From de6cc6a9c3d142aae2ccb27336bed0875d61b326 Mon Sep 17 00:00:00 2001 From: Sven Lafebre Date: Mon, 9 Nov 2026 01:26:34 +0100 Subject: [PATCH] Fix #6252: selecting an object gives new art its fill, stroke and weight With New Art Has Basic Appearance on (the default), the selection never reached the fill and stroke proxies, so new art kept the colours set last. A select.* command or layer.target that changes the selection now loads the proxies and the weight from what the selection shows in them. A paint or weight the selected objects don't share stays as it was, and art without paint (a placed image) changes nothing. The template is left alone, and only a changed selection loads, so a sampled paint stays. --- crates/engine/src/cmd/newart.rs | 34 ++++++- crates/engine/src/session/execute.rs | 6 ++ crates/engine/src/tests/newart.rs | 130 +++++++++++++++++++++++++++ crates/engine/src/tests/xform.rs | 13 +++ docs/mcp.md | 6 +- 5 files changed, 184 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/newart.rs b/crates/engine/src/cmd/newart.rs index 34db64081..c7bd4ffbf 100644 --- a/crates/engine/src/cmd/newart.rs +++ b/crates/engine/src/cmd/newart.rs @@ -5,8 +5,8 @@ //! `stroke.set` whatever is selected), a graphic style clicked with nothing selected (more fills //! and strokes, effects, opacity and blend mode, and the link to the style) or, with the Appearance //! panel's New Art Has Basic Appearance off, the whole appearance of the last selection. With the -//! option on (the default) an inherited appearance gives new art its top stroke's options only. -//! `paint.default` resets it. +//! option on (the default) an inherited appearance gives new art its top stroke's options only, +//! and a change of selection loads only the proxies. `paint.default` resets it. use std::borrow::Cow; @@ -25,7 +25,7 @@ pub fn specs() -> Vec { "New Art Has Basic Appearance", ["Window", "Appearance"], None, - "{on?: bool (omitted: toggle)} the Appearance panel option (the preference newArtBasic, on by default): on, new art takes one fill and stroke (with the Stroke panel's options); off, the whole appearance of the last selected object (every fill, stroke and effect, opacity and blend mode) → {on}", + "{on?: bool (omitted: toggle)} the Appearance panel option (the preference newArtBasic, on by default): on, new art takes one fill and stroke (with the Stroke panel's options), the fill, stroke and weight the selection shares when it last changed; off, the whole appearance of the last selected object (every fill, stroke and effect, opacity and blend mode) → {on}", always, set_basic ), @@ -213,6 +213,34 @@ impl Session { let (ap, opacity, blend) = (super::style::captured(n), n.opacity, n.blend); self.take_new_art(ap, opacity, blend, style, true); } + + /// After a command changed the selection, with New Art Has Basic Appearance on: the fill and + /// stroke proxies and the weight take what the selection shows in them, so new art looks like + /// it. A paint the selected objects don't share (a "?" proxy) stays as it was, and art without + /// paint (a placed image, whose None would make new art invisible) changes nothing. The + /// template is left alone, and without a stroke to show the weight stays too. + pub(crate) fn load_selection_paints(&mut self) { + if !self.prefs.new_art_basic { + return; + } + let has_paint = self.proxy_source(false).is_some_and(|(n, _)| matches!(n.kind, NodeKind::Text(_)) || !n.appearance.items.is_empty()); + if !has_paint { + return; + } + let (fill_mixed, stroke_mixed) = self.proxy_mixed(); + let (fill, stroke) = self.proxy_paints(); + let weight_mixed = self.active().is_some_and(|st| st.stroke_mixed().weight); + let weight = self.shown_stroke().filter(|st| st.paint != Paint::None && !weight_mixed).map(|st| st.width); + if !fill_mixed { + self.paint.fill = unplaced(&fill); + } + if !stroke_mixed { + self.paint.stroke = unplaced(&stroke); + } + if let Some(width) = weight { + self.paint.stroke_width = width; + } + } } fn set_basic(s: &mut Session, p: &Value) -> Result { diff --git a/crates/engine/src/session/execute.rs b/crates/engine/src/session/execute.rs index a8cc8ff5a..f982121b5 100644 --- a/crates/engine/src/session/execute.rs +++ b/crates/engine/src/session/execute.rs @@ -113,6 +113,9 @@ impl Session { }) .flatten(); let selected_before = select_first.as_ref().and(self.active()).map(|d| (d.uid, d.selection.clone(), d.active_layer)); + // Only a selection command that changes the selection loads its paints for new art: other + // commands, and clicking what is already selected, keep a sampled paint in the proxies. + let selection_was = (id.starts_with("select.") || id == "layer.target").then(|| self.active().map(|d| d.selection.clone())).flatten(); let run = |s: &mut Session| { if let Some(ids) = &select_first { s.execute("select.set", &serde_json::json!({ "ids": ids }))?; @@ -147,6 +150,9 @@ impl Session { }); if !restored { self.inherit_new_art(); + if selection_was.is_some_and(|was| self.active().is_some_and(|d| d.selection != was)) { + self.load_selection_paints(); + } } for uid in std::mem::take(&mut self.batch_paint_docs) { if let Some(st) = self.docs.iter_mut().find(|d| d.uid == uid) diff --git a/crates/engine/src/tests/newart.rs b/crates/engine/src/tests/newart.rs index 0f41a5b9d..583a5f43d 100644 --- a/crates/engine/src/tests/newart.rs +++ b/crates/engine/src/tests/newart.rs @@ -141,6 +141,136 @@ fn a_graphic_style_clicked_with_nothing_selected_styles_the_next_object() { assert!(s.execute("graphicStyle.apply", &json!({"name": name, "ids": []})).is_err(), "explicit ids that match nothing"); } +/// A rectangle with this fill, stroke and weight, selected. +fn painted(s: &mut Session, x: f64, fill: &str, stroke: &str, weight: f64) -> NodeId { + let id = rect(s, x); + run(s, "paint.setFill", json!({"color": fill})); + run(s, "paint.setStroke", json!({"color": stroke})); + run(s, "stroke.set", json!({"weight": weight})); + id +} + +fn select(s: &mut Session, ids: &[NodeId]) { + run(s, "select.set", json!({"ids": ids.iter().map(|id| id.0).collect::>()})); +} + +#[test] +fn selecting_an_object_gives_new_art_its_fill_stroke_and_weight_with_basic_appearance_on() { + let mut s = session(); + assert!(s.prefs.new_art_basic, "on by default"); + let green = painted(&mut s, 20.0, "#ff0000", "#00ff00", 5.0); + painted(&mut s, 120.0, "#0000ff", "#000000", 2.0); + select(&mut s, &[green]); + run(&mut s, "select.none", json!({})); + let id = rect(&mut s, 220.0); + let (n, from) = (node(&s, id).appearance, node(&s, green).appearance); + assert_eq!((n.fill_paint(), n.stroke_paint(), n.stroke_width()), (from.fill_paint(), from.stroke_paint(), 5.0)); +} + +#[test] +fn with_basic_appearance_on_only_the_basic_fill_and_stroke_follow_the_selection() { + let mut s = session(); + let src = fancy(&mut s); + painted(&mut s, 120.0, "#00ff00", "#000000", 2.0); + select(&mut s, &[src]); + run(&mut s, "select.none", json!({})); + let id = rect(&mut s, 220.0); + let n = node(&s, id); + assert_eq!(n.appearance.fill_paint(), node(&s, src).appearance.fill_paint(), "the top fill"); + assert_eq!((n.appearance.items.len(), n.appearance.effects.len(), n.opacity), (2, 0, 1.0), "one fill and stroke, nothing else"); +} + +#[test] +fn a_paint_the_selected_objects_do_not_share_stays_as_it_was() { + let mut s = session(); + let a = painted(&mut s, 20.0, "#ff0000", "#00ff00", 5.0); + let b = painted(&mut s, 120.0, "#0000ff", "#00ff00", 5.0); + painted(&mut s, 220.0, "#ffff00", "#000000", 1.0); + let before = s.paint.fill.clone(); + select(&mut s, &[a, b]); + assert_eq!(s.paint.fill, before, "the fills differ"); + assert_eq!((s.paint.stroke.clone(), s.paint.stroke_width), (node(&s, a).appearance.stroke_paint(), 5.0), "the strokes are shared"); +} + +#[test] +fn art_without_paint_leaves_the_proxies_as_they_were() { + let mut s = session(); + painted(&mut s, 20.0, "#ff0000", "#00ff00", 5.0); + let (fill, stroke) = (s.paint.fill.clone(), s.paint.stroke.clone()); + let dir = super::links::Folder::new("new-art-image"); + let path = dir.file("pic.png"); + super::links::write(&path, &super::links::png(4, 4, super::links::RED)); + let image = super::links::place(&mut s, &path); + run(&mut s, "select.none", json!({})); + select(&mut s, &[image]); + assert_eq!((s.paint.fill.clone(), s.paint.stroke.clone(), s.paint.stroke_width), (fill, stroke, 5.0), "new art stays visible"); +} + +#[test] +fn an_object_without_a_stroke_clears_the_stroke_and_keeps_the_weight() { + let mut s = session(); + let bare = rect(&mut s, 20.0); + run(&mut s, "paint.setStroke", json!({"none": true})); + painted(&mut s, 120.0, "#0000ff", "#000000", 7.0); + select(&mut s, &[bare]); + assert_eq!((s.paint.stroke.clone(), s.paint.stroke_width), (Paint::None, 7.0)); +} + +#[test] +fn a_selected_gradient_loads_without_its_place_on_the_object() { + let mut s = session(); + let src = rect(&mut s, 20.0); + let stops = json!([{"offset": 0, "color": "#ffffff"}, {"offset": 1, "color": "#000000"}]); + run(&mut s, "paint.setFill", json!({"gradient": {"stops": stops, "start": [20, 40], "end": [50, 40]}})); + painted(&mut s, 120.0, "#0000ff", "#000000", 2.0); + select(&mut s, &[src]); + assert!(matches!(s.paint.fill, Paint::Gradient(_)), "{:?}", s.paint.fill); + run(&mut s, "select.none", json!({})); + let id = rect(&mut s, 220.0); + assert!(matches!(node(&s, id).appearance.fill_paint(), Paint::Gradient(_))); +} + +#[test] +fn selecting_leaves_the_stroke_panels_options_for_new_art_alone() { + let mut s = session(); + let plain = painted(&mut s, 20.0, "#ff0000", "#00ff00", 5.0); + run(&mut s, "select.none", json!({})); + run(&mut s, "stroke.set", json!({"cap": "round", "dash": [4, 2]})); + select(&mut s, &[plain]); + run(&mut s, "select.none", json!({})); + let id = rect(&mut s, 120.0); + let st = node(&s, id).appearance.stroke().unwrap().clone(); + assert_eq!((st.paint.clone(), st.width), (node(&s, plain).appearance.stroke_paint(), 5.0)); + assert_eq!((st.cap, st.dash.map(|d| d.pattern)), (LineCap::Round, Some(vec![4.0, 2.0]))); +} + +#[test] +fn a_sampled_paint_stays_until_the_selection_changes() { + let mut s = session(); + let a = painted(&mut s, 20.0, "#ff0000", "#00ff00", 5.0); + let sampled = Paint::solid(vectorcraft_color::Color::WHITE); + s.paint.fill = sampled.clone(); + run(&mut s, "transparency.set", json!({"opacity": 50})); + assert_eq!(s.paint.fill, sampled, "not a selection command"); + select(&mut s, &[a]); + assert_eq!(s.paint.fill, sampled, "the same selection again"); + run(&mut s, "select.none", json!({})); + assert_eq!(s.paint.fill, sampled, "nothing selected shows nothing to load"); + select(&mut s, &[a]); + assert_eq!(s.paint.fill, node(&s, a).appearance.fill_paint(), "selected anew"); +} + +#[test] +fn targeting_a_layer_loads_the_paints_of_its_contents() { + let mut s = session(); + let a = painted(&mut s, 20.0, "#ff0000", "#00ff00", 5.0); + let layer = s.doc().unwrap().doc.layers[0].id; + run(&mut s, "select.none", json!({})); + run(&mut s, "paint.setFill", json!({"color": "#0000ff"})); + run(&mut s, "layer.target", json!({"id": layer.0})); + assert_eq!(s.paint.fill, node(&s, a).appearance.fill_paint()); +} + #[test] fn the_option_round_trips_with_the_preferences() { let mut s = session(); diff --git a/crates/engine/src/tests/xform.rs b/crates/engine/src/tests/xform.rs index 4de6abd39..b096922e9 100644 --- a/crates/engine/src/tests/xform.rs +++ b/crates/engine/src/tests/xform.rs @@ -1024,6 +1024,19 @@ fn lasso_picks_gradient_mesh_points() { /// Shift-drag a marquee with the Selection tool (#483): the selected objects it reaches are /// deselected and the others selected, the rest of the selection staying as it was. +#[test] +fn a_selection_tool_click_gives_new_art_the_objects_fill() { + let mut s = session(); + let a = rect(&mut s, 100.0, 100.0, 50.0, 50.0); + s.execute("paint.setFill", &json!({"color": "#00ff00"})).unwrap(); + rect(&mut s, 200.0, 100.0, 50.0, 50.0); + s.execute("paint.setFill", &json!({"color": "#0000ff"})).unwrap(); + s.execute("select.none", &json!({})).unwrap(); + gesture(&mut s, "selection", &[(125.0, 125.0), (125.0, 125.0)], Mods::default()); + assert_eq!(s.doc().unwrap().selection.objects, vec![a]); + assert_eq!(s.paint.fill, s.doc().unwrap().doc.node(a).unwrap().appearance.fill_paint()); +} + #[test] fn selection_shift_marquee_toggles_objects() { let mut s = session(); diff --git a/docs/mcp.md b/docs/mcp.md index f2b9cd0f3..9b08d672f 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -4126,8 +4126,10 @@ New objects take the fill and stroke proxies (`paint.setFill` / `paint.setStroke selected) on top of a template. With nothing selected, `stroke.set` / `stroke.setAdvanced` (cap, join, dashes, arrowheads, profile…) and `graphicStyle.apply` (every fill, stroke and effect, opacity, blend mode and the link to the style; `add: true` stacks it) set that template instead of editing art. With the Appearance panel's New Art Has Basic -Appearance off (`appearance.setNewArtBasic {on: false}`, the preference `newArtBasic`) new art takes the whole -appearance of the last selected object. `appearance.newArt` reports what the next object gets; `paint.default` +Appearance on (the default), a `select.*` command or `layer.target` that changes the selection loads the proxies and +the weight from it: only a paint or weight the selected objects share, and art without paint (a placed image) changes +nothing. With the option off (`appearance.setNewArtBasic {on: false}`, the preference `newArtBasic`) new art takes the +whole appearance of the last selected object. `appearance.newArt` reports what the next object gets; `paint.default` resets it. `appearance.setNewArtBasic` rejects an `on` value that is not a boolean. Omitting `on` toggles the current preference.