From 8c0750e8483aa357e0cb06298e1d992e3b80d694 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 28 Oct 2026 09:33:45 -0700 Subject: [PATCH 1/3] Edit Colors: Saturate without an intensity is an error instead of a pass at 0 edit.colors.saturate's params text names intensity as required, but a call without it, or with a string such as "40", read the intensity as 0 and still ran Saturate. It changed no visible color but recorded a Saturate undo step. edit.colors.saturate now reads intensity with f64_req, as shape.ellipse reads its box. A missing, null or non-number intensity is the error "missing number `intensity`", which changes nothing and records no undo step. A number outside -100..100 still counts as its nearest end, and the params text says so. Saturate returns a color whose saturation stays the same, as at an intensity of 0, as it was. The HSB round trip rewrote most such colors in their last bit, so they counted as changed and lost their swatch link. In the same file, edit.colors.adjustBalance read a channel that is not a number as 0, and color.harmony read steps or amount that is not a number as its default. Both now refuse it with check_numbers, as the commands docs/mcp.md lists for #1952 do. A null is still read as a key left out. Docs: docs/mcp.md, docs/target-app-parity.md. --- crates/engine/src/cmd/colorcmds.rs | 12 ++++-- crates/engine/src/tests/colorguide.rs | 13 +++++++ crates/engine/src/tests/editcolors.rs | 54 +++++++++++++++++++++++++++ docs/mcp.md | 7 +++- docs/target-app-parity.md | 1 + 5 files changed, 81 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/colorcmds.rs b/crates/engine/src/cmd/colorcmds.rs index 539785ab1..54dcf2811 100644 --- a/crates/engine/src/cmd/colorcmds.rs +++ b/crates/engine/src/cmd/colorcmds.rs @@ -70,7 +70,7 @@ pub fn specs() -> Vec { "Saturate…", ["Edit", "Edit Colors"], None, - "{intensity: -100..100 (%), fill?, stroke?, includeImages?, includePatterns?, ids?} scale saturation by (1 + intensity/100), reaching what edit.colors.invert does → {changed}", + "{intensity: -100..100 (%; a number outside the range counts as its nearest end), fill?, stroke?, includeImages?, includePatterns?, ids?} scale saturation by (1 + intensity/100), reaching what edit.colors.invert does → {changed}", has_selection, saturate ), @@ -345,18 +345,21 @@ fn run_pass(s: &mut Session, p: &Value, label: &str, pass: Recolor) -> Result Result { - let i = (f64_or(p, "intensity", 0.0).clamp(-100.0, 100.0) / 100.0) as f32; + let i = (f64_req(p, "intensity", "edit.colors.saturate")?.clamp(-100.0, 100.0) / 100.0) as f32; recolor(s, p, "Saturate", &move |c| { let [h, sat, v] = c.to_hsb(); - if sat <= 0.0 { + let to = (sat * (1.0 + i)).clamp(0.0, 1.0); + // A color whose saturation stays the same is returned as it is, so it keeps its swatch link. + if sat <= 0.0 || to == sat { return c; } - keep_model(c, Color::from_hsb(h, (sat * (1.0 + i)).clamp(0.0, 1.0), v)) + keep_model(c, Color::from_hsb(h, to, v)) }) } fn adjust_balance(s: &mut Session, p: &Value) -> Result { const C: &str = "edit.colors.adjustBalance"; + check_numbers(p, &["r", "g", "b", "c", "m", "y", "k", "gray", "tint"], C)?; let given = |keys: &[&str]| keys.iter().any(|k| p.get(*k).is_some()); let mode = match str_param(p, "mode") { Some(m) => m.to_ascii_lowercase(), @@ -513,6 +516,7 @@ fn harmony(s: &mut Session, p: &Value) -> Result { let ids: Vec = Harmony::ALL.iter().map(|h| h.id()).collect(); bad(C, format!("unknown rule `{rule}` ({})", ids.join("|"))) })?; + check_numbers(p, &["steps", "amount"], C)?; let mut opts = GuideOptions::default(); if let Some(n) = p.get("steps").and_then(Value::as_u64) { opts.steps = n.min(u32::MAX as u64) as u32; diff --git a/crates/engine/src/tests/colorguide.rs b/crates/engine/src/tests/colorguide.rs index d96c4f414..1033edf67 100644 --- a/crates/engine/src/tests/colorguide.rs +++ b/crates/engine/src/tests/colorguide.rs @@ -47,6 +47,19 @@ fn harmony_query_returns_the_group_and_its_grid() { assert!(s.execute("color.harmony", &json!({"color": "#ff0000", "rule": "triad", "variation": "loud"})).is_err()); } +/// The Color Guide refuses `steps` or `amount` that is not a number, which it would otherwise +/// replace with its default. +#[test] +fn harmony_refuses_steps_or_amount_that_is_not_a_number() { + let mut s = Session::new(); + for p in [json!({"color": "#ff0000", "rule": "triad", "steps": "2"}), json!({"color": "#ff0000", "rule": "triad", "amount": "100"})] { + let e = s.execute("color.harmony", &p).unwrap_err().to_string(); + assert!(e.contains("must be a number"), "{p}: {e}"); + } + let r = s.execute("color.harmony", &json!({"color": "#ff0000", "rule": "triad", "steps": null, "amount": null})).unwrap(); + assert_eq!(r["grid"][0].as_array().unwrap().len(), 9, "a null is read as a key left out"); +} + #[test] fn new_swatch_saves_several_colours_in_one_step() { let mut s = session(); diff --git a/crates/engine/src/tests/editcolors.rs b/crates/engine/src/tests/editcolors.rs index d17ded2a7..ced41b72f 100644 --- a/crates/engine/src/tests/editcolors.rs +++ b/crates/engine/src/tests/editcolors.rs @@ -70,6 +70,60 @@ fn balance_previews_and_commits_one_step() { assert_eq!(fill_of(&s, a).to_hex(), "#808080"); } +/// Saturate needs its intensity: a call without one, or with one that is not a number, is an error +/// that changes nothing and records no undo step. A number outside −100..100 counts as its nearest end. +#[test] +fn saturate_needs_a_number_intensity() { + let mut s = session(); + let a = rect(&mut s, json!("#cc6666")); + let undo = s.doc().unwrap().history.undo.len(); + for p in [json!({}), json!({"intensity": "40"}), json!({"intensity": null})] { + let e = s.execute("edit.colors.saturate", &p).unwrap_err().to_string(); + assert!(e.contains("missing number `intensity`"), "{p}: {e}"); + } + assert_eq!(fill_of(&s, a).to_hex(), "#cc6666"); + assert_eq!(s.doc().unwrap().history.undo.len(), undo, "no undo step"); + s.execute("edit.colors.saturate", &json!({"intensity": 100})).unwrap(); + let full = fill_of(&s, a).to_hex(); + assert_ne!(full, "#cc6666"); + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("edit.colors.saturate", &json!({"intensity": 250})).unwrap(); + assert_eq!(fill_of(&s, a).to_hex(), full, "250 counts as 100"); +} + +/// Saturate at 0 leaves every color as it was: a swatch-linked fill keeps its exact color and its link. +#[test] +fn saturate_keeps_a_color_it_does_not_change() { + let mut s = session(); + s.execute("swatch.new", &json!({"name": "Sky", "color": "#3a7bd5", "global": true})).unwrap(); + let a = rect(&mut s, json!("#3a7bd5")); + s.execute("paint.setFill", &json!({"ids": [a.0], "swatch": "Sky"})).unwrap(); + let b = rect(&mut s, json!({"c": 20, "m": 40, "y": 0, "k": 10})); + let paint = |s: &Session, id: NodeId| s.doc().unwrap().doc.node(id).unwrap().appearance.fill_paint().clone(); + let (sky, cmyk) = (paint(&s, a), paint(&s, b)); + let v = s.execute("edit.colors.saturate", &json!({"ids": [a.0, b.0], "intensity": 0})).unwrap(); + assert_eq!(v["changed"], 0); + assert_eq!(paint(&s, a), sky); + assert_eq!(paint(&s, b), cmyk); +} + +/// Adjust Color Balance refuses a channel that is not a number, which it would otherwise read as 0. +#[test] +fn adjust_balance_refuses_a_channel_that_is_not_a_number() { + let mut s = session(); + let a = rect(&mut s, json!("#808080")); + let undo = s.doc().unwrap().history.undo.len(); + for p in [json!({"mode": "rgb", "r": "40"}), json!({"c": "10"}), json!({"gray": true}), json!({"tint": "5"})] { + let e = s.execute("edit.colors.adjustBalance", &p).unwrap_err().to_string(); + assert!(e.contains("must be a number"), "{p}: {e}"); + } + assert_eq!(fill_of(&s, a).to_hex(), "#808080"); + assert_eq!(s.doc().unwrap().history.undo.len(), undo, "no undo step"); + // A null is read as a channel left out. + s.execute("edit.colors.adjustBalance", &json!({"mode": "rgb", "r": 20, "g": null})).unwrap(); + assert_eq!(fill_of(&s, a).to_hex(), "#b38080"); +} + // ---------- meshes, images and patterns ---------- fn mesh_colors(s: &Session, id: NodeId) -> Vec { diff --git a/docs/mcp.md b/docs/mcp.md index f6e269456..bf6bb35cf 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1129,7 +1129,8 @@ and `radial` in `shape.polarGrid`; `size` and the `width`, `height` and `fitMinP `end` in `type.step`; `by` in `type.size.increase` and `type.size.decrease`; `size` in `type.ruby`; `width`, `height` and `fitMinPercent` in `text.areaOptions`; `x` and `y` in `symbol.place`; `radius`, `density` and `scale` in `symbol.spray`; `radius` and `intensity` in `symbol.adjust`; `to` in `swatch.move`; `weight` and `opacity` in -`appearance.setItem`; `angle` in `paint.editGradient` and in the `gradient` of a command that takes a paint; `t` in +`appearance.setItem`; `r`, `g`, `b`, `c`, `m`, `y`, `k`, `gray` and `tint` in `edit.colors.adjustBalance`; `steps` +and `amount` in `color.harmony`; `angle` in `paint.editGradient` and in the `gradient` of a command that takes a paint; `t` in `paint.freeform.splitLine`; `tolerance` and `gap` in `shapeBuilder.merge`; `gap` in `shapeBuilder.regions`; and `width` and `tolerance` in `livePaint.strokeEdge`. A null is read as a key left out. A gradient stop that names a swatch refuses a `tint` that is not a number with `` stop 0: `tint` must be a number ``. @@ -1716,7 +1717,9 @@ the scope and what differs from the public documentation. fills, strokes, text, gradient stops, gradient-mesh points, embedded images (a recoloured copy of the image; linked images are left alone) and the tiles of pattern fills and strokes (a recoloured copy saved as a new pattern swatch, e.g. "Dots 2"; the original pattern is untouched). `includeImages: false` / `includePatterns: false` leave images or -patterns out. The Blend commands also grade gradient meshes, which keep their shading. +patterns out. The Blend commands also grade gradient meshes, which keep their shading. `edit.colors.saturate` needs +`intensity`, a number from -100 to 100 (a number outside the range counts as its nearest end). Without one, or with a +value that is not a number, it answers `` missing number `intensity` `` and changes nothing. ```json {"name":"run_command","arguments":{"command":"recolor.colors","params":{}}} diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 8f3b67c20..7d35e2f4d 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -845,6 +845,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Colour:** a Color Picker (colour field, H/S/B/R/G/B channel slider, HSB/RGB/Lab/CMYK fields, hex, web-only snapping, out-of-gamut correction); the Color panel shows a selected colour in its own model and, with nothing selected, the new-art defaults in the mode picked last, which it keeps between sessions; Alt-click paints the other proxy; proxies look through groups and show "?" for mixed paint; Invert, Complement, Apply Last Color (`,`) and Apply Last Gradient (`.`) keep the colour model; every paint command feeds the recent colours. The Color Guide has 21 harmony rules, Steps and Variation options and Limit to Library. Adjust Color Balance (its Global mode shifts tints) and Saturate preview live; Edit Colors and Recolor also reach meshes, images and patterns. Recolor Artwork has colour reduction, five recolour methods, presets, Limit to Library, a colour-wheel Edit tab and Edit Color Group; a local Color Themes panel saves five-colour themes. Fill/Stroke chips open swatch and mixer popovers (the Properties panel has the Control bar's Fill and Stroke chips, Stroke link and weight spinner, with nothing selected too), panels have Illustrator's Window menu keys (Align Shift+F7, Transform Shift+F8, Pathfinder Cmd+Shift+F9, Layers F7, Character Cmd+T…; every panel can be given one in Edit › Keyboard Shortcuts), and Eyedropper Options pick up and apply chosen attributes (Alt and Shift+Alt, image pixels). An ICC CMYK profile converts straight into the RGB working space (not through sRGB, so a wider space such as Wide Gamut RGB keeps colours sRGB can't hold) with the colour settings' black-point compensation, on the canvas and in RGB exports alike (the built-in Generic CMYK compensates both ways, so RGB black separated to a rich black displays as black again). Fills and strokes can overprint (Attributes panel, Overprint Black options), and CMYK documents blend in inks on screen, in PDF and when flattening. Document Color Mode converts the art, symbols, pattern tiles and swatches through the colour settings: Gray stays Gray, RGB greys separate into four-colour greys and a rich black as Illustrator does, or go on the black plate only with `grays: "black"` (also for `document.open {colorMode}`). View › Proof Setup › Color blindness – Tritanopia-type turns Proof Colors on and proofs the canvas with the tritanopia simulation of Brettel, Viénot and Mollon (1997). The Separations Preview panel's Proof Setup list offers the same target (`view.proofSetup {target: "tritanopia"}`). Grays keep their value. - **Recolor Artwork layout (#3416):** the Edit chip list uses the modal scroll helper and its actual remaining height; the tab's minimum-height space scrolls with the chips. `tests_recolor` covers 5/20/200/400 colors, window resizing, 100/125/150/200% UI scaling, scrolling to and editing the final chip, Preview, Cancel, Confirm and Undo, alongside the existing #1733 drag regressions. Scores and estimates unchanged. + - **Edit Colors parameters:** `edit.colors.saturate` needs a number `intensity`, the value from -100% to 100% that Saturate takes ([Adjusting colors](https://web.archive.org/web/20230128232631/https://helpx.adobe.com/illustrator/using/adjusting-colors.html)), and answers "missing number `intensity`" without one; a number outside the range counts as its nearest end. `edit.colors.adjustBalance` refuses a channel and `color.harmony` refuses `steps` or `amount` that is not a number. `tests::editcolors` and `tests::colorguide` cover them. - **Color Picker layout (#2008):** a centered draggable header, equal-width actions above aligned HSB/RGB/Hex and Lab/CMYK fields, compact field and slider, and warnings below the preview. No horizontal scrolling; narrow windows stack the sections with vertical scrolling. Tests in `dialogs/color_picker.rs` cover actual pointer/key input, typed values before OK/Enter, Cancel/Escape rollback, one Undo step, mode-size stability, warnings and layout at two UI scales. The screen sampler sits above the field and the upstream two-grid Tab order is retained. AccessKit regressions check the action names and each channel slider's displayed value, units and range. German and enlarged English UI renders were reviewed locally; no reference images are shipped. This is a layout improvement, with no change to scores or remaining-hour estimates. - **Eyedropper sources (#1251):** ordinary and Shift clicks, Shift+Alt appearance append, raster pixels and gradient-stop sampling can read visible locked/template art and sources outside isolation. Sampling leaves the source and selection intact; Alt reverse still uses editable hits, and hidden art stays excluded. Engine/tool/hit regressions cover the editing boundary, color models and one-step Undo/Redo. The general object/image workflow is described in [public HelpX](https://helpx.adobe.com/illustrator/using/tool-techniques/eyedropper-tool.html); the lock/isolation expectations come from the user report and these regressions, without a new full-parity measurement. - **Eyedropper colors in a CMYK document (#2276):** a CMYK image (a four-component JPEG, or an 8- or 16-bit chunky CMYK TIFF) gives its ink amounts as they are, and an RGB image's pixel or an RGB color given to `paint.setFill`, `paint.setStroke`, `paint.sampleColor` or `swatch.new {color}` (hex input and gradient stops included) is separated through the color settings' CMYK working space and intent, as Document Color Mode converts it. With the default Generic CMYK, RGB 83/170/223 separates to C59 M12 Y3 K0, where the profile-free formula gave C63 M24 Y0 K13. From 78c51b86e6b3c4a74e4cf5c335086bb113b30de3 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 28 Oct 2026 10:27:16 -0700 Subject: [PATCH 2/3] Docs: Edit Colors and Color Guide read a null parameter as one left out --- docs/target-app-parity.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 7d35e2f4d..37cb26538 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -845,7 +845,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Colour:** a Color Picker (colour field, H/S/B/R/G/B channel slider, HSB/RGB/Lab/CMYK fields, hex, web-only snapping, out-of-gamut correction); the Color panel shows a selected colour in its own model and, with nothing selected, the new-art defaults in the mode picked last, which it keeps between sessions; Alt-click paints the other proxy; proxies look through groups and show "?" for mixed paint; Invert, Complement, Apply Last Color (`,`) and Apply Last Gradient (`.`) keep the colour model; every paint command feeds the recent colours. The Color Guide has 21 harmony rules, Steps and Variation options and Limit to Library. Adjust Color Balance (its Global mode shifts tints) and Saturate preview live; Edit Colors and Recolor also reach meshes, images and patterns. Recolor Artwork has colour reduction, five recolour methods, presets, Limit to Library, a colour-wheel Edit tab and Edit Color Group; a local Color Themes panel saves five-colour themes. Fill/Stroke chips open swatch and mixer popovers (the Properties panel has the Control bar's Fill and Stroke chips, Stroke link and weight spinner, with nothing selected too), panels have Illustrator's Window menu keys (Align Shift+F7, Transform Shift+F8, Pathfinder Cmd+Shift+F9, Layers F7, Character Cmd+T…; every panel can be given one in Edit › Keyboard Shortcuts), and Eyedropper Options pick up and apply chosen attributes (Alt and Shift+Alt, image pixels). An ICC CMYK profile converts straight into the RGB working space (not through sRGB, so a wider space such as Wide Gamut RGB keeps colours sRGB can't hold) with the colour settings' black-point compensation, on the canvas and in RGB exports alike (the built-in Generic CMYK compensates both ways, so RGB black separated to a rich black displays as black again). Fills and strokes can overprint (Attributes panel, Overprint Black options), and CMYK documents blend in inks on screen, in PDF and when flattening. Document Color Mode converts the art, symbols, pattern tiles and swatches through the colour settings: Gray stays Gray, RGB greys separate into four-colour greys and a rich black as Illustrator does, or go on the black plate only with `grays: "black"` (also for `document.open {colorMode}`). View › Proof Setup › Color blindness – Tritanopia-type turns Proof Colors on and proofs the canvas with the tritanopia simulation of Brettel, Viénot and Mollon (1997). The Separations Preview panel's Proof Setup list offers the same target (`view.proofSetup {target: "tritanopia"}`). Grays keep their value. - **Recolor Artwork layout (#3416):** the Edit chip list uses the modal scroll helper and its actual remaining height; the tab's minimum-height space scrolls with the chips. `tests_recolor` covers 5/20/200/400 colors, window resizing, 100/125/150/200% UI scaling, scrolling to and editing the final chip, Preview, Cancel, Confirm and Undo, alongside the existing #1733 drag regressions. Scores and estimates unchanged. - - **Edit Colors parameters:** `edit.colors.saturate` needs a number `intensity`, the value from -100% to 100% that Saturate takes ([Adjusting colors](https://web.archive.org/web/20230128232631/https://helpx.adobe.com/illustrator/using/adjusting-colors.html)), and answers "missing number `intensity`" without one; a number outside the range counts as its nearest end. `edit.colors.adjustBalance` refuses a channel and `color.harmony` refuses `steps` or `amount` that is not a number. `tests::editcolors` and `tests::colorguide` cover them. + - **Edit Colors parameters:** `edit.colors.saturate` needs a number `intensity`, the value from -100% to 100% that Saturate takes ([Adjusting colors](https://web.archive.org/web/20230128232631/https://helpx.adobe.com/illustrator/using/adjusting-colors.html)), and answers "missing number `intensity`" without one; a number outside the range counts as its nearest end. `edit.colors.adjustBalance` refuses a channel and `color.harmony` refuses `steps` or `amount` that is not a number; both read `null` as a value left out. `tests::editcolors` and `tests::colorguide` cover them. - **Color Picker layout (#2008):** a centered draggable header, equal-width actions above aligned HSB/RGB/Hex and Lab/CMYK fields, compact field and slider, and warnings below the preview. No horizontal scrolling; narrow windows stack the sections with vertical scrolling. Tests in `dialogs/color_picker.rs` cover actual pointer/key input, typed values before OK/Enter, Cancel/Escape rollback, one Undo step, mode-size stability, warnings and layout at two UI scales. The screen sampler sits above the field and the upstream two-grid Tab order is retained. AccessKit regressions check the action names and each channel slider's displayed value, units and range. German and enlarged English UI renders were reviewed locally; no reference images are shipped. This is a layout improvement, with no change to scores or remaining-hour estimates. - **Eyedropper sources (#1251):** ordinary and Shift clicks, Shift+Alt appearance append, raster pixels and gradient-stop sampling can read visible locked/template art and sources outside isolation. Sampling leaves the source and selection intact; Alt reverse still uses editable hits, and hidden art stays excluded. Engine/tool/hit regressions cover the editing boundary, color models and one-step Undo/Redo. The general object/image workflow is described in [public HelpX](https://helpx.adobe.com/illustrator/using/tool-techniques/eyedropper-tool.html); the lock/isolation expectations come from the user report and these regressions, without a new full-parity measurement. - **Eyedropper colors in a CMYK document (#2276):** a CMYK image (a four-component JPEG, or an 8- or 16-bit chunky CMYK TIFF) gives its ink amounts as they are, and an RGB image's pixel or an RGB color given to `paint.setFill`, `paint.setStroke`, `paint.sampleColor` or `swatch.new {color}` (hex input and gradient stops included) is separated through the color settings' CMYK working space and intent, as Document Color Mode converts it. With the default Generic CMYK, RGB 83/170/223 separates to C59 M12 Y3 K0, where the profile-free formula gave C63 M24 Y0 K13. From ace60bd542436d982eddd2f3edb5fd8ecba07044 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 09:06:58 +0700 Subject: [PATCH 3/3] Edit Colors: import f64_req and check_numbers in colorcmds.rs (#4299 made the imports explicit) --- crates/engine/src/cmd/colorcmds.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/colorcmds.rs b/crates/engine/src/cmd/colorcmds.rs index 54dcf2811..49b729c8a 100644 --- a/crates/engine/src/cmd/colorcmds.rs +++ b/crates/engine/src/cmd/colorcmds.rs @@ -19,7 +19,8 @@ use vectorcraft_doc::{Document, ImageBlob, Node, NodeId, NodeKind}; use super::edit::selected_roots; use super::{ - CommandSpec, EngineError, Result, Session, always, bad, bool_or, cmd, color_value, f64_or, has_selection, ids_param, str_param, unique_name, + CommandSpec, EngineError, Result, Session, always, bad, bool_or, check_numbers, cmd, color_value, f64_or, f64_req, has_selection, ids_param, + str_param, unique_name, }; pub fn specs() -> Vec {