From db468928d4e291983e70452a9d1c3a7827a70da0 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 4 Nov 2026 03:44:21 -0800 Subject: [PATCH] Appearance: Add New Fill copies the whole active fill row With a fill row active, Add New Fill copied only the row's paint and reset its opacity, blend mode, visibility, overprint and effects. The new row is now a copy of the whole active row, as the command's description says. --- crates/engine/src/cmd/appearance.rs | 4 ++-- crates/engine/src/tests/appearance.rs | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 5ba393f58..4531f22cd 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -551,9 +551,9 @@ fn add_item(s: &mut Session, p: &Value, fill: bool) -> Result { for (id, row) in ids.iter().zip(&per) { if let Some(n) = d.node_mut(*id) { if let Some(i) = n.appearance.item_of_kind(*row, true) - && let Some(paint) = n.appearance.paint_at(Some(i), true).cloned() + && let Some(row) = n.appearance.fill_at(Some(i)).cloned() { - n.appearance.insert_item(i + 1, AppearanceItem::Fill(FillLayer::new(paint))); + n.appearance.insert_item(i + 1, AppearanceItem::Fill(row)); if active == Some(*id) { above = Some(i + 1); } diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index 284a809de..c07fc8eb7 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -323,6 +323,28 @@ fn add_fill_copies_the_active_fill_row_right_above_it() { assert_eq!(s.appearance_item(), Some(0)); } +/// The copy of the active fill row keeps the row's opacity, blend mode, visibility, overprint and +/// effects as well as its paint. +#[test] +fn add_fill_copies_the_whole_active_fill_row() { + let mut s = session(); + let id = rect(&mut s, 100.0); + run(&mut s, "paint.setFill", json!({"color": "#ff0000"})); + run(&mut s, "appearance.setItem", json!({"index": 0, "opacity": 40, "blend": "multiply", "visible": false})); + run(&mut s, "effect.apply", json!({"effect": "distort.roughen", "item": 0})); + run(&mut s, "object.setOverprint", json!({"fill": true, "item": 0})); + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + run(&mut s, "appearance.addFill", json!({})); + let n = node(&s, id); + assert_eq!(n.appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "fill", "stroke"]); + let (AppearanceItem::Fill(row), AppearanceItem::Fill(copy)) = (&n.appearance.items[0], &n.appearance.items[1]) else { + panic!("the first two rows are not fills") + }; + assert_eq!((copy.opacity, copy.blend, copy.visible, copy.overprint), (0.4, BlendMode::Multiply, false, true)); + assert_eq!(copy.effects.len(), 1); + assert_eq!(copy, row); +} + /// In a mixed selection the active fill row goes above itself in the first object; an object that /// doesn't share that row gets a copy of its top fill on top, as without an active row. #[test]