diff --git a/Cargo.lock b/Cargo.lock index c6f4f49d69..92e9da9848 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4096,6 +4096,7 @@ dependencies = [ "pdfcraft-cos", "pdfcraft-fonts", "pdfcraft-model", + "pdfcraft-render", "thiserror 2.0.21", "unicode-bidi", ] diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index 46d4cee891..84dd404590 100644 --- a/crates/edit/src/images.rs +++ b/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 Vec { +/// 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, 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; diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 546f1ed4d6..daf29de783 100644 --- a/crates/edit/src/tests.rs +++ b/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::>().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";