craft-drawingml: a gradient keeps what the file says, and no more (#1063)

- shape is None when the file has neither a:lin nor a:path, and
  rotate_with_shape is None when it has no rotWithShape: each app applies its
  own default (PowerPoint draws such a gradient top to bottom and rotated with
  the shape; Word left to right and not rotated).
- The stops are kept in file order, so a write gives them back as they were;
  Gradient::sorted_stops gives them by position for drawing.

The writer writes no a:lin, a:path or rotWithShape where there is none. A
model saved before reads as it did (rotateWithShape missing is Some(true)).

crates/drawingml/src/chart/write.rs

@@ -45,7 +45,7 @@ fn simple_fill(w: &mut Writer, f: &Fill) {
         }
         Fill::Gradient(_) | Fill::Picture(_) | Fill::Group | Fill::Background => {
             if let Fill::Gradient(g) = f
-                && let Some(s) = g.stops.first()
+                && let Some(s) = g.sorted_stops().first()
             {
                 dml::solid(w, &s.color);
             }

crates/drawingml/src/lib.rs

@@ -56,6 +56,10 @@ pub(crate) fn yes() -> bool {
     true
 }
 
+pub(crate) fn some_yes() -> Option<bool> {
+    Some(true)
+}
+
 macro_rules! id_type {
     ($(#[$doc:meta])* $name:ident) => {
         $(#[$doc])*

crates/drawingml/src/read.rs

@@ -121,7 +121,7 @@ pub fn pattern(f: &El) -> PatternFill {
 }
 
 pub fn gradient(f: &El) -> Gradient {
-    let mut stops: Vec<GradientStop> = f
+    let stops: Vec<GradientStop> = f
         .child(Local("gsLst"))
         .map(|l| {
             l.children(Local("gs"))
@@ -130,15 +130,14 @@ pub fn gradient(f: &El) -> Gradient {
                 .collect()
         })
         .unwrap_or_default();
-    stops.sort_by(|a, b| a.pos.total_cmp(&b.pos));
     let shape = if let Some(l) = f.child(Local("lin")) {
-        GradientShape::Linear { angle: ang(l, "ang").unwrap_or(0.0), scaled: l.attr_local("scaled").and_then(value::to_bool).unwrap_or(false) }
+        Some(GradientShape::Linear { angle: ang(l, "ang").unwrap_or(0.0), scaled: l.attr_local("scaled").and_then(value::to_bool).unwrap_or(false) })
     } else if let Some(p) = f.child(Local("path")) {
         let r = p.child(Local("fillToRect"));
         let g = |n: &str| r.and_then(|r| pct(r, n)).unwrap_or(0.0);
-        GradientShape::Path { path: p.attr_local("path").unwrap_or("circle").to_string(), focus: [g("l"), g("t"), g("r"), g("b")] }
+        Some(GradientShape::Path { path: p.attr_local("path").unwrap_or("circle").to_string(), focus: [g("l"), g("t"), g("r"), g("b")] })
     } else {
-        GradientShape::Linear { angle: 90.0, scaled: false }
+        None
     };
     let tile = f.child(Local("tileRect")).map(|r| [pct(r, "l"), pct(r, "t"), pct(r, "r"), pct(r, "b")].map(|v| v.unwrap_or(0.0).clamp(-10.0, 10.0)));
     Gradient {
@@ -146,7 +145,7 @@ pub fn gradient(f: &El) -> Gradient {
         stops,
         shape,
         flip: f.attr_local("flip").and_then(GradientFlip::from_xml),
-        rotate_with_shape: f.attr_local("rotWithShape").and_then(value::to_bool).unwrap_or(true),
+        rotate_with_shape: f.attr_local("rotWithShape").and_then(value::to_bool),
     }
 }
 

crates/drawingml/src/style.rs

@@ -120,12 +120,19 @@ pub enum GradientShape {
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Gradient {
+    /// The stops in the order the file gives them, so a write gives them back in that order.
+    /// [`Gradient::sorted_stops`] is them by position.
     pub stops: Vec<GradientStop>,
-    pub shape: GradientShape,
+    /// `a:lin` or `a:path`. `None` when the file has neither: each reader of the model applies
+    /// its own default (PowerPoint draws a top-to-bottom linear gradient, Word a left-to-right one).
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub shape: Option<GradientShape>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub flip: Option<GradientFlip>,
-    #[serde(default = "crate::yes")]
-    pub rotate_with_shape: bool,
+    /// `rotWithShape`. `None` when the file does not say; each reader of the model applies its own
+    /// default. A model saved before this was optional reads as `Some(true)`.
+    #[serde(default = "crate::some_yes")]
+    pub rotate_with_shape: Option<bool>,
     /// `a:tileRect`: the box the gradient is laid out in, as insets from the shape's box
     /// (fractions; negative reaches outside it). PowerPoint's "from corner" radial presets use it
     /// to centre the gradient on the corner.
@@ -133,6 +140,16 @@ pub struct Gradient {
     pub tile: [f64; 4],
 }
 
+impl Gradient {
+    /// The stops by position (a stable sort: stops at one position keep their order), for
+    /// drawing.
+    pub fn sorted_stops(&self) -> Vec<GradientStop> {
+        let mut stops = self.stops.clone();
+        stops.sort_by(|a, b| a.pos.total_cmp(&b.pos));
+        stops
+    }
+}
+
 fn is_zero4(v: &[f64; 4]) -> bool {
     v.iter().all(|x| *x == 0.0)
 }

crates/drawingml/src/theme.rs

@@ -109,8 +109,8 @@ pub fn default_format() -> FormatScheme {
                 GradientStop { pos: 0.0, color: ph().with(ColorTransform::LumMod(a)).with(ColorTransform::SatMod(103000)) },
                 GradientStop { pos: 1.0, color: ph().with(ColorTransform::LumMod(b)) },
             ],
-            shape: GradientShape::Linear { angle: 90.0, scaled: false },
-            rotate_with_shape: true,
+            shape: Some(GradientShape::Linear { angle: 90.0, scaled: false }),
+            rotate_with_shape: Some(true),
             flip: None,
         })
     };

crates/drawingml/src/write.rs

@@ -294,7 +294,10 @@ fn shadow(w: &mut Writer, tag: &str, s: &crate::style::Shadow) {
 }
 
 fn gradient(w: &mut Writer, g: &crate::style::Gradient) {
-    w.tag("a:gradFill").attr_opt("flip", g.flip.map(GradientFlip::xml)).attr("rotWithShape", if g.rotate_with_shape { "1" } else { "0" }).open();
+    w.tag("a:gradFill")
+        .attr_opt("flip", g.flip.map(GradientFlip::xml))
+        .attr_opt("rotWithShape", g.rotate_with_shape.map(|r| if r { "1" } else { "0" }))
+        .open();
     // No stops: no `a:gsLst`, so the gradient keeps taking them from its style's fill.
     let mut stops = g.stops.clone();
     if stops.len() == 1 {
@@ -312,8 +315,11 @@ fn gradient(w: &mut Writer, g: &crate::style::Gradient) {
         w.close();
     }
     match &g.shape {
-        GradientShape::Linear { angle, scaled } => w.tag("a:lin").attr("ang", ang(*angle)).attr("scaled", if *scaled { "1" } else { "0" }).empty(),
-        GradientShape::Path { path, focus } => {
+        None => {}
+        Some(GradientShape::Linear { angle, scaled }) => {
+            w.tag("a:lin").attr("ang", ang(*angle)).attr("scaled", if *scaled { "1" } else { "0" }).empty()
+        }
+        Some(GradientShape::Path { path, focus }) => {
             let p = match path.as_str() {
                 "rect" | "shape" | "circle" => path.as_str(),
                 _ => "circle",
@@ -875,4 +881,38 @@ mod tests {
         assert_eq!(write(ColorBase::System { name: "notAColor".into(), last }), r#"<a:sysClr val="windowText" lastClr="123456"/>"#);
         assert_eq!(write(ColorBase::System { name: "3dLight".into(), last }), r#"<a:sysClr val="3dLight" lastClr="123456"/>"#);
     }
+
+    /// A gradient is written as it was read: its stops in file order, and no `a:lin`, `a:path` or
+    /// `rotWithShape` where the file had none (each app has its own default for them).
+    #[test]
+    fn a_gradient_writes_back_what_it_read() {
+        let src = br#"<a:gradFill xmlns:a="x"><a:gsLst><a:gs pos="100000"><a:srgbClr val="0000FF"/></a:gs><a:gs pos="0"><a:srgbClr val="FF0000"/></a:gs></a:gsLst></a:gradFill>"#;
+        let g = crate::read::gradient(&crate::xmlx::parse(src).unwrap());
+        assert_eq!((g.shape.clone(), g.rotate_with_shape), (None, None));
+        assert_eq!(g.stops.iter().map(|s| s.pos).collect::<Vec<_>>(), [1.0, 0.0], "file order");
+        assert_eq!(g.sorted_stops().iter().map(|s| s.pos).collect::<Vec<_>>(), [0.0, 1.0]);
+        let mut w = Writer::new();
+        gradient(&mut w, &g);
+        let out = w.finish();
+        assert!(out.starts_with("<a:gradFill>"), "{out}");
+        assert!(!out.contains("a:lin") && !out.contains("a:path"), "{out}");
+        assert!(out.find("0000FF") < out.find("FF0000"), "{out}");
+        let back = crate::read::gradient(&crate::xmlx::parse(out.replace("<a:gradFill>", r#"<a:gradFill xmlns:a="x">"#).as_bytes()).unwrap());
+        assert_eq!(back, g);
+        // Stated ones are kept.
+        let src = br#"<a:gradFill xmlns:a="x" rotWithShape="0"><a:lin ang="0" scaled="1"/></a:gradFill>"#;
+        let g = crate::read::gradient(&crate::xmlx::parse(src).unwrap());
+        assert_eq!((g.shape, g.rotate_with_shape), (Some(GradientShape::Linear { angle: 0.0, scaled: true }), Some(false)));
+    }
+
+    /// A model saved before `shape` and `rotateWithShape` were optional reads as it did.
+    #[test]
+    fn a_gradient_saved_before_reads_the_same() {
+        let old = r#"{"stops":[],"shape":{"kind":"linear","angle":90.0}}"#;
+        let g: crate::style::Gradient = serde_json::from_str(old).unwrap();
+        assert_eq!((g.shape, g.rotate_with_shape), (Some(GradientShape::Linear { angle: 90.0, scaled: false }), Some(true)));
+        let none = crate::style::Gradient { stops: vec![], shape: None, flip: None, rotate_with_shape: None, tile: [0.0; 4] };
+        let back: crate::style::Gradient = serde_json::from_str(&serde_json::to_string(&none).unwrap()).unwrap();
+        assert_eq!(back, none);
+    }
 }