diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 89b67604..7e794f70 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -43,7 +43,7 @@ pub fn specs() -> Vec { "Clear Appearance", ["Window", "Appearance"], None, - "{ids?} leave each object one empty fill and stroke (None) and reset its opacity and blend mode", + "{ids?} leave each object one empty fill and stroke (None; a group none of its own) and reset its opacity and blend mode", has_selection, clear_appearance ), @@ -52,7 +52,7 @@ pub fn specs() -> Vec { "Reduce to Basic Appearance", ["Window", "Appearance"], None, - "{ids?} keep only the topmost visible fill and stroke, without their own opacity, blend mode or effects, and drop the object's effects", + "{ids?} keep only the topmost visible fill and stroke, without their own opacity, blend mode or effects (a group keeps no rows it lacked), and drop the object's effects", has_selection, reduce_basic ), @@ -290,8 +290,9 @@ fn clear_appearance(s: &mut Session, p: &Value) -> Result { s.edit("Clear Appearance", |d, _| { for id in &ids { if let Some(n) = d.node_mut(*id) { - // No fill, no stroke, and the object's Opacity row back to Default. - n.appearance = Appearance::basic(Paint::None, Paint::None, 1.0); + // No fill, no stroke, and the object's Opacity row back to Default. A group keeps no + // fill or stroke rows of its own. + n.appearance = if is_group(n) { Appearance::default() } else { Appearance::basic(Paint::None, Paint::None, 1.0) }; n.opacity = 1.0; n.blend = BlendMode::Normal; } @@ -302,6 +303,10 @@ fn clear_appearance(s: &mut Session, p: &Value) -> Result { ok() } +fn is_group(n: &Node) -> bool { + matches!(n.kind, NodeKind::Group { .. }) +} + fn reduce_basic(s: &mut Session, p: &Value) -> Result { let ids = appearance_targets(s, p)?; s.edit("Reduce to Basic Appearance", |d, _| { @@ -309,9 +314,12 @@ fn reduce_basic(s: &mut Session, p: &Value) -> Result { if let Some(n) = d.node_mut(*id) { // The topmost visible fill and stroke stay, without their own transparency or effects. let top = |fill: bool| n.appearance.items.iter().rev().find(|i| i.visible() && i.is_fill() == fill).cloned(); - let fill = top(true).map_or(Paint::None, |f| f.paint().clone()); - let stroke = top(false); - n.appearance = Appearance { items: vec![AppearanceItem::Fill(FillLayer::new(fill))], effects: vec![] }; + let (fill, stroke) = (top(true), top(false)); + // Other objects always keep a fill row (None when there was none); a group only + // keeps the rows it had. + let fill = + (fill.is_some() || !is_group(n)).then(|| AppearanceItem::Fill(FillLayer::new(fill.map_or(Paint::None, |f| f.paint().clone())))); + n.appearance = Appearance { items: fill.into_iter().collect(), effects: vec![] }; if let Some(AppearanceItem::Stroke(mut st)) = stroke { st.effects.clear(); st.opacity = 1.0; diff --git a/crates/engine/src/tests_appearance.rs b/crates/engine/src/tests_appearance.rs index 30ed6917..d55627a6 100644 --- a/crates/engine/src/tests_appearance.rs +++ b/crates/engine/src/tests_appearance.rs @@ -385,3 +385,24 @@ fn item_transparency_is_one_step_and_checks_the_blend_mode() { assert!(s.execute("transparency.set", &json!({"blend": "nope"})).is_err()); assert_eq!(stroke(&node(&s, id), 1).blend, BlendMode::Normal); } + +#[test] +fn clear_and_reduce_keep_a_groups_stack_empty() { + let mut s = session(); + let a = rect(&mut s, 100.0); + let b = rect(&mut s, 250.0); + let members = node(&s, a).appearance; + 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.reduceToBasic", json!({})); + assert!(node(&s, g).appearance.items.is_empty()); + run(&mut s, "appearance.addStroke", json!({})); + run(&mut s, "appearance.reduceToBasic", json!({})); + assert_eq!(node(&s, g).appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["stroke"]); + run(&mut s, "transparency.set", json!({"opacity": 50})); + run(&mut s, "appearance.clear", json!({})); + let n = node(&s, g); + assert!(n.appearance.items.is_empty() && n.opacity == 1.0); + // The members keep their own appearance. + assert_eq!(node(&s, a).appearance, members); +} diff --git a/crates/ui-egui/src/panels/layers.rs b/crates/ui-egui/src/panels/layers.rs index af0a676f..585c4ba8 100644 --- a/crates/ui-egui/src/panels/layers.rs +++ b/crates/ui-egui/src/panels/layers.rs @@ -290,14 +290,14 @@ fn row( } } -/// A real rendered thumbnail, cached by node identity (unchanged nodes keep their `Arc` -/// allocation, so the address is a free change detector). Only rendered for visible rows. /// Whether an object's target circle is filled: its appearance is not basic or its transparency /// (opacity, blend mode, isolation, knockout or an opacity mask) is not the default. fn has_styled_target(n: &Node) -> bool { !n.appearance.is_basic() || !n.has_default_transparency() } +/// A real rendered thumbnail, cached by node identity (unchanged nodes keep their `Arc` +/// allocation, so the address is a free change detector). Only rendered for visible rows. fn real_thumb(ui: &Ui, doc: &vectorcraft_doc::Document, n: &Node, r: egui::Rect) -> bool { use std::cell::RefCell; use std::collections::HashMap;