Image Arrange: keep the image's clipping and position through later edits

- Remove Crop rebuilds the image's placement from its drawn CTM instead of
  pattern-matching cm ops. After an Arrange under a translated CTM it used to
  keep the absolute placement and draw the image displaced.
- Arrange refuses a move that would change which clips apply to the image
  (moving it past a top-level clip it isn't under, or out from under one
  without redrawing it), instead of silently cropping it.
- The exact-clip test now checks the moved copy carries its own clip and
  hit-tests both images.
- Cargo.lock: add pdfcraft-edit's pdfcraft-render dev-dependency, which the
  previous lockfile commit left out (CI failed --locked).

Cargo.lock

@@ -4096,6 +4096,7 @@ dependencies = [
  "pdfcraft-cos",
  "pdfcraft-fonts",
  "pdfcraft-model",
+ "pdfcraft-render",
  "thiserror 2.0.21",
  "unicode-bidi",
 ]

crates/edit/src/images.rs

@@ -995,11 +995,14 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran
     let mut dest_effective_state = EffectiveGState::default();
     let mut dest_ctm = Matrix::IDENTITY;
     let mut scope_start_ctm = Matrix::IDENTITY;
+    let mut outer_clips = Vec::new();
+    let mut dest_clips = Vec::new();
 
     let max_scan = scan_target.max(target.saturating_add(1)).max(scope_start.saturating_add(1));
     for (i, op) in ops.iter().enumerate() {
         if i == scope_start {
             scope_start_ctm = state.placement.matrix;
+            outer_clips = state.placement.clips.clone();
         }
         if i == target {
             source_effective_state = tracker.current_state.clone();
@@ -1008,6 +1011,7 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran
         if i == scan_target {
             dest_effective_state = tracker.current_state.clone();
             dest_ctm = state.placement.matrix;
+            dest_clips = state.placement.clips.clone();
         }
         if i >= max_scan {
             break;
@@ -1018,6 +1022,7 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran
     if scan_target >= ops.len() {
         dest_effective_state = tracker.current_state.clone();
         dest_ctm = state.placement.matrix;
+        dest_clips = state.placement.clips.clone();
     }
     if dest_ctm.invert().is_none() {
         return Err(EditError::Invalid("unsupported graphics state: non-invertible CTM".into()));
@@ -1036,7 +1041,25 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran
         }
     };
 
-    if extracted.clip_ops.is_empty() && !img.clips.is_empty() {
+    // The clips the moved unit draws itself: all of the image's when they are rebuilt from
+    // `img.clips`, else those its own scope set after the clips around it.
+    let all_clips = extracted.clip_ops.is_empty();
+    let reemitted = if all_clips {
+        img.clips.as_slice()
+    } else if img.clips.starts_with(&outer_clips) {
+        img.clips.get(outer_clips.len()..).unwrap_or_default()
+    } else {
+        return Err(EditError::Invalid("unsupported graphics state: the image clipping path cannot be cleanly preserved".into()));
+    };
+    // At the destination it is clipped by the clips there and the ones it draws. Moving past a
+    // clip the image isn't under (or out from under one it is) would change what part shows.
+    if !dest_clips.iter().all(|c| img.clips.contains(c)) || !img.clips.iter().all(|c| dest_clips.contains(c) || reemitted.contains(c)) {
+        return Err(EditError::Invalid(
+            "unsupported graphics state: moving the image past a clipping path would change what part of it shows".into(),
+        ));
+    }
+
+    if all_clips && !img.clips.is_empty() {
         for clip in &img.clips {
             let ops = clip_path_to_ops(clip);
             if !ops.is_empty() {
@@ -1218,53 +1241,42 @@ fn image_exclusive_scope(ops: &[Op], target: usize) -> Result<Option<(usize, usi
     Ok(isolated)
 }
 
-fn extract_image_drawing_ops(ops: &[Op], start: usize, end: usize, scope_matrix: Matrix) -> Vec<Op> {
+/// The scope `start..=end` without its clipping paths, for drawing in the CTM `scope_matrix`
+/// in effect before it. Its `cm`s go too, and one `cm` before the image `Do` at `target` puts
+/// it back at `draw`, so clips any earlier move put around it (`cm inverse`, path, `W n`,
+/// absolute `cm`) can't leave the image displaced.
+fn extract_image_drawing_ops(ops: &[Op], start: usize, end: usize, target: usize, scope_matrix: Matrix, draw: Matrix) -> Result<Vec<Op>, EditError> {
+    let inverse = scope_matrix.invert().ok_or_else(|| EditError::Invalid("unsupported graphics state: non-invertible CTM".into()))?;
+    let placement = draw.then(&inverse);
+    if !placement.0.iter().all(|v| v.is_finite()) {
+        return Err(EditError::Invalid("unsupported graphics state: invalid transform coordinates".into()));
+    }
+    let is_path = |o: &Op| matches!(o.op.as_slice(), b"m" | b"l" | b"c" | b"v" | b"y" | b"h" | b"re");
     let mut drawing = Vec::new();
     let mut i = start.saturating_add(1);
     while i < end {
-        let op = match ops.get(i) {
-            Some(o) => o,
-            None => break,
-        };
-        if op.is("cm")
-            && scope_matrix != Matrix::IDENTITY
-            && let (Some(m), Some(inv)) = (op.nums::<6>(), scope_matrix.invert())
-            && Matrix(m) == inv
-        {
-            let mut k = i.saturating_add(1);
-            while ops.get(k).is_some_and(|o| matches!(o.op.as_slice(), b"m" | b"l" | b"c" | b"v" | b"y" | b"h" | b"re")) {
+        let Some(op) = ops.get(i) else { break };
+        if op.is("cm") {
+            i = i.saturating_add(1);
+            continue;
+        }
+        if is_path(op) {
+            let mut k = i;
+            while ops.get(k).is_some_and(is_path) {
                 k = k.saturating_add(1);
             }
-            if k > i.saturating_add(1)
-                && ops.get(k).is_some_and(|o| matches!(o.op.as_slice(), b"W" | b"W*"))
-                && ops.get(k.saturating_add(1)).is_some_and(|o| o.is("n"))
-            {
-                i = i.saturating_add(1);
+            if ops.get(k).is_some_and(|o| o.is("W") || o.is("W*")) && ops.get(k.saturating_add(1)).is_some_and(|o| o.is("n")) {
+                i = k.saturating_add(2);
                 continue;
             }
         }
-        if matches!(op.op.as_slice(), b"m" | b"l" | b"c" | b"v" | b"y" | b"h" | b"re") {
-            let mut k = i;
-            while ops.get(k).is_some_and(|o| matches!(o.op.as_slice(), b"m" | b"l" | b"c" | b"v" | b"y" | b"h" | b"re")) {
-                k = k.saturating_add(1);
-            }
-            if k > i && ops.get(k).is_some_and(|o| matches!(o.op.as_slice(), b"W" | b"W*")) && ops.get(k.saturating_add(1)).is_some_and(|o| o.is("n"))
-            {
-                i = k.saturating_add(2);
-                if scope_matrix != Matrix::IDENTITY
-                    && ops.get(i).is_some_and(|next| next.is("cm"))
-                    && let Some(m) = ops.get(i).and_then(|next| next.nums::<6>())
-                    && Matrix(m) == scope_matrix
-                {
-                    i = i.saturating_add(1);
-                }
-                continue;
-            }
+        if i == target && placement != Matrix::IDENTITY {
+            drawing.push(Op::new("cm", placement.0.map(pdfcraft_content::num).to_vec()));
         }
         drawing.push(op.clone());
         i = i.saturating_add(1);
     }
-    drawing
+    Ok(drawing)
 }
 
 /// Remove any private clipping path / crop on image `index` on `page`.
@@ -1290,7 +1302,7 @@ pub fn remove_image_crop(doc: &mut Document, page: usize, index: usize) -> Resul
         .position(|op| op.span.start == span && op.is("Do"))
         .ok_or_else(|| EditError::Invalid("the image operation could not be located".into()))?;
     if let Some((start, end, scope_matrix)) = image_exclusive_scope(&ops, target)? {
-        let drawing_ops = extract_image_drawing_ops(&ops, start, end, scope_matrix);
+        let drawing_ops = extract_image_drawing_ops(&ops, start, end, target, scope_matrix, Matrix(img.draw_matrix))?;
         let start_op = ops.get(start).ok_or_else(|| EditError::Invalid("the image operation is missing".into()))?;
         let end_op = ops.get(end).ok_or_else(|| EditError::Invalid("the image operation is missing".into()))?;
         let range = start_op.span.start..end_op.span.end;

crates/edit/src/tests.rs

@@ -2241,14 +2241,72 @@ fn image_arrange_preserves_exact_non_rectangular_clipping_path() {
 
     // 2. Appearance: polygon geometry and even-odd rule are preserved
     assert!(str_content.contains("W*"), "Appearance: even-odd W* fill rule preserved");
-    assert!(str_content.contains("120 120 m 160 120 l 140 160 l h W* n"), "Appearance: exact polygon clip preserved");
+    // The original group still clips the image left behind, so the moved copy must carry its own.
+    let flat = str_content.split_whitespace().collect::<Vec<_>>().join(" ");
+    assert_eq!(flat.matches("120 120 m 160 120 l 140 160 l h W* n").count(), 2, "Appearance: exact polygon clip preserved in both units");
     let after = images::page_images(&doc, 0).unwrap();
     assert_eq!(after.len(), 2, "both images preserved");
+    for image in &after {
+        assert!(image.contains([140.0, 130.0]), "Appearance: inside the triangle shows");
+        assert!(!image.contains([125.0, 155.0]), "Appearance: inside its box but outside the triangle stays clipped");
+    }
 
     // 3. Preservation: External Text and remaining original group content preserved
     assert!(str_content.contains("External Text"), "Preservation: external text preserved");
 }
 
+#[test]
+fn image_remove_crop_after_arrange_keeps_position_under_outer_transform() {
+    // Arrange writes `cm inverse`, the clip, then the absolute placement; Remove Crop used to drop
+    // only the first, which drew the image under the outer translation twice.
+    let content = "1 0 0 1 50 50 cm q 0 0 50 50 re W n 100 0 0 100 0 0 cm /Im0 Do Q BT /F1 12 Tf 72 700 Td (External Text) Tj ET";
+    let mut doc = image_page_with(content);
+    assert_eq!(images::page_images(&doc, 0).unwrap()[0].rect, [50.0, 50.0, 150.0, 150.0]);
+
+    images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap();
+    let mut doc = reopen(&doc);
+    let moved = images::page_images(&doc, 0).unwrap();
+    assert_eq!(moved[0].rect, [50.0, 50.0, 150.0, 150.0]);
+    assert_eq!(moved[0].visible_rect, Some([50.0, 50.0, 100.0, 100.0]));
+
+    images::remove_image_crop(&mut doc, 0, 0).unwrap();
+    let doc = reopen(&doc);
+    let after = images::page_images(&doc, 0).unwrap();
+    assert_eq!(after.len(), 1);
+    assert_eq!(after[0].rect, [50.0, 50.0, 150.0, 150.0], "the image stays where it was");
+    assert_eq!(after[0].visible_rect, Some([50.0, 50.0, 150.0, 150.0]), "the crop is gone");
+}
+
+#[test]
+fn image_arrange_refuses_to_move_past_a_clip_it_is_not_under() {
+    // The text after the image is drawn under a top-level clip; moving the image after it would
+    // crop the image to that clip.
+    let content = "q 100 0 0 100 50 50 cm /Im0 Do Q 0 0 60 60 re W n BT /F1 12 Tf 72 700 Td (External Text) Tj ET";
+    let mut doc = image_page_with(content);
+    let before = page_content_bytes(&doc, 0);
+    assert_eq!(images::page_images(&doc, 0).unwrap()[0].visible_rect, Some([50.0, 50.0, 150.0, 150.0]));
+
+    for order in [images::ArrangeOrder::BringToFront, images::ArrangeOrder::BringForward] {
+        let err = images::arrange_image(&mut doc, 0, 0, order).unwrap_err();
+        assert!(err.to_string().contains("clipping path"), "{err}");
+    }
+    assert_eq!(page_content_bytes(&doc, 0), before, "nothing changes");
+}
+
+#[test]
+fn image_arrange_send_to_back_keeps_a_shared_clip() {
+    // The image after the text is clipped by a top-level clip set before both; behind the text it
+    // is no longer under that clip, so the moved unit has to draw it itself.
+    let content = "BT /F1 12 Tf 72 700 Td (External Text) Tj ET 0 0 60 60 re W n q 100 0 0 100 50 50 cm /Im0 Do Q";
+    let mut doc = image_page_with(content);
+    images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::SendToBack).unwrap();
+    let doc = reopen(&doc);
+    let bytes = page_content_bytes(&doc, 0);
+    let str_content = String::from_utf8_lossy(&bytes);
+    assert!(str_content.rfind("/Im0 Do").unwrap() < str_content.find("External Text").unwrap(), "the image moved behind the text");
+    assert_eq!(images::page_images(&doc, 0).unwrap()[0].visible_rect, Some([50.0, 50.0, 60.0, 60.0]), "still cropped to the clip");
+}
+
 #[test]
 fn unwrapped_image_moves_successfully() {
     let content = "100 0 0 100 50 50 cm /Im0 Do BT /F1 12 Tf 72 700 Td (Middle Text) Tj ET";