diff --git a/codex-rs/artifact-presentation/src/presentation_artifact.rs b/codex-rs/artifact-presentation/src/presentation_artifact.rs index ebece2238..93b551b3c 100644 --- a/codex-rs/artifact-presentation/src/presentation_artifact.rs +++ b/codex-rs/artifact-presentation/src/presentation_artifact.rs @@ -37,6 +37,7 @@ use std::io::Read; use std::io::Write; use std::path::Path; use std::path::PathBuf; +use std::process::Command; use thiserror::Error; use uuid::Uuid; use zip::ZipArchive; @@ -1326,14 +1327,27 @@ impl PresentationArtifactManager { let args: AddTableArgs = parse_args(&request.action, &request.args)?; let artifact_id = required_artifact_id(&request)?; let document = self.get_document_mut(&artifact_id, &request.action)?; + let rows = coerce_table_rows(args.rows, &request.action)?; + let mut frame: Rect = args.position.into(); + let (column_widths, row_heights) = normalize_table_dimensions( + &rows, + frame, + args.column_widths, + args.row_heights, + &request.action, + )?; + frame.width = column_widths.iter().sum(); + frame.height = row_heights.iter().sum(); let element_id = document.next_element_id(); let slide = document.get_slide_mut(args.slide_index, &request.action)?; slide .elements .push(PresentationElement::Table(TableElement { element_id: element_id.clone(), - frame: args.position.into(), - rows: coerce_table_rows(args.rows, &request.action)?, + frame, + rows, + column_widths, + row_heights, style: args.style, merges: Vec::new(), z_order: slide.elements.len(), @@ -2437,6 +2451,18 @@ impl PresentationDocument { .collect() }) .collect(), + column_widths: imported_table + .column_widths + .iter() + .copied() + .map(emu_to_points) + .collect(), + row_heights: imported_table + .rows + .iter() + .map(|row| row.height.map_or(400_000, |height| height)) + .map(emu_to_points) + .collect(), style: None, merges: Vec::new(), z_order: slide.elements.len(), @@ -2821,15 +2847,18 @@ impl PresentationSlide { } } PresentationElement::Table(table) => { - let row_count = table.rows.len().max(1) as u32; - let column_count = - table.rows.iter().map(std::vec::Vec::len).max().unwrap_or(1) as u32; - let column_width = points_to_emu(table.frame.width / column_count.max(1)); - let mut builder = TableBuilder::new(vec![column_width; column_count as usize]) - .position( - points_to_emu(table.frame.left), - points_to_emu(table.frame.top), - ); + let mut builder = TableBuilder::new( + table + .column_widths + .iter() + .copied() + .map(points_to_emu) + .collect(), + ) + .position( + points_to_emu(table.frame.left), + points_to_emu(table.frame.top), + ); for (row_index, row) in table.rows.into_iter().enumerate() { let cells = row .into_iter() @@ -2838,9 +2867,12 @@ impl PresentationSlide { build_table_cell(cell, &table.merges, row_index, column_index) }) .collect::>(); - builder = builder.add_row(TableRow::new(cells)); + let mut table_row = TableRow::new(cells); + if let Some(height) = table.row_heights.get(row_index) { + table_row = table_row.with_height(points_to_emu(*height)); + } + builder = builder.add_row(table_row); } - let _ = row_count; content = content.table(builder.build()); } PresentationElement::Chart(chart) => { @@ -2993,6 +3025,8 @@ struct TableElement { element_id: String, frame: Rect, rows: Vec>, + column_widths: Vec, + row_heights: Vec, style: Option, merges: Vec, z_order: usize, @@ -3609,6 +3643,8 @@ struct AddTableArgs { slide_index: u32, position: PositionArgs, rows: Vec>, + column_widths: Option>, + row_heights: Option>, style: Option, } @@ -4099,6 +4135,57 @@ fn coerce_table_rows( .collect()) } +fn normalize_table_dimensions( + rows: &[Vec], + frame: Rect, + column_widths: Option>, + row_heights: Option>, + action: &str, +) -> Result<(Vec, Vec), PresentationArtifactError> { + let column_count = rows.iter().map(std::vec::Vec::len).max().unwrap_or(1); + let normalized_column_widths = match column_widths { + Some(widths) => { + if widths.len() != column_count { + return Err(PresentationArtifactError::InvalidArgs { + action: action.to_string(), + message: format!( + "`column_widths` must contain {column_count} entries for this table" + ), + }); + } + widths + } + None => split_points(frame.width, column_count), + }; + let normalized_row_heights = match row_heights { + Some(heights) => { + if heights.len() != rows.len() { + return Err(PresentationArtifactError::InvalidArgs { + action: action.to_string(), + message: format!( + "`row_heights` must contain {} entries for this table", + rows.len() + ), + }); + } + heights + } + None => split_points(frame.height, rows.len()), + }; + Ok((normalized_column_widths, normalized_row_heights)) +} + +fn split_points(total: u32, count: usize) -> Vec { + if count == 0 { + return Vec::new(); + } + let base = total / count as u32; + let remainder = total % count as u32; + (0..count) + .map(|index| base + u32::from(index < remainder as usize)) + .collect() +} + fn parse_alignment(value: &str, action: &str) -> Result { match value { "left" => Ok(TextAlignment::Left), @@ -4129,13 +4216,24 @@ fn normalize_theme(args: ThemeArgs, action: &str) -> Result Result { - let position: PositionArgs = serde_json::from_value(value.clone()).map_err(|error| { + #[derive(Deserialize)] + struct SlideSizeArgs { + width: u32, + height: u32, + } + + let slide_size: SlideSizeArgs = serde_json::from_value(value.clone()).map_err(|error| { PresentationArtifactError::InvalidArgs { action: action.to_string(), message: format!("invalid slide_size: {error}"), } })?; - Ok(position.into()) + Ok(Rect { + left: 0, + top: 0, + width: slide_size.width, + height: slide_size.height, + }) } fn apply_layout_to_slide( @@ -4516,6 +4614,8 @@ fn inspect_document( "slide": index + 1, "rows": table.rows.len(), "cols": table.rows.iter().map(std::vec::Vec::len).max().unwrap_or(0), + "columnWidths": table.column_widths, + "rowHeights": table.row_heights, "preview": table.rows.first().map(|row| row.iter().map(|cell| cell.text.clone()).collect::>().join(" | ")), "style": table.style, "bbox": [table.frame.left, table.frame.top, table.frame.width, table.frame.height], @@ -4744,6 +4844,8 @@ fn resolve_anchor( "slideIndex": slide_index, "rows": table.rows.len(), "cols": table.rows.iter().map(std::vec::Vec::len).max().unwrap_or(0), + "columnWidths": table.column_widths, + "rowHeights": table.row_heights, "bbox": [table.frame.left, table.frame.top, table.frame.width, table.frame.height], "bboxUnit": "points", }), @@ -4804,10 +4906,10 @@ fn build_pptx_bytes(document: &PresentationDocument, action: &str) -> Result, document: &PresentationDocument, ) -> Result, String> { @@ -4837,6 +4939,20 @@ fn patch_pptx_hyperlinks( writer .start_file(&name, options) .map_err(|error| error.to_string())?; + if name == "ppt/presentation.xml" { + writer + .write_all( + update_presentation_xml_dimensions(bytes, document.slide_size)?.as_bytes(), + ) + .map_err(|error| error.to_string())?; + continue; + } + if let Some(slide_number) = parse_slide_xml_path(&name) { + writer + .write_all(update_slide_xml(bytes, &document.slides[slide_number - 1])?.as_bytes()) + .map_err(|error| error.to_string())?; + continue; + } if let Some(slide_number) = parse_slide_relationships_path(&name) && let Some(relationships) = pending_slide_relationships.remove(&slide_number) { @@ -4868,6 +4984,42 @@ fn patch_pptx_hyperlinks( .map(Cursor::into_inner) } +fn update_presentation_xml_dimensions( + existing_bytes: Vec, + slide_size: Rect, +) -> Result { + let existing = String::from_utf8(existing_bytes).map_err(|error| error.to_string())?; + let updated = replace_self_closing_xml_tag( + &existing, + "p:sldSz", + &format!( + r#""#, + points_to_emu(slide_size.width), + points_to_emu(slide_size.height) + ), + )?; + replace_self_closing_xml_tag( + &updated, + "p:notesSz", + &format!( + r#""#, + points_to_emu(slide_size.height), + points_to_emu(slide_size.width) + ), + ) +} + +fn replace_self_closing_xml_tag(xml: &str, tag: &str, replacement: &str) -> Result { + let start = xml + .find(&format!("<{tag} ")) + .ok_or_else(|| format!("presentation xml is missing `<{tag} .../>`"))?; + let end = xml[start..] + .find("/>") + .map(|offset| start + offset + 2) + .ok_or_else(|| format!("presentation xml tag `{tag}` is not self-closing"))?; + Ok(format!("{}{replacement}{}", &xml[..start], &xml[end..])) +} + fn slide_hyperlink_relationships(slide: &PresentationSlide) -> Vec { let mut ordered = slide.elements.iter().collect::>(); ordered.sort_by_key(|element| element.z_order()); @@ -4898,6 +5050,13 @@ fn parse_slide_relationships_path(path: &str) -> Option { .ok() } +fn parse_slide_xml_path(path: &str) -> Option { + path.strip_prefix("ppt/slides/slide")? + .strip_suffix(".xml")? + .parse::() + .ok() +} + fn update_slide_relationships_xml( existing_bytes: Vec, relationships: &[String], @@ -4922,6 +5081,68 @@ fn slide_relationships_xml(relationships: &[String]) -> String { ) } +fn update_slide_xml(existing_bytes: Vec, slide: &PresentationSlide) -> Result { + let existing = String::from_utf8(existing_bytes).map_err(|error| error.to_string())?; + let table_xml = slide_table_xml(slide); + if table_xml.is_empty() { + return Ok(existing); + } + existing + .contains("") + .then(|| existing.replace("", &format!("{table_xml}\n"))) + .ok_or_else(|| "slide xml is missing a closing ``".to_string()) +} + +fn slide_table_xml(slide: &PresentationSlide) -> String { + let mut ordered = slide.elements.iter().collect::>(); + ordered.sort_by_key(|element| element.z_order()); + let mut table_index = 0_usize; + ordered + .into_iter() + .filter_map(|element| { + let PresentationElement::Table(table) = element else { + return None; + }; + table_index += 1; + let rows = table + .rows + .clone() + .into_iter() + .enumerate() + .map(|(row_index, row)| { + let cells = row + .into_iter() + .enumerate() + .map(|(column_index, cell)| { + build_table_cell(cell, &table.merges, row_index, column_index) + }) + .collect::>(); + let mut table_row = TableRow::new(cells); + if let Some(height) = table.row_heights.get(row_index) { + table_row = table_row.with_height(points_to_emu(*height)); + } + Some(table_row) + }) + .collect::>>()?; + Some(ppt_rs::generator::table::generate_table_xml( + &ppt_rs::generator::table::Table::new( + rows, + table + .column_widths + .iter() + .copied() + .map(points_to_emu) + .collect(), + points_to_emu(table.frame.left), + points_to_emu(table.frame.top), + ), + 300 + table_index, + )) + }) + .collect::>() + .join("\n") +} + fn write_preview_images( document: &PresentationDocument, output_dir: &Path, @@ -4938,13 +5159,74 @@ fn write_preview_images( path: pptx_path.clone(), message: error.to_string(), })?; - document - .to_ppt_rs() - .save_as_png(output_dir) + render_pptx_to_pngs(&pptx_path, output_dir, action) +} + +fn render_pptx_to_pngs( + pptx_path: &Path, + output_dir: &Path, + action: &str, +) -> Result<(), PresentationArtifactError> { + let soffice_cmd = if cfg!(target_os = "macos") + && Path::new("/Applications/LibreOffice.app/Contents/MacOS/soffice").exists() + { + "/Applications/LibreOffice.app/Contents/MacOS/soffice" + } else { + "soffice" + }; + let conversion = Command::new(soffice_cmd) + .arg("--headless") + .arg("--convert-to") + .arg("pdf") + .arg(pptx_path) + .arg("--outdir") + .arg(output_dir) + .output() .map_err(|error| PresentationArtifactError::ExportFailed { + path: pptx_path.to_path_buf(), + message: format!("{action}: failed to execute LibreOffice: {error}"), + })?; + if !conversion.status.success() { + return Err(PresentationArtifactError::ExportFailed { + path: pptx_path.to_path_buf(), + message: format!( + "{action}: LibreOffice conversion failed: {}", + String::from_utf8_lossy(&conversion.stderr) + ), + }); + } + + let pdf_path = output_dir.join( + pptx_path + .file_stem() + .and_then(|stem| stem.to_str()) + .map(|stem| format!("{stem}.pdf")) + .ok_or_else(|| PresentationArtifactError::ExportFailed { + path: pptx_path.to_path_buf(), + message: format!("{action}: preview pptx filename is invalid"), + })?, + ); + let prefix = output_dir.join("slide"); + let conversion = Command::new("pdftoppm") + .arg("-png") + .arg(&pdf_path) + .arg(&prefix) + .output() + .map_err(|error| PresentationArtifactError::ExportFailed { + path: pdf_path.clone(), + message: format!("{action}: failed to execute pdftoppm: {error}"), + })?; + std::fs::remove_file(&pdf_path).ok(); + if !conversion.status.success() { + return Err(PresentationArtifactError::ExportFailed { path: output_dir.to_path_buf(), - message: format!("{action}: {error}"), - }) + message: format!( + "{action}: pdftoppm conversion failed: {}", + String::from_utf8_lossy(&conversion.stderr) + ), + }); + } + Ok(()) } pub(crate) fn write_preview_image( diff --git a/codex-rs/artifact-presentation/src/tests.rs b/codex-rs/artifact-presentation/src/tests.rs index bf702d50a..aef6afeb2 100644 --- a/codex-rs/artifact-presentation/src/tests.rs +++ b/codex-rs/artifact-presentation/src/tests.rs @@ -133,6 +133,48 @@ fn manager_can_import_exported_presentation() -> Result<(), Box Result<(), Box> { + let temp_dir = tempfile::tempdir()?; + let mut manager = PresentationArtifactManager::default(); + let created = manager.execute( + PresentationArtifactRequest { + artifact_id: None, + action: "create".to_string(), + args: serde_json::json!({ + "name": "Custom Size", + "slide_size": { "width": 960, "height": 540 } + }), + }, + temp_dir.path(), + )?; + manager.execute( + PresentationArtifactRequest { + artifact_id: Some(created.artifact_id.clone()), + action: "add_slide".to_string(), + args: serde_json::json!({}), + }, + temp_dir.path(), + )?; + let export_path = temp_dir.path().join("custom-size.pptx"); + manager.execute( + PresentationArtifactRequest { + artifact_id: Some(created.artifact_id), + action: "export_pptx".to_string(), + args: serde_json::json!({ "path": export_path }), + }, + temp_dir.path(), + )?; + + let presentation_xml = zip_entry_text( + &temp_dir.path().join("custom-size.pptx"), + "ppt/presentation.xml", + )?; + assert!(presentation_xml.contains(r#"cx="12192000" cy="6858000""#)); + assert!(presentation_xml.contains(r#"p:notesSz cx="6858000" cy="12192000""#)); + Ok(()) +} + #[test] fn image_fit_contain_preserves_aspect_ratio() { let image = ImageElement { @@ -1288,6 +1330,8 @@ fn manager_supports_table_cell_updates_and_merges() -> Result<(), Box Result<(), Box>()), + Some(vec![90, 150]) + ); + assert_eq!( + resolved + .resolved_record + .as_ref() + .and_then(|record| record.get("rowHeights")) + .and_then(serde_json::Value::as_array) + .map(|heights| heights + .iter() + .filter_map(serde_json::Value::as_u64) + .collect::>()), + Some(vec![40, 80]) + ); let merged = manager.execute( PresentationArtifactRequest { @@ -1353,5 +1429,25 @@ fn manager_supports_table_cell_updates_and_merges() -> Result<(), Box"#), + "{slide_xml}" + ); + assert!(slide_xml.contains(r#""#)); + assert!(slide_xml.contains(r#""#)); + assert!(slide_xml.contains(r#""#)); Ok(()) } diff --git a/codex-rs/core/templates/tools/presentation_artifact.md b/codex-rs/core/templates/tools/presentation_artifact.md index 4628a2bd8..7f635b999 100644 --- a/codex-rs/core/templates/tools/presentation_artifact.md +++ b/codex-rs/core/templates/tools/presentation_artifact.md @@ -55,9 +55,14 @@ Supported actions: Example create: `{"action":"create","args":{"name":"Quarterly Update"}}` +Example create with custom slide size: +`{"action":"create","args":{"name":"Quarterly Update","slide_size":{"width":960,"height":540}}}` + Example edit: `{"artifact_id":"presentation_x","action":"add_text_shape","args":{"slide_index":0,"text":"Revenue up 24%","position":{"left":48,"top":72,"width":260,"height":80}}}` +Table creation also accepts optional `column_widths` and `row_heights` arrays in points when you need explicit table sizing instead of even splits. + Example export: `{"artifact_id":"presentation_x","action":"export_pptx","args":{"path":"artifacts/q2-update.pptx"}}`