Merge pull request #5665 from d8abyte-dk/plugins-compound-guide-filter-and-color-profile

Plug-ins: compound guides aren't filter inputs; colors follow the working RGB space

crates/plugins/src/manifest.rs

@@ -117,26 +117,31 @@ pub enum ParamSpec {
     /// `plugin.run`, is a color object as paints hold one (`{"model": "rgb", "r", "g", "b"}`, or
     /// `cmyk`, `gray`, `lab`) or a hex string (`"#3a7bd5"`); the plug-in always gets a color object.
     Color {
-        #[serde(default, deserialize_with = "color_default")]
-        default: vectorcraft_color::Color,
+        /// As the manifest gives it (`null`: black), read with [`color_value`] when used, so a hex
+        /// default follows the working RGB space in use then, not the one at install.
+        #[serde(default)]
+        default: Value,
     },
 }
 
-/// A color parameter's value: a color object or a hex string, its components clamped into range.
+/// A color parameter's value: a color object, or a hex string (display sRGB, converted into the
+/// active working RGB space as the color picker's choices are), its components clamped into range.
 pub fn color_value(v: &Value) -> Option<vectorcraft_color::Color> {
+    use vectorcraft_color::{Color, cms};
     let c = match v {
-        Value::String(s) => vectorcraft_color::Color::from_hex(s)?,
+        Value::String(s) => match Color::from_hex(s)? {
+            Color::Rgb { r, g, b } => {
+                let [r, g, b] = cms::active().srgb_to_rgb([r, g, b]);
+                Color::rgb(r, g, b)
+            }
+            other => other,
+        },
         Value::Object(_) => serde_json::from_value(v.clone()).ok()?,
         _ => return None,
     };
     Some(crate::objects::clamp_color(c))
 }
 
-fn color_default<'de, D: serde::Deserializer<'de>>(d: D) -> std::result::Result<vectorcraft_color::Color, D::Error> {
-    let v = Value::deserialize(d)?;
-    color_value(&v).ok_or_else(|| serde::de::Error::custom("a color default must be a color object or a hex string such as \"#3a7bd5\""))
-}
-
 fn int_min() -> i64 {
     -1_000_000
 }
@@ -176,7 +181,7 @@ impl ParamSpec {
             ParamSpec::Choice { options, default } => json!(default.clone().or_else(|| options.first().cloned()).unwrap_or_default()),
             ParamSpec::Text { default, max_length } => json!(default.chars().take(*max_length).collect::<String>()),
             ParamSpec::NumberList { min, max, default, .. } => json!(default.iter().map(|x| x.clamp(*min, *max)).collect::<Vec<f64>>()),
-            ParamSpec::Color { default } => serde_json::to_value(default).unwrap_or_default(),
+            ParamSpec::Color { default } => serde_json::to_value(color_value(default).unwrap_or_default()).unwrap_or_default(),
         }
     }
 
@@ -196,6 +201,9 @@ impl ParamSpec {
                 }
                 Ok(())
             }
+            ParamSpec::Color { default } if !default.is_null() && color_value(default).is_none() => {
+                bad("a color default must be a color object or a hex string such as \"#3a7bd5\"")
+            }
             ParamSpec::Text { default, max_length } if !(1..=MAX_TEXT).contains(max_length) || default.chars().count() > *max_length => {
                 bad(&format!("`maxLength` must be 1-{MAX_TEXT}, and the default no longer"))
             }
@@ -344,7 +352,7 @@ impl Manifest {
                 ParamSpec::NumberList { min, max, min_items, max_items, .. } => {
                     format!("{k}: {min_items} to {max_items} numbers {min}..{max} ({})", p.spec.default_value())
                 }
-                ParamSpec::Color { default } => format!("{k}: color object or hex ({})", default.to_hex()),
+                ParamSpec::Color { default } => format!("{k}: color object or hex ({})", color_value(default).unwrap_or_default().to_hex()),
             })
             .collect();
         format!("{{{}}}", parts.join(", "))
