diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index be963244e0..b54ecd5bf5 100644 --- a/crates/edit/src/images.rs +++ b/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'); diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 9d74024daa..af3c6c3908 100644 --- a/crates/edit/src/tests.rs +++ b/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() { diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 1b625e1290..1865032206 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/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, Vec)> = None; + let (has_system_cb, os_image_res): (bool, Option, arboard::Error>>) = (false, None); - let image_data: Option<(String, Vec)> = 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)> = 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() { diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index f7630f955d..9055414169 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/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"); +} +