From bd80829a2786b8ee6907061e254515d0a23fc1c1 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 24 Oct 2026 10:30:49 +0700 Subject: [PATCH] Batch r2-1 review: a paint keeps its own opacity through unbind and rebind color_alpha is the part of a solid paint's opacity that is its typed colour's alpha, not a flag: binding a colour variable takes only that part out, unbinding puts the variable's alpha back in, and REST import records it. A 50% paint bound to a 50% variable stays at 25% bound or not. --- crates/color/src/lib.rs | 21 +++++++++++++-------- crates/engine/src/bound_alpha_tests.rs | 11 +++++++++++ crates/engine/src/cmd/edit.rs | 4 ++-- crates/engine/src/cmd/styles.rs | 19 +++++++++---------- crates/format/src/fig/assets_tests.rs | 2 +- crates/format/src/fig/export_tests.rs | 2 +- crates/format/src/rest/props.rs | 5 ++++- crates/format/src/rest/tests.rs | 1 + 8 files changed, 42 insertions(+), 23 deletions(-) diff --git a/crates/color/src/lib.rs b/crates/color/src/lib.rs index 84149dfb..1fff7667 100644 --- a/crates/color/src/lib.rs +++ b/crates/color/src/lib.rs @@ -559,10 +559,11 @@ 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, + /// The part of the opacity that is the alpha of a typed solid colour (see [`Paint::solid`]), + /// the rest being an opacity set on its own: a colour variable bound to the paint replaces + /// this part only. 1: all of it is the paint's own. + #[serde(default = "one", rename = "colorAlpha", skip_serializing_if = "is_one")] + pub color_alpha: f32, } /// Paints compare by what they paint; where the opacity came from doesn't matter. @@ -576,6 +577,10 @@ fn yes() -> bool { true } +fn is_one(x: &f32) -> bool { + *x == 1.0 +} + impl Paint { pub fn solid(color: Color) -> Self { Self { @@ -583,12 +588,12 @@ impl Paint { opacity: color.a, visible: true, blend: BlendMode::Normal, - color_alpha: color.a < 1.0, + color_alpha: color.a, } } pub fn gradient(g: Gradient) -> Self { - Self { kind: PaintKind::Gradient(g), opacity: 1.0, visible: true, blend: BlendMode::Normal, color_alpha: false } + Self { kind: PaintKind::Gradient(g), opacity: 1.0, visible: true, blend: BlendMode::Normal, color_alpha: 1.0 } } pub fn image(hash: impl Into, mode: ImageScaleMode) -> Self { @@ -604,13 +609,13 @@ impl Paint { opacity: 1.0, visible: true, blend: BlendMode::Normal, - color_alpha: false, + color_alpha: 1.0, } } /// 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, color_alpha: false } + Self { kind: PaintKind::Pattern(PatternPaint::new(source)), opacity: 1.0, visible: true, blend: BlendMode::Normal, color_alpha: 1.0 } } /// The solid colour (with the paint opacity folded into alpha), if solid. diff --git a/crates/engine/src/bound_alpha_tests.rs b/crates/engine/src/bound_alpha_tests.rs index c54f0fb7..fb15425d 100644 --- a/crates/engine/src/bound_alpha_tests.rs +++ b/crates/engine/src/bound_alpha_tests.rs @@ -151,3 +151,14 @@ fn binding_keeps_an_opacity_set_on_the_paint() { ex(&mut s, "variable.bind", json!({"ids": [r.0], "field": "fill", "variable": "tint"})); assert!(near(alpha(&s, r), 0.25), "{}", alpha(&s, r)); } + +#[test] +fn an_unbind_and_rebind_keeps_the_opacity_set_on_the_paint() { + // A paint at 50% bound to a half-transparent colour shows a quarter, unbound or bound again. + let (mut s, r) = setup("#FF000080", "#FF000080"); + ex(&mut s, "fill.set", json!({"ids": [r.0], "index": 0, "opacity": 0.5})); + for variable in [json!("tint"), json!(null), json!("tint"), json!(null)] { + ex(&mut s, "variable.bind", json!({"ids": [r.0], "field": "fill", "variable": variable})); + assert!(near(alpha(&s, r), HALF / 2.0), "{variable}: {}", alpha(&s, r)); + } +} diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 60a96b31..5ed92aec 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1375,12 +1375,12 @@ fn paint_set(s: &mut Session, p: &Value, fills: bool) -> Result { 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; + slot.color_alpha = c.a; } } 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; + slot.color_alpha = 1.0; } if let Some(v) = boolean(p, "visible") { slot.visible = v; diff --git a/crates/engine/src/cmd/styles.rs b/crates/engine/src/cmd/styles.rs index dd93bac3..e5ac145e 100644 --- a/crates/engine/src/cmd/styles.rs +++ b/crates/engine/src/cmd/styles.rs @@ -804,10 +804,10 @@ pub fn bind(s: &mut Session, p: &Value) -> Result { ok() } -/// A colour variable bound to a solid paint owns its whole colour, alpha included. Binding restarts -/// an opacity that holds the alpha a typed colour had (see `Paint::solid`) at 1, so the variable's -/// alpha applies once; an opacity set on its own, before or while bound, multiplies on top and -/// stays. Unbinding moves the colour's alpha back into the opacity, where a typed colour keeps it. +/// A colour variable bound to a solid paint owns its whole colour, alpha included. Binding takes the +/// alpha a typed colour had (see `Paint::solid`) out of the opacity, so the variable's alpha applies +/// once; an opacity set on its own, before or while bound, multiplies on top and 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 }; @@ -820,13 +820,12 @@ pub(crate) fn rebind_paint_alpha(fills: &mut [Paint], strokes: &mut [Paint], fie && let PaintKind::Solid { color } = &mut p.kind { if bind { - if p.color_alpha { - p.opacity = 1.0; - } - p.color_alpha = false; - } else if color.a < 1.0 { + // What's left is the opacity set on its own (unknown under a colour alpha of 0: 1). + p.opacity = if p.color_alpha > 0.0 { (p.opacity / p.color_alpha).clamp(0.0, 1.0) } else { 1.0 }; + p.color_alpha = 1.0; + } else { p.opacity *= color.a; - p.color_alpha = true; + p.color_alpha *= color.a; } color.a = 1.0; } diff --git a/crates/format/src/fig/assets_tests.rs b/crates/format/src/fig/assets_tests.rs index b48dd9d3..a271a908 100644 --- a/crates/format/src/fig/assets_tests.rs +++ b/crates/format/src/fig/assets_tests.rs @@ -106,7 +106,7 @@ fn themed() -> Document { 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, color_alpha: false }, + Paint { kind: PaintKind::Gradient(g), opacity: 1.0, visible: true, blend: BlendMode::Normal, color_alpha: 1.0 }, ]; n.strokes = vec![solid(0.0, 0.0, 0.0), solid(1.0, 0.0, 0.0)]; n.effects = vec![Effect { diff --git a/crates/format/src/fig/export_tests.rs b/crates/format/src/fig/export_tests.rs index ae7da3ed..2edefcaa 100644 --- a/crates/format/src/fig/export_tests.rs +++ b/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), color_alpha: false } + Paint { kind, opacity: r.pick(&[1.0, 0.5, 0.25]), visible: r.chance(85), blend: r.pick(&BLENDS), color_alpha: 1.0 } } fn effect(r: &mut Lcg) -> Effect { diff --git a/crates/format/src/rest/props.rs b/crates/format/src/rest/props.rs index ff06c737..f5be3d27 100644 --- a/crates/format/src/rest/props.rs +++ b/crates/format/src/rest/props.rs @@ -123,10 +123,13 @@ pub(super) fn paints(cx: &mut Ctx, v: &Value, k: &str) -> Option> { fn paint(cx: &mut Ctx, p: &Value) -> Option { let ty = text(p, "type")?; let mut opacity = num(p, "opacity").unwrap_or(1.0).clamp(0.0, 1.0) as f32; + // The colour's alpha goes into the opacity as for a typed colour (`Paint::solid`). + let mut color_alpha = 1.0; let kind = match ty { "SOLID" => { let c = p.get("color").and_then(color).unwrap_or(Color::BLACK); opacity *= c.a; + color_alpha = c.a; PaintKind::Solid { color: c.with_alpha(1.0) } } "GRADIENT_LINEAR" => PaintKind::Gradient(gradient(GradientKind::Linear, p)), @@ -139,7 +142,7 @@ fn paint(cx: &mut Ctx, p: &Value) -> Option { return None; } }; - Some(Paint { kind, opacity, visible: flag(p, "visible").unwrap_or(true), blend: paint_blend(p), color_alpha: false }) + Some(Paint { kind, opacity, visible: flag(p, "visible").unwrap_or(true), blend: paint_blend(p), color_alpha }) } /// Gradient handles are already in the node's unit box (start, end, width), like ours. diff --git a/crates/format/src/rest/tests.rs b/crates/format/src/rest/tests.rs index 1b3f85b6..f9acf2ea 100644 --- a/crates/format/src/rest/tests.rs +++ b/crates/format/src/rest/tests.rs @@ -162,6 +162,7 @@ fn paints_gradients_and_image_placeholders() { let s = &named(&doc, "Solid").fills[0]; assert!(matches!(s.kind, PaintKind::Solid { color } if color.r == 1.0 && color.a == 1.0)); assert!(close(f64::from(s.opacity), 0.25), "paint opacity × colour alpha"); + assert!(close(f64::from(s.color_alpha), 0.5), "the colour's part, which a bound variable replaces"); assert!(!s.visible && s.blend == BlendMode::Screen); let PaintKind::Gradient(g) = &named(&doc, "Linear").fills[0].kind else { panic!() }; assert_eq!((g.kind, g.from, g.to, g.width), (GradientKind::Linear, (0.0, 0.5), (1.0, 0.5), (0.0, 1.0)));