From dea68e6350b43b148773f2c67d82610098923534 Mon Sep 17 00:00:00 2001 From: Combiler01 Date: Thu, 22 Oct 2026 23:55:33 +0530 Subject: [PATCH] Fixes Suggest by Reviewers --- crates/automation/tests/it/automation.rs | 28 +++-- crates/edit/src/images.rs | 106 ++++++++++-------- crates/edit/src/tests.rs | 96 ++++++++++++++-- crates/engine/src/lib.rs | 66 +++++++++++ crates/engine/src/tests.rs | 64 +++++++++++ crates/ui-egui/src/canvas.rs | 12 +- crates/ui-egui/src/content_ui.rs | 137 +++++++++++++++-------- crates/ui-egui/src/edit_text_ui.rs | 24 ++-- crates/ui-egui/src/image_pick.rs | 1 + crates/ui-egui/src/lib.rs | 4 +- crates/ui-egui/tests/it/editing.rs | 89 +++++++++++++++ 11 files changed, 499 insertions(+), 128 deletions(-) diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 74a93173d6..bc09179f3a 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -3040,14 +3040,11 @@ fn editing_page_images_through_tools() { assert_eq!(cropped["visible_rect"], json!([10.0, 10.0, 25.0, 30.0])); ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 1, "action": "rotate" })); ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 1, "action": "flip_horizontal" })); - let r = ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 1, "action": "bring_to_front" })); - assert_eq!(r["undo"], "Bring to front"); - let r = ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 1, "action": "send_to_back" })); - assert_eq!(r["undo"], "Send to back"); - let r = ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 1, "action": "bring_forward" })); - assert_eq!(r["undo"], "Bring forward"); - let r = ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 1, "action": "send_backward" })); - assert_eq!(r["undo"], "Send backward"); + // A single image is already in front and at the back; no-op arrange is refused without dirtying undo. + assert!(a.call("image_edit", &json!({ "doc": made, "page": 1, "image": 1, "action": "bring_to_front" })).is_err()); + assert!(a.call("image_edit", &json!({ "doc": made, "page": 1, "image": 1, "action": "send_to_back" })).is_err()); + assert!(a.call("image_edit", &json!({ "doc": made, "page": 1, "image": 1, "action": "bring_forward" })).is_err()); + assert!(a.call("image_edit", &json!({ "doc": made, "page": 1, "image": 1, "action": "send_backward" })).is_err()); let r = ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 1, "action": "remove_crop" })); assert_eq!(r["undo"], "Remove crop"); // Any angle: the box grows to the turned picture's bounds, and it undoes like a quarter turn. @@ -3069,6 +3066,21 @@ fn editing_page_images_through_tools() { assert!(matches!(a.call("image_edit", &json!({ "doc": made, "page": 1, "image": 1, "action": "delete" })), Err(ToolError::InvalidArgs(_)))); } +#[test] +fn arranging_page_images_through_tools() { + let dir = workdir("arrange-page-images"); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap(); + ok(&mut a, "doc_export_images", json!({ "doc": doc, "folder": "src", "dpi": 18, "pages": [1, 2] })); + let made = ok(&mut a, "doc_create", json!({ "from": "images", "paths": ["src/a_page_1.png"] }))["doc"].as_u64().unwrap(); + ok(&mut a, "page_add_image", json!({ "doc": made, "page": 1, "path": "src/a_page_2.png", "rect": [50, 50, 100, 100] })); + assert_eq!(ok(&mut a, "page_images", json!({ "doc": made, "page": 1 }))["count"], 2); + let r = ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 1, "action": "bring_to_front" })); + assert_eq!(r["undo"], "Bring to front"); + let r = ok(&mut a, "image_edit", json!({ "doc": made, "page": 1, "image": 2, "action": "send_to_back" })); + assert_eq!(r["undo"], "Send to back"); +} + #[test] fn enhance_camera_image_through_tools() { let dir = workdir("enhance-camera"); diff --git a/crates/edit/src/images.rs b/crates/edit/src/images.rs index 4e8cc70c6e..6d15dcdc35 100644 --- a/crates/edit/src/images.rs +++ b/crates/edit/src/images.rs @@ -775,8 +775,6 @@ impl ExtendedGStateTracker { if let Some(name) = op.name(0) { if let Some(dict) = ext_gstates.get(name).map(|v| doc.resolve(v)).and_then(|v| v.as_dict().cloned()) { self.current_state.merge_dict(&dict, doc, Some(name)); - } else if self.current_state.ca.is_none() { - self.current_state.ca = Some(0.5); } self.active_gs_ops.retain(|(o, _)| o.name(0) != Some(name)); self.active_gs_ops.push((op.clone(), ctm)); @@ -977,39 +975,47 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran let (scan_start, scan_end) = if is_wrapped { (1, ops.len().saturating_sub(1)) } else { (0, ops.len()) }; let units = content_units(&ops, scan_start, scan_end); let Some(target_idx) = units.iter().position(|u| u.start <= target && target < u.end) else { - return Ok(()); + return Err(EditError::Invalid("the image content unit could not be located".into())); }; if units.len() <= 1 { - return Ok(()); + return Err(EditError::Invalid("the page has only one content unit to arrange".into())); } let (dest_unit, insert_after) = match order { ArrangeOrder::BringToFront => { if target_idx == units.len().saturating_sub(1) { - return Ok(()); + return Err(EditError::Invalid("the image is already in front".into())); } - let Some(u) = units.last() else { return Ok(()) }; + let Some(u) = units.last() else { + return Err(EditError::Invalid("the image is already in front".into())); + }; (u, true) } ArrangeOrder::SendToBack => { if target_idx == 0 { - return Ok(()); + return Err(EditError::Invalid("the image is already at the back".into())); } - let Some(u) = units.first() else { return Ok(()) }; + let Some(u) = units.first() else { + return Err(EditError::Invalid("the image is already at the back".into())); + }; (u, false) } ArrangeOrder::BringForward => { if target_idx == units.len().saturating_sub(1) { - return Ok(()); + return Err(EditError::Invalid("the image is already in front".into())); } - let Some(u) = units.get(target_idx.saturating_add(1)) else { return Ok(()) }; + let Some(u) = units.get(target_idx.saturating_add(1)) else { + return Err(EditError::Invalid("the image is already in front".into())); + }; (u, true) } ArrangeOrder::SendBackward => { if target_idx == 0 { - return Ok(()); + return Err(EditError::Invalid("the image is already at the back".into())); } - let Some(u) = target_idx.checked_sub(1).and_then(|i| units.get(i)) else { return Ok(()) }; + let Some(u) = target_idx.checked_sub(1).and_then(|i| units.get(i)) else { + return Err(EditError::Invalid("the image is already at the back".into())); + }; (u, false) } }; @@ -1030,36 +1036,36 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran // other content and leaves the remaining tagged content intact. let marked_unit = ops.get(target_unit.start).is_some_and(|o| o.is("BDC") || o.is("BMC")); - let (mut target_span, scope_start) = if has_other_paint { + let target_op = ops.get(target).ok_or_else(|| EditError::Invalid("the image operation could not be located".into()))?; + let (mut target_span, scope_start, rem_start, rem_end) = 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 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, sub_start) + let start_op = ops.get(sub_start).unwrap_or(target_op); + let end_op = ops.get(sub_end).unwrap_or(target_op); + (start_op.span.start..end_op.span.end, sub_start, sub_start, sub_end.saturating_add(1)) } else { - let target_op = &ops[target]; - (target_op.span.start..target_op.span.end, target) + (target_op.span.start..target_op.span.end, target, target, target.saturating_add(1)) } } else if let Ok(Some((sub_start, sub_end, _))) = image_exclusive_scope(&ops, target) && sub_start >= target_unit.start && sub_end < target_unit.end && !marked_unit { - 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, sub_start) + let start_op = ops.get(sub_start).unwrap_or(target_op); + let end_op = ops.get(sub_end).unwrap_or(target_op); + (start_op.span.start..end_op.span.end, sub_start, sub_start, sub_end.saturating_add(1)) } else if marked_unit { // A marked-content unit holding only this image moves whole, with its BDC…EMC. // The whole sequence is removed; the image's state is rebuilt from the ops inside it. 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, target_unit.start.saturating_add(1).min(target)) + (first_op.span.start..last_op.span.end, target_unit.start.saturating_add(1).min(target), target_unit.start, target_unit.end) } else { 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, target_unit.start) + (first_op.span.start..last_op.span.end, target_unit.start, target_unit.start, target_unit.end) }; let (dest_pos, scan_target) = if insert_after { @@ -1082,9 +1088,14 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran let mut scope_start_ctm = Matrix::IDENTITY; let mut outer_clips = Vec::new(); let mut dest_clips = Vec::new(); + let mut ctm_before_removal = Matrix::IDENTITY; + let mut ctm_after_removal = Matrix::IDENTITY; - let max_scan = scan_target.max(target.saturating_add(1)).max(scope_start.saturating_add(1)); + let max_scan = scan_target.max(target.saturating_add(1)).max(scope_start.saturating_add(1)).max(rem_end); for (i, op) in ops.iter().enumerate() { + if i == rem_start { + ctm_before_removal = state.placement.matrix; + } if i == scope_start { scope_start_ctm = state.placement.matrix; outer_clips = state.placement.clips.clone(); @@ -1103,6 +1114,9 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran } state.apply(op)?; tracker.step(op, &ext_gstates, doc, state.placement.matrix)?; + if i == rem_end.saturating_sub(1) { + ctm_after_removal = state.placement.matrix; + } } if scan_target >= ops.len() { dest_effective_state = tracker.current_state.clone(); @@ -1164,12 +1178,10 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran let mut reconstructed_state = dest_effective_state.clone(); for (op, _) in &source_gs_ops { - if let Some(name) = op.name(0) { - if let Some(dict) = ext_gstates.get(name).map(|v| doc.resolve(v)).and_then(|v| v.as_dict().cloned()) { - reconstructed_state.merge_dict(&dict, doc, Some(name)); - } else if reconstructed_state.ca.is_none() { - reconstructed_state.ca = Some(0.5); - } + if let Some(name) = op.name(0) + && let Some(dict) = ext_gstates.get(name).map(|v| doc.resolve(v)).and_then(|v| v.as_dict().cloned()) + { + reconstructed_state.merge_dict(&dict, doc, Some(name)); } } @@ -1180,18 +1192,7 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran } let mut unit_ops = Vec::new(); - let marked_open = if marked_unit { - if has_other_paint { - let open = ops.get(target_unit.start); - let has_mcid = - open.is_some_and(|o| o.is("BDC") && o.operands.get(1).and_then(|dict| dict.as_dict()).is_some_and(|d| d.get(b"MCID").is_some())); - if has_mcid { Some(Op::new("BMC", vec![Object::name("Artifact")])) } else { open.cloned() } - } else { - ops.get(target_unit.start).cloned() - } - } else { - None - }; + let marked_open = if marked_unit { ops.get(target_unit.start).cloned() } else { None }; if let Some(open) = &marked_open { unit_ops.push(open.clone()); } @@ -1254,15 +1255,27 @@ pub fn arrange_image(doc: &mut Document, page: usize, index: usize, order: Arran serialized.extend_from_slice(&serialize_ops(&unit_ops)); serialized.push(b'\n'); + let leaking = if let Some(inv) = ctm_before_removal.invert() { ctm_after_removal.then(&inv) } else { Matrix::IDENTITY }; + let removal_replacement = if leaking != Matrix::IDENTITY { + let leak_ops = vec![Op::new("cm", leaking.0.map(pdfcraft_content::num).to_vec())]; + let mut s = Vec::new(); + s.push(b'\n'); + s.extend_from_slice(&serialize_ops(&leak_ops)); + s.push(b'\n'); + s + } else { + Vec::new() + }; + let mut edits = Vec::new(); if dest_pos >= target_span.end { - edits.push((target_span, Vec::new())); + edits.push((target_span, removal_replacement)); edits.push((dest_pos..dest_pos, serialized)); } else if dest_pos <= target_span.start { edits.push((dest_pos..dest_pos, serialized)); - edits.push((target_span, Vec::new())); + edits.push((target_span, removal_replacement)); } else { - return Ok(()); + return Err(EditError::Invalid("no change in image stacking order".into())); } let changed = pieces.splice(edits); @@ -1636,6 +1649,9 @@ fn crop_image_impl( image: &PageImage, keep: [f64; 4], ) -> Result<(), EditError> { + if image.is_form { + return Err(EditError::Invalid("cropping grouped Form artwork is not supported".into())); + } if !keep.iter().all(|v| v.is_finite() && v.abs() <= 1e12) { return Err(EditError::Invalid("the crop box must be finite and no larger than 1e12".into())); } diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 161585e53c..677f6a0133 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -2675,9 +2675,9 @@ fn image_arrange_stacking_order_and_remove_crop() { assert!(close(initial_images[0].rect, [100.0, 100.0, 200.0, 200.0])); assert!(close(initial_images[1].rect, [200.0, 200.0, 350.0, 350.0])); - // Boundary case: image 0 is already at the back; SendToBack and SendBackward are no-ops. - images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::SendToBack).unwrap(); - images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::SendBackward).unwrap(); + // Boundary case: image 0 is already at the back; SendToBack and SendBackward report an error. + assert!(images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::SendToBack).is_err()); + assert!(images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::SendBackward).is_err()); let imgs = images::page_images(&doc, 0).unwrap(); assert_eq!(imgs.len(), 2); assert!(close(imgs[0].rect, initial_images[0].rect)); @@ -2849,8 +2849,8 @@ fn image_arrange_moves_a_bmc_sequence_as_a_whole() { assert!(body < art && art < img && img < emc, "text first, then the whole artifact sequence: {s}"); } -/// An image that shares its marked-content sequence with other content is extracted into its own -/// tag (/Artifact) when arranged, keeping the remaining content in its original tag. +/// An image that shares its marked-content sequence with other content keeps its tag and properties +/// (e.g. /Figure with MCID) when arranged, keeping the remaining content in its original tag. #[test] fn image_arrange_splits_a_shared_marked_sequence_into_own_tag() { let content = @@ -2864,16 +2864,17 @@ fn image_arrange_splits_a_shared_marked_sequence_into_own_tag() { let caption = s.find("(Caption)").expect("caption preserved"); let emc1 = s.find("EMC").expect("first EMC"); assert!(fig < caption && caption < emc1, "caption stays in Figure sequence: {s}"); - // The moved image is placed after Body, wrapped in its own Artifact sequence. + // The moved image is placed after Body, wrapped in its own sequence retaining the Figure tag and MCID. let body = s.find("(Body)").expect("body text"); - let art = s.find("/Artifact BMC").expect("image has its own Artifact tag"); + let fig2 = s.rfind("/Figure <> BDC").expect("image retains Figure tag and MCID"); let img = s.find("/Im0 Do").expect("image moved"); let emc2 = s.rfind("EMC").expect("second EMC"); - assert!(emc1 < body && body < art && art < img && img < emc2, "image moved after body in own tag: {s}"); + assert!(emc1 < body && body < fig2 && fig2 < img && img < emc2, "image moved after body in own tag: {s}"); + assert_ne!(fig, fig2); assert_eq!(images::page_images(&doc, 0).unwrap().len(), 1); } -/// Send backward also splits out the image into its own tag when it shares its sequence with text, +/// Send backward also splits out the image while keeping its tag when it shares its sequence with text, /// placing it before the preceding unit and keeping the remaining content tagged. #[test] fn image_arrange_send_backward_splits_shared_marked_sequence() { @@ -2887,9 +2888,10 @@ fn image_arrange_send_backward_splits_shared_marked_sequence() { let img = s.find("/Im0 Do").expect("image"); let cell = s.find("(Cell)").expect("cell text"); assert!(img < header && header < cell, "image sent backward before header, while cell stays: {s}"); - let table = s.find("/Table <> BDC").expect("table tag preserved"); + let table = s.rfind("/Table <> BDC").expect("table tag preserved"); + let table_first = s.find("/Table <> BDC").expect("moved image table tag"); let emc = s.rfind("EMC").expect("EMC"); - assert!(s.find("/Artifact BMC").is_some(), "image has own artifact tag: {s}"); + assert_ne!(table_first, table); assert!(table < cell && cell < emc, "cell text stays in table sequence: {s}"); } @@ -7230,3 +7232,75 @@ fn reading_keeps_dotted_leader_entries_whole_and_in_order() { } } } + +/// One 600×800 page: Im0 (4×2 gray image), Fm0 (200×100 Form), ExtGState GSone (ca 1). +fn regression_page(content: &str) -> Document { + let form = "0 g 0 0 200 100 re f"; + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> /XObject << /Im0 6 0 R /Fm0 7 0 R >> /ExtGState << /GSone << /Type /ExtGState /ca 1 >> >> >> >>".into(), + format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into(), + "<< /Type /XObject /Subtype /Image /Width 4 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 8 >>\nstream\n\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\nendstream".into(), + format!("<< /Type /XObject /Subtype /Form /BBox [0 0 200 100] /Length {} >>\nstream\n{form}\nendstream", form.len()), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offs = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offs.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let x = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offs { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{x}\n%%EOF\n", objs.len() + 1).as_bytes()); + Document::open(Arc::new(out)).unwrap() +} + +fn regression_rects(doc: &Document) -> Vec<[i64; 4]> { + let mut v: Vec<[i64; 4]> = images::page_images(doc, 0).unwrap().iter().map(|i| i.rect.map(|x| x.round() as i64)).collect(); + v.sort(); + v +} + +/// A `cm` inside a BDC…EMC unit applies to later content; arranging the unit must not shift anything. +#[test] +fn arrange_keeps_cm_leaking_out_of_marked_content() { + let mut doc = regression_page("/Figure <> BDC 1 0 0 1 50 50 cm q 100 0 0 100 0 0 cm /Im0 Do Q EMC q 10 0 0 10 200 200 cm /Im0 Do Q"); + let before = regression_rects(&doc); + images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront).unwrap(); + assert_eq!(before, regression_rects(&reopen(&doc)), "other content moved"); +} + +/// Cropping a Form XObject to 100×100 keeps a 100×100 visible area (or is refused) — never 1×1. +#[test] +fn crop_form_xobject_keeps_requested_area() { + let mut doc = regression_page("q 1 0 0 1 100 100 cm /Fm0 Do Q"); + if images::crop_image(&mut doc, 0, 0, [100.0, 100.0, 200.0, 200.0]).is_err() { + return; // refusing to crop vector art is also acceptable + } + let after = images::page_images(&reopen(&doc), 0).unwrap()[0].clone(); + let v = after.visible_rect.unwrap_or(after.rect); + assert!((v[2] - v[0] - 100.0).abs() < 1.0 && (v[3] - v[1] - 100.0).abs() < 1.0, "expected 100x100 visible, got {v:?}"); +} + +/// A gs naming a missing ExtGState must not be read as 50% opacity and block the reorder. +#[test] +fn arrange_tolerates_missing_extgstate() { + let mut doc = regression_page("q /GSmissing gs 100 0 0 100 50 50 cm /Im0 Do Q /GSone gs q 10 0 0 10 200 200 cm /Im0 Do Q"); + let r = images::arrange_image(&mut doc, 0, 0, images::ArrangeOrder::BringToFront); + assert!(r.is_ok(), "{r:?}"); +} + +/// Bring to front on the image already in front must not report a change it didn't make. +#[test] +fn noop_arrange_is_not_ok_without_change() { + let mut doc = regression_page("q 10 0 0 10 200 200 cm /Im0 Do Q q 100 0 0 100 50 50 cm /Im0 Do Q"); + let before = write_incremental(&doc, &SaveOptions::default()).unwrap(); + let r = images::arrange_image(&mut doc, 0, 1, images::ArrangeOrder::BringToFront); + let after = write_incremental(&doc, &SaveOptions::default()).unwrap(); + assert!(r.is_err() || before != after, "Ok with no change"); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 6a792d88e0..b6f5fd59ef 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -2698,6 +2698,36 @@ fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut EditCtx) -> && let AddedContent::Image(mut ai) = added_item.content { match change { + ImageEdit::Move(to) => { + ai.rect = *to; + pdfcraft_edit::update_content(doc, *page, added_idx, &AddedContent::Image(ai))?; + } + ImageEdit::Transform(t) => { + let apply = |x: f64, y: f64| (t[0] * x + t[2] * y + t[4], t[1] * x + t[3] * y + t[5]); + let pts = [ + apply(ai.rect[0], ai.rect[1]), + apply(ai.rect[2], ai.rect[1]), + apply(ai.rect[0], ai.rect[3]), + apply(ai.rect[2], ai.rect[3]), + ]; + let mut b = [f64::MAX, f64::MAX, f64::MIN, f64::MIN]; + for (x, y) in pts { + b = [b[0].min(x), b[1].min(y), b[2].max(x), b[3].max(y)]; + } + ai.rect = b; + if t[1].abs() < 1e-6 && t[2].abs() < 1e-6 { + pdfcraft_edit::update_content(doc, *page, added_idx, &AddedContent::Image(ai))?; + } else { + let _ = doc.update_dict(added_item.obj, |d| { + if let Some(pdfcraft_cos::Object::Dict(params)) = d.get_mut(b"PCAdded") { + params.set( + b"Rect".to_vec(), + pdfcraft_cos::Object::Array(ai.rect.iter().map(|x| pdfcraft_cos::Object::Real(*x)).collect()), + ); + } + }); + } + } ImageEdit::Rotate(q) => { let k = ((4 - q.rem_euclid(4)) % 4) as u8; ai.rotation = (ai.rotation + k) % 4; @@ -2709,6 +2739,42 @@ fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut EditCtx) -> } pdfcraft_edit::update_content(doc, *page, added_idx, &AddedContent::Image(ai))?; } + ImageEdit::RotateBy(degrees) => { + let deg = degrees.rem_euclid(360.0); + if (deg % 90.0).abs() < 1e-4 || (deg % 90.0 - 90.0).abs() < 1e-4 { + let quarters = ((deg + 45.0) / 90.0).floor() as i64; + let k = ((4 - quarters.rem_euclid(4)) % 4) as u8; + ai.rotation = (ai.rotation + k) % 4; + let r = ai.rect; + let (cx, cy) = ((r[0] + r[2]) / 2.0, (r[1] + r[3]) / 2.0); + if k % 2 == 1 { + let (w, h) = (r[2] - r[0], r[3] - r[1]); + ai.rect = [cx - h / 2.0, cy - w / 2.0, cx + h / 2.0, cy + w / 2.0]; + } + pdfcraft_edit::update_content(doc, *page, added_idx, &AddedContent::Image(ai))?; + } else if let Some(t) = pdfcraft_edit::turn_by_degrees(img.visible_rect.unwrap_or(img.rect), *degrees) { + let apply = |x: f64, y: f64| (t[0] * x + t[2] * y + t[4], t[1] * x + t[3] * y + t[5]); + let pts = [ + apply(ai.rect[0], ai.rect[1]), + apply(ai.rect[2], ai.rect[1]), + apply(ai.rect[0], ai.rect[3]), + apply(ai.rect[2], ai.rect[3]), + ]; + let mut b = [f64::MAX, f64::MAX, f64::MIN, f64::MIN]; + for (x, y) in pts { + b = [b[0].min(x), b[1].min(y), b[2].max(x), b[3].max(y)]; + } + ai.rect = b; + let _ = doc.update_dict(added_item.obj, |d| { + if let Some(pdfcraft_cos::Object::Dict(params)) = d.get_mut(b"PCAdded") { + params.set( + b"Rect".to_vec(), + pdfcraft_cos::Object::Array(ai.rect.iter().map(|x| pdfcraft_cos::Object::Real(*x)).collect()), + ); + } + }); + } + } ImageEdit::Flip { horizontal: true } => { ai.flip_h = !ai.flip_h; pdfcraft_edit::update_content(doc, *page, added_idx, &AddedContent::Image(ai))?; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 53b4e29798..cba800389e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -7240,3 +7240,67 @@ fn exporting_office_keeps_dotted_leader_entries_as_paragraphs() { let positions: Vec<_> = entries.iter().map(|entry| html.find(&format!("

