From edb984a75a1b0782d6d10da1d25a39c21272ea2a Mon Sep 17 00:00:00 2001 From: Jerome CAMUS Date: Fri, 23 Oct 2026 15:49:05 +0200 Subject: [PATCH 1/3] Fix #2091 copy/paste masks between layers Edit > Copy with masks selected copied the layer, so Paste duplicated it instead of adding the mask. Masks now go to their own clipboard and Paste appends them, with unique names, to the selected unlocked layers. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/edit.rs | 14 ++++++++ crates/engine/src/cmd/effect.rs | 1 + crates/engine/src/cmd/prop_groups.rs | 53 ++++++++++++++++++++++++++++ crates/engine/src/doc_state.rs | 3 ++ crates/engine/src/tests/menu_cmds.rs | 50 ++++++++++++++++++++++++++ crates/ui-egui/src/menus/frontend.rs | 2 ++ 6 files changed, 123 insertions(+) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index b33f4cc5c..594f01d29 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -25,6 +25,7 @@ fn has_clip(s: &Session) -> std::result::Result<(), String> { && s.state.key_clipboard.is_empty() && s.state.effect_clipboard.is_empty() && s.state.contents_clipboard.is_empty() + && s.state.mask_clipboard.is_empty() && s.state.link_clipboard.is_none() && s.state.value_clipboard.is_empty() { @@ -285,6 +286,7 @@ fn clear_clipboards(s: &mut Session) { s.state.key_clipboard.clear(); s.state.effect_clipboard.clear(); s.state.contents_clipboard.clear(); + s.state.mask_clipboard.clear(); s.state.link_clipboard = None; s.state.value_clipboard.clear(); s.state.keyframe_text = None; @@ -311,6 +313,15 @@ fn copy(s: &mut Session, p: &Value) -> Result { if p.get("layers").is_none() && !super::effect::selected_effects(s).is_empty() { return s.execute("effect.copy", json!({})); } + // Masks selected → copy them (not their layer), for Paste onto other layers (#2091). + if p.get("layers").is_none() { + let masks = super::prop_groups::copy_masks(s); + if !masks.is_empty() { + clear_clipboards(s); + s.state.mask_clipboard = masks; + return Ok(json!({"masks": s.state.mask_clipboard.len()})); + } + } // Shape items selected in a shape layer's Contents → copy them (not their layer). if p.get("layers").is_none() { let items = super::prop_groups::copy_contents(s); @@ -597,6 +608,9 @@ fn paste(s: &mut Session, p: &Value) -> Result { if !s.state.contents_clipboard.is_empty() { return super::prop_groups::paste_contents(s, p); } + if !s.state.mask_clipboard.is_empty() { + return super::prop_groups::paste_masks(s, p); + } let cid = super::comp_id(s, p)?; let clip = s.state.clipboard.clone(); let new = s.edit("Paste", None, |proj, st| { diff --git a/crates/engine/src/cmd/effect.rs b/crates/engine/src/cmd/effect.rs index 7b64015cd..f41ba3f9c 100644 --- a/crates/engine/src/cmd/effect.rs +++ b/crates/engine/src/cmd/effect.rs @@ -851,6 +851,7 @@ fn copy(s: &mut Session, p: &Value) -> Result { s.state.key_clipboard.clear(); s.state.keyframe_text = None; s.state.contents_clipboard.clear(); + s.state.mask_clipboard.clear(); s.state.link_clipboard = None; s.state.clip_is_keys = false; let n = groups.len(); diff --git a/crates/engine/src/cmd/prop_groups.rs b/crates/engine/src/cmd/prop_groups.rs index 6f7d74aef..8976e89fc 100644 --- a/crates/engine/src/cmd/prop_groups.rs +++ b/crates/engine/src/cmd/prop_groups.rs @@ -135,6 +135,59 @@ pub(crate) fn copy_contents(s: &Session) -> Vec { selected_contents(s).into_iter().filter_map(|(lid, uid)| c.layer(lid)?.props.find_group(uid).cloned()).collect() } +/// Masks selected on the active comp's layers, top to bottom (#2091). +pub(crate) fn selected_masks(s: &Session) -> Vec<(LayerId, Uid)> { + let Some(c) = s.active_comp().filter(|_| !s.state.selected_props.is_empty()) else { return vec![] }; + c.layers + .iter() + .flat_map(|l| { + let masks = l.masks().into_iter().flat_map(|m| m.groups()); + masks.filter(|m| s.state.selected_props.contains(&(l.id, m.uid))).map(|m| (l.id, m.uid)) + }) + .collect() +} + +/// Copies of the selected masks (Edit ▸ Copy), top to bottom. +pub(crate) fn copy_masks(s: &Session) -> Vec { + let Some(c) = s.active_comp() else { return vec![] }; + selected_masks(s).into_iter().filter_map(|(lid, uid)| c.layer(lid)?.props.find_group(uid).cloned()).collect() +} + +/// Edit ▸ Paste with masks on the clipboard: each is added, with a unique name, below the masks of +/// every selected unlocked layer that can have masks, and the copies are selected (#2091). +pub(crate) fn paste_masks(s: &mut Session, p: &Value) -> Result { + let c = "edit.paste"; + let clip = s.state.mask_clipboard.clone(); + let (cid, ids) = layers_p(s, p)?; + let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let targets: Vec = ids.into_iter().filter(|l| comp.layer(*l).is_some_and(|l| l.masks().is_some() && !l.switches.locked)).collect(); + if targets.is_empty() { + return Err(bad(c, "select an unlocked layer that can have masks to paste masks into")); + } + let label = match clip.as_slice() { + [g] => format!("Paste {}", g.name), + _ => "Paste".to_string(), + }; + let pasted = s.edit(&label, None, |proj, st| { + let mut next = proj.next_id; + let mut out = vec![]; + for lid in &targets { + let masks = layer_mut(proj, cid, *lid)?.props.sub_mut("masks").ok_or_else(|| bad(c, "the layer can't have masks"))?; + for g in &clip { + let mut g = g.clone(); + g.reassign_uids(&mut next); + g.name = super::effect::unique_name(masks, &g.name); + out.push((*lid, g.uid)); + masks.children.push(g.into()); + } + } + proj.next_id = next + 1; + st.selected_props = out.clone(); + Ok(out) + })?; + Ok(json!({"masks": pasted.iter().map(|(_, u)| *u).collect::>()})) +} + /// Edit ▸ Cut's removal of the selected shape items (once copied). pub(crate) fn cut_contents(s: &mut Session, items: &[(LayerId, Uid)]) -> Result { let cid = s.active_comp_id().ok_or(EngineError::NoComp)?; diff --git a/crates/engine/src/doc_state.rs b/crates/engine/src/doc_state.rs index c11f575b0..7b4889dba 100644 --- a/crates/engine/src/doc_state.rs +++ b/crates/engine/src/doc_state.rs @@ -98,6 +98,9 @@ pub struct EditorState { /// a shape layer's Contents); Paste adds them to the selected shape layers. #[serde(skip)] pub contents_clipboard: Vec, + /// Copied masks (Edit ▸ Copy with masks selected); Paste adds them to the selected layers. + #[serde(skip)] + pub mask_clipboard: Vec, /// Selected mask vertices (viewer Selection tool / pen). #[serde(default)] pub selected_vertices: Vec, diff --git a/crates/engine/src/tests/menu_cmds.rs b/crates/engine/src/tests/menu_cmds.rs index cfbbb4966..95ed5d8d8 100644 --- a/crates/engine/src/tests/menu_cmds.rs +++ b/crates/engine/src/tests/menu_cmds.rs @@ -1943,3 +1943,53 @@ fn old_guides_load_as_pixel_guides_and_percent_round_trips() { // A zero-sized comp never divides by zero. assert!(g.px(0, 0).is_finite()); } + +#[test] +fn copy_paste_masks_between_layers() { + let mut s = comp(); + let a = solid(&mut s, "#ff0000"); + let b = solid(&mut s, "#00ff00"); + let n = s.active_comp().unwrap().layers.len(); + s.execute("layer.select", json!({"layers": [a]})).unwrap(); + s.execute("layer.addMask", json!({})).unwrap(); + s.execute("layer.addMask", json!({"shape": "ellipse"})).unwrap(); + let names = |s: &Session, id: u64| layer(s, id).masks().unwrap().groups().map(|m| m.name.clone()).collect::>(); + let uids = |s: &Session, id: u64| layer(s, id).masks().unwrap().groups().map(|m| m.uid).collect::>(); + let first = uids(&s, a)[0]; + s.execute("prop.select", json!({"layer": a, "prop": first})).unwrap(); + // Copy the mask: it is the mask that is copied, not its layer. + assert_eq!(s.execute("edit.copy", json!({})).unwrap()["masks"], json!(1)); + assert!(s.state.clipboard.is_empty(), "not the layer"); + assert!(s.is_enabled("edit.paste")); + s.execute("layer.select", json!({"layers": [b]})).unwrap(); + let r = s.execute("edit.paste", json!({})).unwrap(); + assert_eq!(s.active_comp().unwrap().layers.len(), n, "no new layer"); + assert_eq!(names(&s, b), ["Mask 1"]); + let pasted = r["masks"][0].as_u64().unwrap(); + assert_eq!(uids(&s, b), [pasted]); + assert_ne!(pasted, first); + assert_eq!(s.state.selected_props, vec![(effectcraft_project::LayerId(b), pasted)]); + assert_eq!(names(&s, a), ["Mask 1", "Mask 2"], "the source keeps its masks"); + // Pasting again onto the same layer keeps names unique. + s.execute("edit.paste", json!({})).unwrap(); + assert_eq!(names(&s, b), ["Mask 1", "Mask 2"]); + // One undo step per paste. + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(names(&s, b), ["Mask 1"]); +} + +#[test] +fn paste_masks_skips_locked_layers() { + let mut s = comp(); + let a = solid(&mut s, "#ff0000"); + let b = solid(&mut s, "#00ff00"); + s.execute("layer.select", json!({"layers": [a]})).unwrap(); + s.execute("layer.addMask", json!({})).unwrap(); + let mask = layer(&s, a).masks().unwrap().groups().next().unwrap().uid; + s.execute("prop.select", json!({"layer": a, "prop": mask})).unwrap(); + s.execute("edit.copy", json!({})).unwrap(); + s.execute("layer.select", json!({"layers": [b]})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [b], "switch": "lock", "value": true})).unwrap(); + assert!(s.execute("edit.paste", json!({})).is_err()); + assert_eq!(layer(&s, b).masks().unwrap().groups().count(), 0); +} diff --git a/crates/ui-egui/src/menus/frontend.rs b/crates/ui-egui/src/menus/frontend.rs index 1bd11dabe..c3af37656 100644 --- a/crates/ui-egui/src/menus/frontend.rs +++ b/crates/ui-egui/src/menus/frontend.rs @@ -21,6 +21,8 @@ fn clipboard_text(s: &effectcraft_engine::Session) -> String { n(st.effect_clipboard.len(), "effect", "effects") } else if !st.contents_clipboard.is_empty() { n(st.contents_clipboard.len(), "shape item", "shape items") + } else if !st.mask_clipboard.is_empty() { + n(st.mask_clipboard.len(), "mask", "masks") } else if st.link_clipboard.is_some() { "EffectCraft: property links".into() } else { From 685448a0b28c49dd78ab05d0aa0db4d8bf9d55a3 Mon Sep 17 00:00:00 2001 From: Jerome CAMUS Date: Fri, 23 Oct 2026 16:06:08 +0200 Subject: [PATCH 2/3] Fix #2091 copy a named layer even when a mask is selected edit.copy {"layer": id} copied the selected mask instead of the layer: the mask branch only checked for `layers`, but `layer` is an explicit target too. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/edit.rs | 5 +++-- crates/engine/src/tests/menu_cmds.rs | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 594f01d29..8a5068e8f 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -313,8 +313,9 @@ fn copy(s: &mut Session, p: &Value) -> Result { if p.get("layers").is_none() && !super::effect::selected_effects(s).is_empty() { return s.execute("effect.copy", json!({})); } - // Masks selected → copy them (not their layer), for Paste onto other layers (#2091). - if p.get("layers").is_none() { + // Masks selected → copy them (not their layer), for Paste onto other layers (#2091), unless + // a layer is named explicitly. + if p.get("layers").is_none() && p.get("layer").is_none_or(Value::is_null) { let masks = super::prop_groups::copy_masks(s); if !masks.is_empty() { clear_clipboards(s); diff --git a/crates/engine/src/tests/menu_cmds.rs b/crates/engine/src/tests/menu_cmds.rs index 95ed5d8d8..bf4daf1a4 100644 --- a/crates/engine/src/tests/menu_cmds.rs +++ b/crates/engine/src/tests/menu_cmds.rs @@ -1993,3 +1993,16 @@ fn paste_masks_skips_locked_layers() { assert!(s.execute("edit.paste", json!({})).is_err()); assert_eq!(layer(&s, b).masks().unwrap().groups().count(), 0); } + +#[test] +fn copy_with_an_explicit_layer_ignores_the_selected_mask() { + let mut s = comp(); + let a = solid(&mut s, "#ff0000"); + s.execute("layer.select", json!({"layers": [a]})).unwrap(); + s.execute("layer.addMask", json!({})).unwrap(); + let mask = layer(&s, a).masks().unwrap().groups().next().unwrap().uid; + s.execute("prop.select", json!({"layer": a, "prop": mask})).unwrap(); + s.execute("edit.copy", json!({"layer": a})).unwrap(); + assert_eq!(s.state.clipboard.len(), 1, "the named layer is copied"); + assert!(s.state.mask_clipboard.is_empty()); +} From 73af2379d009a3c5978bb7d3267ec2d56a618b67 Mon Sep 17 00:00:00 2001 From: Jerome CAMUS Date: Fri, 30 Oct 2026 08:38:28 +0100 Subject: [PATCH 3/3] Fix #2091 clear stale key and vertex selections around mask copy/paste Selecting a mask group now clears the previous key selection, so Copy/Cut act on the mask. Pasting masks clears the source mask's vertex selection, so Clear removes the pasted mask rather than points of the source. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/prop.rs | 5 ++--- crates/engine/src/cmd/prop_groups.rs | 2 ++ crates/engine/src/tests/menu_cmds.rs | 29 ++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index 50c261020..b4432dd2f 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -794,6 +794,8 @@ fn select_prop(s: &mut Session, p: &Value) -> Result { if !add { s.state.selected_props.clear(); s.state.selected_vertices.clear(); + // Groups (masks, effects) have no keys of their own: don't leave the old selection's behind. + s.state.selected_keys.clear(); } if !s.state.selected_layers.contains(&lid) { s.state.selected_layers = vec![lid]; @@ -811,9 +813,6 @@ fn select_prop(s: &mut Session, p: &Value) -> Result { && let Some(pr) = s.active_comp().and_then(|c| c.layer(lid)).and_then(|l| l.props.find(uid)) { let keys: Vec = pr.keys.iter().map(|k| KeyRef { layer: lid, prop: uid, time: k.time }).collect(); - if !add { - s.state.selected_keys.clear(); - } s.state.selected_keys.extend(keys); } Ok(Value::Null) diff --git a/crates/engine/src/cmd/prop_groups.rs b/crates/engine/src/cmd/prop_groups.rs index 8976e89fc..b3e9baf14 100644 --- a/crates/engine/src/cmd/prop_groups.rs +++ b/crates/engine/src/cmd/prop_groups.rs @@ -183,6 +183,8 @@ pub(crate) fn paste_masks(s: &mut Session, p: &Value) -> Result { } proj.next_id = next + 1; st.selected_props = out.clone(); + // The source mask's points (selected with it) must not outlive its selection. + st.selected_vertices.clear(); Ok(out) })?; Ok(json!({"masks": pasted.iter().map(|(_, u)| *u).collect::>()})) diff --git a/crates/engine/src/tests/menu_cmds.rs b/crates/engine/src/tests/menu_cmds.rs index bf4daf1a4..d30fc156a 100644 --- a/crates/engine/src/tests/menu_cmds.rs +++ b/crates/engine/src/tests/menu_cmds.rs @@ -2006,3 +2006,32 @@ fn copy_with_an_explicit_layer_ignores_the_selected_mask() { assert_eq!(s.state.clipboard.len(), 1, "the named layer is copied"); assert!(s.state.mask_clipboard.is_empty()); } + +#[test] +fn selecting_a_mask_drops_stale_key_selection_so_copy_copies_the_mask() { + let mut s = comp(); + let a = solid(&mut s, "#ff0000"); + s.execute("layer.select", json!({"layers": [a]})).unwrap(); + s.execute("layer.addMask", json!({})).unwrap(); + let mask = layer(&s, a).masks().unwrap().groups().next().unwrap().uid; + s.state.selected_keys = vec![crate::KeyRef { layer: effectcraft_project::LayerId(a), prop: 1, time: Default::default() }]; + s.execute("prop.select", json!({"layer": a, "prop": mask})).unwrap(); + assert!(s.state.selected_keys.is_empty()); + assert_eq!(s.execute("edit.copy", json!({})).unwrap()["masks"], json!(1)); +} + +#[test] +fn pasting_a_mask_onto_its_own_layer_drops_the_source_vertex_selection() { + let mut s = comp(); + let a = solid(&mut s, "#ff0000"); + s.execute("layer.select", json!({"layers": [a]})).unwrap(); + s.execute("layer.addMask", json!({})).unwrap(); + let mask = layer(&s, a).masks().unwrap().groups().next().unwrap().uid; + s.execute("prop.select", json!({"layer": a, "prop": mask})).unwrap(); + assert!(!s.state.selected_vertices.is_empty()); + s.execute("edit.copy", json!({})).unwrap(); + s.execute("edit.paste", json!({})).unwrap(); + assert!(s.state.selected_vertices.is_empty()); + s.execute("edit.clear", json!({})).unwrap(); + assert_eq!(layer(&s, a).masks().unwrap().groups().count(), 1, "the pasted mask goes, the source stays"); +}