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"); +}