diff --git a/crates/engine/src/cmd/recolor.rs b/crates/engine/src/cmd/recolor.rs index 5ff152a85..47357437e 100644 --- a/crates/engine/src/cmd/recolor.rs +++ b/crates/engine/src/cmd/recolor.rs @@ -555,24 +555,31 @@ fn randomize(_: &mut Session, p: &Value) -> Result { let rows = parse_rows(p, C)?; let mut rng = Rng::new(super::menucmds::whole_or(p, "seed", 0, C)?); let (order, saturation_brightness) = (switch_or(p, "order", true, C)?, switch_or(p, "saturationBrightness", false, C)?); - let mut news: Vec = rows.iter().filter_map(Row::new_color).collect(); + // Each new colour with its row's tint: a tint moves with its colour, and goes when the colour changes. + let mut news: Vec<(Color, f32)> = rows.iter().filter_map(|r| r.new_color().map(|c| (c, r.tint))).collect(); if order { rng.shuffle(&mut news); } if saturation_brightness { - news = news.into_iter().map(|c| rng.saturation_brightness(c)).collect(); + news = news + .into_iter() + .map(|(c, tint)| { + let n = rng.saturation_brightness(c); + (n, if ColorKey::of(&n) == ColorKey::of(&c) { tint } else { 1.0 }) + }) + .collect(); } let mut news = news.into_iter(); let map: Vec = rows .iter() .map(|r| { - let to = if r.new_color().is_some() { news.next() } else { r.to }; + let (to, tint) = if r.new_color().is_some() { news.next().map_or((None, 1.0), |(c, t)| (Some(c), t)) } else { (r.to, r.tint) }; let mut row = json!({"from": r.from.iter().map(Match::to_json).collect::>(), "to": to.as_ref().map(key_str)}); if r.exclude { row["exclude"] = json!(true); } - if r.tint < 1.0 { - row["tint"] = json!(f64::from((r.tint * 10_000.0).round()) / 100.0); + if tint < 1.0 { + row["tint"] = json!(f64::from((tint * 10_000.0).round()) / 100.0); } row }) diff --git a/crates/engine/src/tests/recolor.rs b/crates/engine/src/tests/recolor.rs index ecf71f91a..1be156606 100644 --- a/crates/engine/src/tests/recolor.rs +++ b/crates/engine/src/tests/recolor.rs @@ -538,7 +538,7 @@ fn recolor_to_a_library_spot_colour_links_it_and_adds_the_swatch() { } /// A row's `tint` gives the art linked to the row's spot color that share of its tint; -/// randomize keeps it on the row, and a tint that is not a number is refused. +/// randomize without a change keeps it on the row, and a tint that is not a number is refused. #[test] fn a_rows_tint_tints_the_spot_color_the_art_links_to() { let mut s = session(); @@ -564,6 +564,31 @@ fn a_rows_tint_tints_the_spot_color_the_art_links_to() { assert!(e.to_string().contains("`tint` must be a number"), "{e}"); } +/// Randomize order moves a row's tint with its new color; changing saturation and brightness +/// drops the tint of every color it changes. +#[test] +fn randomize_moves_a_tint_with_its_color_and_drops_it_when_the_color_changes() { + let mut s = session(); + let map = json!([ + {"from": ["rgb 255 0 0"], "to": "cmyk 0 100 0 0", "tint": 40}, + {"from": ["rgb 0 255 0"], "to": "rgb 0 255 0"}, + {"from": ["rgb 0 0 255"], "to": "rgb 0 0 255"}, + ]); + let mut moved = false; + for seed in 0..20 { + let r = run(&mut s, "recolor.randomize", json!({"map": map, "seed": seed})); + for row in r["map"].as_array().unwrap() { + let spot = row["to"] == json!("cmyk 0 100 0 0"); + assert_eq!(row.get("tint").cloned(), spot.then(|| json!(40.0)), "seed {seed}: the tint goes with its color: {r}"); + } + moved |= r["map"][0]["to"] != map[0]["to"]; + } + assert!(moved, "some seed moves the tinted color to another row"); + let sb = run(&mut s, "recolor.randomize", json!({"map": map, "order": false, "saturationBrightness": true})); + assert_ne!(sb["map"][0]["to"], map[0]["to"], "{sb}"); + assert!(sb["map"][0].get("tint").is_none(), "a changed color has no tint: {sb}"); +} + #[test] fn model_param_chooses_the_model_of_the_new_colour() { // RGB art in a CMYK document: by default a CMYK replacement comes out RGB (#2170). diff --git a/crates/ui-egui/src/dialogs/recolor.rs b/crates/ui-egui/src/dialogs/recolor.rs index 7041f96ca..0deb4007b 100644 --- a/crates/ui-egui/src/dialogs/recolor.rs +++ b/crates/ui-egui/src/dialogs/recolor.rs @@ -215,6 +215,24 @@ fn set_to(row: &mut Value, c: &Color) { } } +/// Set for the frame in which Randomly change color order shuffled the rows' new colors. +const SHUFFLED: &str = "__shuffled"; + +/// Give each row of `rows` whose current colors and new color are those of the same row of +/// `before` that row's tint again, when it has none: a change that snaps back to the same new +/// color keeps the tint of it. +fn keep_tints(before: &[Value], rows: &mut [Value]) { + for (r, b) in rows.iter_mut().zip(before) { + if r["from"] == b["from"] + && r["to"] == b["to"] + && r.get("tint").is_none() + && let Some(t) = b.get("tint") + { + r["tint"] = t.clone(); + } + } +} + fn excluded(row: &Value) -> bool { row["exclude"].as_bool().unwrap_or(false) } @@ -274,6 +292,10 @@ fn body(app: &mut VectorcraftApp, ui: &mut Ui, d: &mut Dialog) -> bool { } if changed { snap(app, d, &mut rs); + // A shuffle by `recolor.randomize` has already moved each tint with its color. + if d.fields.remove(SHUFFLED).is_none() { + keep_tints(&rows(d), &mut rs); + } d.fields.insert("rows".into(), Value::Array(rs)); } if d.fields.get("group").is_some_and(Value::is_string) || d.bool("__newGroup") { @@ -299,6 +321,7 @@ pub(super) fn with_new_color(app: &VectorcraftApp, mut d: Dialog, i: usize, c: C if let Some(r) = rs.get_mut(i) { set_to(r, &c); snap(app, &d, &mut rs); + keep_tints(&rows(&d), &mut rs); d.fields.insert("rows".into(), Value::Array(rs)); } d @@ -484,6 +507,9 @@ fn assign_tab(app: &mut VectorcraftApp, ui: &mut Ui, d: &mut Dialog, rows: &mut match app.session.execute("recolor.randomize", &p) { Ok(r) => { *rows = r["map"].as_array().cloned().unwrap_or_default(); + if !sb { + d.fields.insert(SHUFFLED.into(), json!(true)); + } changed = true; } Err(e) => app.status(e.to_string()), @@ -782,8 +808,8 @@ fn move_color(rows: &mut [Value], i: usize, hsb: [f32; 3], linked: bool) { let mut colors = before.clone(); move_on_wheel(&mut colors, k, hsb, linked); for ((&j, c), old) in active.iter().zip(&colors).zip(&before) { - if c != old { - rows[j]["to"] = key(c); + if key(c) != key(old) { + set_to(&mut rows[j], c); } } } @@ -868,13 +894,17 @@ fn edit_tab(ui: &mut Ui, d: &mut Dialog, rows: &mut [Value]) -> bool { } /// Give the rows `active` the colours of `rule` from the base colour `b` of row `base` (the base row -/// first, then the others in order, the rule's colours repeating), each in its row's model. +/// first, then the others in order, the rule's colours repeating), each in its row's model. A row +/// whose new colour changes loses its tint. fn apply_rule(rows: &mut [Value], rule: Harmony, base: usize, b: Color, active: &[usize]) { let colors = rule.apply(b); let order = std::iter::once(base).chain(active.iter().copied().filter(|i| *i != base)); for (n, i) in order.enumerate() { let old = to_color(&rows[i]).unwrap_or(b); - rows[i]["to"] = key(&keep_model(old, colors[n % colors.len()])); + let c = keep_model(old, colors[n % colors.len()]); + if key(&c) != rows[i]["to"] { + set_to(&mut rows[i], &c); + } } } @@ -937,6 +967,41 @@ mod tests { assert_eq!([&r[0]["to"], &r[1]["to"], &r[2]["to"]], ["rgb 255 0 0", "rgb 0 255 0", "rgb 0 0 255"]); } + /// A rule or a wheel move that gives a row a new colour drops the row's tint; a row whose + /// colour stays keeps it. + #[test] + fn a_new_colour_from_the_edit_tab_drops_the_rows_tint() { + let tinted = || { + let mut r = rows(); + for row in &mut r { + row["tint"] = json!(40.0); + } + r + }; + let mut r = tinted(); + apply_rule(&mut r, Harmony::Triad, 0, Color::rgb(1.0, 0.0, 0.0), &[0, 1, 2]); + assert_eq!(r[0]["tint"], json!(40.0), "the base row's colour stays"); + assert!(r[1].get("tint").is_none() && r[2].get("tint").is_none(), "{r:?}"); + let mut r = tinted(); + move_color(&mut r, 0, [120.0, 1.0, 1.0], false); + assert!(r[0].get("tint").is_none(), "the moved row: {:?}", r[0]); + assert_eq!(r[1]["tint"], json!(40.0), "unlinked: the other rows keep theirs"); + } + + /// The Color Picker's OK keeps a row's tint when the row's new color stays, and drops it otherwise. + #[test] + fn a_picked_color_keeps_the_tint_only_of_the_same_color() { + let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()); + app.run("file.new", json!({"width": 100, "height": 100})).unwrap(); + app.run("ui.recolorDialog", json!({"colors": ["#ff0000"]})).unwrap(); + let mut d = app.ui.dialog.clone().unwrap(); + d.fields.insert("rows".into(), json!([{"from": ["rgb 255 0 0"], "to": "rgb 255 0 0", "tint": 40.0}])); + let same = with_new_color(&app, d.clone(), 0, Color::rgb(1.0, 0.0, 0.0)); + assert_eq!(super::rows(&same)[0]["tint"], json!(40.0)); + let other = with_new_color(&app, d, 0, Color::rgb(0.0, 1.0, 0.0)); + assert!(super::rows(&other)[0].get("tint").is_none(), "{:?}", super::rows(&other)); + } + /// Runs one frame of the open dialog at `time` and returns row `i`'s new color chip, when the /// Recolor Artwork dialog drew it. fn frame(app: &mut VectorcraftApp, ctx: &egui::Context, time: f64, events: Vec, i: usize) -> Option { diff --git a/crates/ui-egui/src/tests/recolor.rs b/crates/ui-egui/src/tests/recolor.rs index c4aa85d6e..b5c5f874d 100644 --- a/crates/ui-egui/src/tests/recolor.rs +++ b/crates/ui-egui/src/tests/recolor.rs @@ -346,6 +346,85 @@ fn the_tint_slider_sets_a_spot_rows_tint() { } } +/// A spot row's tint goes when the Edit tab gives the row another new color: after a click on the +/// Brightness slider snaps the row to a second spot color, OK links the art to that one at full +/// strength. A click that snaps back to the same spot color keeps the tint. +#[test] +fn a_new_color_from_the_edit_tab_drops_a_spot_rows_tint() { + let mut app = app(); + let lib = library_of( + &mut app, + &[json!({"name": "Mix Ink", "color": "#ff00ff", "spot": true}), json!({"name": "Mix Dark", "color": "#800080", "spot": true})], + ); + art(&mut app, &["#ff00ff"]); + let mut h = assign_harness(app, json!({"library": lib})); + let mut rows = field(h.state(), "rows"); + rows[0]["tint"] = json!(40.0); + set(h.state_mut(), "rows", rows); + set(h.state_mut(), "tab", json!("edit")); + h.run_steps(4); + click_slider(&mut h, "Brightness", 0.9); + let row = field(h.state(), "rows")[0].clone(); + assert_eq!((&row["to"], &row["tint"]), (&json!("rgb 255 0 255"), &json!(40.0)), "snapped back to the same spot color"); + click_slider(&mut h, "Brightness", 0.4); + let row = field(h.state(), "rows")[0].clone(); + assert_eq!(row["to"], json!("rgb 128 0 128"), "snapped to the darker spot color"); + assert!(row.get("tint").is_none(), "the row's tint went with its old color: {row}"); + h.get_by_label("OK").click(); + h.run_steps(2); + match first_fill(h.state()) { + vectorcraft_color::Paint::Solid { swatch, tint, .. } => { + assert_eq!(swatch.as_deref(), Some("Mix Dark"), "linked to the new spot color"); + assert!((tint - 1.0).abs() < 1e-4, "at full strength: {tint}"); + } + p => panic!("{p:?}"), + } +} + +/// Randomize order moves a tint with its color, also between rows of the same new color: after +/// each click exactly one row is tinted, and some click moves the tint to the other row. +#[test] +fn randomize_order_moves_a_tint_between_rows_of_the_same_color() { + let mut app = app(); + art(&mut app, &["#ff0000", "#0000ff"]); + let mut h = assign_harness(app, json!({})); + let mut rows = field(h.state(), "rows"); + for r in rows.as_array_mut().unwrap() { + r["to"] = json!("rgb 0 255 0"); + } + rows[0]["tint"] = json!(40.0); + set(h.state_mut(), "rows", rows); + h.run_steps(2); + let mut moved = false; + for _ in 0..8 { + h.get_by_label("Randomly change color order").click(); + h.run_steps(2); + let rows = field(h.state(), "rows"); + let tinted: Vec = (0..2).filter(|i| rows[*i].get("tint").is_some()).collect(); + assert_eq!(tinted.len(), 1, "one tinted row: {rows}"); + moved |= tinted == [1]; + } + assert!(moved, "some click moves the tint to the other row"); +} + +/// With a one-color library, Randomly change saturation and brightness snaps a spot row back to +/// its spot color, which keeps its tint. +#[test] +fn a_saturation_change_that_snaps_back_keeps_the_tint() { + let mut app = app(); + let lib = library_of(&mut app, &[json!({"name": "Mix Ink", "color": "#ff00ff", "spot": true})]); + art(&mut app, &["#ff00ff"]); + let mut h = assign_harness(app, json!({"library": lib})); + let mut rows = field(h.state(), "rows"); + rows[0]["tint"] = json!(40.0); + set(h.state_mut(), "rows", rows); + h.run_steps(2); + h.get_by_label("Randomly change saturation and brightness").click(); + h.run_steps(2); + let row = field(h.state(), "rows")[0].clone(); + assert_eq!((&row["to"], &row["tint"]), (&json!("rgb 255 0 255"), &json!(40.0)), "{row}"); +} + /// Makes the swatches of the `swatch.new` params `swatches`, saves them as a library and loads it, /// removes them from the document again, and returns the library's id. fn library_of(app: &mut VectorcraftApp, swatches: &[Value]) -> String { diff --git a/docs/mcp.md b/docs/mcp.md index 5363e96fb..f88a146aa 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1929,7 +1929,8 @@ darker than the row's average become tints and shades) or `hueShift` (the most s the others turn by the same hue). `limitTo` snaps new colours to a swatch library's nearest colour; `recolor.randomize {map, order?, saturationBrightness?, seed?}` shuffles or varies new colours; a row with `exclude: true` keeps its colours. With `limitTo`, a row's `tint` (a percentage, default 100) gives the art -linked to the global or spot swatch its new color is that share of its tint; `tint` must be a number. `swatch.editGroup {group, colors, rename?}` rewrites a colour group in one undo +linked to the global or spot swatch its new color is that share of its tint; `tint` must be a number. `recolor.randomize` +moves a row's `tint` with its new color and removes it from a color it changes. `swatch.editGroup {group, colors, rename?}` rewrites a colour group in one undo step (art linked to its global swatches follows), and `recolor.apply` with `group` recolours the art and rewrites the group together. Art linked to one of the group's global swatches stays linked when the swatch is rewritten with its row's new color (the default without `groupColors` when the rows follow the group's order), so a later `swatch.edit` still