Fixes As Requested [image transformer,Image extraction,stale cached image]

crates/edit/src/images.rs

@@ -613,69 +613,81 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran
             )
     });
 
-    let (target_span, unit_ops) = if has_other_paint {
-        if let Ok(Some((sub_start, sub_end, sub_placement))) = image_exclusive_scope(&ops, target)
+    let (dest_pos, scan_target) = if insert_after {
+        let Some(op) = dest_unit.end.checked_sub(1).and_then(|i| ops.get(i)) else { return Ok(()) };
+        (op.span.end, dest_unit.end)
+    } else {
+        let Some(op) = ops.get(dest_unit.start) else { return Ok(()) };
+        (op.span.start, dest_unit.start)
+    };
+
+    let mut state = ImageState::new(Matrix::IDENTITY, p.crop(doc), Vec::new());
+    for (i, op) in ops.iter().enumerate() {
+        if i == scan_target {
+            break;
+        }
+        state.apply(op)?;
+    }
+    let dest_ctm = state.placement.matrix;
+    let Some(dest_inv) = dest_ctm.invert() else { return Ok(()); };
+
+    let target_span = if has_other_paint {
+        if let Ok(Some((sub_start, sub_end, _))) = image_exclusive_scope(&ops, target)
             && sub_start >= target_unit.start
             && sub_end < target_unit.end
         {
-            let u_ops = if sub_placement == Matrix::IDENTITY {
-                ops.get(sub_start..=sub_end).unwrap_or_default().to_vec()
-            } else {
-                let mut out = Vec::new();
-                out.push(Op::new("q", vec![]));
-                out.push(Op::new("cm", sub_placement.0.map(pdfcraft_content::num).to_vec()));
-                if let Some(inner) = ops.get(sub_start.saturating_add(1)..=sub_end) {
-                    out.extend_from_slice(inner);
-                }
-                out
-            };
             let start_op = ops.get(sub_start).unwrap_or(&ops[target]);
             let end_op = ops.get(sub_end).unwrap_or(&ops[target]);
-            (start_op.span.start..end_op.span.end, u_ops)
+            start_op.span.start..end_op.span.end
         } else {
-            let u_ops = vec![
-                Op::new("q", vec![]),
-                Op::new("cm", img.draw_matrix.map(pdfcraft_content::num).to_vec()),
-                ops.get(target).cloned().unwrap_or_else(|| Op::new("Do", vec![Object::name(&img.name)])),
-                Op::new("Q", vec![]),
-            ];
             let target_op = &ops[target];
-            (target_op.span.start..target_op.span.end, u_ops)
+            target_op.span.start..target_op.span.end
         }
     } else {
-        let (scope_start, scope_end, placement) = image_transform_scope(&ops, target)?.unwrap_or((target, target, Matrix(img.draw_matrix)));
-        let u_ops = if scope_start != scope_end && target_unit.start == scope_start && target_unit.end == scope_end.saturating_add(1) {
-            if placement == Matrix::IDENTITY {
-                ops.get(scope_start..=scope_end).unwrap_or_default().to_vec()
-            } else {
-                let mut out = Vec::new();
-                out.push(Op::new("q", vec![]));
-                out.push(Op::new("cm", placement.0.map(pdfcraft_content::num).to_vec()));
-                if let Some(inner) = ops.get(scope_start.saturating_add(1)..=scope_end) {
-                    out.extend_from_slice(inner);
-                }
-                out
-            }
-        } else {
-            vec![
-                Op::new("q", vec![]),
-                Op::new("cm", img.draw_matrix.map(pdfcraft_content::num).to_vec()),
-                ops.get(target).cloned().unwrap_or_else(|| Op::new("Do", vec![Object::name(&img.name)])),
-                Op::new("Q", vec![]),
-            ]
-        };
         let Some(first_op) = ops.get(target_unit.start) else { return Ok(()) };
         let Some(last_op) = target_unit.end.checked_sub(1).and_then(|i| ops.get(i)) else { return Ok(()) };
-        (first_op.span.start..last_op.span.end, u_ops)
+        first_op.span.start..last_op.span.end
     };
 
-    let dest_pos = if insert_after {
-        let Some(op) = dest_unit.end.checked_sub(1).and_then(|i| ops.get(i)) else { return Ok(()) };
-        op.span.end
+    let is_clipped = img.visible_rect.is_some() && img.visible_rect != Some(img.rect);
+    if has_other_paint && is_clipped && img.clips.len() > 1 {
+        // Safely decline extraction when complex multi-path clipping cannot be preserved
+        return Ok(());
+    }
+
+    let mut unit_ops = Vec::new();
+    unit_ops.push(Op::new("q", vec![]));
+    if is_clipped {
+        if dest_ctm != Matrix::IDENTITY {
+            unit_ops.push(Op::new("cm", dest_inv.0.map(pdfcraft_content::num).to_vec()));
+        }
+        if let Some(clip) = img.visible_rect {
+            let cx0 = clip[0];
+            let cy0 = clip[1];
+            let cw = clip[2] - clip[0];
+            let ch = clip[3] - clip[1];
+            if cw > 0.0 && ch > 0.0 {
+                unit_ops.push(Op::new("re", vec![
+                    pdfcraft_content::num(cx0),
+                    pdfcraft_content::num(cy0),
+                    pdfcraft_content::num(cw),
+                    pdfcraft_content::num(ch),
+                ]));
+                unit_ops.push(Op::new("W", vec![]));
+                unit_ops.push(Op::new("n", vec![]));
+            }
+        }
+        if Matrix(img.draw_matrix) != Matrix::IDENTITY {
+            unit_ops.push(Op::new("cm", img.draw_matrix.map(pdfcraft_content::num).to_vec()));
+        }
     } else {
-        let Some(op) = ops.get(dest_unit.start) else { return Ok(()) };
-        op.span.start
-    };
+        let local = Matrix(img.draw_matrix).then(&dest_inv);
+        if local != Matrix::IDENTITY {
+            unit_ops.push(Op::new("cm", local.0.map(pdfcraft_content::num).to_vec()));
+        }
+    }
+    unit_ops.push(ops.get(target).cloned().unwrap_or_else(|| Op::new("Do", vec![Object::name(&img.name)])));
+    unit_ops.push(Op::new("Q", vec![]));
 
     let mut serialized = Vec::new();
     serialized.push(b'\n');

crates/edit/src/tests.rs

@@ -1805,6 +1805,47 @@ fn image_crop_preserves_outer_cm_for_subsequent_drawings() {
     assert_eq!(after[1].rect, [50.0, 50.0, 150.0, 150.0], "subsequent image must retain global cm");
 }
 
+#[test]
+fn image_arrange_preserves_position_under_active_ctm() {
+    let content = "100 0 0 100 50 50 cm /Im0 Do /Im0 Do";
+    let mut doc = image_page_with(content);
+    let initial = images::page_images(&doc, 0).unwrap();
+    assert_eq!(initial.len(), 2);
+    assert_eq!(initial[0].rect, [50.0, 50.0, 150.0, 150.0]);
+    assert_eq!(initial[1].rect, [50.0, 50.0, 150.0, 150.0]);
+
+    // Move first image to front: must not apply destination CTM twice!
+    images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap();
+    let doc = reopen(&doc);
+    let after = images::page_images(&doc, 0).unwrap();
+    assert_eq!(after.len(), 2);
+    assert_eq!(after[0].rect, [50.0, 50.0, 150.0, 150.0], "first image position preserved");
+    assert_eq!(after[1].rect, [50.0, 50.0, 150.0, 150.0], "second image position preserved");
+}
+
+#[test]
+fn image_arrange_preserves_shared_group_clipping_bounds() {
+    let content = "q 120 120 40 40 re W n 100 0 0 100 100 100 cm /Im0 Do /Im0 Do Q BT /F1 12 Tf 72 700 Td (External Text) Tj ET";
+    let mut doc = image_page_with(content);
+    let initial = images::page_images(&doc, 0).unwrap();
+    assert_eq!(initial.len(), 2);
+    assert_eq!(initial[0].rect, [100.0, 100.0, 200.0, 200.0]);
+    assert_eq!(initial[0].visible_rect, Some([120.0, 120.0, 160.0, 160.0]));
+    assert_eq!(initial[1].rect, [100.0, 100.0, 200.0, 200.0]);
+    assert_eq!(initial[1].visible_rect, Some([120.0, 120.0, 160.0, 160.0]));
+
+    // Bring image 0 to front: moves it past the external text while preserving clipping bounds
+    images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap();
+    let doc = reopen(&doc);
+    let after = images::page_images(&doc, 0).unwrap();
+    assert_eq!(after.len(), 2, "both images preserved");
+    assert_eq!(after[0].visible_rect, Some([120.0, 120.0, 160.0, 160.0]), "image 0 retains clip");
+    assert_eq!(after[1].visible_rect, Some([120.0, 120.0, 160.0, 160.0]), "image 1 retains clip");
+    let bytes = page_content_bytes(&doc, 0);
+    let str_content = String::from_utf8_lossy(&bytes);
+    assert!(str_content.contains("External Text"), "external text preserved");
+}
+
 
 #[test]
 fn form_artwork_edits_only_the_selected_placement() {

crates/ui-egui/src/content_ui.rs

@@ -774,9 +774,8 @@ impl crate::PdfCraftApp {
         {
             if self.system_clipboard
                 && let Ok(mut cb) = arboard::Clipboard::new()
-                && cb.get_image().is_ok()
             {
-                return true;
+                return cb.get_image().is_ok();
             }
         }
         self.last_copied_image.is_some() || self.last_copied_file.is_some()
@@ -793,34 +792,39 @@ impl crate::PdfCraftApp {
         }
 
         #[cfg(not(target_arch = "wasm32"))]
-        let os_png = if self.system_clipboard {
-            if let Ok(mut cb) = arboard::Clipboard::new()
-                && let Ok(img) = cb.get_image()
-            {
-                let matches_internal = self.last_copied_image.as_ref().is_some_and(|(w, h, rgba)| {
-                    *w as usize == img.width && *h as usize == img.height && rgba.as_slice() == img.bytes.as_ref()
-                });
-                if !matches_internal {
-                    crate::create_ui::png_from_rgba(img.width, img.height, &img.bytes).ok().map(|bytes| {
-                        (img.width as u32, img.height as u32, img.bytes.to_vec(), bytes)
-                    })
-                } else {
-                    None
-                }
+        let (has_system_cb, os_image_res) = if self.system_clipboard {
+            if let Ok(mut cb) = arboard::Clipboard::new() {
+                (true, Some(cb.get_image()))
             } else {
-                None
+                (false, None)
             }
         } else {
-            None
+            (false, None)
         };
         #[cfg(target_arch = "wasm32")]
-        let os_png: Option<(u32, u32, Vec<u8>, Vec<u8>)> = None;
+        let (has_system_cb, os_image_res): (bool, Option<Result<arboard::ImageData<'static>, arboard::Error>>) = (false, None);
 
-        let image_data: Option<(String, Vec<u8>)> = if let Some((w, h, rgba, png_bytes)) = os_png {
-            self.last_copied_image = Some((w, h, rgba));
-            self.last_copied_file = Some(("clipboard_image.png".to_string(), png_bytes.clone()));
-            self.last_copied_image_rect = None;
-            Some(("clipboard_image.png".to_string(), png_bytes))
+        let image_data: Option<(String, Vec<u8>)> = if has_system_cb {
+            match os_image_res {
+                Some(Ok(img)) => {
+                    let matches_internal = self.last_copied_image.as_ref().is_some_and(|(w, h, rgba)| {
+                        *w as usize == img.width && *h as usize == img.height && rgba.as_slice() == img.bytes.as_ref()
+                    });
+                    if matches_internal && let Some((name, bytes)) = self.last_copied_file.clone() {
+                        Some((name, bytes))
+                    } else if let Ok(png_bytes) = crate::create_ui::png_from_rgba(img.width, img.height, &img.bytes) {
+                        self.last_copied_image = Some((img.width as u32, img.height as u32, img.bytes.to_vec()));
+                        self.last_copied_file = Some(("clipboard_image.png".to_string(), png_bytes.clone()));
+                        self.last_copied_image_rect = None;
+                        Some(("clipboard_image.png".to_string(), png_bytes))
+                    } else {
+                        None
+                    }
+                }
+                // When system clipboard is active and accessible but contains text or non-image data:
+                // Do NOT fall back to stale in-memory cached image!
+                _ => None,
+            }
         } else if let Some((name, bytes)) = self.last_copied_file.clone() {
             Some((name, bytes))
         } else if let Some((w, h, rgba)) = self.last_copied_image.clone() {

crates/ui-egui/tests/editing.rs

@@ -2717,6 +2717,7 @@ fn image_edge_handle_stretch_and_crop_mode() {
     h.event(egui::Event::PointerButton { pos: c, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() });
     h.run_steps(2);
     h.get_by_label("Remove crop").click();
+    h.run_steps(4);
     let images = h.state().session.get(doc_id).unwrap().page_images(0);
     assert_eq!(images[0].visible_rect, Some(images[0].rect), "crop is removed");
 }
@@ -4479,3 +4480,26 @@ fn image_copy_cut_paste_keyboard_and_context_menu() {
     let after_paste2 = h.state().session.get(id).unwrap().page_images(0).len();
     assert_eq!(after_paste2, 2, "pasted via context menu");
 }
+
+#[test]
+fn stale_clipboard_plain_text_does_not_paste_image() {
+    let mut h = harness(1, |_| {});
+    h.run_steps(4);
+    // Put an image into internal cache
+    h.state_mut().last_copied_image = Some((10, 10, vec![0; 400]));
+    h.state_mut().last_copied_file = Some(("cached.png".to_string(), vec![1, 2, 3]));
+    h.state_mut().system_clipboard = true;
+
+    // Set actual system clipboard to plain text
+    #[cfg(not(target_arch = "wasm32"))]
+    if let Ok(mut cb) = arboard::Clipboard::new() {
+        cb.set_text("just plain text").ok();
+    }
+
+    // can_paste_image should now be false because the system clipboard has text, not an image!
+    assert!(!h.state().can_paste_image(), "cannot paste image when system clipboard has text");
+
+    // paste_image should fail and return false without pasting
+    assert!(!h.state_mut().paste_image(0, None), "must not paste stale cached image");
+}
+