Merge pull request #1588

Preserve bullet sizes authored in points

crates/engine/src/cmd/format/paragraph.rs

@@ -55,6 +55,7 @@ pub(super) fn bullets(s: &mut Session, p: &Value) -> Result<Value> {
             }
             if size.is_some() {
                 para.props.bullet_size = size;
+                para.props.bullet_size_points = None;
                 para.props.bullet_size_follow_text = None;
             }
             if para.props.indent.is_none_or(|i| i >= 0.0) && para.props.margin_left.is_none_or(|m| m == 0.0) {
@@ -85,6 +86,7 @@ pub(super) fn numbering(s: &mut Session, p: &Value) -> Result<Value> {
             }
             if size.is_some() {
                 para.props.bullet_size = size;
+                para.props.bullet_size_points = None;
                 para.props.bullet_size_follow_text = None;
             }
             para.props.margin_left = Some(27.0 + 36.0 * para.level as f64);

crates/engine/src/tests.rs

@@ -3223,6 +3223,7 @@ fn numbering_takes_color_and_size() {
         props.bullet_font_follow_text = Some(true);
         props.bullet_color_follow_text = Some(true);
         props.bullet_size_follow_text = Some(true);
+        props.bullet_size_points = Some(12.0);
     }
     s.execute("format.numbering", &json!({"on": true, "scheme": "romanUcPeriod", "start": 3, "color": "accent2", "size": 150})).unwrap();
     let para = |s: &Session| {
@@ -3236,9 +3237,11 @@ fn numbering_takes_color_and_size() {
     assert_eq!(pp.bullet_font_follow_text, Some(true));
     assert_eq!(pp.bullet_color_follow_text, None);
     assert_eq!(pp.bullet_size_follow_text, None);
+    assert_eq!(pp.bullet_size_points, None);
     assert_eq!(pp.bullet_color, Some(deckcraft_model::ColorRef::scheme(deckcraft_color::SchemeSlot::Accent2)));
     s.execute("format.bullets", &json!({"on": true, "char": "★", "size": 9000})).unwrap();
     assert_eq!(para(&s).bullet_size, Some(4.0));
+    assert_eq!(para(&s).bullet_size_points, None);
     assert_eq!(para(&s).bullet_font_follow_text, None);
 }
 

crates/model/src/text.rs

@@ -242,6 +242,8 @@ pub struct ParaProps {
     pub bullet_color_follow_text: Option<bool>,
     /// Bullet size as a fraction of the text size.
     pub bullet_size: Option<f64>,
+    /// Absolute bullet font size in points, independent of the paragraph's run size.
+    pub bullet_size_points: Option<f64>,
     pub bullet_size_follow_text: Option<bool>,
     pub tabs: Option<Vec<TabStop>>,
     pub default_tab: Option<f64>,
@@ -286,11 +288,13 @@ impl ParaProps {
                 self.bullet_color.clone_from(&p.bullet_color);
             }
         }
-        if self.bullet_size_follow_text == Some(false) || self.bullet_size.is_some() {
+        if self.bullet_size_follow_text == Some(false) || self.bullet_size.is_some() || self.bullet_size_points.is_some() {
             self.bullet_size_follow_text = None;
         }
-        if self.bullet_size.is_none() && self.bullet_size_follow_text.is_none() {
-            if p.bullet_size_follow_text == Some(true) {
+        if self.bullet_size.is_none() && self.bullet_size_points.is_none() && self.bullet_size_follow_text.is_none() {
+            if p.bullet_size_points.is_some() {
+                self.bullet_size_points = p.bullet_size_points;
+            } else if p.bullet_size_follow_text == Some(true) {
                 self.bullet_size_follow_text = Some(true);
             } else {
                 self.bullet_size = p.bullet_size;
@@ -310,6 +314,33 @@ impl ParaProps {
     }
 }
 
+#[cfg(test)]
+mod bullet_size_tests {
+    use super::ParaProps;
+
+    #[test]
+    fn bullet_size_inheritance_keeps_percentage_and_points_as_one_mode() {
+        let parent = ParaProps { bullet_size_points: Some(12.0), ..Default::default() };
+        let mut child = ParaProps { bullet_size: Some(2.0), ..Default::default() };
+        child.inherit(&parent);
+        assert_eq!(child.bullet_size, Some(2.0));
+        assert_eq!(child.bullet_size_points, None);
+
+        let parent = ParaProps { bullet_size: Some(1.5), ..Default::default() };
+        let mut child = ParaProps { bullet_size_points: Some(14.0), ..Default::default() };
+        child.inherit(&parent);
+        assert_eq!(child.bullet_size, None);
+        assert_eq!(child.bullet_size_points, Some(14.0));
+
+        let parent = ParaProps { bullet_size_points: Some(12.0), ..Default::default() };
+        let mut child = ParaProps { bullet_size_follow_text: Some(true), ..Default::default() };
+        child.inherit(&parent);
+        assert_eq!(child.bullet_size, None);
+        assert_eq!(child.bullet_size_points, None);
+        assert_eq!(child.bullet_size_follow_text, Some(true));
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub enum Caps {

crates/pptx/src/read/dml.rs

@@ -23,6 +23,16 @@ pub fn pct(e: &El, name: &str) -> Option<f64> {
     e.f64(name).map(|v| v / 100_000.0)
 }
 
+fn bullet_size_points(value: Option<&str>) -> Option<f64> {
+    value.and_then(|v| v.trim().parse::<u32>().ok()).filter(|v| (100..=400_000).contains(v)).map(|v| v as f64 / 100.0)
+}
+
+fn bullet_size_percent(value: Option<&str>) -> Option<f64> {
+    let value = value?.trim();
+    let (value, scale) = if let Some(value) = value.strip_suffix('%') { (value.trim(), 100.0) } else { (value, 100_000.0) };
+    value.parse::<f64>().ok().filter(|v| v.is_finite()).map(|v| v / scale)
+}
+
 /// 1/60 000 degree → degrees.
 pub fn ang(e: &El, name: &str) -> Option<f64> {
     e.f64(name).map(|v| v / 60_000.0)
@@ -454,9 +464,22 @@ pub fn ppr(imp: &mut Imp, part: &Part, p: &El) -> (Option<u8>, ParaProps, Option
             "buFont" => pp.bullet_font = e.attr("typeface").map(String::from),
             "buFontTx" => pp.bullet_font_follow_text = Some(true),
             "buClr" => pp.bullet_color = color(e),
+            "buSzPct" => {
+                pp.bullet_size_points = None;
+                pp.bullet_size_follow_text = None;
+                pp.bullet_size = bullet_size_percent(e.attr("val")).map(|v| v.clamp(0.25, 4.0));
+            }
+            "buSzPts" => {
+                pp.bullet_size = None;
+                pp.bullet_size_follow_text = None;
+                pp.bullet_size_points = bullet_size_points(e.attr("val"));
+            }
             "buClrTx" => pp.bullet_color_follow_text = Some(true),
-            "buSzPct" => pp.bullet_size = pct(e, "val").map(|v| v.clamp(0.25, 4.0)),
-            "buSzTx" => pp.bullet_size_follow_text = Some(true),
+            "buSzTx" => {
+                pp.bullet_size = None;
+                pp.bullet_size_points = None;
+                pp.bullet_size_follow_text = Some(true);
+            }
             "tabLst" => {
                 pp.tabs = Some(
                     e.children_named("tab")
@@ -692,6 +715,21 @@ mod tests {
         assert_eq!(color(&d.root).unwrap().base, ColorBase::Rgb { rgb: Rgba::rgb(255, 0, 0) });
     }
 
+    #[test]
+    fn bullet_point_size_trims_schema_whitespace() {
+        let d = parse(br#"<a:buSzPts xmlns:a="x" val=" 1400 "/>"#).unwrap();
+        assert_eq!(bullet_size_points(d.root.attr("val")), Some(14.0));
+        assert_eq!(bullet_size_points(Some("1400")), Some(14.0));
+    }
+
+    #[test]
+    fn bullet_percentage_size_accepts_numeric_and_percent_forms() {
+        assert_eq!(bullet_size_percent(Some("100000")), Some(1.0));
+        assert_eq!(bullet_size_percent(Some("100%")), Some(1.0));
+        assert_eq!(bullet_size_percent(Some(" 125% ")), Some(1.25));
+        assert_eq!(bullet_size_percent(Some("invalid")), None);
+    }
+
     #[test]
     fn miter_limit_accepts_both_percentage_forms_with_schema_bounds() {
         assert_eq!(parse_miter_limit("200000"), Some(200_000));

crates/pptx/src/write/dml.rs

@@ -543,6 +543,7 @@ pub fn ppr(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, level: Option<u8>, p:
         || p.bullet_color.is_some()
         || p.bullet_color_follow_text.is_some()
         || p.bullet_size.is_some()
+        || p.bullet_size_points.is_some()
         || p.bullet_size_follow_text.is_some()
         || p.tabs.is_some()
         || def.is_some();
@@ -567,8 +568,10 @@ pub fn ppr(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, level: Option<u8>, p:
         color(w, c);
         w.close("a:buClr");
     }
-    if p.bullet_size.is_none() && p.bullet_size_follow_text == Some(true) {
+    if p.bullet_size.is_none() && p.bullet_size_points.is_none() && p.bullet_size_follow_text == Some(true) {
         w.empty0("a:buSzTx");
+    } else if let Some(s) = p.bullet_size_points.filter(|s| s.is_finite() && (1.0..=4000.0).contains(s)) {
+        w.val("a:buSzPts", ((s * 100.0).round() as u32).to_string());
     } else if let Some(s) = p.bullet_size {
         w.val("a:buSzPct", pct(s.clamp(0.25, 4.0)));
     }

crates/pptx/tests/it/roundtrip.rs

@@ -490,6 +490,33 @@ fn shapes_fills_lines_effects_round_trip() {
     assert_eq!(by_name(&q.slides[1], "Group").id, by_name(&p.slides[1], "Group").id);
 }
 
+#[test]
+fn bullet_size_in_points_survives_two_pptx_round_trips() {
+    let mut p = Presentation::default();
+    let mut textbox = shape(&mut p, "Point-sized bullet", 20.0, 20.0, 400.0, 100.0);
+    textbox.text = Some(TextBody {
+        paragraphs: [12.0, 500.0, 4000.0]
+            .into_iter()
+            .map(|size| Paragraph {
+                props: deckcraft_model::text::ParaProps {
+                    bullet: Some(Bullet::Char { char: "•".into() }),
+                    bullet_size_points: Some(size),
+                    ..Default::default()
+                },
+                runs: vec![Run::with("Bullet text", RunProps::default())],
+                ..Default::default()
+            })
+            .collect(),
+        ..Default::default()
+    });
+    Arc::make_mut(&mut p.slides[0]).shapes.push(textbox);
+
+    let q = round(&round(&p));
+    let props = &by_name(&q.slides[0], "Point-sized bullet").text.as_ref().expect("text").paragraphs;
+    assert_eq!(props.iter().map(|p| p.props.bullet_size_points).collect::<Vec<_>>(), vec![Some(12.0), Some(500.0), Some(4000.0)]);
+    assert!(props.iter().all(|p| p.props.bullet_size.is_none()));
+}
+
 #[test]
 fn line_miter_limit_survives_two_pptx_round_trips() {
     let mut p = Presentation::default();

crates/text/src/shape.rs

@@ -76,7 +76,7 @@ pub(crate) fn shape_para(
             if let Some(c) = &props.bullet_color {
                 rp.fill = Some(Fill::solid(c.clone()));
             }
-            rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0));
+            rp.size = Some(props.bullet_size_points.unwrap_or(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)));
             rp.underline = None;
             rp.strike = None;
             rp.highlight = None;
@@ -105,7 +105,7 @@ pub(crate) fn shape_para(
             if let Some(c) = &props.bullet_color {
                 rp.fill = Some(Fill::solid(c.clone()));
             }
-            rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0));
+            rp.size = Some(props.bullet_size_points.unwrap_or(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)));
             rp.underline = None;
             let mut st = make_style(ctx, &rp, scale, opts.prompt_color);
             let label = autonum(scheme, start_at.saturating_sub(1).saturating_add(number.unwrap_or(1)));