From 36d239d2b1179cded620f3a18e4e0035e3f6621d Mon Sep 17 00:00:00 2001 From: komo1iddin <45395275+komo1iddin@users.noreply.github.com> Date: Sun, 18 Oct 2026 15:47:08 +0800 Subject: [PATCH] M7: fold repeated group moves of a cropped image into one clip cm (#2476) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving an image whose crop is its own (`q re W n cm Do Q`) as part of a group put a fresh `cm` before the clip path on every move, so twenty moves left twenty `cm` operators in front of one `re`. It never nested deeper, but the content stream grew with every drag. When a `cm` already sits right before that clip (one an earlier move added, or the file's own), `translate_images` now replaces it with the product of the new and old matrices. The CTM at the clip is `c · prev · outer` either way (ISO 32000-1 8.3.4, 8.4.4). A previous `cm` that is not finite, or a product larger than 1e12, keeps the old behaviour and inserts a separate `cm`. The new test moves an image cropped smaller than itself twenty times and checks that the image and its visible crop both moved by the full offset, that the operator count stays at its value after the first move, and that the caption after it is untouched. It fails without this change (32 operators against 13). Co-authored-by: Claude Opus 5.5 --- crates/edit/src/images.rs | 15 ++++++++++++--- crates/edit/src/tests.rs | 25 +++++++++++++++++++++++++ docs/editing-images.md | 3 ++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index fb6212588..62bef8bc5 100644 --- a/crates/edit/src/images.rs +++ b/crates/edit/src/images.rs @@ -784,7 +784,16 @@ pub(crate) fn translate_images(doc: &mut Document, page: usize, indexes: &[usize return Err(EditError::Invalid("the image move is too large".into())); }; plans.remove(&(stream, index)); - clips.insert((stream, at), c); + // A `cm` an earlier move put before the clip takes this move too: `c · prev` is the + // same CTM as `prev` followed by another `cm c`, without one more operator per move. + let folded = at + .checked_sub(1) + .and_then(|i| Some((i, c.then(&Matrix(ops.get(i).filter(|op| op.is("cm"))?.nums::<6>()?))))) + .filter(|(_, m)| m.0.iter().all(|v| v.is_finite() && v.abs() <= 1e12)); + match folded { + Some((i, m)) => clips.insert((stream, i), (m, false)), + None => clips.insert((stream, at), (c, true)), + }; } // An image already alone in `q cm Do Q` moves by changing that `cm`, not by nesting another. let mut folds = std::collections::BTreeMap::new(); @@ -799,8 +808,8 @@ pub(crate) fn translate_images(doc: &mut Document, page: usize, indexes: &[usize folds.insert(at, folded); } let changed = crate::text::splice(&pieces, &ops, |index| { - if let Some(c) = clips.remove(&(stream, index)) { - return (vec![Op::new("cm", c.0.map(Object::Real).to_vec())], true); + if let Some((c, keep)) = clips.remove(&(stream, index)) { + return (vec![Op::new("cm", c.0.map(Object::Real).to_vec())], keep); } if let Some(m) = folds.remove(&index) { return (vec![Op::new("cm", m.0.map(Object::Real).to_vec())], false); diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 88e3d7806..7930a7ead 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -1921,6 +1921,31 @@ fn repeated_image_edits_change_one_transform_instead_of_nesting() { } } +#[test] +fn repeated_group_moves_of_a_cropped_image_change_one_clip_transform() { + // The crop is smaller than the image, so a clip left behind would shrink `visible_rect`. + let mut doc = image_page_with("q 150 120 100 50 re W n 200 0 0 100 100 100 cm /Im0 Do Q BT /F1 12 Tf 72 700 Td (Caption) Tj ET"); + let operators = |doc: &Document| pdfcraft_content::parse(&page_content_bytes(doc, 0)).ops.len(); + let start = images::page_images(&doc, 0).unwrap()[0].clone(); + let mut first = 0; + for i in 0..20 { + images::translate_images(&mut doc, 0, &[0], [1.0, -2.0]).unwrap(); + if i == 0 { + first = operators(&doc); + } + } + let moved = |r: [f64; 4]| [r[0] + 20.0, r[1] - 40.0, r[2] + 20.0, r[3] - 40.0]; + let now = images::page_images(&doc, 0).unwrap()[0].clone(); + assert!(close(now.rect, moved(start.rect)), "{:?}", now.rect); + assert!(close(now.visible_rect.unwrap(), moved(start.visible_rect.unwrap())), "the crop moves with the image: {:?}", now.visible_rect); + assert_eq!(operators(&doc), first, "twenty group moves add no more operators than one"); + assert_eq!(q_depth(&doc), 1); + let doc = reopen(&doc); + assert!(close(images::page_images(&doc, 0).unwrap()[0].visible_rect.unwrap(), moved(start.visible_rect.unwrap()))); + let caption: Vec<_> = text::text_lines(&doc, 0).unwrap().into_iter().map(|l| l.text).collect(); + assert_eq!(caption, ["Caption"]); +} + #[test] fn page_images_move_turn_replace_and_delete() { let mut doc = image_page(); diff --git a/docs/editing-images.md b/docs/editing-images.md index eba918ef3..67d3613bf 100644 --- a/docs/editing-images.md +++ b/docs/editing-images.md @@ -8,7 +8,8 @@ together with its pixels, including nested graphics states, intervening state op and paths split across content streams. A clip shared with other content stays where it is. When the figure already sits alone in a `q … cm … Q` (its own placement, or the one an earlier edit added), moving or resizing it again changes that `cm`, so repeated edits don't -nest one more graphics state each time. +nest one more graphics state each time. Moving a privately cropped figure again likewise +changes the `cm` before its clip instead of adding another. PDF figures are not always bitmap images. Imported diagrams often use a **Form XObject**, which packages text, vector paths, and nested images into one object. PdfCraft selects