@@ -635,6 +643,40 @@ mod tests {
         assert!(Manifest::parse(br#"{"id":"a","name":"x","kind":"filter","params":{"c":{"type":"color","default":"nope"}}}"#).is_err());
     }
 
+    /// A hex color is display sRGB: in a wide-gamut working RGB space it becomes the working RGB
+    /// color that shows as that hex, read when used, while the manifest keeps the default as written.
+    #[test]
+    fn hex_colors_follow_the_working_rgb_space() {
+        const CHILD: &str = "VECTORCRAFT_TEST_PLUGIN_COLOR_PROFILE";
+        if std::env::var_os(CHILD).is_none() {
+            // Color settings are process-wide: exercise this profile in a separate test process.
+            let output = std::process::Command::new(std::env::current_exe().unwrap())
+                .args(["--exact", "manifest::tests::hex_colors_follow_the_working_rgb_space", "--nocapture"])
+                .env(CHILD, "1")
+                .output()
+                .unwrap();
+            let stdout = String::from_utf8_lossy(&output.stdout);
+            assert!(
+                output.status.success() && stdout.contains("test result: ok. 1 passed; 0 failed;"),
+                "isolated color profile test failed:\n{stdout}\n{}",
+                String::from_utf8_lossy(&output.stderr),
+            );
+            return;
+        }
+        use vectorcraft_color::{Color, cms};
+        let m =
+            Manifest::parse(br##"{"id":"org.example.grid","name":"Grid","kind":"filter","params":{"line":{"type":"color","default":"#3a7bd5"}}}"##)
+                .unwrap();
+        cms::set_active(&cms::ColorSettings { rgb: cms::WIDE_GAMUT_RGB.into(), ..Default::default() }).unwrap();
+        for c in [m.defaults()["line"].clone(), m.resolve_params(&json!({"line": "#3a7bd5"})).unwrap()["line"].clone()] {
+            let c: Color = serde_json::from_value(c).unwrap();
+            assert_ne!(c, Color::rgb8(0x3a, 0x7b, 0xd5), "converted into the working space");
+            assert_eq!(c.to_hex(), "#3a7bd5", "shows as the hex given");
+        }
+        assert_eq!(serde_json::to_value(&m).unwrap()["params"]["line"]["default"], json!("#3a7bd5"), "kept as written");
+        cms::set_active(&cms::ColorSettings::default()).unwrap();
+    }
+
     #[test]
     fn up_to_256_parameters_with_at_most_64_outside_groups() {
         let manifest = |n: usize, grouped: usize| {

crates/plugins/src/objects.rs

@@ -621,8 +621,9 @@ pub fn selectable(n: &Node) -> bool {
 }
 
 /// The paths and compound paths a filter reads from the selection `selected`, in paint order:
-/// selected ones, and those inside selected groups and layers; hidden, locked, guide and clipping
-/// paths are left out (so are paths inside compound paths, blends and other live objects).
+/// selected ones, and those inside selected groups and layers; hidden, locked, guide (compound
+/// guides too) and clipping paths are left out (so are paths inside compound paths, blends and
+/// other live objects).
 pub fn filter_targets(doc: &Document, selected: &[NodeId]) -> Vec<NodeId> {
     fn visit(list: &[Arc<Node>], sel: &HashSet<NodeId>, inside: bool, out: &mut Vec<NodeId>) {
         for n in list {
@@ -631,7 +632,7 @@ pub fn filter_targets(doc: &Document, selected: &[NodeId]) -> Vec<NodeId> {
             }
             let picked = inside || sel.contains(&n.id);
             match &n.kind {
-                NodeKind::Path { guide: false, clipping: false, .. } | NodeKind::Compound { .. } if picked => out.push(n.id),
+                NodeKind::Path { guide: false, clipping: false, .. } | NodeKind::Compound { .. } if picked && !is_guide(n) => out.push(n.id),
                 NodeKind::Group { children, .. } | NodeKind::Layer { children, .. } => visit(children, sel, picked, out),
                 _ => {}
             }

crates/plugins/src/objects/tests.rs

@@ -151,6 +151,19 @@ fn geometry_type_and_strokes_change() {
     assert!(n.appearance.stroke().is_none());
 }
 
+/// A compound path made a guide is left out of a later filter's inputs, as guide paths are; a
+/// plain compound path is still one of them.
+#[test]
+fn compound_guides_are_not_filter_targets() {
+    let (mut d, _, layer) = doc();
+    let two = PathData::new(square(0.0).subpaths.into_iter().chain(square(2.0).subpaths).collect());
+    let out = json!({"objects": [{"type": "compound", "path": two.clone(), "guide": true}, {"type": "compound", "path": two}]});
+    let made = apply_output(&mut d, &[], decode_json(out).unwrap(), Some(layer), &keep).unwrap();
+    let targets = filter_targets(&d, &[layer]);
+    assert!(!targets.contains(&made[0]), "the compound guide");
+    assert!(targets.contains(&made[1]), "the plain compound path");
+}
+
 /// `guide` and `locked` make objects guides (a compound path's members too) and lock them, or undo
 /// both; locked objects and guides are not [`selectable`]. Other values are refused.
 #[test]

crates/ui-egui/src/dialogs/form.rs

@@ -638,12 +638,14 @@ fn value_field(ui: &mut egui::Ui, k: &str, label: &str, spec: &ParamSpec, cur: &
 }
 
 /// A plug-in `color` parameter's button, showing `cur` (a color object or hex) and opening the color
-/// picker → the color picked, as the color object the plug-in gets.
+/// picker → the color picked, as the color object the plug-in gets: the picker's display sRGB in the
+/// working RGB space, so the color shows as it was picked whatever the working profile.
 pub(crate) fn plugin_color_button(ui: &mut egui::Ui, id: impl std::hash::Hash + std::fmt::Debug, cur: &Value) -> Option<Value> {
     let [r, g, b, _] = vectorcraft_plugins::manifest::color_value(cur).unwrap_or_default().to_rgba8(1.0);
     let mut rgb = [r, g, b];
     let changed = ui.push_id(id, |ui| ui.color_edit_button_srgb(&mut rgb).changed()).inner;
-    changed.then(|| serde_json::to_value(vectorcraft_color::Color::rgb8(rgb[0], rgb[1], rgb[2])).unwrap_or_default())
+    let picked = || crate::screen_color::sampled(&vectorcraft_color::cms::active(), rgb.map(|v| f32::from(v) / 255.0));
+    changed.then(|| serde_json::to_value(picked()).unwrap_or_default())
 }
 
 /// A `numberList` value as its field shows it: "45, 135".

docs/plugins.md

@@ -171,8 +171,10 @@ pointers are offsets into the module's memory.
   each clamped to `min..max`; a list of too few or too many is an error, and `maxItems` is at most 256) and `color`
   (`{"type": "color", "default": "#3a7bd5"}`: a color button that opens a color picker; the default, black when left
   out, and a value given to `plugin.run` may be a color object as paints hold one or a hex string, and the plug-in
-  always gets a color object, its components clamped, such as `{"model": "rgb", "r": 0.23, "g": 0.48, "b": 0.84}`; the
-  picker gives RGB). Hosts from before `color` refuse a manifest that uses it. A plug-in declares up to 256
+  always gets a color object, its components clamped, such as `{"model": "rgb", "r": 0.23, "g": 0.48, "b": 0.84}`; a
+  hex string and the picker's choice are display colors, given as the working RGB color that shows as them, so in a
+  working RGB space other than sRGB the numbers differ from the hex). Hosts from before `color` refuse a manifest that
+  uses it. A plug-in declares up to 256
   parameters, at most 64 of them outside groups.
 
 Each parameter may also say how dialogs show it. Hosts from before these keys ignore them, so a plug-in that
@@ -194,8 +196,8 @@ parameters uses `label`, `unit`, `help` and `visibleWhen`, and lists the paramet
 ### Input
 
 `input` is UTF-8 JSON: `{"objects": [...]}`. An object filter gets the selected paths and compound paths in paint
-order (bottom first): the selected ones and those inside selected groups and layers; hidden, locked, guide and clipping
-paths are left out, and so are other kinds of object (type, images, symbols, live blends…), which a filter never
+order (bottom first): the selected ones and those inside selected groups and layers; hidden, locked, guide (compound
+guides too) and clipping paths are left out, and so are other kinds of object (type, images, symbols, live blends…), which a filter never
 touches. With nothing selected the list is empty (a generator then adds its own objects). A live effect gets one
 object: its geometry (`type`, `path`, `fillRule`, `bounds`) and its `fills` and `strokes`, as a filter gets them (hosts
 from before these give geometry only). The paints are read only: an effect may draw as wide as a stroke or use a