From 4fdb3e1f9b297082dd0bd6eb5d61d97b547b61d7 Mon Sep 17 00:00:00 2001 From: Taha-Mahmoodi <85902429+Taha-Mahmoodi@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:55:35 +0430 Subject: [PATCH] Fix #3188: arranging art inside a clipping mask keeps the clipping path at the bottom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a clip group or a clipping layer the bottom child is the clipping path. Object › Arrange moved the selected children with no regard for that slot: Send to Back (and Send Backward from just above it) put the art under the clipping path, so the art became the clipping path, lost its role as art, and the group drew nothing. Bring Forward / to Front on the clipping path itself would likewise have made art the clip. Layers-panel moves already keep the clipping path at the bottom (layer.rs, `index.max(1)`); arrange didn't. - object.rs `arrange`: in a parent that clips, the clipping path is left where it is and the art is arranged within the slots above it (Back goes to just above the clipping path, Backward stops there). - tests: Back, Backward, several objects, and Forward/Front on the clipping path in a clip group (with a render check); Send to Back in a clipping layer. --- crates/engine/src/cmd/object.rs | 16 ++++++---- crates/engine/src/tests_clip.rs | 44 ++++++++++++++++++++++++++++ crates/engine/src/tests_layerclip.rs | 14 +++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 186630a88..7522106c4 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -639,18 +639,24 @@ fn arrange(s: &mut Session, how: Arrange) -> Result { None => by_parent.push((par, vec![*id])), } } - for (par, group) in by_parent { + for (par, mut group) in by_parent { + // In a clip group or a clipping layer the bottom child is the clipping path: it stays + // there, and the art above it never goes below it (#3188). + let floor = usize::from(par.and_then(|p| d.node(p)).is_some_and(vectorcraft_doc::Node::clips)); + if floor > 0 { + group.retain(|id| d.position(*id).is_some_and(|(_, idx, _)| idx >= floor)); + } for (k, id) in group.iter().enumerate() { let Some((_, idx, len)) = d.position(*id) else { continue }; - if len == 0 { + if len <= floor { continue; } let last = len - 1; let to = match how { - Arrange::Front => last.saturating_sub(k), - Arrange::Back => k.min(last), + Arrange::Front => last.saturating_sub(k).max(floor), + Arrange::Back => (floor + k).min(last), Arrange::Forward => (idx + 1).min(last.saturating_sub(k)).max(idx), - Arrange::Backward => idx.saturating_sub(1).max(k.min(idx)), + Arrange::Backward => idx.saturating_sub(1).max(floor + k.min(idx.saturating_sub(floor))), }; if to != idx { d.move_node(*id, par, to)?; diff --git a/crates/engine/src/tests_clip.rs b/crates/engine/src/tests_clip.rs index f3ea91f8c..2cc16daee 100644 --- a/crates/engine/src/tests_clip.rs +++ b/crates/engine/src/tests_clip.rs @@ -170,3 +170,47 @@ fn imported_multi_shape_and_even_odd_clip_paths_clip() { assert!(!opaque(&r, 70, 75), "the even-odd centre"); assert_outputs_agree(&mut s, &[(15, 20), (75, 20), (45, 20), (15, 43), (70, 56), (70, 75)]); } + +/// The children of node `g`, bottom first. +fn child_ids(s: &Session, g: NodeId) -> Vec { + s.doc().unwrap().doc.children(Some(g)).unwrap().iter().map(|n| n.id).collect() +} + +#[test] +fn arranging_inside_a_clip_group_keeps_the_clipping_path_at_the_bottom() { + // #3188: Send to Back on clipped art made it the clipping path, and the group showed nothing. + let mut s = session(); + let rect = + |s: &mut Session, x: f64, y: f64, w: f64| id_of(&s.execute("shape.rectangle", &json!({"x": x, "y": y, "width": w, "height": w})).unwrap()); + let a = rect(&mut s, 0.0, 0.0, 50.0); + let b = rect(&mut s, 50.0, 50.0, 50.0); + let clip = ellipse(&mut s, 20.0, 60.0); + select(&mut s, &[a, b, clip]); + let g = id_of(&s.execute("object.clippingMask.make", &json!({})).unwrap()); + assert_eq!(child_ids(&s, g), [clip, a, b]); + let send = |s: &mut Session, ids: &[NodeId], cmd: &str| { + select(s, ids); + s.execute(cmd, &json!({})).unwrap(); + }; + + send(&mut s, &[b], "object.arrange.sendToBack"); + assert_eq!(child_ids(&s, g), [clip, b, a], "the art goes just above the clipping path"); + let r = render(&s); + assert!(opaque(&r, 40, 40) && opaque(&r, 60, 60), "both pieces of art still show inside the clip"); + assert!(!opaque(&r, 5, 5), "and are still clipped"); + + send(&mut s, &[a], "object.arrange.sendBackward"); + assert_eq!(child_ids(&s, g), [clip, a, b]); + send(&mut s, &[a], "object.arrange.sendBackward"); + assert_eq!(child_ids(&s, g), [clip, a, b], "the lowest art stays above the clipping path"); + send(&mut s, &[a, b], "object.arrange.sendToBack"); + assert_eq!(child_ids(&s, g), [clip, a, b], "several pieces of art keep their order"); + + // The clipping path itself stays where it is: moving it up would make art the clip. + for cmd in ["object.arrange.bringForward", "object.arrange.bringToFront"] { + send(&mut s, &[clip], cmd); + assert_eq!(child_ids(&s, g), [clip, a, b], "{cmd}"); + } + send(&mut s, &[clip, a], "object.arrange.bringToFront"); + assert_eq!(child_ids(&s, g), [clip, b, a], "the art selected with it still moves"); +} diff --git a/crates/engine/src/tests_layerclip.rs b/crates/engine/src/tests_layerclip.rs index fd79992bd..21c694d3e 100644 --- a/crates/engine/src/tests_layerclip.rs +++ b/crates/engine/src/tests_layerclip.rs @@ -120,3 +120,17 @@ fn the_top_object_must_be_able_to_clip() { let err = s.execute("layer.clippingMask.toggle", &json!({"id": sub.0})); assert!(err.is_err(), "an empty sublayer has nothing to clip by"); } + +#[test] +fn send_to_back_in_a_clipping_layer_keeps_the_clipping_path_at_the_bottom() { + // #3188, for a clipping layer: the art went under the clipping path and became the clip. + let mut s = session(); + let (art, clip) = art_and_clip(&mut s); + assert!(toggle(&mut s)); + let top = rect(&mut s, 0.0, 0.0, 40.0, 40.0); + select(&mut s, &[top]); + s.execute("object.arrange.sendToBack", &json!({})).unwrap(); + let ids: Vec = layer(&s).children().unwrap().iter().map(|n| n.id).collect(); + assert_eq!(ids, [clip, top, art]); + assert!(opaque(&render(&s), 50, 50), "the art inside the clip still shows"); +}