From b54ec601d836a4de2af9a783453d48c21ab4428f Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Wed, 14 Oct 2026 15:42:23 -0400 Subject: [PATCH] Fix up #508: read 3-D settings with the XML parser; keep only known presets Shape3d::parse scanned the kept a:scene3d/a:sp3d XML as strings and copied the camera, light rig, direction, material and bevel names from the file into the XML it writes back unescaped. It now parses the fragment with the importer's XML reader (exported from deckcraft-pptx as parse_xml) and keeps a value only when it is in the format's lists (ST_PresetCameraType, ST_LightRigType, ST_LightRigDirection, ST_PresetMaterialType, ST_BevelPresetType), else the default; lengths are clamped and the extrusion/contour colours are re-serialized (escaped) by the XML writer. The Format Shape pane rows in ui-parity.md that earlier merges had duplicated are folded into one. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/three_d.rs | 280 ++++++++++++++++++++++--------- crates/pptx/src/lib.rs | 4 + docs/ui-parity.md | 6 +- 3 files changed, 209 insertions(+), 81 deletions(-) diff --git a/crates/engine/src/cmd/three_d.rs b/crates/engine/src/cmd/three_d.rs index fe53450b..881a39d1 100644 --- a/crates/engine/src/cmd/three_d.rs +++ b/crates/engine/src/cmd/three_d.rs @@ -62,6 +62,126 @@ pub const LIGHTS: &[&str] = &[ "brightRoom", ]; +/// Every camera preset the file format defines (`ST_PresetCameraType`); a file's camera is kept +/// only when it is one of these. +pub const ALL_CAMERAS: &[&str] = &[ + "legacyObliqueTopLeft", + "legacyObliqueTop", + "legacyObliqueTopRight", + "legacyObliqueLeft", + "legacyObliqueFront", + "legacyObliqueRight", + "legacyObliqueBottomLeft", + "legacyObliqueBottom", + "legacyObliqueBottomRight", + "legacyPerspectiveTopLeft", + "legacyPerspectiveTop", + "legacyPerspectiveTopRight", + "legacyPerspectiveLeft", + "legacyPerspectiveFront", + "legacyPerspectiveRight", + "legacyPerspectiveBottomLeft", + "legacyPerspectiveBottom", + "legacyPerspectiveBottomRight", + "orthographicFront", + "isometricTopUp", + "isometricTopDown", + "isometricBottomUp", + "isometricBottomDown", + "isometricLeftUp", + "isometricLeftDown", + "isometricRightUp", + "isometricRightDown", + "isometricOffAxis1Left", + "isometricOffAxis1Right", + "isometricOffAxis1Top", + "isometricOffAxis2Left", + "isometricOffAxis2Right", + "isometricOffAxis2Top", + "isometricOffAxis3Left", + "isometricOffAxis3Right", + "isometricOffAxis3Bottom", + "isometricOffAxis4Left", + "isometricOffAxis4Right", + "isometricOffAxis4Bottom", + "obliqueTopLeft", + "obliqueTop", + "obliqueTopRight", + "obliqueLeft", + "obliqueRight", + "obliqueBottomLeft", + "obliqueBottom", + "obliqueBottomRight", + "perspectiveFront", + "perspectiveLeft", + "perspectiveRight", + "perspectiveAbove", + "perspectiveBelow", + "perspectiveAboveLeftFacing", + "perspectiveAboveRightFacing", + "perspectiveContrastingLeftFacing", + "perspectiveContrastingRightFacing", + "perspectiveHeroicLeftFacing", + "perspectiveHeroicRightFacing", + "perspectiveHeroicExtremeLeftFacing", + "perspectiveHeroicExtremeRightFacing", + "perspectiveRelaxed", + "perspectiveRelaxedModerately", +]; + +/// Every light rig the file format defines (`ST_LightRigType`). +pub const ALL_LIGHTS: &[&str] = &[ + "legacyFlat1", + "legacyFlat2", + "legacyFlat3", + "legacyFlat4", + "legacyNormal1", + "legacyNormal2", + "legacyNormal3", + "legacyNormal4", + "legacyHarsh1", + "legacyHarsh2", + "legacyHarsh3", + "legacyHarsh4", + "threePt", + "balanced", + "soft", + "harsh", + "flood", + "contrasting", + "morning", + "sunrise", + "sunset", + "chilly", + "freezing", + "flat", + "twoPt", + "glow", + "brightRoom", +]; + +/// Light rig directions (`ST_LightRigDirection`). +pub const LIGHT_DIRS: &[&str] = &["tl", "t", "tr", "l", "r", "bl", "b", "br"]; + +/// Every material the file format defines (`ST_PresetMaterialType`). +pub const ALL_MATERIALS: &[&str] = &[ + "legacyMatte", + "legacyPlastic", + "legacyMetal", + "legacyWireframe", + "matte", + "plastic", + "metal", + "warmMatte", + "translucentPowder", + "powder", + "dkEdge", + "softEdge", + "clear", + "flat", + "softmetal", +]; + /// One bevel: preset, width and height in points. #[derive(Clone, Debug, PartialEq)] pub struct Bevel { @@ -107,92 +227,54 @@ impl Default for Shape3d { } } -/// The start tags of `raw` as (local name, tag text), in order. -fn tags(raw: &str) -> Vec<(&str, &str)> { - let mut out = Vec::new(); - let mut rest = raw; - while let Some(i) = rest.find('<') { - let Some(t) = rest.get(i + 1..) else { break }; - let end = t.find('>').unwrap_or(t.len()); - let tag = t.get(..end).unwrap_or(""); - rest = t.get(end..).unwrap_or(""); - if tag.starts_with('/') || tag.starts_with('?') || tag.starts_with('!') { - continue; - } - let name = tag.split(|c: char| c.is_whitespace() || c == '/').next().unwrap_or(""); - let local = name.rsplit(':').next().unwrap_or(name); - out.push((local, tag)); - } - out -} - -fn attr<'a>(tag: &'a str, name: &str) -> Option<&'a str> { - let key = format!(" {name}=\""); - let s = tag.find(&key)? + key.len(); - let rest = tag.get(s..)?; - rest.get(..rest.find('"')?) -} - -fn num(tag: &str, name: &str) -> Option { - attr(tag, name).and_then(|v| v.parse::().ok()).filter(|v| v.is_finite()) -} - const EMU: f64 = 12700.0; const ANGLE: f64 = 60000.0; +/// Largest length kept, in points (PowerPoint's own limit). +const MAX_PT: f64 = 1584.0; -/// An `a:`-prefixed element `local` (with its children) cut from `raw`, if there is one. -fn element(raw: &str, local: &str) -> Option { - let open = format!(""); - let i = raw.find(&open)?; - let j = raw.get(i..)?.find(&close)? + i + close.len(); - raw.get(i..j).map(str::to_string) +/// `value` when it is one of `known`, else `default`. Values from a file are written back into +/// XML as they are, so nothing outside these lists may pass. +fn known(value: Option<&str>, known: &[&str], default: &str) -> String { + value.filter(|v| known.contains(v)).unwrap_or(default).to_string() +} + +fn num(e: &deckcraft_pptx::XmlEl, name: &str) -> Option { + e.attr(name).and_then(|v| v.trim().parse::().ok()).filter(|v| v.is_finite()) } impl Shape3d { - /// Reads kept 3-D XML; unknown parts take their defaults. + /// Reads kept 3-D XML (`a:scene3d` and `a:sp3d`); unknown or invalid parts take their defaults. pub fn parse(raw: &str) -> Shape3d { let mut s = Shape3d::default(); - let mut in_camera = false; - for (name, tag) in tags(raw) { - match name { - "camera" => { - s.camera = attr(tag, "prst").unwrap_or("orthographicFront").to_string(); - in_camera = !tag.ends_with('/'); - } - "rot" if in_camera => { - let d = |n| num(tag, n).unwrap_or(0.0) / ANGLE; - s.rot = Some((d("lon"), d("lat"), d("rev"))); - in_camera = false; - } - "lightRig" => { - in_camera = false; - s.light = attr(tag, "rig").unwrap_or("threePt").to_string(); - s.light_dir = attr(tag, "dir").unwrap_or("t").to_string(); - } - "sp3d" => { - in_camera = false; - s.depth = num(tag, "extrusionH").unwrap_or(0.0) / EMU; - s.contour = num(tag, "contourW").unwrap_or(0.0) / EMU; - s.z = num(tag, "z").unwrap_or(0.0) / EMU; - s.material = attr(tag, "prstMaterial").unwrap_or("warmMatte").to_string(); - } - "bevelT" | "bevelB" => { - let b = Bevel { - preset: attr(tag, "prst").unwrap_or("circle").to_string(), - w: num(tag, "w").unwrap_or(76200.0) / EMU, - h: num(tag, "h").unwrap_or(76200.0) / EMU, - }; - if name == "bevelT" { - s.top = Some(b); - } else { - s.bottom = Some(b); - } - } - _ => {} + let Ok(doc) = deckcraft_pptx::parse_xml(format!("{raw}").as_bytes()) else { return s }; + let root = &doc.root; + if let Some(cam) = root.path(&["scene3d", "camera"]) { + s.camera = known(cam.attr("prst"), ALL_CAMERAS, "orthographicFront"); + if let Some(rot) = cam.child("rot") { + let d = |n| num(rot, n).unwrap_or(0.0) / ANGLE; + s.rot = Some((d("lon"), d("lat"), d("rev"))); } } - s.colors = ["extrusionClr", "contourClr"].iter().filter_map(|n| element(raw, n)).collect(); + if let Some(l) = root.path(&["scene3d", "lightRig"]) { + s.light = known(l.attr("rig"), ALL_LIGHTS, "threePt"); + s.light_dir = known(l.attr("dir"), LIGHT_DIRS, "t"); + } + if let Some(sp) = root.child("sp3d") { + let pt = |n, lo: f64| (num(sp, n).unwrap_or(0.0) / EMU).clamp(lo, MAX_PT); + s.depth = pt("extrusionH", 0.0); + s.contour = pt("contourW", 0.0); + s.z = pt("z", -MAX_PT); + s.material = known(sp.attr("prstMaterial"), ALL_MATERIALS, "warmMatte"); + for (name, slot) in [("bevelT", &mut s.top), ("bevelB", &mut s.bottom)] { + if let Some(b) = sp.child(name) { + let len = |n| (num(b, n).unwrap_or(76200.0) / EMU).clamp(0.0, MAX_PT); + *slot = Some(Bevel { preset: known(b.attr("prst"), BEVELS, "circle"), w: len("w"), h: len("h") }); + } + } + // Colours are re-serialized by the XML writer (escaped); only `a:`-prefixed ones, + // since they are written back inside our own `a:sp3d`. + s.colors = sp.elements().filter(|c| (c.is("extrusionClr") || c.is("contourClr")) && c.prefix() == "a").map(|c| c.to_xml()).collect(); + } s } @@ -302,3 +384,49 @@ pub fn store(e: &mut deckcraft_model::Effects, s: &Shape3d) { e.raw3d = s.to_xml(); e.bevel = if e.raw3d.is_some() { s.top.as_ref().map(|b| b.preset.clone()) } else { None }; } + +#[cfg(test)] +mod tests { + use super::*; + + const NS: &str = r#"xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main""#; + + /// Preset names from a file are written back into XML; anything outside the format's lists + /// (here, values that would close the tag and inject markup once unescaped) falls back to the + /// default instead. + #[test] + fn unknown_presets_from_a_file_are_not_written_back() { + let evil = "x"/><a:evil/>"; + let raw = format!( + r#""# + ); + let s = Shape3d::parse(&raw); + assert_eq!( + (s.camera.as_str(), s.light.as_str(), s.light_dir.as_str(), s.material.as_str()), + ("orthographicFront", "threePt", "t", "warmMatte") + ); + assert_eq!(s.top.as_ref().map(|b| b.preset.as_str()), Some("circle")); + assert_eq!(s.depth, 1.0); + let xml = s.to_xml().expect("not flat"); + assert!(!xml.contains("{xml}").as_bytes()).expect("well-formed"); + assert_eq!(doc.root.path(&["sp3d", "extrusionClr", "srgbClr"]).and_then(|c| c.attr("val")), Some(r#"x"/>"#)); + } + + #[test] + fn known_presets_round_trip() { + let raw = format!( + r#""# + ); + let s = Shape3d::parse(&raw); + assert_eq!(s.camera, "legacyObliqueTopLeft"); + assert_eq!(s.rot, Some((10.0, 20.0, 0.0))); + assert_eq!((s.light.as_str(), s.light_dir.as_str(), s.material.as_str()), ("legacyHarsh2", "tl", "legacyMetal")); + assert_eq!((s.z, s.contour), (-1.0, 1.0)); + assert_eq!(s.bottom, Some(Bevel { preset: "artDeco".into(), w: 3.0, h: 1.0 })); + assert_eq!(Shape3d::parse(&s.to_xml().expect("3-D")), s); + // Not XML at all: defaults. + assert_eq!(Shape3d::parse("