Merge pull request #6267 from slafebre/fix/6252-new-art-follows-selection

Fix #6252: selecting an object gives new art its fill, stroke and weight

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<CommandSpec> {
             "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<Value> {

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 }))?;
@@ -150,6 +153,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)

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::<Vec<_>>()}));
+}
+
+#[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();

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();

docs/mcp.md

@@ -4154,8 +4154,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.