Merge pull request #3206 from Taha-Mahmoodi/fix/3188-clip-arrange

Fix #3188: arranging art inside a clipping mask keeps the clipping path at the bottom

crates/engine/src/cmd/object.rs

@@ -639,18 +639,24 @@ fn arrange(s: &mut Session, how: Arrange) -> Result<Value> {
                 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)?;

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

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