From af74427ef448507ac760c79899f61ae6e3c123d3 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 4 Nov 2026 12:36:56 -0800 Subject: [PATCH] Appearance: edits to the active row reach the matching row in each selected object Paint and effect edits matched the active row across all selected objects at once, so one object without an equal row made every object edit the row at the raw index. ItemTarget::per_target and effect_targets now match each object against the first one on its own, as items_per_target does. effect_targets also looks up the row in the stack it belongs to when deciding whether an effect edit is object-level, so an effect on a row that sits lower in another object's stack no longer fails with "no appearance item". --- crates/engine/src/cmd/appearance.rs | 26 +++++++------ crates/engine/src/cmd/effectcmd.rs | 24 ++++++------ crates/engine/src/tests/appearance.rs | 53 +++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 8fb5734ef..6507001c7 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -276,22 +276,24 @@ impl ItemTarget { /// This target for each of `ids` (in their order): an aligned item (a row of the first /// target's stack, or for the active item of the first selected object's, see /// [`ItemTarget::anchor`]) is, in each other target, the item equal to it there (see - /// [`SharedStack`]); `None` for a target that has none, which the edit leaves alone. A row the - /// targets don't share, and any other target, stands for itself in every target. + /// [`SharedStack`]), each target matched against that stack on its own, whatever the other + /// targets have. In a target without an equal item, and for any other target, the row stands + /// for itself. Every entry is `Some`. pub(crate) fn per_target(self, s: &Session, ids: &[NodeId]) -> Vec> { let ItemTarget::Item { index, explicit, aligned: true } = self else { return vec![Some(self); ids.len()] }; let Some(st) = s.active().filter(|_| ids.len() >= 2) else { return vec![Some(self); ids.len()] }; let k = self.anchor(s, ids); - let mut order = ids.to_vec(); - order.swap(0, k); - let Some(shared) = shared_stack(&st.doc, &order) else { return vec![Some(self); ids.len()] }; - if !shared.has_item(index) { - return vec![Some(self); ids.len()]; - } - let mut per: Vec> = - (0..order.len()).map(|j| shared.item_in(index, j).map(|index| ItemTarget::Item { index, explicit, aligned: true })).collect(); - per.swap(0, k); - per + let Some(first) = ids.get(k).and_then(|id| st.doc.node(*id)) else { return vec![Some(self); ids.len()] }; + ids.iter() + .enumerate() + .map(|(j, id)| match st.doc.node(*id) { + Some(n) if j != k => { + let at = SharedStack::of(&[&*first.appearance, &*n.appearance]).item_in(index, 1); + Some(at.map_or(self, |index| ItemTarget::Item { index, explicit, aligned: true })) + } + _ => Some(self), + }) + .collect() } /// Where among `ids` (the targets, in paint order) the object whose stack this item is a row of diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index 54d0176a3..75ffc5021 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -8,11 +8,12 @@ use std::sync::Arc; use serde_json::{Value, json}; +use vectorcraft_doc::appearance::SharedStack; use vectorcraft_doc::{Document, Effect, Node, NodeKind, StrokeLayer}; use vectorcraft_geom::{FillRule, PathData}; use vectorcraft_render::effects; -use super::appearance::{ItemTarget, aligned, appearance_targets, index_param, item_indices, item_target, item_target_at, shared_stack}; +use super::appearance::{ItemTarget, aligned, appearance_targets, index_param, item_indices, item_target, item_target_at}; use super::expand::has_seeded; use super::rasterfx; use super::{CommandSpec, EngineError, NodeId, Result, Session, always, bad, cmd, fileio, has_doc, str_param}; @@ -105,24 +106,23 @@ fn ids_json(ids: &[NodeId]) -> Value { /// Each target of an effect edit aimed at `item` and effect `index`: the object, the item whose /// effects it edits (`None`: the object's own) and the effect's index there. With `aligned` (see /// [`ItemTarget::per_target`]) `item` and `index` are the first target's, and each other target -/// edits its equal item, or its equal object-level effect; a target without them is left out. +/// edits its equal item, or its equal object-level effect, each target matched on its own whatever +/// the other targets have; in a target without them, `item` and `index` stand for themselves. pub(crate) fn effect_targets(s: &Session, p: &Value, item: ItemTarget, index: usize, cmd: &str) -> Result, usize)>> { let roots = appearance_targets(s, p)?; let d = &s.doc()?.doc; let per = item.per_target(s, &roots); - let shared = if aligned(p) { shared_stack(d, &roots) } else { None }; + let first = roots.first().and_then(|f| d.node(*f)); + // Whether `item` names an item in the stack it is a row of (else the edit is object-level there). + let anchor = roots.get(item.anchor(s, &roots)).and_then(|a| d.node(*a)); + let anchor_level = anchor.map(|a| item.effects_item(&a.appearance, cmd)).transpose()?.flatten(); let mut out = Vec::with_capacity(roots.len()); - for (k, (id, item)) in roots.iter().zip(per).enumerate() { + for (id, item) in roots.iter().zip(per) { let (Some(item), Some(n)) = (item, d.node(*id)) else { continue }; let at = item.effects_item(&n.appearance, cmd)?; - // An object-level effect the targets share is, in each, where it is there. - let first = roots.first().and_then(|f| d.node(*f)); - let first_level = first.map(|f| item.effects_item(&f.appearance, cmd)).transpose()?.flatten(); - let index = match (&shared, at, first_level) { - (Some(sh), None, None) if sh.has_effect(index) => match sh.effect_in(index, k) { - Some(i) => i, - None => continue, - }, + // An object-level effect this target shares with the first is where it is in this target. + let index = match (first.filter(|_| aligned(p)), at, anchor_level) { + (Some(f), None, None) => SharedStack::of(&[&*f.appearance, &*n.appearance]).effect_in(index, 1).unwrap_or(index), _ => index, }; out.push((*id, at, index)); diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index 9bed4eb34..ada5fdd2f 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -554,6 +554,49 @@ fn add_fill_copies_the_active_row_in_each_object_that_has_it() { } } +/// A paint edit of the active row changes the equal row of each object that has one, even when +/// another selected object has none; that object's row stands for itself, as with only it and the +/// first selected object selected. +#[test] +fn paint_edits_change_the_active_row_in_each_object_that_has_it() { + let mut s = session(); + let [a, b, c] = [rect(&mut s, 50.0), rect(&mut s, 200.0), rect(&mut s, 300.0)]; + run(&mut s, "select.set", json!({"ids": [a.0, b.0, c.0]})); + run(&mut s, "appearance.addFill", json!({})); + run(&mut s, "appearance.setItem", json!({"index": 0, "color": "#00ff00"})); + run(&mut s, "appearance.setItem", json!({"index": 2, "color": "#ff0000"})); // [Fill green, Stroke, Fill red] + // b gets a yellow fill below the others: [Fill yellow, Fill green, Stroke, Fill red]. + run(&mut s, "appearance.addFill", json!({"ids": [b.0]})); + run(&mut s, "appearance.moveItem", json!({"ids": [b.0], "from": 3, "to": 0})); + run(&mut s, "appearance.setItem", json!({"ids": [b.0], "index": 0, "color": "#ffff00"})); + run(&mut s, "appearance.setItem", json!({"ids": [c.0], "index": 0, "color": "#0000ff"})); // [Fill blue, Stroke, Fill red] + run(&mut s, "select.set", json!({"ids": [a.0, b.0, c.0]})); + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + run(&mut s, "paint.setFill", json!({"color": "#ff00ff"})); + let (na, nb, nc) = (node(&s, a), node(&s, b), node(&s, c)); + assert_eq!(fill_paint(&na, 0).color().unwrap().to_hex(), "#ff00ff"); + assert_eq!(fill_paint(&nb, 1).color().unwrap().to_hex(), "#ff00ff", "{:?}", nb.appearance); + assert_eq!(fill_paint(&nb, 0).color().unwrap().to_hex(), "#ffff00", "{:?}", nb.appearance); + assert_eq!(fill_paint(&nc, 0).color().unwrap().to_hex(), "#ff00ff", "{:?}", nc.appearance); +} + +/// An object-level effect is removed from each object that has an equal one, even when another +/// selected object has none. +#[test] +fn removing_an_effect_reaches_each_object_that_has_it() { + let mut s = session(); + let [a, b, c] = [rect(&mut s, 50.0), rect(&mut s, 200.0), rect(&mut s, 300.0)]; + run(&mut s, "effect.apply", json!({"ids": [a.0], "effect": "stylize.feather", "item": null})); + run(&mut s, "effect.apply", json!({"ids": [a.0, b.0], "effect": "distort.twist", "item": null})); + // a has [feather, twist], b [twist], c none. + run(&mut s, "select.set", json!({"ids": [a.0, b.0, c.0]})); + run(&mut s, "effect.remove", json!({"index": 1, "item": null, "aligned": true})); + let ids = |id: NodeId| node(&s, id).appearance.effects.iter().map(|e| e.id.clone()).collect::>(); + assert_eq!(ids(a), ["stylize.feather"]); + assert!(ids(b).is_empty(), "b loses its twist: {:?}", ids(b)); + assert!(ids(c).is_empty()); +} + /// b `[Fill red, Fill white, Stroke]` above a `[Fill white, Stroke]`, selected b first: b's rows 1 /// and 2 equal a's rows 0 and 1. fn reversed_pair(s: &mut Session) -> (NodeId, NodeId) { @@ -916,6 +959,16 @@ fn shared_effects_and_new_fills_reach_every_selected_object() { assert!(s.execute("appearance.moveItem", &json!({"from": 1, "to": 0, "aligned": true})).is_err()); } +/// An effect applied to the first object's stroke row goes on the equal stroke of the other +/// object, which sits lower in a longer stack there. +#[test] +fn an_effect_on_a_shared_row_reaches_it_at_another_index() { + let mut s = session(); + let (a, b) = mixed_circles(&mut s, true); + run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 1, "aligned": true})); + assert_eq!((stroke(&node(&s, a), 1).effects.len(), stroke(&node(&s, b), 2).effects.len()), (1, 1)); +} + /// Rows deleted together, one shared (the stroke) and one not (a's fill): each object loses its /// own copy of the shared row, and the other object keeps the items at those indices of a's stack. #[test]