mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
fix more
This commit is contained in:
@@ -1058,12 +1058,12 @@ fn export_text_body_xml(
|
||||
style.alignment.unwrap_or(TextAlignment::Left),
|
||||
rich_text,
|
||||
);
|
||||
let left_inset = points_to_emu(insets.left);
|
||||
let right_inset = points_to_emu(insets.right);
|
||||
let top_inset = points_to_emu(insets.top);
|
||||
let bottom_inset = points_to_emu(insets.bottom);
|
||||
format!(
|
||||
r#"<p:txBody><a:bodyPr wrap="{wrap}" lIns="{}" rIns="{}" tIns="{}" bIns="{}" anchor="{anchor}">{auto_fit}</a:bodyPr><a:lstStyle/>{paragraphs}</p:txBody>"#,
|
||||
points_to_emu(insets.left),
|
||||
points_to_emu(insets.right),
|
||||
points_to_emu(insets.top),
|
||||
points_to_emu(insets.bottom),
|
||||
r#"<p:txBody><a:bodyPr wrap="{wrap}" lIns="{left_inset}" rIns="{right_inset}" tIns="{top_inset}" bIns="{bottom_inset}" anchor="{anchor}">{auto_fit}</a:bodyPr><a:lstStyle/>{paragraphs}</p:txBody>"#
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1110,12 +1110,12 @@ fn export_table_cell_text_body_xml(
|
||||
TextAutoFitMode::ResizeShapeToFitText => "<a:spAutoFit/>".to_string(),
|
||||
};
|
||||
let insets = cell.rich_text.layout.insets.map(|insets| {
|
||||
let left_inset = points_to_emu(insets.left);
|
||||
let right_inset = points_to_emu(insets.right);
|
||||
let top_inset = points_to_emu(insets.top);
|
||||
let bottom_inset = points_to_emu(insets.bottom);
|
||||
format!(
|
||||
r#" lIns="{}" rIns="{}" tIns="{}" bIns="{}""#,
|
||||
points_to_emu(insets.left),
|
||||
points_to_emu(insets.right),
|
||||
points_to_emu(insets.top),
|
||||
points_to_emu(insets.bottom),
|
||||
r#" lIns="{left_inset}" rIns="{right_inset}" tIns="{top_inset}" bIns="{bottom_inset}""#,
|
||||
)
|
||||
});
|
||||
let paragraphs = export_text_paragraphs_xml(
|
||||
|
||||
@@ -153,7 +153,7 @@ fn render_slide_image(
|
||||
}
|
||||
|
||||
let mut ordered = slide.elements.iter().collect::<Vec<_>>();
|
||||
ordered.sort_by_key(|element| element.z_order());
|
||||
ordered.sort_by_key(|element| (preview_render_layer(element), element.z_order()));
|
||||
for element in ordered {
|
||||
render_element(&mut canvas, document, slide, element, render_scale)?;
|
||||
}
|
||||
@@ -1123,11 +1123,13 @@ fn layout_text_lines(glyphs: &[StyledGlyph], config: TextRenderConfig) -> Vec<Te
|
||||
let mut current_spaces = 0;
|
||||
for glyph in glyphs {
|
||||
if glyph.ch == '\n' {
|
||||
lines.push(TextLine {
|
||||
glyphs: std::mem::take(&mut current),
|
||||
width: current_width,
|
||||
spaces: current_spaces,
|
||||
});
|
||||
push_text_line(
|
||||
&mut lines,
|
||||
&mut current,
|
||||
&mut current_width,
|
||||
&mut current_spaces,
|
||||
config.font_px,
|
||||
);
|
||||
current_width = 0;
|
||||
current_spaces = 0;
|
||||
continue;
|
||||
@@ -1137,13 +1139,21 @@ fn layout_text_lines(glyphs: &[StyledGlyph], config: TextRenderConfig) -> Vec<Te
|
||||
&& !current.is_empty()
|
||||
&& current_width.saturating_add(glyph_width) > config.width
|
||||
{
|
||||
lines.push(TextLine {
|
||||
glyphs: std::mem::take(&mut current),
|
||||
width: current_width,
|
||||
spaces: current_spaces,
|
||||
});
|
||||
push_text_line(
|
||||
&mut lines,
|
||||
&mut current,
|
||||
&mut current_width,
|
||||
&mut current_spaces,
|
||||
config.font_px,
|
||||
);
|
||||
current_width = 0;
|
||||
current_spaces = 0;
|
||||
if matches!(glyph.ch, ' ' | '\t') {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if current.is_empty() && matches!(glyph.ch, ' ' | '\t') {
|
||||
continue;
|
||||
}
|
||||
if glyph.ch == ' ' {
|
||||
current_spaces += 1;
|
||||
@@ -1152,15 +1162,41 @@ fn layout_text_lines(glyphs: &[StyledGlyph], config: TextRenderConfig) -> Vec<Te
|
||||
current.push(glyph.clone());
|
||||
}
|
||||
if !current.is_empty() || lines.is_empty() {
|
||||
lines.push(TextLine {
|
||||
glyphs: current,
|
||||
width: current_width,
|
||||
spaces: current_spaces,
|
||||
});
|
||||
push_text_line(
|
||||
&mut lines,
|
||||
&mut current,
|
||||
&mut current_width,
|
||||
&mut current_spaces,
|
||||
config.font_px,
|
||||
);
|
||||
}
|
||||
lines
|
||||
}
|
||||
|
||||
fn push_text_line(
|
||||
lines: &mut Vec<TextLine>,
|
||||
current: &mut Vec<StyledGlyph>,
|
||||
current_width: &mut u32,
|
||||
current_spaces: &mut usize,
|
||||
font_px: u32,
|
||||
) {
|
||||
while let Some(glyph) = current.last() {
|
||||
if !matches!(glyph.ch, ' ' | '\t') {
|
||||
break;
|
||||
}
|
||||
let trimmed = current.pop().expect("last glyph must exist");
|
||||
*current_width = current_width.saturating_sub(measure_glyph_width(&trimmed, font_px));
|
||||
if trimmed.ch == ' ' {
|
||||
*current_spaces = current_spaces.saturating_sub(1);
|
||||
}
|
||||
}
|
||||
lines.push(TextLine {
|
||||
glyphs: std::mem::take(current),
|
||||
width: *current_width,
|
||||
spaces: *current_spaces,
|
||||
});
|
||||
}
|
||||
|
||||
fn draw_bitmap_glyph(
|
||||
image: &mut RgbaImage,
|
||||
left: u32,
|
||||
@@ -1637,6 +1673,17 @@ fn parse_rgba(hex: &str, alpha: u8) -> Rgba<u8> {
|
||||
Rgba([0, 0, 0, alpha])
|
||||
}
|
||||
|
||||
fn preview_render_layer(element: &PresentationElement) -> usize {
|
||||
match element {
|
||||
PresentationElement::Text(_)
|
||||
| PresentationElement::Shape(_)
|
||||
| PresentationElement::Connector(_)
|
||||
| PresentationElement::Image(ImageElement { payload: None, .. }) => 0,
|
||||
PresentationElement::Image(_) | PresentationElement::Chart(_) => 1,
|
||||
PresentationElement::Table(_) => 2,
|
||||
}
|
||||
}
|
||||
|
||||
fn shape_path(
|
||||
geometry: ShapeGeometry,
|
||||
width: u32,
|
||||
@@ -1645,9 +1692,10 @@ fn shape_path(
|
||||
let width = width.max(1) as f32;
|
||||
let height = height.max(1) as f32;
|
||||
match geometry {
|
||||
ShapeGeometry::Rectangle | ShapeGeometry::RoundedRectangle => {
|
||||
ShapeGeometry::Rectangle => {
|
||||
polygon_path(&[(0.0, 0.0), (width, 0.0), (width, height), (0.0, height)])
|
||||
}
|
||||
ShapeGeometry::RoundedRectangle => rounded_rectangle_path(width, height),
|
||||
ShapeGeometry::Ellipse => ellipse_path(width as u32, height as u32),
|
||||
ShapeGeometry::Triangle => polygon_path(&[(width / 2.0, 0.0), (width, height), (0.0, height)]),
|
||||
ShapeGeometry::RightTriangle => polygon_path(&[(0.0, 0.0), (width, height), (0.0, height)]),
|
||||
@@ -1799,6 +1847,55 @@ fn ellipse_path(width: u32, height: u32) -> Result<TinyPath, PresentationArtifac
|
||||
polygon_path(&points)
|
||||
}
|
||||
|
||||
fn rounded_rectangle_path(width: f32, height: f32) -> Result<TinyPath, PresentationArtifactError> {
|
||||
let radius = (width.min(height) * 0.18).max(2.0);
|
||||
let radius = radius.min(width / 2.0).min(height / 2.0);
|
||||
let control = radius * (4.0 * (2.0_f32.sqrt() - 1.0) / 3.0);
|
||||
let mut builder = PathBuilder::new();
|
||||
builder.move_to(radius, 0.0);
|
||||
builder.line_to(width - radius, 0.0);
|
||||
builder.cubic_to(
|
||||
width - radius + control,
|
||||
0.0,
|
||||
width,
|
||||
radius - control,
|
||||
width,
|
||||
radius,
|
||||
);
|
||||
builder.line_to(width, height - radius);
|
||||
builder.cubic_to(
|
||||
width,
|
||||
height - radius + control,
|
||||
width - radius + control,
|
||||
height,
|
||||
width - radius,
|
||||
height,
|
||||
);
|
||||
builder.line_to(radius, height);
|
||||
builder.cubic_to(
|
||||
radius - control,
|
||||
height,
|
||||
0.0,
|
||||
height - radius + control,
|
||||
0.0,
|
||||
height - radius,
|
||||
);
|
||||
builder.line_to(0.0, radius);
|
||||
builder.cubic_to(
|
||||
0.0,
|
||||
radius - control,
|
||||
radius - control,
|
||||
0.0,
|
||||
radius,
|
||||
0.0,
|
||||
);
|
||||
builder.close();
|
||||
builder.finish().ok_or_else(|| PresentationArtifactError::RenderFailed {
|
||||
action: "render_preview".to_string(),
|
||||
message: "failed to build rounded rectangle path".to_string(),
|
||||
})
|
||||
}
|
||||
|
||||
fn regular_polygon_path(
|
||||
sides: usize,
|
||||
width: f32,
|
||||
@@ -2171,3 +2268,79 @@ fn resolve_render_slide_index(
|
||||
}
|
||||
Ok(0)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn wrapped_lines_skip_leading_spaces() {
|
||||
let text = "One strong visual lands the story.";
|
||||
let style = TextStyle::default();
|
||||
let glyphs = styled_glyphs(text, &style, &RichTextState::default(), None);
|
||||
let prefix = "One strong visual lands the";
|
||||
let width = glyphs
|
||||
.iter()
|
||||
.take(prefix.chars().count())
|
||||
.map(|glyph| measure_glyph_width(glyph, 14))
|
||||
.sum();
|
||||
let lines = layout_text_lines(
|
||||
&glyphs,
|
||||
TextRenderConfig {
|
||||
width,
|
||||
font_px: 14,
|
||||
line_height: 18,
|
||||
baseline_offset: 12,
|
||||
wrap: TextWrapMode::Square,
|
||||
vertical_alignment: TextVerticalAlignment::Top,
|
||||
alignment: TextAlignment::Left,
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(lines.len(), 2);
|
||||
assert_eq!(
|
||||
lines[0].glyphs.iter().map(|glyph| glyph.ch).collect::<String>(),
|
||||
prefix
|
||||
);
|
||||
assert_eq!(
|
||||
lines[1].glyphs.iter().map(|glyph| glyph.ch).collect::<String>(),
|
||||
"story."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn wrapped_lines_trim_trailing_spaces() {
|
||||
let text = "One strong visual lands the story.";
|
||||
let style = TextStyle::default();
|
||||
let glyphs = styled_glyphs(text, &style, &RichTextState::default(), None);
|
||||
let prefix = "One strong visual lands the ";
|
||||
let width = glyphs
|
||||
.iter()
|
||||
.take(prefix.chars().count())
|
||||
.map(|glyph| measure_glyph_width(glyph, 14))
|
||||
.sum();
|
||||
let lines = layout_text_lines(
|
||||
&glyphs,
|
||||
TextRenderConfig {
|
||||
width,
|
||||
font_px: 14,
|
||||
line_height: 18,
|
||||
baseline_offset: 12,
|
||||
wrap: TextWrapMode::Square,
|
||||
vertical_alignment: TextVerticalAlignment::Top,
|
||||
alignment: TextAlignment::Left,
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(lines.len(), 2);
|
||||
assert_eq!(
|
||||
lines[0].glyphs.iter().map(|glyph| glyph.ch).collect::<String>(),
|
||||
"One strong visual lands the"
|
||||
);
|
||||
assert_eq!(
|
||||
lines[1].glyphs.iter().map(|glyph| glyph.ch).collect::<String>(),
|
||||
"story."
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -132,7 +132,7 @@ fn manager_can_import_exported_presentation() -> Result<(), Box<dyn std::error::
|
||||
)?;
|
||||
manager.execute(
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: Some(artifact_id),
|
||||
artifact_id: Some(artifact_id.clone()),
|
||||
action: "add_shape".to_string(),
|
||||
args: serde_json::json!({
|
||||
"slide_index": 0,
|
||||
@@ -489,7 +489,7 @@ fn exported_text_shapes_preserve_text_styling() -> Result<(), Box<dyn std::error
|
||||
)?;
|
||||
manager.execute(
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: Some(artifact_id),
|
||||
artifact_id: Some(artifact_id.clone()),
|
||||
action: "add_shape".to_string(),
|
||||
args: serde_json::json!({
|
||||
"slide_index": 0,
|
||||
@@ -4311,6 +4311,86 @@ fn export_preview_uses_native_renderer_for_single_slide() -> Result<(), Box<dyn
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_preview_matches_pptx_stacking_and_rounded_shapes()
|
||||
-> Result<(), Box<dyn std::error::Error>> {
|
||||
let temp_dir = tempfile::tempdir()?;
|
||||
let image_path = temp_dir.path().join("stacking-source.png");
|
||||
image::RgbaImage::from_pixel(40, 40, image::Rgba([0x2A, 0x80, 0xD7, 0xFF]))
|
||||
.save(&image_path)?;
|
||||
|
||||
let mut manager = PresentationArtifactManager::default();
|
||||
let created = manager.execute(
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: None,
|
||||
action: "create".to_string(),
|
||||
args: serde_json::json!({ "name": "Preview stacking parity" }),
|
||||
},
|
||||
temp_dir.path(),
|
||||
)?;
|
||||
let artifact_id = created.artifact_id;
|
||||
manager.execute(
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: Some(artifact_id.clone()),
|
||||
action: "add_slide".to_string(),
|
||||
args: serde_json::json!({
|
||||
"background_fill": "#D9EAF7"
|
||||
}),
|
||||
},
|
||||
temp_dir.path(),
|
||||
)?;
|
||||
manager.execute(
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: Some(artifact_id.clone()),
|
||||
action: "add_image".to_string(),
|
||||
args: serde_json::json!({
|
||||
"slide_index": 0,
|
||||
"path": image_path,
|
||||
"position": { "left": 160, "top": 80, "width": 180, "height": 180 },
|
||||
"fit": "stretch"
|
||||
}),
|
||||
},
|
||||
temp_dir.path(),
|
||||
)?;
|
||||
manager.execute(
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: Some(artifact_id.clone()),
|
||||
action: "add_shape".to_string(),
|
||||
args: serde_json::json!({
|
||||
"slide_index": 0,
|
||||
"geometry": "rounded_rectangle",
|
||||
"position": { "left": 120, "top": 200, "width": 220, "height": 120 },
|
||||
"fill": "#FFFFFF"
|
||||
}),
|
||||
},
|
||||
temp_dir.path(),
|
||||
)?;
|
||||
|
||||
let target_path = temp_dir.path().join("stacking-preview.png");
|
||||
let response = manager.execute(
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: Some(artifact_id),
|
||||
action: "export_preview".to_string(),
|
||||
args: serde_json::json!({
|
||||
"path": target_path,
|
||||
"slide_index": 0
|
||||
}),
|
||||
},
|
||||
temp_dir.path(),
|
||||
)?;
|
||||
let preview = image::open(
|
||||
response
|
||||
.exported_paths
|
||||
.first()
|
||||
.expect("exported preview path present"),
|
||||
)?;
|
||||
|
||||
assert_eq!(preview.get_pixel(200, 220).0, [0x2A, 0x80, 0xD7, 0xFF]);
|
||||
assert_eq!(preview.get_pixel(200, 290).0, [0xFF, 0xFF, 0xFF, 0xFF]);
|
||||
assert_eq!(preview.get_pixel(122, 202).0, [0xD9, 0xEA, 0xF7, 0xFF]);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn export_preview_writes_all_slides_with_stable_names() -> Result<(), Box<dyn std::error::Error>> {
|
||||
let temp_dir = tempfile::tempdir()?;
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_core::CodexAuth;
|
||||
use codex_core::features::Feature;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
@@ -46,7 +47,12 @@ async fn presentation_artifact_render_preview_returns_inline_image() -> anyhow::
|
||||
cwd,
|
||||
session_configured,
|
||||
..
|
||||
} = test_codex().build(&server).await?;
|
||||
} = test_codex()
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::Artifact);
|
||||
})
|
||||
.build(&server)
|
||||
.await?;
|
||||
|
||||
let call_id = "presentation-render-preview";
|
||||
let arguments = serde_json::json!({
|
||||
@@ -206,6 +212,7 @@ async fn presentation_artifact_render_preview_fails_for_text_only_model() -> any
|
||||
let TestCodex { codex, cwd, .. } = test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::Artifact);
|
||||
config.model = Some(model_slug.to_string());
|
||||
})
|
||||
.build(&server)
|
||||
|
||||
Reference in New Issue
Block a user