From dc8808fe5ef1b75fbd4a829cb5174cb68b123722 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 3 Nov 2026 10:01:03 -0800 Subject: [PATCH] Read compound effects when opening effect dialogs --- crates/engine/src/cmd/appearance.rs | 17 ++ crates/ui-egui/src/dialogs/effect.rs | 30 +++- crates/ui-egui/src/panels/tests_appearance.rs | 158 ++++++++++++++++++ crates/ui-egui/src/panels/tests_effectedit.rs | 8 +- 4 files changed, 203 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index dc83d24bd..92ee0c03a 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -178,6 +178,23 @@ pub(crate) struct ActiveItem { } impl Session { + /// The method returns explicit target parameters and the selected appearance effect list. + pub fn appearance_effect_target(&self, params: &Value) -> Option<(Value, &[vectorcraft_doc::Effect])> { + let mut ids = appearance_targets(self, params).ok()?; + let item = item_target(self, params, "effect.apply").ok()?; + let anchor = item.anchor(self, &ids); + let node = self.active()?.doc.node(*ids.get(anchor)?)?; + let at = item.effects_item(&node.appearance, "effect.apply").ok()?; + let effects = node.appearance.effects_at(at)?; + ids.swap(0, anchor); + let ids: Vec = ids.into_iter().map(|id| id.0).collect(); + let aligned = match item { + ItemTarget::Item { aligned, .. } => aligned, + ItemTarget::Top => aligned(params), + }; + Some((json!({"ids": ids, "item": at, "aligned": aligned}), effects.as_slice())) + } + /// The Appearance panel's active fill/stroke: a paint-order index into the first selected /// object's stack, while the selection it was chosen for is unchanged. pub fn appearance_item(&self) -> Option { diff --git a/crates/ui-egui/src/dialogs/effect.rs b/crates/ui-egui/src/dialogs/effect.rs index 85282add1..15d0b7004 100644 --- a/crates/ui-egui/src/dialogs/effect.rs +++ b/crates/ui-egui/src/dialogs/effect.rs @@ -43,6 +43,7 @@ fn heading_label(d: &Dialog) -> String { /// Fields: `__effect`, `__label`, `__item?` (as given to `effect.dialog`), `__target` and /// `__index` (the applied effect's item and position). OK edits it; `discard: true` then OK adds /// a new one. +/// `__editTarget` stores the targets used when editing the applied effect. pub const EXISTS: &str = "effectExists"; pub(super) const EXISTS_SPEC: DialogSpec = DialogSpec { @@ -67,7 +68,8 @@ pub fn open(app: &mut VectorcraftApp, p: &Value) -> Result { // The list the effect goes to: `item` as given, else the active item's (the object's without // one). A new effect keeps `item` unresolved, so `effect.apply` targets each object as usual. let target = p.get("item").cloned().unwrap_or_else(|| json!(app.session.appearance_item())); - let applied = target_effects(app, &target); + let (edit_target, applied) = target_effects(app, p); + let target = edit_target.get("item").cloned().unwrap_or(target); let mut fields = Map::new(); fields.insert("__effect".into(), json!(id)); fields.insert("__label".into(), json!(info.label.trim_end_matches('…'))); @@ -85,12 +87,14 @@ pub fn open(app: &mut VectorcraftApp, p: &Value) -> Result { return Err(format!("effect {k} is `{}`, not `{id}`", e.id)); } fields.insert("__item".into(), target); + fields.insert("__editTarget".into(), edit_target); fields.insert("__index".into(), json!(k)); fields.extend(params_of(e)); } None => { if let Some(k) = applied.iter().rposition(|e| e.id == id) { fields.insert("__target".into(), target); + fields.insert("__editTarget".into(), edit_target); fields.insert("__index".into(), json!(k)); app.ui.dialog = Some(Dialog { kind: EXISTS.into(), fields }); return Ok(json!({ "pending": EXISTS })); @@ -106,11 +110,14 @@ pub fn open(app: &mut VectorcraftApp, p: &Value) -> Result { Ok(Value::Null) } -/// The function reads effects at `item` (an index or null) from the targeted node or first selected object. -fn target_effects(app: &VectorcraftApp, item: &Value) -> Vec { - let Some(st) = app.session.active() else { return vec![] }; - let node = st.selection.subjects().first().and_then(|id| st.doc.node(*id)); - node.and_then(|n| n.appearance.effects_at(item.as_u64().map(|i| i as usize))).cloned().unwrap_or_default() +/// The function returns target parameters and effects at `item` (an index or null). +/// An omitted `item` uses the active row. +fn target_effects(app: &VectorcraftApp, p: &Value) -> (Value, Vec) { + let mut params = json!({"aligned": p.get("aligned").and_then(Value::as_bool).unwrap_or(false)}); + if let Some(item) = p.get("item") { + params["item"] = item.clone(); + } + app.session.appearance_effect_target(¶ms).map(|(target, effects)| (target, effects.to_vec())).unwrap_or_else(|| (json!({}), vec![])) } /// An applied effect's values over its defaults. @@ -128,6 +135,7 @@ fn confirm_exists(app: &mut VectorcraftApp, d: &Dialog) -> Result let target = fields.remove("__target").unwrap_or(Value::Null); if d.bool("discard") { fields.remove("__index"); + fields.remove("__editTarget"); let id = d.str("__effect"); let defaults = vectorcraft_effects::default_params(&id); fields.extend(defaults.and_then(|v| v.as_object().cloned()).unwrap_or_default()); @@ -136,7 +144,10 @@ fn confirm_exists(app: &mut VectorcraftApp, d: &Dialog) -> Result } } else { let k = fields.get("__index").and_then(Value::as_u64).unwrap_or(0) as usize; - fields.extend(target_effects(app, &target).get(k).map(params_of).unwrap_or_default()); + let edit_target = fields.get("__editTarget").cloned().unwrap_or_else(|| json!({"item": target})); + let applied = app.session.appearance_effect_target(&edit_target).map(|(_, effects)| effects.to_vec()).unwrap_or_default(); + let params = applied.get(k).map(params_of).unwrap_or_default(); + fields.extend(params); fields.insert("__item".into(), target); } fields.insert("preview".into(), json!(true)); @@ -804,7 +815,10 @@ fn command(d: &Dialog) -> (&'static str, Value) { let params = form::params(d); let item = d.fields.get("__item").cloned(); if let Some(index) = d.fields.get("__index") { - let mut p = json!({"index": index, "item": item.unwrap_or(Value::Null), "params": params}); + let mut p = d.fields.get("__editTarget").filter(|p| p.is_object()).cloned().unwrap_or_else(|| json!({})); + p["index"] = index.clone(); + p["item"] = item.unwrap_or(Value::Null); + p["params"] = params; let id = d.str("__effect"); // The displayed style comes from the first target. Preserve other targets' Warp IDs // unless the user deliberately chooses Style, including after a preview rollback. diff --git a/crates/ui-egui/src/panels/tests_appearance.rs b/crates/ui-egui/src/panels/tests_appearance.rs index dc56d99e8..ba4eebdda 100644 --- a/crates/ui-egui/src/panels/tests_appearance.rs +++ b/crates/ui-egui/src/panels/tests_appearance.rs @@ -870,6 +870,164 @@ fn transform_effect_name_edits_the_displayed_appearance_target() { } } +#[test] +fn transform_effect_dialog_reads_the_selected_compound_members_owner() { + for item in [None, Some(0)] { + let mut app = app_with_rect(); + let member = app.session.doc().unwrap().selection.subjects()[0]; + let other = app.run("shape.rectangle", json!({"x": 30, "y": 30, "width": 20, "height": 20})).unwrap()["id"].clone(); + app.run("select.set", json!({"ids": [member.0, other]})).unwrap(); + let compound = app.run("object.compoundPath.make", json!({})).unwrap()["id"].as_u64().unwrap(); + app.run("effect.apply", json!({"effect": "distort.transform", "item": item, "params": {"moveH": 12, "copies": 2}})).unwrap(); + app.run("select.set", json!({"ids": [member.0]})).unwrap(); + app.run("effect.dialog", json!({"effect": "distort.transform", "item": item, "index": 0})).unwrap(); + let d = app.ui.dialog.as_mut().unwrap(); + assert_eq!(d.f64("moveH", 0.0), 12.0); + assert_eq!(d.f64("copies", 0.0), 2.0); + d.fields.insert("moveH".into(), json!(35)); + crate::tests_labels::painted_text(&mut app, |app, ui| crate::dialogs::show(app, ui.ctx())); + crate::dialogs::confirm(&mut app).unwrap(); + let effects = &app.session.doc().unwrap().doc.node(vectorcraft_doc::NodeId(compound)).unwrap().appearance; + assert_eq!(effects.effects_at(item).unwrap()[0].params["moveH"].as_f64(), Some(35.0)); + assert_eq!(effects.effects_at(item).unwrap()[0].params["copies"].as_f64(), Some(2.0)); + app.run("edit.undo", json!({})).unwrap(); + app.run("effect.dialog", json!({"effect": "distort.transform", "item": item})).unwrap(); + assert_eq!(app.ui.dialog.as_ref().unwrap().kind, "effectExists"); + crate::dialogs::confirm(&mut app).unwrap(); + let d = app.ui.dialog.as_mut().unwrap(); + assert_eq!(d.f64("moveH", 0.0), 12.0); + d.fields.insert("moveH".into(), json!(60)); + crate::tests_labels::painted_text(&mut app, |app, ui| crate::dialogs::show(app, ui.ctx())); + crate::dialogs::cancel(&mut app); + let effects = &app.session.doc().unwrap().doc.node(vectorcraft_doc::NodeId(compound)).unwrap().appearance; + assert_eq!(effects.effects_at(item).unwrap()[0].params["moveH"].as_f64(), Some(12.0)); + } +} + +#[test] +fn transform_effect_dialog_keeps_the_active_items_selection_anchor() { + for item in [None, Some(json!(0)), Some(Value::Null)] { + let mut app = app_with_rect(); + let back = app.session.doc().unwrap().selection.subjects()[0]; + for at in [json!(0), Value::Null] { + app.run("effect.apply", json!({"effect": "distort.transform", "item": at, "params": {"moveH": 99}})).unwrap(); + } + let front = app.run("shape.rectangle", json!({"x": 30, "y": 30, "width": 20, "height": 20})).unwrap()["id"].clone(); + for at in [json!(0), Value::Null] { + app.run("effect.apply", json!({"effect": "distort.transform", "item": at, "params": {"moveH": 12}})).unwrap(); + } + app.run("select.set", json!({"ids": [front, back.0]})).unwrap(); + app.run("appearance.setActiveItem", json!({"index": 0})).unwrap(); + let mut p = json!({"effect": "distort.transform"}); + if let Some(item) = &item { + p["item"] = item.clone(); + } + app.run("effect.dialog", p).unwrap(); + assert_eq!(app.ui.dialog.as_ref().unwrap().kind, "effectExists"); + crate::dialogs::confirm(&mut app).unwrap(); + let expected = if item.is_none() { 12.0 } else { 99.0 }; + assert_eq!(app.ui.dialog.as_ref().unwrap().f64("moveH", 0.0), expected); + } +} + +#[test] +fn explicit_effect_items_use_paint_order_with_a_selected_compound_member() { + let mut app = app_with_rect(); + let back = app.session.doc().unwrap().selection.subjects()[0]; + for item in [json!(0), Value::Null] { + app.run("effect.apply", json!({"effect": "distort.transform", "item": item, "params": {"moveH": 99}})).unwrap(); + } + let member = app.run("shape.rectangle", json!({"x": 20, "y": 20, "width": 80, "height": 80})).unwrap()["id"].clone(); + let inner = app.run("shape.rectangle", json!({"x": 40, "y": 40, "width": 20, "height": 20})).unwrap()["id"].clone(); + app.run("select.set", json!({"ids": [member, inner]})).unwrap(); + app.run("object.compoundPath.make", json!({})).unwrap(); + for item in [json!(0), Value::Null] { + app.run("effect.apply", json!({"effect": "distort.transform", "item": item, "params": {"moveH": 12}})).unwrap(); + } + app.run("select.set", json!({"ids": [member, back.0]})).unwrap(); + for item in [json!(0), Value::Null] { + app.run("effect.dialog", json!({"effect": "distort.transform", "item": item, "index": 0})).unwrap(); + assert_eq!(app.ui.dialog.as_ref().unwrap().f64("moveH", 0.0), 99.0); + crate::dialogs::cancel(&mut app); + } +} + +#[test] +fn editing_an_active_effect_keeps_aligned_rows_in_mixed_stacks() { + let mut app = app_with_rect(); + let back = app.session.doc().unwrap().selection.subjects()[0]; + app.run("appearance.addStroke", json!({})).unwrap(); + app.run("appearance.moveItem", json!({"from": 2, "to": 0})).unwrap(); + app.run("effect.apply", json!({"effect": "distort.transform", "item": 0, "params": {"moveH": 99}})).unwrap(); + app.run("effect.apply", json!({"effect": "distort.transform", "item": 1, "params": {"moveH": 12}})).unwrap(); + let front = app.run("shape.rectangle", json!({"x": 30, "y": 30, "width": 20, "height": 20})).unwrap()["id"].as_u64().unwrap(); + app.run("effect.apply", json!({"effect": "distort.transform", "item": 0, "params": {"moveH": 12}})).unwrap(); + app.run("select.set", json!({"ids": [front, back.0]})).unwrap(); + app.run("appearance.setActiveItem", json!({"index": 0})).unwrap(); + app.run("effect.dialog", json!({"effect": "distort.transform", "index": 0})).unwrap(); + let d = app.ui.dialog.as_mut().unwrap(); + assert_eq!(d.f64("moveH", 0.0), 12.0); + d.fields.insert("moveH".into(), json!(35)); + app.run("appearance.setActiveItem", json!({"index": null})).unwrap(); + crate::tests_labels::painted_text(&mut app, |app, ui| crate::dialogs::show(app, ui.ctx())); + crate::dialogs::confirm(&mut app).unwrap(); + let doc = &app.session.doc().unwrap().doc; + let front = &doc.node(vectorcraft_doc::NodeId(front)).unwrap().appearance; + let back = &doc.node(back).unwrap().appearance; + assert_eq!(front.effects_at(Some(0)).unwrap()[0].params["moveH"], 35); + assert_eq!(back.effects_at(Some(1)).unwrap()[0].params["moveH"], 35); + assert_eq!(back.effects_at(Some(0)).unwrap()[0].params["moveH"], 99); +} + +#[test] +fn editing_an_effect_keeps_its_original_item_when_the_active_row_changes() { + for prompt in [false, true] { + let mut app = app_with_rect(); + let id = app.session.doc().unwrap().selection.subjects()[0]; + app.run("effect.apply", json!({"effect": "distort.transform", "item": null, "params": {"moveH": 99}})).unwrap(); + app.run("effect.apply", json!({"effect": "distort.transform", "item": 0, "params": {"moveH": 12}})).unwrap(); + app.run("appearance.setActiveItem", json!({"index": 0})).unwrap(); + let mut p = json!({"effect": "distort.transform"}); + if !prompt { + p["index"] = json!(0); + } + app.run("effect.dialog", p).unwrap(); + app.run("appearance.setActiveItem", json!({"index": null})).unwrap(); + if prompt { + crate::dialogs::confirm(&mut app).unwrap(); + } + let d = app.ui.dialog.as_mut().unwrap(); + assert_eq!(d.f64("moveH", 0.0), 12.0); + d.fields.insert("moveH".into(), json!(35)); + crate::tests_labels::painted_text(&mut app, |app, ui| crate::dialogs::show(app, ui.ctx())); + crate::dialogs::confirm(&mut app).unwrap(); + let appearance = &app.session.doc().unwrap().doc.node(id).unwrap().appearance; + assert_eq!(appearance.effects_at(Some(0)).unwrap()[0].params["moveH"].as_f64(), Some(35.0)); + assert_eq!(appearance.effects[0].params["moveH"].as_f64(), Some(99.0)); + } +} + +#[test] +fn new_effects_keep_implicit_item_targeting_until_confirmation() { + for prompt in [false, true] { + let mut app = app_with_rect(); + let id = app.session.doc().unwrap().selection.subjects()[0]; + app.run("effect.apply", json!({"effect": "distort.transform", "item": null, "params": {"moveH": 99}})).unwrap(); + app.run("effect.apply", json!({"effect": "distort.transform", "item": 0, "params": {"moveH": 12}})).unwrap(); + app.run("appearance.setActiveItem", json!({"index": 0})).unwrap(); + app.run("effect.dialog", json!({"effect": if prompt { "distort.transform" } else { "distort.roughen" }})).unwrap(); + app.run("appearance.setActiveItem", json!({"index": null})).unwrap(); + if prompt { + app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true)); + crate::dialogs::confirm(&mut app).unwrap(); + } + crate::dialogs::confirm(&mut app).unwrap(); + let appearance = &app.session.doc().unwrap().doc.node(id).unwrap().appearance; + assert_eq!(appearance.effects_at(Some(0)).unwrap().len(), 1); + assert_eq!(appearance.effects.len(), 2); + } +} + #[test] fn transform_effect_typed_scale_changes_the_painted_object() { for layer_target in [false, true] { diff --git a/crates/ui-egui/src/panels/tests_effectedit.rs b/crates/ui-egui/src/panels/tests_effectedit.rs index a27a60cb6..48dfdfc25 100644 --- a/crates/ui-egui/src/panels/tests_effectedit.rs +++ b/crates/ui-egui/src/panels/tests_effectedit.rs @@ -59,7 +59,10 @@ fn an_effect_name_opens_its_dialog_prefilled_and_ok_edits_it_in_place() { let texts = frame_events(&ctx, &mut app, vec![], appearance::show); click(&ctx, &mut app, text_rect(&texts, "Roughen").center(), appearance::show); let d = app.ui.dialog.clone().expect("the effect's dialog"); - assert_eq!((d.kind.as_str(), d.f64("size", 0.0), d.fields["__index"].clone()), ("effect", 9.0, json!(0))); + assert_eq!( + (d.kind.as_str(), d.f64("size", 0.0), d.fields["__index"].clone(), d.fields["__item"].clone()), + ("effect", 9.0, json!(0), Value::Null) + ); // The dialog previews; OK keeps the new value on the same effect as one undo step. let mut out = ctx.run_ui(Default::default(), |ui| crate::dialogs::show(&mut app, ui.ctx())); out.textures_delta.clear(); @@ -115,9 +118,10 @@ fn choosing_an_applied_effect_asks_to_edit_it_or_add_another() { assert_eq!(app.run("effect.dialog", json!({"effect": "distort.roughen"})).unwrap(), json!({"pending": "effectExists"})); crate::dialogs::confirm(&mut app).unwrap(); let d = app.ui.dialog.clone().unwrap(); - assert_eq!((d.kind.as_str(), d.f64("size", 0.0), d.fields["__index"].clone(), d.fields["__item"].clone()), ("effect", 9.0, json!(0), json!(0))); + assert_eq!((d.kind.as_str(), d.f64("size", 0.0), d.fields["__index"].clone()), ("effect", 9.0, json!(0))); crate::dialogs::confirm(&mut app).unwrap(); assert_eq!(effects(&app, Some(0)).len(), 1); + assert!(effects(&app, None).is_empty()); // "Add New": a fresh dialog with the defaults, which adds a second one. app.run("effect.dialog", json!({"effect": "distort.roughen"})).unwrap(); app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true));