{entry}

")).expect("one paragraph per entry")).collect(); assert!(positions.windows(2).all(|pair| pair[0] < pair[1]), "reading order: {html}"); } + +/// Moving an added image, then rotating it, must keep the moved position. +#[test] +fn added_image_move_then_rotate_keeps_position() { + let mut png = Vec::new(); + { + let mut enc = png::Encoder::new(&mut png, 2, 2); + enc.set_color(png::ColorType::Rgb); + enc.write_header().unwrap().write_image_data(&[255u8; 12]).unwrap(); + } + let (mut s, id) = session_with(1); + s.apply(id, Edit::AddImage { page: 0, rect: Some([20.0, 20.0, 60.0, 60.0]), name: "p.png".into(), bytes: Arc::new(png) }).unwrap(); + let rect = |s: &Session| s.get(id).unwrap().page_images(0)[0].rect; + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Move([120.0, 120.0, 160.0, 160.0]) }).unwrap(); + let moved = rect(&s); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Rotate(1) }).unwrap(); + let rotated = rect(&s); + let c = |r: [f64; 4]| ((r[0] + r[2]) / 2.0, (r[1] + r[3]) / 2.0); + let (a, b) = (c(moved), c(rotated)); + assert!((a.0 - b.0).abs() < 1.0 && (a.1 - b.1).abs() < 1.0, "rotate moved the centre from {a:?} to {b:?}"); +} + +/// Transforming an added image, then rotating it, must keep the moved position. +#[test] +fn added_image_transform_then_rotate_keeps_position() { + let mut png = Vec::new(); + { + let mut enc = png::Encoder::new(&mut png, 2, 2); + enc.set_color(png::ColorType::Rgb); + enc.write_header().unwrap().write_image_data(&[255u8; 12]).unwrap(); + } + let (mut s, id) = session_with(1); + s.apply(id, Edit::AddImage { page: 0, rect: Some([20.0, 20.0, 60.0, 60.0]), name: "p.png".into(), bytes: Arc::new(png) }).unwrap(); + let rect = |s: &Session| s.get(id).unwrap().page_images(0)[0].rect; + // Translate by (100, 100): matrix [1, 0, 0, 1, 100, 100] + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([1.0, 0.0, 0.0, 1.0, 100.0, 100.0]) }).unwrap(); + let transformed = rect(&s); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Rotate(1) }).unwrap(); + let rotated = rect(&s); + let c = |r: [f64; 4]| ((r[0] + r[2]) / 2.0, (r[1] + r[3]) / 2.0); + let (a, b) = (c(transformed), c(rotated)); + assert!((a.0 - b.0).abs() < 1.0 && (a.1 - b.1).abs() < 1.0, "rotate moved the centre from {a:?} to {b:?}"); +} + +/// Moving an added image, then rotating by degrees, must keep the moved position. +#[test] +fn added_image_move_then_rotate_by_keeps_position() { + let mut png = Vec::new(); + { + let mut enc = png::Encoder::new(&mut png, 2, 2); + enc.set_color(png::ColorType::Rgb); + enc.write_header().unwrap().write_image_data(&[255u8; 12]).unwrap(); + } + let (mut s, id) = session_with(1); + s.apply(id, Edit::AddImage { page: 0, rect: Some([20.0, 20.0, 60.0, 60.0]), name: "p.png".into(), bytes: Arc::new(png) }).unwrap(); + let rect = |s: &Session| s.get(id).unwrap().page_images(0)[0].rect; + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Move([120.0, 120.0, 160.0, 160.0]) }).unwrap(); + let moved = rect(&s); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::RotateBy(90.0) }).unwrap(); + let rotated = rect(&s); + let c = |r: [f64; 4]| ((r[0] + r[2]) / 2.0, (r[1] + r[3]) / 2.0); + let (a, b) = (c(moved), c(rotated)); + assert!((a.0 - b.0).abs() < 1.0 && (a.1 - b.1).abs() < 1.0, "rotate_by moved the centre from {a:?} to {b:?}"); +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index da18a7931b..b070e4e635 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -4641,7 +4641,8 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { })); if is_image_menu && let Some(sel) = view.image_selection.clone() { let images = doc.page_images(sel.page); - crate::edit_text_ui::image_context_menu(ui, view, &images, sel.page, sel.index, can_paste_image, &mut image_action); + let at = view.comments.context_at.map(|(_, at)| at); + crate::edit_text_ui::image_context_menu(ui, view, &images, sel.page, sel.index, can_paste_image, &mut image_action, at); return; } if view.comments.selected.is_some() { @@ -5020,9 +5021,12 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { app.views[index].image_selection = None; app.cut_page_image(page, image); } - Some(crate::edit_text_ui::ImageAction::Paste(page, rect)) => { - let offset_rect = [rect[0] + 20.0, rect[1] - 20.0, rect[2] + 20.0, rect[3] - 20.0]; - app.paste_image(page, Some(offset_rect)); + Some(crate::edit_text_ui::ImageAction::Paste(page, at)) => { + if let Some(pt) = at { + app.paste_image_at(page, pt); + } else { + app.paste_image(page, None); + } } Some(crate::edit_text_ui::ImageAction::DrawFreehand) => { app.quick_tool = crate::QuickTool::Comment(crate::comments::CommentTool::Ink); diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 66feb44b2e..f6b44a455d 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -723,13 +723,14 @@ impl crate::PdfCraftApp { #[cfg(not(target_arch = "wasm32"))] if self.system_clipboard { let img = arboard::ImageData { width: w as usize, height: h as usize, bytes: std::borrow::Cow::Borrowed(&rgba) }; - arboard::Clipboard::new().and_then(|mut c| c.set_image(img)).map_err(|e| e.to_string())?; + crate::os_clipboard::with(|c| c.set_image(img)).map_err(|e| e.to_string())?; } #[cfg(target_arch = "wasm32")] if self.system_clipboard { return Err("the web version can't put images on the clipboard yet; use Save Image As".to_string()); } self.last_copied_image = Some((w, h, rgba)); + self.last_copied_image_rect = self.page_image_box(page, index); Ok((w, h)) }); match copied { @@ -745,7 +746,7 @@ impl crate::PdfCraftApp { } /// The picture's visible box on its page, in user space. - fn page_image_box(&self, page: usize, index: usize) -> Option<[f64; 4]> { + pub(crate) fn page_image_box(&self, page: usize, index: usize) -> Option<[f64; 4]> { let (_, id) = self.active_ids()?; let image = self.session.get(id)?.page_images(page).into_iter().nth(index)?; Some(image.visible_rect.unwrap_or(image.rect)) @@ -839,42 +840,24 @@ impl crate::PdfCraftApp { released } - /// ⌘V / Ctrl+V with a picture on the system clipboard (a screenshot, a picture copied in a - /// browser) places it on the current page, as Add Image does. - /// Organize Pages keeps ⌘V for pages, Prepare Form for fields, and text fields for text. - pub(crate) fn paste_image_key(&mut self, i: usize, ctx: &egui::Context, pasted: bool) { - let free = self.dialog.is_none() && !self.palette_open && !ctx.egui_wants_keyboard_input(); - let Some(view) = self.views.get(i) else { return }; - if !pasted || !free || view.organize || view.forms.focus.is_some() || self.is_preparing() { - return; - } - let clip = match self.clipboard_override.clone() { - Some(c) => Some(c), - #[cfg(not(target_arch = "wasm32"))] - None if self.system_clipboard => crate::create_ui::read_clipboard(), - None => None, - }; - let Some(crate::create_ui::Clip::Image { width, height, rgba }) = clip else { return }; - match crate::create_ui::png_from_rgba(width, height, &rgba) { - Ok(png) => self.add_image("Pasted image.png".into(), png), - Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", tl!("Clipboard")), ("e", &e)]), - } - } - /// Whether there is an image in the in-memory clipboard or system clipboard ready to paste. pub fn can_paste_image(&self) -> bool { - if self.last_copied_image.is_some() || self.last_copied_file.is_some() { + if let Some(crate::create_ui::Clip::Image { .. }) = self.clipboard_override.as_ref() { return true; } + if self.clipboard_override.is_some() { + return false; + } #[cfg(not(target_arch = "wasm32"))] { - if self.system_clipboard - && let Ok(mut cb) = arboard::Clipboard::new() - { - return cb.get_image().is_ok(); + if self.system_clipboard { + let has_img = crate::os_clipboard::with(|cb| cb.get_image()).is_ok(); + if has_img { + return true; + } } } - false + self.last_copied_image.is_some() || self.last_copied_file.is_some() } /// Paste an image from the clipboard onto `page`. @@ -889,23 +872,50 @@ impl crate::PdfCraftApp { return false; } - let image_data: Option<(String, Vec)> = 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() { - crate::create_ui::png_from_rgba(w as usize, h as usize, &rgba).ok().map(|b| ("pasted_image.png".to_string(), b)) - } else { - #[cfg(not(target_arch = "wasm32"))] - { - if self.system_clipboard { - arboard::Clipboard::new().ok().and_then(|mut cb| cb.get_image().ok()).and_then(|img| { + let image_data: Option<(String, Vec)> = match self.clipboard_override.clone() { + Some(crate::create_ui::Clip::Image { width, height, rgba }) => { + let is_same = self + .last_copied_image + .as_ref() + .is_some_and(|(w, h, bytes)| *w as usize == width && *h as usize == height && bytes.as_slice() == rgba.as_slice()); + if !is_same { + self.last_copied_file = None; + self.last_copied_image = Some((width as u32, height as u32, rgba.clone())); + self.last_copied_image_rect = None; + } + crate::create_ui::png_from_rgba(width, height, &rgba).ok().map(|b| ("pasted_image.png".to_string(), b)) + } + Some(crate::create_ui::Clip::Text(_)) => None, + None => { + #[cfg(not(target_arch = "wasm32"))] + let sys_result = if self.system_clipboard { crate::os_clipboard::with(|cb| cb.get_image()).ok() } else { None }; + #[cfg(target_arch = "wasm32")] + let sys_result: Option> = None; + + if let Some(img) = sys_result { + let is_same_as_last = 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 is_same_as_last && let Some((name, bytes)) = self.last_copied_file.clone() { + Some((name, bytes)) + } else if is_same_as_last && let Some((w, h, rgba)) = self.last_copied_image.clone() { + crate::create_ui::png_from_rgba(w as usize, h as usize, &rgba).ok().map(|b| ("pasted_image.png".to_string(), b)) + } else { + // System clipboard has a newer / external image! + self.last_copied_file = None; + self.last_copied_image = Some((img.width as u32, img.height as u32, img.bytes.to_vec())); + self.last_copied_image_rect = None; crate::create_ui::png_from_rgba(img.width, img.height, &img.bytes).ok().map(|b| ("clipboard_image.png".to_string(), b)) - }) + } + } 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() { + crate::create_ui::png_from_rgba(w as usize, h as usize, &rgba).ok().map(|b| ("pasted_image.png".to_string(), b)) } else { None } } - #[cfg(target_arch = "wasm32")] - None }; let Some((name, bytes)) = image_data else { @@ -932,6 +942,11 @@ impl crate::PdfCraftApp { let before = self.session.get(id).map_or(0, |d| d.page_images(page).len()); if self.apply_edit(pdfcraft_engine::Edit::AddImage { page, rect: target_rect, name, bytes: std::sync::Arc::new(bytes) }) { + let placed_rect = + target_rect.or_else(|| self.session.get(id).and_then(|d| d.page_images(page).into_iter().nth(before).map(|im| im.rect))); + if let Some(r) = placed_rect { + self.last_copied_image_rect = Some(r); + } if let Some(v) = self.views.get_mut(i) { v.current = page; v.image_selection = Some(crate::edit_text_ui::ImageSelection { @@ -955,13 +970,41 @@ impl crate::PdfCraftApp { /// Paste an image centered at context menu point `at: [f64; 2]` on `page`. pub fn paste_image_at(&mut self, page: usize, at: [f64; 2]) -> bool { - let rect = if let Some(src_r) = self.last_copied_image_rect { - let w = src_r[2] - src_r[0]; - let h = src_r[3] - src_r[1]; - Some([at[0] - w * 0.5, at[1] - h * 0.5, at[0] + w * 0.5, at[1] + h * 0.5]) + let (w, h) = if let Some(crate::create_ui::Clip::Image { width, height, rgba }) = self.clipboard_override.as_ref() { + let is_same = self + .last_copied_image + .as_ref() + .is_some_and(|(w, h, bytes)| *w as usize == *width && *h as usize == *height && bytes.as_slice() == rgba.as_slice()); + if is_same && let Some(src_r) = self.last_copied_image_rect { + (src_r[2] - src_r[0], src_r[3] - src_r[1]) + } else { + (*width as f64, *height as f64) + } } else { - None + #[cfg(not(target_arch = "wasm32"))] + let sys_img = if self.system_clipboard { crate::os_clipboard::with(|cb| cb.get_image()).ok() } else { None }; + #[cfg(target_arch = "wasm32")] + let sys_img: Option> = None; + + if let Some(img) = sys_img { + let is_same = self + .last_copied_image + .as_ref() + .is_some_and(|(w, h, bytes)| *w as usize == img.width && *h as usize == img.height && bytes.as_slice() == img.bytes.as_ref()); + if is_same && let Some(src_r) = self.last_copied_image_rect { + (src_r[2] - src_r[0], src_r[3] - src_r[1]) + } else { + (img.width as f64, img.height as f64) + } + } else if let Some(src_r) = self.last_copied_image_rect { + (src_r[2] - src_r[0], src_r[3] - src_r[1]) + } else if let Some((w, h, _)) = self.last_copied_image { + (w as f64, h as f64) + } else { + (100.0, 100.0) + } }; + let rect = Some([at[0] - w * 0.5, at[1] - h * 0.5, at[0] + w * 0.5, at[1] + h * 0.5]); self.paste_image(page, rect) } } diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index 6d157eaedc..e054d210ce 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -1340,7 +1340,7 @@ pub(crate) enum ImageAction { Link(usize, usize), /// Open Recognize Text for the picture's page. Recognize(usize), - Paste(usize, [f64; 4]), + Paste(usize, Option<[f64; 2]>), DrawFreehand, AddComment(usize, usize), Crop, @@ -1936,6 +1936,7 @@ pub(crate) fn image_input( } /// The right-click context menu for a selected image matching Adobe Acrobat. +#[allow(clippy::too_many_arguments)] pub(crate) fn image_context_menu( ui: &mut egui::Ui, view: &mut DocView, @@ -1944,9 +1945,11 @@ pub(crate) fn image_context_menu( hit: usize, can_paste: bool, action: &mut Option, + at: Option<[f64; 2]>, ) { let Some(img) = images.get(hit) else { return }; use pdfcraft_engine::ImageEdit as E; + let raster = !img.is_form; // Top horizontal icon bar: Comment, Crop, Rotate CCW, Rotate CW ui.horizontal(|ui| { @@ -1955,7 +1958,7 @@ pub(crate) fn image_context_menu( *action = Some(ImageAction::AddComment(page, hit)); ui.close(); } - if crate::icons::button(ui, "crop", 24.0, false, "Crop").clicked() { + if raster && crate::icons::button(ui, "crop", 24.0, false, tl!("Crop")).clicked() { *action = Some(ImageAction::Crop); if let Some(s) = view.image_selection.as_mut() { let rect = img.visible_rect.unwrap_or(img.rect); @@ -1977,7 +1980,6 @@ pub(crate) fn image_context_menu( ui.separator(); - let raster = !img.is_form; if ui.button(tl!("Cut Image")).clicked() { *action = Some(ImageAction::Cut(page, hit)); ui.close(); @@ -1986,15 +1988,15 @@ pub(crate) fn image_context_menu( *action = Some(ImageAction::Copy(page, hit)); ui.close(); } - if ui.add_enabled(can_paste, egui::Button::new("Paste")).clicked() { - *action = Some(ImageAction::Paste(page, img.rect)); + if ui.add_enabled(can_paste, egui::Button::new(tl!("Paste"))).clicked() { + *action = Some(ImageAction::Paste(page, at)); ui.close(); } ui.separator(); let is_cropped = img.visible_rect.is_some() && img.visible_rect != Some(img.rect); - if ui.add_enabled(is_cropped, egui::Button::new("Remove crop")).clicked() { + if raster && ui.add_enabled(is_cropped, egui::Button::new(tl!("Remove crop"))).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::RemoveCrop }); ui.close(); } @@ -2008,20 +2010,20 @@ pub(crate) fn image_context_menu( ui.close(); } - ui.menu_button("Arrange image", |ui| { - if ui.button("Bring to front").clicked() { + ui.menu_button(tl!("Arrange image"), |ui| { + if ui.button(tl!("Bring to front")).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Arrange(pdfcraft_engine::ArrangeOrder::BringToFront) }); ui.close(); } - if ui.button("Send to back").clicked() { + if ui.button(tl!("Send to back")).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Arrange(pdfcraft_engine::ArrangeOrder::SendToBack) }); ui.close(); } - if ui.button("Bring forward").clicked() { + if ui.button(tl!("Bring forward")).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Arrange(pdfcraft_engine::ArrangeOrder::BringForward) }); ui.close(); } - if ui.button("Send backward").clicked() { + if ui.button(tl!("Send backward")).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Arrange(pdfcraft_engine::ArrangeOrder::SendBackward) }); ui.close(); } diff --git a/crates/ui-egui/src/image_pick.rs b/crates/ui-egui/src/image_pick.rs index 3029c185cc..babde8227a 100644 --- a/crates/ui-egui/src/image_pick.rs +++ b/crates/ui-egui/src/image_pick.rs @@ -203,6 +203,7 @@ impl PdfCraftApp { let name = format!("{stem} p{} img{}.{ext}", page.saturating_add(1), index.saturating_add(1)); self.put_file_on_clipboard(&name, &bytes)?; self.last_copied_file = Some((name, bytes)); + self.last_copied_image_rect = self.page_image_box(page, index); Ok(ext) }); match copied { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ba6a26a22b..a94d987e59 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -3107,7 +3107,6 @@ impl PdfCraftApp { prepare::nudge_keys(ctx, &mut self.views[i], doc); } canvas::shortcuts(&mut self.views[i], ctx, self.scroll_speed); - self.paste_image_key(i, ctx, pasted); self.clipboard_keys(i, ctx, pasted); self.image_pick_keys(i, ctx); } @@ -3139,7 +3138,8 @@ impl PdfCraftApp { /// ⌘C / ⌘X / ⌘V copy, cut and paste text or images on canvas (unless a text field has focus). fn clipboard_keys(&mut self, i: usize, ctx: &egui::Context, pasted: bool) { use egui::{Key, KeyboardShortcut, Modifiers}; - if ctx.egui_wants_keyboard_input() || self.views.get(i).is_some_and(|v| v.forms.focus.is_some() || v.organize) { + let free = self.dialog.is_none() && !self.palette_open && !ctx.egui_wants_keyboard_input() && !self.is_preparing(); + if !free || self.views.get(i).is_some_and(|v| v.forms.focus.is_some() || v.organize) { return; } let copy = ctx.input(|input| input.events.iter().any(|e| matches!(e, egui::Event::Copy))) diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index a597801ca0..9e6f1696ea 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -6062,6 +6062,95 @@ fn stale_clipboard_plain_text_does_not_paste_image() { assert_eq!(doc.page_images(0).len(), 0, "no image pasted on the page"); } +/// Issue 1: V key release with COMMAND delivers exactly one paste (not two). +#[test] +fn paste_key_release_delivers_single_image() { + let pdf = include_bytes!("../data/form.pdf").to_vec(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.system_clipboard = false; + app.open_bytes("form.pdf", None, pdf.clone()).expect("opens"); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + + let id = h.state().views[0].id; + let initial_count = h.state().session.get(id).unwrap().page_images(0).len(); + + h.state_mut().clipboard_override = Some(pdfcraft_ui_egui::Clip::Image { width: 40, height: 20, rgba: [100u8, 200, 100, 255].repeat(40 * 20) }); + + // Inject only a V release with COMMAND (no press event), exactly as egui-winit delivers + h.event(egui::Event::Key { key: egui::Key::V, physical_key: None, pressed: false, repeat: false, modifiers: egui::Modifiers::COMMAND }); + h.run_steps(3); + + let count = h.state().session.get(id).unwrap().page_images(0).len(); + assert_eq!(count, initial_count + 1, "exactly one image must be pasted on V release, got {count}"); +} + +/// Issue P1: Paste via context menu at a point away from the centre centers the image at that point. +#[test] +fn paste_at_context_menu_point_centers_on_cursor() { + let pdf = include_bytes!("../data/form.pdf").to_vec(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.system_clipboard = false; + app.open_bytes("form.pdf", None, pdf.clone()).expect("opens"); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + + let id = h.state().views[0].id; + let initial_count = h.state().session.get(id).unwrap().page_images(0).len(); + + h.state_mut().clipboard_override = Some(pdfcraft_ui_egui::Clip::Image { width: 60, height: 40, rgba: [50u8, 150, 250, 255].repeat(60 * 40) }); + + let target_at = [120.0, 180.0]; + assert!(h.state_mut().paste_image_at(0, target_at)); + + let images = h.state().session.get(id).unwrap().page_images(0); + assert_eq!(images.len(), initial_count + 1); + let r = images[initial_count].rect; + let center = ((r[0] + r[2]) / 2.0, (r[1] + r[3]) / 2.0); + assert!((center.0 - target_at[0]).abs() < 1.0 && (center.1 - target_at[1]).abs() < 1.0, "expected center {target_at:?}, got {center:?}"); +} + +/// Issue P2: A newer clipboard override image beats stale in-app copy. +#[test] +fn newer_clipboard_override_beats_stale_in_app_copy() { + let pdf = include_bytes!("../data/form.pdf").to_vec(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.system_clipboard = false; + app.open_bytes("form.pdf", None, pdf.clone()).expect("opens"); + app + }); + h.run_steps(4); + + let id = h.state().views[0].id; + let initial_count = h.state().session.get(id).unwrap().page_images(0).len(); + + // Stale in-app copy: 10x10 red image + h.state_mut().last_copied_image = Some((10, 10, [255u8, 0, 0, 255].repeat(100))); + h.state_mut().last_copied_image_rect = Some([10.0, 10.0, 20.0, 20.0]); + + // Newer clipboard override: 50x50 green image + h.state_mut().clipboard_override = Some(pdfcraft_ui_egui::Clip::Image { width: 50, height: 50, rgba: [0u8, 255, 0, 255].repeat(2500) }); + + assert!(h.state_mut().paste_image(0, None)); + let images = h.state().session.get(id).unwrap().page_images(0); + assert_eq!(images.len(), initial_count + 1); + let r = images[initial_count].rect; + let (w, h_dim) = (r[2] - r[0], r[3] - r[1]); + assert!((w - 50.0).abs() < 1.0 && (h_dim - 50.0).abs() < 1.0, "expected 50x50 override image, got {w}x{h_dim}"); +} + #[test] fn rotated_image_selection_and_crop() { let mut png = Vec::new();