From 1b3e7110a365bbf17cbdb7bc3929dc6810bd80cb Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sun, 4 Oct 2026 17:50:46 +0700 Subject: [PATCH] M3.21: active-row edits of the other kind stay on the painted leaves, item transparency is one undo step - A fill row no longer turns stroke edits (stroke.set, setAdvanced, paint.setStroke, gradients) into a new stroke on a selected group's own stack: an active row only stands for edits of its kind. - Removing, duplicating or moving items of other objects (`ids`) no longer re-points the active row. - transparency.set with an item touches the item only for opacity/blend (isolate or knockout alone is one undo step) and rejects unknown blend modes. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/appearance.rs | 31 ++++++++++++------ crates/engine/src/cmd/gradient.rs | 6 ++-- crates/engine/src/cmd/paint.rs | 46 +++++++++++++++------------ crates/engine/src/cmd/stroke.rs | 4 +-- crates/engine/src/tests_appearance.rs | 44 +++++++++++++++++++++++++ crates/tools/src/xform/gradient.rs | 7 ++-- crates/ui-egui/src/panels/mod.rs | 3 +- 7 files changed, 101 insertions(+), 40 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 18a00cdc..89b67604 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -119,9 +119,12 @@ impl Session { (a.index < n.appearance.items.len()).then_some(a.index) } - /// Re-point the active item after its stack changed (`None` drops it). - fn remap_appearance_item(&mut self, f: impl FnOnce(usize) -> Option) { - if let Some(a) = &mut self.active_appearance_item { + /// Re-point the active item after an edit of `ids` changed its stack (`None` drops it). Edits + /// of other objects leave it alone. + fn remap_appearance_item(&mut self, ids: &[NodeId], f: impl FnOnce(usize) -> Option) { + if let Some(a) = &mut self.active_appearance_item + && a.objects.first().is_some_and(|o| ids.contains(o)) + { match f(a.index) { Some(i) => a.index = i, None => self.active_appearance_item = None, @@ -163,6 +166,15 @@ impl ItemTarget { } } + /// The target of a fill (`fill`) or stroke edit: the panel's active item stands only for edits + /// of its own kind, so a fill row leaves stroke edits on the topmost stroke of the painted + /// leaves (as without an active item) and vice versa. Explicit items are kept (and checked). + pub(crate) fn of_kind(self, s: &Session, fill: bool) -> Self { + let ItemTarget::Item { index, explicit: false } = self else { return self }; + let fits = s.active().and_then(|st| st.doc.node(*st.selection.objects.first()?)?.appearance.items.get(index).map(|it| it.is_fill() == fill)); + if fits == Some(true) { self } else { ItemTarget::Top } + } + /// The item of `ap` a fill (`fill`) or stroke edit changes; `None` = the topmost one. pub(crate) fn resolve(self, ap: &Appearance, fill: bool, cmd: &str) -> Result> { match self { @@ -219,13 +231,14 @@ pub(crate) fn edit_items( }) } -/// Whether an edit aimed at `item` on `ids` changes a stroke: the `stroke` param, else the kind of -/// the targeted item (on the first target), else `default`. -pub(crate) fn edits_stroke(s: &Session, p: &Value, item: ItemTarget, ids: &[NodeId], default: bool) -> Result { +/// Whether an edit aimed at `item` changes a stroke: the `stroke` param, else the kind of the +/// targeted item (on the first target object), else `default`. +pub(crate) fn edits_stroke(s: &Session, p: &Value, item: ItemTarget, default: bool) -> Result { if let Some(b) = p.get("stroke").and_then(Value::as_bool) { return Ok(b); } let ItemTarget::Item { index, .. } = item else { return Ok(default) }; + let ids = appearance_targets(s, p)?; let d = &s.doc()?.doc; Ok(ids.first().and_then(|id| d.node(*id)?.appearance.items.get(index)).map_or(default, |it| !it.is_fill())) } @@ -362,7 +375,7 @@ fn remove_item(s: &mut Session, p: &Value) -> Result { } Ok(()) })?; - s.remap_appearance_item(|a| match a.cmp(&idx) { + s.remap_appearance_item(&ids, |a| match a.cmp(&idx) { std::cmp::Ordering::Less => Some(a), std::cmp::Ordering::Equal => None, std::cmp::Ordering::Greater => Some(a - 1), @@ -384,7 +397,7 @@ fn duplicate_item(s: &mut Session, p: &Value) -> Result { } if any { Ok(()) } else { Err(bad("appearance.duplicateItem", format!("no item at index {idx}"))) } })?; - s.remap_appearance_item(|a| Some(if a > idx { a + 1 } else { a })); + s.remap_appearance_item(&ids, |a| Some(if a > idx { a + 1 } else { a })); ok() } @@ -408,7 +421,7 @@ fn move_item(s: &mut Session, p: &Value) -> Result { Ok(()) })?; // The moved row stays the active one; the rows it passed shift by one. - s.remap_appearance_item(|a| { + s.remap_appearance_item(&ids, |a| { Some(if a == from { landed } else if from < a && a <= landed { diff --git a/crates/engine/src/cmd/gradient.rs b/crates/engine/src/cmd/gradient.rs index 5cace287..b3da6652 100644 --- a/crates/engine/src/cmd/gradient.rs +++ b/crates/engine/src/cmd/gradient.rs @@ -114,8 +114,9 @@ pub(crate) fn apply_gradient_edit(paint: &Paint, p: &Value, bounds: Option Result { const C: &str = "paint.editGradient"; let item = item_target(s, p, C)?; + let stroke = edits_stroke(s, p, item, !s.fill_active)?; + let item = item.of_kind(s, !stroke); let ids = item.targets(s, p)?; - let stroke = edits_stroke(s, p, item, &ids, !s.fill_active)?; // Validate against the defaults first so bad params fail without touching the document. let default_paint = if stroke { s.paint.stroke.clone() } else { s.paint.fill.clone() }; let new_default = apply_gradient_edit(&default_paint, p, None).map_err(|e| bad(C, e))?; @@ -147,6 +148,8 @@ fn set_gradient_geom(s: &mut Session, p: &Value) -> Result { let start = point_param(p, "start").ok_or_else(|| bad(C, "missing start [x,y]"))?; let end = point_param(p, "end").ok_or_else(|| bad(C, "missing end [x,y]"))?; let item = item_target(s, p, C)?; + let stroke = edits_stroke(s, p, item, false)?; + let item = item.of_kind(s, !stroke); let targets = match item { ItemTarget::Top => leaf_targets(s, &ids_param(p, "ids").map_or_else(|| selected_roots(s), Ok)?)?, _ => item.targets(s, p)?, @@ -154,7 +157,6 @@ fn set_gradient_geom(s: &mut Session, p: &Value) -> Result { if targets.is_empty() { return Err(EngineError::Other("nothing selected".into())); } - let stroke = edits_stroke(s, p, item, &targets, false)?; let geom = GradientGeom { start, end, aspect: 1.0 }; edit_items(s, &targets, item, C, "Gradient", !stroke, |n, index| { if matches!(n.kind, NodeKind::Text(_)) { diff --git a/crates/engine/src/cmd/paint.rs b/crates/engine/src/cmd/paint.rs index 2d671b77..b1ce6c74 100644 --- a/crates/engine/src/cmd/paint.rs +++ b/crates/engine/src/cmd/paint.rs @@ -1,7 +1,7 @@ //! Fill and stroke paint (the toolbar proxies and their defaults) and the Transparency panel. use serde_json::{Value, json}; -use vectorcraft_color::{Color, Gradient, GradientKind, GradientPaint, Paint}; +use vectorcraft_color::{BlendMode, Color, Gradient, GradientKind, GradientPaint, Paint}; use vectorcraft_doc::{Appearance, NodeKind}; use super::appearance::{ItemTarget, appearance_targets, edit_items, item_target}; @@ -104,7 +104,7 @@ fn set_paint(s: &mut Session, p: &Value, fill: bool) -> Result { s.paint.stroke = paint.clone(); } s.fill_active = fill; - let item = item_target(s, p, cmd)?; + let item = item_target(s, p, cmd)?.of_kind(s, fill); let ids = item.targets(s, p)?; edit_items(s, &ids, item, cmd, if fill { "Fill Color" } else { "Stroke Color" }, fill, |n, index| { if index.is_none() @@ -173,30 +173,36 @@ fn transparency(s: &mut Session, p: &Value) -> Result { let mut q = p.as_object().cloned().unwrap_or_default(); q.remove("item"); let opacity = q.remove("opacity").and_then(|v| v.as_f64()).map(|o| (o / 100.0).clamp(0.0, 1.0)); - if let ItemTarget::Item { index, explicit } = item { - // Opacity and blend belong to the targeted fill/stroke; isolate/knockout stay object-level. - if explicit { - let d = &s.doc()?.doc; - if !appearance_targets(s, p)?.iter().any(|id| d.node(*id).is_some_and(|n| index < n.appearance.items.len())) { - return Err(bad(C, format!("no appearance item {index}"))); - } + let ItemTarget::Item { index, explicit } = item else { + if let Some(o) = opacity { + q.insert("opacity".into(), json!(o)); } - let mut set = json!({ "index": index }); - for key in ["blend", "ids", "id"] { + return s.execute("object.setProps", &Value::Object(q)); + }; + // Opacity and blend belong to the targeted fill/stroke; isolate/knockout stay object-level. + if explicit { + let d = &s.doc()?.doc; + if !appearance_targets(s, p)?.iter().any(|id| d.node(*id).is_some_and(|n| index < n.appearance.items.len())) { + return Err(bad(C, format!("no appearance item {index}"))); + } + } + let blend = q.remove("blend"); + if let Some(b) = &blend + && b.as_str().and_then(BlendMode::parse).is_none() + { + return Err(bad(C, format!("unknown blend mode {b}"))); + } + if opacity.is_some() || blend.is_some() { + let mut set = json!({ "index": index, "opacity": opacity, "blend": blend }); + for key in ["ids", "id"] { if let Some(v) = p.get(key) { set[key] = v.clone(); } } - if let Some(o) = opacity { - set["opacity"] = json!(o); - } - q.remove("blend"); s.execute("appearance.setItem", &set)?; - if q.keys().all(|k| k == "ids" || k == "id") { - return ok(); - } - } else if let Some(o) = opacity { - q.insert("opacity".into(), json!(o)); + } + if q.keys().all(|k| k == "ids" || k == "id") { + return ok(); } s.execute("object.setProps", &Value::Object(q)) } diff --git a/crates/engine/src/cmd/stroke.rs b/crates/engine/src/cmd/stroke.rs index ef645d84..e78b621f 100644 --- a/crates/engine/src/cmd/stroke.rs +++ b/crates/engine/src/cmd/stroke.rs @@ -35,7 +35,7 @@ fn stroke_set(s: &mut Session, p: &Value) -> Result { if let Some(w) = p.get("weight").and_then(Value::as_f64) { s.paint.stroke_width = w.max(0.0); } - let item = item_target(s, p, C)?; + let item = item_target(s, p, C)?.of_kind(s, false); let ids = item.targets(s, p)?; let arrow = |k: &str| -> Result>> { match p.get(k) { @@ -139,7 +139,7 @@ fn stroke_advanced(s: &mut Session, p: &Value) -> Result { if scale.is_none() && flip.is_none() && !swap && brush.is_none() { return Err(bad(C, "nothing to change")); } - let item = item_target(s, p, C)?; + let item = item_target(s, p, C)?.of_kind(s, false); let ids = item.targets(s, p)?; edit_items(s, &ids, item, C, "Stroke", false, |n, index| { let Some(st) = n.appearance.stroke_at_mut(index) else { return Ok(()) }; diff --git a/crates/engine/src/tests_appearance.rs b/crates/engine/src/tests_appearance.rs index 413dac6c..30ed6917 100644 --- a/crates/engine/src/tests_appearance.rs +++ b/crates/engine/src/tests_appearance.rs @@ -341,3 +341,47 @@ fn graphic_style_apply_replaces_the_stack() { assert_eq!(node(&s, b).appearance, node(&s, a).appearance); assert!(s.execute("graphicStyle.apply", &json!({"name": "nope"})).is_err()); } + +#[test] +fn an_active_row_of_the_other_kind_leaves_edits_on_the_painted_leaves() { + let mut s = session(); + let a = rect(&mut s, 100.0); + let b = rect(&mut s, 250.0); + run(&mut s, "select.set", json!({"ids": [a.0, b.0]})); + let g = NodeId(run(&mut s, "object.group", json!({}))["id"].as_u64().unwrap()); + run(&mut s, "appearance.addFill", json!({})); + // The group's own fill row is active: stroke edits still go to the members' strokes. + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + run(&mut s, "stroke.set", json!({"weight": 5})); + run(&mut s, "paint.setStroke", json!({"color": "#00ff00"})); + assert_eq!(node(&s, g).appearance.items.len(), 1, "no stroke row added to the group"); + for id in [a, b] { + assert_eq!(stroke(&node(&s, id), 1).width, 5.0); + assert_eq!(stroke(&node(&s, id), 1).paint.color().unwrap().to_hex(), "#00ff00"); + } +} + +#[test] +fn the_active_row_follows_only_edits_of_its_object() { + let mut s = session(); + let other = rect(&mut s, 250.0); + rect(&mut s, 100.0); + run(&mut s, "appearance.setActiveItem", json!({"index": 1})); + run(&mut s, "appearance.removeItem", json!({"index": 0, "ids": [other.0]})); + run(&mut s, "appearance.duplicateItem", json!({"index": 0, "ids": [other.0]})); + assert_eq!(s.appearance_item(), Some(1)); +} + +#[test] +fn item_transparency_is_one_step_and_checks_the_blend_mode() { + let mut s = session(); + let id = rect(&mut s, 100.0); + run(&mut s, "appearance.setActiveItem", json!({"index": 1})); + let steps = |s: &Session| s.doc().unwrap().history.undo.len(); + let before = steps(&s); + run(&mut s, "transparency.set", json!({"knockout": true})); + assert_eq!(steps(&s), before + 1, "isolate/knockout alone do not touch the item"); + assert!(node(&s, id).knockout); + assert!(s.execute("transparency.set", &json!({"blend": "nope"})).is_err()); + assert_eq!(stroke(&node(&s, id), 1).blend, BlendMode::Normal); +} diff --git a/crates/tools/src/xform/gradient.rs b/crates/tools/src/xform/gradient.rs index 2f92d775..af6cfb98 100644 --- a/crates/tools/src/xform/gradient.rs +++ b/crates/tools/src/xform/gradient.rs @@ -7,7 +7,6 @@ use serde_json::{Value, json}; use vectorcraft_color::Paint; -use vectorcraft_doc::AppearanceItem; use vectorcraft_doc::hit::hit_test; use vectorcraft_geom::{Point, Vec2}; @@ -27,10 +26,8 @@ pub struct GradientTool { pub fn annotator(cx: &ToolContext) -> Option<(Point, Point, Vec)> { cx.selection.objects.iter().find_map(|id| { let n = cx.doc.node(*id)?; - let item = cx.appearance_item.and_then(|i| n.appearance.items.get(i)); - let paint = match item { - Some(AppearanceItem::Stroke(s)) => &s.paint, - Some(AppearanceItem::Fill(f)) => &f.paint, + let paint = match cx.appearance_item.and_then(|i| n.appearance.items.get(i)) { + Some(item) => item.paint(), None => &n.appearance.fill()?.paint, }; let Paint::Gradient(g) = paint else { return None }; diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index d57443b1..db6a3ead 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -46,8 +46,7 @@ use crate::{VectorcraftApp, icons}; /// The first selected node (cloned), if any. pub fn first_selected(app: &VectorcraftApp) -> Option { - let st = app.session.active()?; - st.selection.objects.first().and_then(|id| st.doc.node(*id)).cloned() + first_node(app).cloned() } /// Number of selected objects.