diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index 98d7205434..5ed70aed7a 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -256,8 +256,12 @@ fn run_edit_page_image(doc: &mut pdfcraft_cos::Document, page: usize, index: usi ImageEdit::Move(to) if !to.iter().all(|v| v.is_finite()) => { return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("invalid position".into()))); } - ImageEdit::Transform(t) if !t.iter().all(|v| v.is_finite()) => { - return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("the image transform must be finite".into()))); + ImageEdit::Transform(t) => { + let [a, b, c, d, _, _] = *t; + let determinant = a * d - b * c; + if !t.iter().all(|v| v.is_finite()) || !determinant.is_finite() || determinant.abs() < 1e-9 { + return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("the image transform must be finite and nonsingular".into()))); + } } ImageEdit::Crop(keep) if !keep.iter().all(|v| v.is_finite()) => { return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("invalid crop".into()))); @@ -326,7 +330,10 @@ fn run_edit_page_image(doc: &mut pdfcraft_cos::Document, page: usize, index: usi || (v[2] - img.rect[2]).abs() > 1e-4 || (v[3] - img.rect[3]).abs() > 1e-4 }); + let non_axis_transform = matches!(change, ImageEdit::Transform([a, b, c, d, _, _]) + if !((b.abs() < 1e-5 && c.abs() < 1e-5) || (a.abs() < 1e-5 && d.abs() < 1e-5))); let should_promote = has_active_crop + || non_axis_transform || matches!(change, ImageEdit::Crop(_) | ImageEdit::RemoveCrop | ImageEdit::RotateBy(_) | ImageEdit::Arrange(_) | ImageEdit::Delete); if should_promote { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index d8a7b793fc..51a19b7b24 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -9439,7 +9439,7 @@ fn mirroring_transform_on_added_image_flips_and_non_finite_is_refused() { } #[test] -fn transform_on_added_image_folds_quarter_turns_and_rejects_non_axis_aligned() { +fn transform_on_added_image_folds_quarter_turns_and_promotes_non_axis_aligned() { let make_png = |rgb: [u8; 3]| { let mut png = Vec::new(); { @@ -9482,16 +9482,21 @@ fn transform_on_added_image_folds_quarter_turns_and_rejects_non_axis_aligned() { assert!((ai2.rect[2] - 200.0).abs() < 1e-4); assert!((ai2.rect[3] - 150.0).abs() < 1e-4); - // 3. Non-axis-aligned transforms (e.g. 45° rotation or skew) are rejected: + // Arbitrary transforms promote the added image, just like RotateBy. + let initial_matrix = s.get(id).unwrap().page_images(0)[0].matrix; let rot_45 = pdfcraft_edit::turn_by_degrees([100.0, 100.0, 200.0, 150.0], 45.0).unwrap(); - let res_45 = s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform(rot_45) }); - assert!(res_45.is_err(), "non-axis-aligned 45° rotation must be rejected for added image"); - assert!(res_45.unwrap_err().to_string().contains("non-axis-aligned")); - - let skew = [1.0, 0.5, 0.0, 1.0, 0.0, 0.0]; - let res_skew = s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform(skew) }); - assert!(res_skew.is_err(), "skew must be rejected for added image"); - assert!(res_skew.unwrap_err().to_string().contains("non-axis-aligned")); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform(rot_45) }).unwrap(); + let cos = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap(); + assert!(pdfcraft_edit::list_added(&cos).is_empty(), "arbitrary transform promotes the image"); + assert_eq!(s.get(id).unwrap().page_images(0).len(), 1); + let expected = pdfcraft_content::Matrix(initial_matrix).then(&pdfcraft_content::Matrix(rot_45)).0; + let actual = s.get(id).unwrap().page_images(0)[0].matrix; + assert!(actual.into_iter().zip(expected).all(|(a, b)| (a - b).abs() < 1e-3), "{actual:?} != {expected:?}"); + s.undo(id).unwrap(); + let cos = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap(); + assert_eq!(pdfcraft_edit::list_added(&cos).len(), 1, "undo restores added-image editability"); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([1.0, 0.5, 0.0, 1.0, 0.0, 0.0]) }).unwrap(); + assert_eq!(s.get(id).unwrap().page_images(0).len(), 1); } #[test] @@ -9988,3 +9993,19 @@ fn original_image_edit_does_not_match_an_added_sibling_of_the_same_xobject() { assert_eq!(doc.added()[0].content, before_added, "matching an original Do must not select the added sibling's metadata"); assert_eq!(doc.page_images(0)[0].rect, [40.0, 50.0, 140.0, 150.0]); } + +#[test] +fn image_transform_engine_refuses_singular_matrices_without_mutation() { + let mut png = Vec::new(); + let mut enc = png::Encoder::new(&mut png, 1, 1); + enc.set_color(png::ColorType::Rgb); + enc.write_header().unwrap().write_image_data(&[100, 150, 200]).unwrap(); + let mut s = Session::new(); + let bytes = s.create_from_images(&[("pixel.png".into(), png)]).unwrap(); + let id = s.open("image.pdf", None, bytes, None).unwrap(); + for matrix in [[0.0, 0.0, 0.0, 1.0, 0.0, 0.0], [1.0, 1.0, 1.0, 1.0 + 1e-12, 0.0, 0.0], [1e308, 0.0, 0.0, 1e308, 0.0, 0.0]] { + let before = s.get(id).unwrap().bytes.clone(); + assert!(s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform(matrix) }).is_err()); + assert_eq!(s.get(id).unwrap().bytes, before); + } +}