Appearance: shared rows map one by one, and effect dialogs edit each copy (#4174)

Review follow-ups:

- items_per_target maps each requested row on its own. Rows deleted or
  duplicated together, one shared and one not, fell back to the literal
  indices in every object, so the other objects could lose the wrong
  items. The first object takes its own rows; each other object takes
  its copies of the shared ones only.
- effect.dialog takes `aligned` (the panel passes it on a mixed
  selection's shared effect row), and its OK and preview run
  effect.setParams with it, so the dialog edits each object's own copy
  of the effect instead of the first object's index in every object.

Tests: removing a shared stroke with an unshared fill leaves the other
object's fills; an aligned dialog's command carries `aligned`.

crates/doc/src/appearance/tests.rs

@@ -326,11 +326,8 @@ fn mixed_appearances_share_their_equal_rows() {
     assert_eq!(shared.items, vec![vec![1, 1]], "the stroke, not the fills");
     assert_eq!(shared.effects, vec![vec![0, 0]]);
     // The stroke under an extra fill in the second object.
-    let c = Appearance {
-        items: vec![fill(Color::BLACK), fill(Color::WHITE), stroke.clone()],
-        effects: vec![feather.clone()],
-        ..Appearance::default()
-    };
+    let c =
+        Appearance { items: vec![fill(Color::BLACK), fill(Color::WHITE), stroke.clone()], effects: vec![feather.clone()], ..Appearance::default() };
     let shared = SharedStack::of(&[&a, &c]);
     assert_eq!(shared.items, vec![vec![1, 2]]);
     assert_eq!(shared.item_in(1, 1), Some(2));

crates/engine/src/cmd/appearance.rs

@@ -313,12 +313,13 @@ pub(crate) fn appearances_differ(d: &vectorcraft_doc::Document, ids: &[NodeId])
     aps.any(|a| a != first)
 }
 
-/// Indices `idx` of the first target's items (aligned with `aligned`) in each of `ids`: where
-/// each target has the item equal to the first's (see [`SharedStack`]), else the same indices.
-/// A target missing one of them gets only those it has.
+/// Indices `idx` of the first target's items (aligned with `aligned`) in each of `ids`: the first
+/// target's own, and in each other one the items equal to them there (see [`SharedStack`]), each
+/// index on its own: one the targets don't share is left out of the others (never taken as the
+/// same index there). Not aligned, or one target: the same indices in each.
 pub(crate) fn items_per_target(d: &vectorcraft_doc::Document, ids: &[NodeId], idx: &[usize], aligned: bool) -> Vec<Vec<usize>> {
-    match shared_stack(d, ids).filter(|sh| aligned && idx.iter().all(|i| sh.has_item(*i))) {
-        Some(sh) => (0..ids.len()).map(|k| idx.iter().filter_map(|i| sh.item_in(*i, k)).collect()).collect(),
+    match shared_stack(d, ids).filter(|_| aligned) {
+        Some(sh) => (0..ids.len()).map(|k| if k == 0 { idx.to_vec() } else { idx.iter().filter_map(|i| sh.item_in(*i, k)).collect() }).collect(),
         None => vec![idx.to_vec(); ids.len()],
     }
 }

crates/engine/src/tests/appearance.rs

@@ -585,3 +585,16 @@ fn shared_effects_and_new_fills_reach_every_selected_object() {
     // Rows of different stacks aren't reordered together.
     assert!(s.execute("appearance.moveItem", &json!({"from": 1, "to": 0, "aligned": true})).is_err());
 }
+
+/// 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]
+fn removing_shared_and_unshared_rows_removes_only_the_shared_ones_elsewhere() {
+    let mut s = session();
+    let (a, b) = mixed_circles(&mut s, true);
+    // b is [Fill, Fill, Stroke]; a's rows 0 (its fill) and 1 (the shared stroke).
+    let fills_b: Vec<_> = node(&s, b).appearance.items.iter().take(2).cloned().collect();
+    run(&mut s, "appearance.removeItem", json!({"indices": [0, 1], "aligned": true}));
+    assert!(node(&s, a).appearance.items.is_empty(), "a loses both");
+    assert_eq!(node(&s, b).appearance.items, fills_b, "b loses only its stroke, keeping both fills");
+}

crates/ui-egui/src/dialogs/effect.rs

@@ -74,6 +74,10 @@ pub fn open(app: &mut VectorcraftApp, p: &Value) -> Result<Value, String> {
     if let Some(item) = p.get("item") {
         fields.insert("__item".into(), item.clone());
     }
+    // A shared row of a mixed selection: OK edits each object's own copy of the effect.
+    if p.get("aligned").and_then(Value::as_bool) == Some(true) {
+        fields.insert("__aligned".into(), json!(true));
+    }
     match p.get("index").and_then(Value::as_u64) {
         Some(k) => {
             let e = applied.get(k as usize).ok_or_else(|| format!("no effect at index {k}"))?;
@@ -737,6 +741,9 @@ fn command(d: &Dialog) -> (&'static str, Value) {
         if super::warp_effect::is_warp(&id) && d.bool("__warp_style_changed") {
             p["warpStyle"] = json!(id.strip_prefix("warp."));
         }
+        if d.bool("__aligned") {
+            p["aligned"] = json!(true);
+        }
         return ("effect.setParams", p);
     }
     let mut p = json!({"effect": d.str("__effect"), "params": params});
@@ -766,6 +773,27 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result<Value, String> {
 mod tests {
     use super::*;
 
+    /// An applied effect's dialog opened on a shared row of a mixed selection (`aligned`) edits
+    /// each object's own copy on OK; opened otherwise it edits the index as given.
+    #[test]
+    fn a_shared_rows_dialog_edits_each_objects_copy() {
+        let dialog = |aligned: bool| {
+            let mut fields = Map::new();
+            fields.insert("__effect".into(), json!("stylize.feather"));
+            fields.insert("__index".into(), json!(0));
+            fields.insert("__item".into(), Value::Null);
+            fields.insert("radius".into(), json!(8));
+            if aligned {
+                fields.insert("__aligned".into(), json!(true));
+            }
+            Dialog { kind: "effect".into(), fields }
+        };
+        let (cmd, p) = command(&dialog(true));
+        assert_eq!((cmd, &p["aligned"], &p["params"]["radius"]), ("effect.setParams", &json!(true), &json!(8)));
+        assert!(p["params"].get("__aligned").is_none());
+        assert!(command(&dialog(false)).1.get("aligned").is_none());
+    }
+
     #[test]
     fn fields_that_pick_a_value_are_dropdowns_of_its_values() {
         // The values come from the effect's parameter documentation (`joins: "miter"|"round"|…`).

crates/ui-egui/src/menus/ui_commands.rs

@@ -506,7 +506,7 @@ pub const UI_COMMANDS: &[(&str, &str, &str, &str)] = &[
         "effect.dialog",
         "Effect…",
         "",
-        "{effect: id, index?: int (edit that applied effect in place, prefilled; OK runs effect.setParams), item?: appearance item index|null (the fill/stroke whose effects, null the object's; default: the Appearance panel's active item)} open the effect's dialog with live preview; without `index`, an effect already in that list first opens `effectExists` (confirm edits it, discard adds another) → {pending: \"effectExists\"}",
+        "{effect: id, index?: int (edit that applied effect in place, prefilled; OK runs effect.setParams), item?: appearance item index|null (the fill/stroke whose effects, null the object's; default: the Appearance panel's active item), aligned?: bool (index and item are rows of the first selected object's stack, which OK edits in each object where it has them, as effect.setParams does)} open the effect's dialog with live preview; without `index`, an effect already in that list first opens `effectExists` (confirm edits it, discard adds another) → {pending: \"effectExists\"}",
     ),
     ("ui.paramDialog", "Command Dialog", "", "{command, label?, params} open a parameter dialog for any command"),
     (

crates/ui-egui/src/panels/appearance.rs

@@ -774,7 +774,7 @@ fn effect_row(app: &mut VectorcraftApp, ui: &mut Ui, item: Option<usize>, k: usi
     if link(ui, pos2(lx, r.center().y), ("ap-fx-link", item, k), &shown_effect_label(&e.id)) {
         select_row(app, ui.ctx(), this);
         if has_options(&e.id) {
-            app.run("effect.dialog", json!({"effect": e.id, "index": k, "item": item})).ok();
+            app.run("effect.dialog", aligned(app, json!({"effect": e.id, "index": k, "item": item}))).ok();
         } else {
             set_pstate(ui.ctx(), &open_key, !open);
         }
@@ -916,7 +916,7 @@ fn effect_editor(app: &mut VectorcraftApp, ui: &mut Ui, item: Option<usize>, k:
         app.run("effect.setParams", aligned(app, json!({"item": item, "index": k, "params": {key: v}}))).ok();
     }
     if edit {
-        app.run("effect.dialog", json!({"effect": e.id, "index": k, "item": item})).ok();
+        app.run("effect.dialog", aligned(app, json!({"effect": e.id, "index": k, "item": item}))).ok();
     }
 }