From 5a1e9de92ed789abc176ab668fee68378a224ee3 Mon Sep 17 00:00:00 2001 From: japbcoelho Date: Wed, 7 Oct 2026 13:21:35 -0300 Subject: [PATCH] Color parsing: one scale per colour, not per component A CMYK component of 1 or less was read as a fraction even when the other components were percentages, so {"c": 0.7, "m": 0, "y": 12.2, "k": 0} (a pale cream) became C 70 % Y 12.2 % and exported as `0.7019608 0 0.12156863 0 k`. The same applied to RGB arrays. Now a colour is read as percentages (0..100, or 0..255 for RGB) when any component is above 1, and as fractions only when every component is within 0..1. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/style.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index 94bd92c6d..296a8d07b 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -716,13 +716,18 @@ pub fn parse_color(v: &Value) -> Option { use designcraft_color::Color; match v { Value::String(s) => Color::from_hex(s), + // One scale per colour, not per component: {c: 0.7, y: 12.2} is C 0.7 %, Y 12.2 %. Only a colour whose + // components are all within 0..1 is read as fractions (the older form). Value::Array(a) if a.len() >= 3 => { - let f = |i: usize| a[i].as_f64().map(|x| if x > 1.0 { x / 255.0 } else { x } as f32); - Some(Color::rgb(f(0)?, f(1)?, f(2)?)) + let v = [a[0].as_f64()?, a[1].as_f64()?, a[2].as_f64()?]; + let scale = if v.iter().any(|&x| x > 1.0) { 255.0 } else { 1.0 }; + Some(Color::rgb((v[0] / scale) as f32, (v[1] / scale) as f32, (v[2] / scale) as f32)) } Value::Object(o) => { - let g = |k: &str| o.get(k).and_then(Value::as_f64).map(|x| if x > 1.0 { x / 100.0 } else { x } as f32); - Some(Color::cmyk(g("c")?, g("m")?, g("y")?, g("k")?)) + let g = |k: &str| o.get(k).and_then(Value::as_f64); + let v = [g("c")?, g("m")?, g("y")?, g("k")?]; + let scale = if v.iter().any(|&x| x > 1.0) { 100.0 } else { 1.0 }; + Some(Color::cmyk((v[0] / scale) as f32, (v[1] / scale) as f32, (v[2] / scale) as f32, (v[3] / scale) as f32)) } _ => None, } @@ -1040,6 +1045,20 @@ fn rename_in(it: &mut designcraft_doc::Item, from: &str, to: &str) { mod color_tests { use super::*; + #[test] + fn percent_components_at_most_one_stay_percent() { + use designcraft_color::Color; + // {"c": 0.7, ..., "y": 12.2} is a pale cream (C 0.7 %, Y 12.2 %), not C 70 %. + let Some(Color::Cmyk { c, m, y, k }) = parse_color(&json!({"c": 0.7, "m": 0, "y": 12.2, "k": 0})) else { panic!("not CMYK") }; + assert!((c - 0.007).abs() < 1e-6 && m == 0.0 && (y - 0.122).abs() < 1e-6 && k == 0.0, "got {c} {m} {y} {k}"); + // Fractions (every component within 0..1) keep working. + let Some(Color::Cmyk { c, k, .. }) = parse_color(&json!({"c": 0.5, "m": 0, "y": 0, "k": 1})) else { panic!("not CMYK") }; + assert!((c - 0.5).abs() < 1e-6 && (k - 1.0).abs() < 1e-6); + // RGB arrays follow the same rule: [1, 128, 255] is 0..255. + let Some(Color::Rgb { r, g, b }) = parse_color(&json!([1, 128, 255])) else { panic!("not RGB") }; + assert!((r - 1.0 / 255.0).abs() < 1e-6 && (g - 128.0 / 255.0).abs() < 1e-6 && (b - 1.0).abs() < 1e-6); + } + #[test] fn style_groups_rename_every_use() { let mut s = Session::new();