diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 5ba393f58..27f5aee81 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -45,7 +45,7 @@ pub fn specs() -> Vec { "Add New Fill", ["Window", "Appearance"], Some("Cmd+/"), - "{ids?|id?, target?: \"object\"|\"contents\"} add a fill on top of each selected object's own stack, on an image right below its Contents row (a copy of its top fill; type without one: its characters' fill; else the default fill). With a fill row active (appearance.setActiveItem), no `ids` and no `target: \"contents\"`, the new fill is a copy of that row, right above it, and becomes the active row", + "{ids?|id?, target?: \"object\"|\"contents\"} add a fill on top of each selected object's own stack, on an image right below its Contents row, with its top fill's paint (type without one: its characters' fill; else the default fill). With a fill row active (appearance.setActiveItem), no `ids` and no `target: \"contents\"`, the new fill takes that row's paint, goes right above it, and becomes the active row; appearance.duplicateItem copies a whole row", has_doc, |s, p| add_item(s, p, true) ), @@ -54,7 +54,7 @@ pub fn specs() -> Vec { "Add New Stroke", ["Window", "Appearance"], Some("Cmd+Alt+/"), - "{ids?|id?, target?} add a stroke on top of each selected object's own stack, on an image right below its Contents row (a copy of its top stroke, else the default stroke)", + "{ids?|id?, target?} add a stroke on top of each selected object's own stack, on an image right below its Contents row, with its top stroke's paint and weight (else the default stroke)", has_doc, |s, p| add_item(s, p, false) ), @@ -519,8 +519,8 @@ fn set_active_item(s: &mut Session, p: &Value) -> Result { fn add_item(s: &mut Session, p: &Value, fill: bool) -> Result { let ids = require_targets(s, p, if fill { "appearance.addFill" } else { "appearance.addStroke" })?; let defaults = s.paint.clone(); - // The panel's active Fill row, when the new fill goes on the selection: the new fill is a copy - // of that row, right above it in each target that has it (others get a copy of their top fill). + // The panel's active Fill row, when the new fill goes on the selection: the new fill takes that + // row's paint, right above it in each target that has it (others take their top fill's paint). // Where the new item went in on the active item's object, when it is an image. let active = s.active_appearance_item.as_ref().and_then(|a| a.objects.first().copied()); // The active row is a row of the active object's stack, which need not come first in `ids` diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index 284a809de..4fea39bdf 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -3,7 +3,7 @@ use serde_json::{Value, json}; use vectorcraft_color::{BlendMode, Color, GradientKind, Paint}; -use vectorcraft_doc::{AppearanceItem, Node}; +use vectorcraft_doc::{AppearanceItem, FillLayer, Node}; use crate::*; @@ -288,8 +288,8 @@ fn add_fill_and_stroke_copy_the_top_ones() { assert_eq!(node(&s, id).appearance.items.len(), 3); } -/// With a fill row active, Add New Fill puts a copy of that row right above it and makes the copy -/// the active row; the rows above it stay where they are. +/// With a fill row active, Add New Fill puts a fill with that row's paint right above it and makes +/// the new fill the active row; the rows above it stay where they are. #[test] fn add_fill_copies_the_active_fill_row_right_above_it() { let mut s = session(); @@ -323,8 +323,30 @@ fn add_fill_copies_the_active_fill_row_right_above_it() { assert_eq!(s.appearance_item(), Some(0)); } +/// The new fill above the active fill row takes only the row's paint: the row's opacity, blend +/// mode, visibility, overprint and effects are not copied (Duplicate Item copies those). +#[test] +fn add_fill_takes_only_the_active_fill_rows_paint() { + 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!((row.opacity, row.blend, row.visible, row.overprint), (0.4, BlendMode::Multiply, false, true)); + assert_eq!(row.effects.len(), 1); + assert_eq!(copy, &FillLayer::new(row.paint.clone())); +} + /// 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. +/// doesn't share that row gets a fill with its top fill's paint on top, as without an active row. #[test] fn add_fill_above_an_unshared_active_row_tops_the_other_objects() { let mut s = session();