Merge #948: Keep a solid paint's own opacity when binding a colour variable (@Silvertree2010)

crates/color/src/lib.rs

@@ -549,7 +549,7 @@ pub enum PaintKind {
 }
 
 /// One fill or stroke paint.
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct Paint {
     #[serde(flatten)]
     pub kind: PaintKind,
@@ -559,6 +559,17 @@ pub struct Paint {
     pub visible: bool,
     #[serde(default)]
     pub blend: BlendMode,
+    /// The opacity holds the alpha of a typed solid colour (see [`Paint::solid`]) rather than an
+    /// opacity set on its own: a colour variable bound to the paint replaces it.
+    #[serde(default, rename = "colorAlpha", skip_serializing_if = "std::ops::Not::not")]
+    pub color_alpha: bool,
+}
+
+/// Paints compare by what they paint; where the opacity came from doesn't matter.
+impl PartialEq for Paint {
+    fn eq(&self, other: &Self) -> bool {
+        self.kind == other.kind && self.opacity == other.opacity && self.visible == other.visible && self.blend == other.blend
+    }
 }
 
 fn yes() -> bool {
@@ -567,11 +578,17 @@ fn yes() -> bool {
 
 impl Paint {
     pub fn solid(color: Color) -> Self {
-        Self { kind: PaintKind::Solid { color: color.with_alpha(1.0) }, opacity: color.a, visible: true, blend: BlendMode::Normal }
+        Self {
+            kind: PaintKind::Solid { color: color.with_alpha(1.0) },
+            opacity: color.a,
+            visible: true,
+            blend: BlendMode::Normal,
+            color_alpha: color.a < 1.0,
+        }
     }
 
     pub fn gradient(g: Gradient) -> Self {
-        Self { kind: PaintKind::Gradient(g), opacity: 1.0, visible: true, blend: BlendMode::Normal }
+        Self { kind: PaintKind::Gradient(g), opacity: 1.0, visible: true, blend: BlendMode::Normal, color_alpha: false }
     }
 
     pub fn image(hash: impl Into<String>, mode: ImageScaleMode) -> Self {
@@ -587,12 +604,13 @@ impl Paint {
             opacity: 1.0,
             visible: true,
             blend: BlendMode::Normal,
+            color_alpha: false,
         }
     }
 
     /// Layer `source` (a node id) tiled at its own size.
     pub fn pattern(source: u64) -> Self {
-        Self { kind: PaintKind::Pattern(PatternPaint::new(source)), opacity: 1.0, visible: true, blend: BlendMode::Normal }
+        Self { kind: PaintKind::Pattern(PatternPaint::new(source)), opacity: 1.0, visible: true, blend: BlendMode::Normal, color_alpha: false }
     }
 
     /// The solid colour (with the paint opacity folded into alpha), if solid.

crates/engine/src/binding_lists_tests.rs

@@ -46,7 +46,7 @@ fn a_bound_fill_colour_keeps_the_other_fills() {
     let fills = s.doc().unwrap().node(r).unwrap().fills.clone();
     assert_eq!(fills.len(), 2, "the second fill stays");
     assert_eq!(fills[0].kind, PaintKind::Solid { color: Color::from_rgb8_const(0xFF, 0, 0) });
-    assert_eq!(fills[0].opacity, 1.0, "the variable brings the whole colour, alpha included");
+    assert_eq!(fills[0].opacity, 0.5, "the bound paint keeps its opacity");
     assert_eq!(fills[1].kind, PaintKind::Solid { color: Color::from_rgb8_const(0, 0xFF, 0) });
     // Removing the bound (first) fill takes the binding with it.
     ex(&mut s, "fill.remove", json!({"ids": [r], "index": 0}));

crates/engine/src/bound_alpha_tests.rs

@@ -129,3 +129,25 @@ fn code_gives_a_translucent_stroke_its_alpha_once() {
     let code = s.execute("inspect.code", &json!({"id": r.0})).unwrap()["code"].as_str().unwrap_or_default().to_string();
     assert!(code.contains("solid rgba(255, 0, 0, 0.50)"), "{code}");
 }
+
+#[test]
+fn binding_keeps_an_opacity_set_on_the_paint() {
+    // A 15% tint whose colour comes from a variable (#923): the bind replaces the colour only.
+    let mut s = Session::new();
+    ex(&mut s, "variable.createCollection", json!({"name": "Tokens", "modes": ["Light"]}));
+    ex(&mut s, "variable.create", json!({"collection": "Tokens", "name": "red", "type": "color", "value": "#FF0000"}));
+    let props = json!({"fills": [{"type": "solid", "color": "#000000", "opacity": 0.15}]});
+    let r = s.execute("node.create", &json!({"type": "rectangle", "x": 0, "y": 0, "width": 10, "height": 10, "props": props})).unwrap();
+    let r = NodeId(r["id"].as_u64().unwrap());
+    ex(&mut s, "variable.bind", json!({"ids": [r.0], "field": "fill", "variable": "red"}));
+    let p = s.doc().unwrap().node(r).unwrap().fills[0].clone();
+    assert_eq!(p.kind, figcraft_color::PaintKind::Solid { color: figcraft_color::Color::from_rgb8_const(0xFF, 0, 0) });
+    assert!(near(p.opacity, 0.15), "{}", p.opacity);
+    ex(&mut s, "variable.bind", json!({"ids": [r.0], "field": "fill", "variable": null}));
+    assert!(near(s.doc().unwrap().node(r).unwrap().fills[0].opacity, 0.15));
+    // An opacity set on a `#RRGGBBAA` fill is the user's too, and stays.
+    let (mut s, r) = setup("#FF0000", "#FF0000");
+    ex(&mut s, "fill.set", json!({"ids": [r.0], "index": 0, "opacity": 0.25}));
+    ex(&mut s, "variable.bind", json!({"ids": [r.0], "field": "fill", "variable": "tint"}));
+    assert!(near(alpha(&s, r), 0.25), "{}", alpha(&s, r));
+}

crates/engine/src/cmd/edit.rs

@@ -1375,10 +1375,12 @@ fn paint_set(s: &mut Session, p: &Value, fills: bool) -> Result<Value> {
             slot.kind = figcraft_color::PaintKind::Solid { color: c.with_alpha(1.0) };
             if c.a < 1.0 {
                 slot.opacity = c.a;
+                slot.color_alpha = true;
             }
         }
         if let Some(o) = num(p, "opacity") {
             slot.opacity = (if o > 1.0 { o / 100.0 } else { o }).clamp(0.0, 1.0) as f32;
+            slot.color_alpha = false;
         }
         if let Some(v) = boolean(p, "visible") {
             slot.visible = v;

crates/engine/src/cmd/styles.rs

@@ -805,9 +805,9 @@ pub fn bind(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 /// A colour variable bound to a solid paint owns its whole colour, alpha included. Binding restarts
-/// the opacity (which holds the alpha a typed colour had, see `Paint::solid`) at 1, so the
+/// an opacity that holds the alpha a typed colour had (see `Paint::solid`) at 1, so the variable's
-/// variable's alpha applies once; unbinding moves the colour's alpha back into the opacity, where a
+/// alpha applies once; an opacity set on its own, before or while bound, multiplies on top and
-/// typed colour keeps it. An opacity set while bound multiplies on top and stays.
+/// stays. Unbinding moves the colour's alpha back into the opacity, where a typed colour keeps it.
 pub(crate) fn rebind_paint_alpha(fills: &mut [Paint], strokes: &mut [Paint], field: &str, bind: bool) {
     use super::lists::{BoundList, list_binding};
     let Some((list, i, "color")) = list_binding(field) else { return };
@@ -819,7 +819,15 @@ pub(crate) fn rebind_paint_alpha(fills: &mut [Paint], strokes: &mut [Paint], fie
     if let Some(p) = paints.get_mut(i)
         && let PaintKind::Solid { color } = &mut p.kind
     {
-        p.opacity = if bind { 1.0 } else { p.opacity * color.a };
+        if bind {
+            if p.color_alpha {
+                p.opacity = 1.0;
+            }
+            p.color_alpha = false;
+        } else if color.a < 1.0 {
+            p.opacity *= color.a;
+            p.color_alpha = true;
+        }
         color.a = 1.0;
     }
 }

crates/format/src/fig/assets_tests.rs

@@ -104,7 +104,10 @@ fn themed() -> Document {
     add(&mut doc, frame, NodeKind::Rectangle, "Swatch", (10.0, 10.0), (80.0, 60.0), &|n| {
         let mut g = Gradient::default_for(GradientKind::Linear, Color::BLACK);
         g.stops = vec![ColorStop { position: 0.0, color: Color::BLACK }, ColorStop { position: 1.0, color: Color::WHITE }];
-        n.fills = vec![solid(0.5, 0.5, 0.5), Paint { kind: PaintKind::Gradient(g), opacity: 1.0, visible: true, blend: BlendMode::Normal }];
+        n.fills = vec![
+            solid(0.5, 0.5, 0.5),
+            Paint { kind: PaintKind::Gradient(g), opacity: 1.0, visible: true, blend: BlendMode::Normal, color_alpha: false },
+        ];
         n.strokes = vec![solid(0.0, 0.0, 0.0), solid(1.0, 0.0, 0.0)];
         n.effects = vec![Effect {
             kind: EffectKind::DropShadow(Shadow {

crates/format/src/fig/export_tests.rs

@@ -74,7 +74,7 @@ fn paint(r: &mut Lcg, image: &str) -> Paint {
             p.kind
         }
     };
-    Paint { kind, opacity: r.pick(&[1.0, 0.5, 0.25]), visible: r.chance(85), blend: r.pick(&BLENDS) }
+    Paint { kind, opacity: r.pick(&[1.0, 0.5, 0.25]), visible: r.chance(85), blend: r.pick(&BLENDS), color_alpha: false }
 }
 
 fn effect(r: &mut Lcg) -> Effect {

crates/format/src/rest/props.rs

@@ -139,7 +139,7 @@ fn paint(cx: &mut Ctx, p: &Value) -> Option<Paint> {
             return None;
         }
     };
-    Some(Paint { kind, opacity, visible: flag(p, "visible").unwrap_or(true), blend: paint_blend(p) })
+    Some(Paint { kind, opacity, visible: flag(p, "visible").unwrap_or(true), blend: paint_blend(p), color_alpha: false })
 }
 
 /// Gradient handles are already in the node's unit box (start, end, width), like ours.