diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 1b8aa65bad..a8ace8b5d4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -2705,6 +2705,18 @@ fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut EditCtx) -> pdfcraft_edit::stamp(doc, *page, "OCR", pdfcraft_ocr::text_layer(words))?; } Edit::EditPageImage { page, index, change } => { + match change { + 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::Crop(keep) if !keep.iter().all(|v| v.is_finite()) => { + return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("invalid crop".into()))); + } + _ => {} + } let img = pdfcraft_edit::page_images(doc, *page)? .into_iter() .nth(*index) @@ -2739,15 +2751,18 @@ fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut EditCtx) -> ImageEdit::Arrange(order) => pdfcraft_edit::ImageChange::Arrange(*order), ImageEdit::RemoveCrop => pdfcraft_edit::ImageChange::RemoveCrop, }; - let page_obj = pdfcraft_model::pages(doc).into_iter().nth(*page); - let target_stream_obj = page_obj.as_ref().and_then(|p| match p.dict.get(b"Contents") { + let page_model = pdfcraft_model::pages(doc) + .into_iter() + .nth(*page) + .ok_or_else(|| EditError::Edit(pdfcraft_edit::EditError::Invalid(format!("page {} not found", page + 1))))?; + let target_stream_obj = match page_model.dict.get(b"Contents") { None => None, Some(c) => match &*doc.resolve(c) { pdfcraft_cos::Object::Array(a) => a.get(img.stream).and_then(|o| o.as_ref()), _ if img.stream == 0 => c.as_ref(), _ => None, }, - }); + }; let added_items: Vec<(usize, Added)> = pdfcraft_edit::list_added(doc).into_iter().filter(|a| a.page == *page).enumerate().collect(); let matched_added = if let Some(st_ref) = target_stream_obj { @@ -2788,24 +2803,50 @@ fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut EditCtx) -> } if let Some((added_idx, mut ai)) = ai_to_update { + let view = page_model.view_matrix(doc); + let det = view[0] * view[3] - view[1] * view[2]; + if det.abs() < 1e-12 || !det.is_finite() { + return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("invalid page transform".into()))); + } + let (ia, ib, ic, id) = (view[3] / det, -view[1] / det, -view[2] / det, view[0] / det); + let to_display = [ia, ib, ic, id, -(view[4] * ia + view[5] * ic), -(view[4] * ib + view[5] * id)]; + let apply = |m: [f64; 6], x: f64, y: f64| (m[0] * x + m[2] * y + m[4], m[1] * x + m[3] * y + m[5]); + let bbox = |m: [f64; 6], r: [f64; 4]| { + let pts = [apply(m, r[0], r[1]), apply(m, r[2], r[1]), apply(m, r[0], r[3]), apply(m, r[2], r[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)]; + } + b + }; + match change { ImageEdit::Move(to) => { - ai.rect = *to; + if !to.iter().all(|v| v.is_finite()) { + return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("invalid position".into()))); + } + ai.rect = bbox(to_display, *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)]; + let mul = |m1: [f64; 6], m2: [f64; 6]| { + [ + m1[0] * m2[0] + m1[1] * m2[2], + m1[0] * m2[1] + m1[1] * m2[3], + m1[2] * m2[0] + m1[3] * m2[2], + m1[2] * m2[1] + m1[3] * m2[3], + m1[4] * m2[0] + m1[5] * m2[2] + m2[4], + m1[4] * m2[1] + m1[5] * m2[3] + m2[5], + ] + }; + let t_display = mul(mul(view, *t), to_display); + if t_display[0] < 0.0 { + ai.flip_h = !ai.flip_h; } - ai.rect = b; + if t_display[3] < 0.0 { + ai.flip_v = !ai.flip_v; + } + ai.rect = bbox(t_display, ai.rect); pdfcraft_edit::update_content(doc, *page, added_idx, &AddedContent::Image(ai))?; } ImageEdit::Rotate(q) => { @@ -2827,7 +2868,16 @@ fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut EditCtx) -> ai.flip_v = !ai.flip_v; pdfcraft_edit::update_content(doc, *page, added_idx, &AddedContent::Image(ai))?; } - _ => {} + ImageEdit::Replace { .. } => { + let pdfcraft_edit::ImageChange::Replace(new_image) = c else { + return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("expected replace".into()))); + }; + ai.image = new_image; + pdfcraft_edit::update_content(doc, *page, added_idx, &AddedContent::Image(ai))?; + } + ImageEdit::Crop(_) | ImageEdit::RemoveCrop | ImageEdit::RotateBy(_) | ImageEdit::Arrange(_) | ImageEdit::Delete => { + pdfcraft_edit::change_image(doc, *page, *index, &c)?; + } } } else { pdfcraft_edit::change_image(doc, *page, *index, &c)?; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 7af0064a40..7f5a101081 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -30,6 +30,39 @@ pub(crate) fn fixture(n: usize) -> Vec { out } +fn fixture_with_rotation_and_user_unit(rotate: i64, user_unit: f64) -> Vec { + let mut objs: Vec> = vec![b"<< /Type /Catalog /Pages 2 0 R >>".to_vec()]; + objs.push(b"<< /Type /Pages /Kids [4 0 R] /Count 1 /MediaBox [0 0 200 300] >>".to_vec()); + objs.push(b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_vec()); + let mut page_dict = String::from("<< /Type /Page /Parent 2 0 R /Contents 5 0 R /Resources << /Font << /F1 3 0 R >> >>"); + if rotate != 0 { + page_dict.push_str(&format!(" /Rotate {rotate}")); + } + if (user_unit - 1.0).abs() > 1e-6 { + page_dict.push_str(&format!(" /UserUnit {user_unit}")); + } + page_dict.push_str(" >>"); + objs.push(page_dict.into_bytes()); + let body = "BT /F1 24 Tf 20 150 Td (Page 1) Tj ET"; + objs.push(format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len()).into_bytes()); + + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + 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{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + fn session_with(n: usize) -> (Session, DocId) { let mut s = Session::new().with_clock(|| 1_700_000_000); let id = s.open("fixture.pdf", None, Arc::new(fixture(n)), None).expect("opens"); @@ -7545,3 +7578,182 @@ fn added_image_crop_then_move_keeps_crop() { let img = s.get(id).unwrap().page_images(0)[0].clone(); assert_eq!(img.visible_rect, Some([60.0, 60.0, 100.0, 100.0])); } + +#[test] +fn added_image_move_and_transform_on_rotated_and_user_unit_pages() { + 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 png_bytes = Arc::new(png); + + let cases = [(90, 1.0), (180, 1.0), (270, 1.0), (0, 2.0), (90, 2.0)]; + + for (rotate, user_unit) in cases { + let pdf = fixture_with_rotation_and_user_unit(rotate, user_unit); + let mut s = Session::new().with_clock(|| 1_700_000_000); + let id = s.open("test.pdf", None, Arc::new(pdf), None).expect("opens"); + + s.apply(id, Edit::AddImage { page: 0, rect: Some([10.0, 10.0, 50.0, 50.0]), name: "img.png".into(), bytes: png_bytes.clone() }).unwrap(); + + // Target user-space rectangle R + let r = [30.0, 40.0, 80.0, 90.0]; + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Move(r) }).unwrap(); + + let painted = s.get(id).unwrap().page_images(0)[0].rect; + for i in 0..4 { + assert!( + (painted[i] - r[i]).abs() < 1e-3, + "Move: rotate={rotate}, user_unit={user_unit}: painted[{i}] = {}, expected {}", + painted[i], + r[i] + ); + } + + let cos = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap(); + let added = pdfcraft_edit::list_added(&cos); + assert_eq!(added.len(), 1); + let stream_obj = match &*cos.get(added[0].obj) { + pdfcraft_cos::Object::Stream(s) => s.clone(), + other => panic!("expected stream, got {other:?}"), + }; + let pc_rect = stream_obj + .dict + .get(b"PCAdded") + .and_then(|o| cos.resolve(o).as_dict().cloned()) + .and_then(|d| d.get(b"Rect").cloned()) + .and_then(|o| cos.resolve(&o).as_array().cloned()) + .unwrap(); + let pc_box: [f64; 4] = + [pc_rect[0].as_f64().unwrap(), pc_rect[1].as_f64().unwrap(), pc_rect[2].as_f64().unwrap(), pc_rect[3].as_f64().unwrap()]; + let content = String::from_utf8(stream_obj.decoded().unwrap()).unwrap(); + let re_idx = content.find(" re W n").expect("re W n in content stream"); + let prefix = &content[..re_idx]; + let nums: Vec = + prefix.split_whitespace().rev().take(4).collect::>().into_iter().rev().map(|val| val.parse::().unwrap()).collect(); + assert_eq!(nums.len(), 4); + let (rx, ry, rw, rh) = (nums[0], nums[1], nums[2], nums[3]); + assert!((rx - pc_box[0]).abs() < 1e-3, "re x {rx} != pc_box[0] {}", pc_box[0]); + assert!((ry - pc_box[1]).abs() < 1e-3, "re y {ry} != pc_box[1] {}", pc_box[1]); + assert!((rw - (pc_box[2] - pc_box[0])).abs() < 1e-3, "re w {rw} != {}", pc_box[2] - pc_box[0]); + assert!((rh - (pc_box[3] - pc_box[1])).abs() < 1e-3, "re h {rh} != {}", pc_box[3] - pc_box[1]); + + // Transform test: user-space translation by (10, 10) + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([1.0, 0.0, 0.0, 1.0, 10.0, 10.0]) }).unwrap(); + + let expected_transformed = [r[0] + 10.0, r[1] + 10.0, r[2] + 10.0, r[3] + 10.0]; + let painted_transformed = s.get(id).unwrap().page_images(0)[0].rect; + for i in 0..4 { + assert!( + (painted_transformed[i] - expected_transformed[i]).abs() < 1e-3, + "Transform: rotate={rotate}, user_unit={user_unit}: painted[{i}] = {}, expected {}", + painted_transformed[i], + expected_transformed[i] + ); + } + } +} + +#[test] +fn replace_on_added_image_replaces_xobject_and_undo_restores() { + let make_png = |rgb: [u8; 3]| { + let mut png = Vec::new(); + { + let mut enc = png::Encoder::new(&mut png, 2, 2); + enc.set_color(png::ColorType::Rgb); + let data = [rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2]]; + enc.write_header().unwrap().write_image_data(&data).unwrap(); + } + Arc::new(png) + }; + let png1 = make_png([255, 0, 0]); + let png2 = make_png([0, 0, 255]); + + 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: "img1.png".into(), bytes: png1 }).unwrap(); + + let old_obj = s.get(id).unwrap().page_images(0)[0].object.expect("initial image object ref"); + + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Replace { name: "img2.png".into(), bytes: png2 } }).unwrap(); + + let new_obj = s.get(id).unwrap().page_images(0)[0].object.expect("new image object ref"); + assert_ne!(new_obj, old_obj, "after Replace the page references the new XObject"); + + s.undo(id).unwrap(); + + let restored_obj = s.get(id).unwrap().page_images(0)[0].object.expect("restored image object ref"); + assert_eq!(restored_obj, old_obj, "undo restores the old XObject"); + + // Also test replacement on an image with active crop (promoted path) + let png3 = make_png([0, 255, 0]); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Crop([25.0, 25.0, 55.0, 55.0]) }).unwrap(); + let crop_obj = s.get(id).unwrap().page_images(0)[0].object.expect("crop image object ref"); + assert_eq!(crop_obj, old_obj); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Replace { name: "img3.png".into(), bytes: png3 } }).unwrap(); + let promoted_new_obj = s.get(id).unwrap().page_images(0)[0].object.expect("promoted replace object ref"); + assert_ne!(promoted_new_obj, crop_obj); + s.undo(id).unwrap(); + assert_eq!(s.get(id).unwrap().page_images(0)[0].object.expect("undo promoted replace"), crop_obj); +} + +#[test] +fn mirroring_transform_on_added_image_flips_and_non_finite_is_refused() { + let make_png = |rgb: [u8; 3]| { + let mut png = Vec::new(); + { + let mut enc = png::Encoder::new(&mut png, 2, 2); + enc.set_color(png::ColorType::Rgb); + let data = [rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2]]; + enc.write_header().unwrap().write_image_data(&data).unwrap(); + } + Arc::new(png) + }; + let png1 = make_png([255, 0, 0]); + + let (mut s, id) = session_with(1); + s.apply(id, Edit::AddImage { page: 0, rect: Some([0.0, 0.0, 80.0, 40.0]), name: "img1.png".into(), bytes: png1 }).unwrap(); + + let initial_img = s.get(id).unwrap().page_images(0)[0].clone(); + assert_eq!(initial_img.rect, [0.0, 0.0, 80.0, 40.0]); + assert!(initial_img.matrix[0] > 0.0, "initially unflipped"); + + // Mirroring transform: [-1.0, 0.0, 0.0, 1.0, 80.0, 0.0] + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([-1.0, 0.0, 0.0, 1.0, 80.0, 0.0]) }).unwrap(); + + let flipped_img = s.get(id).unwrap().page_images(0)[0].clone(); + assert!(flipped_img.matrix[0] < 0.0, "horizontal mirror flips matrix x-axis, got matrix: {:?}", flipped_img.matrix); + + let cos = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap(); + let added = pdfcraft_edit::list_added(&cos); + assert_eq!(added.len(), 1); + let AddedContent::Image(ai) = &added[0].content else { panic!("expected added image") }; + assert!(ai.flip_h, "PCAdded record must have flip_h = true"); + + // Mirroring horizontally again flips it back + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([-1.0, 0.0, 0.0, 1.0, 80.0, 0.0]) }).unwrap(); + let unflipped_img = s.get(id).unwrap().page_images(0)[0].clone(); + assert!(unflipped_img.matrix[0] > 0.0, "double horizontal mirror restores positive x-axis"); + let cos2 = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap(); + let AddedContent::Image(ai2) = &pdfcraft_edit::list_added(&cos2)[0].content else { panic!("expected added image") }; + assert!(!ai2.flip_h, "PCAdded record must have flip_h = false"); + + // Mirroring vertically: [1.0, 0.0, 0.0, -1.0, 0.0, 40.0] + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([1.0, 0.0, 0.0, -1.0, 0.0, 40.0]) }).unwrap(); + let v_flipped_img = s.get(id).unwrap().page_images(0)[0].clone(); + assert!(v_flipped_img.matrix[3] < 0.0, "vertical mirror flips matrix y-axis, got matrix: {:?}", v_flipped_img.matrix); + let cos3 = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap(); + let AddedContent::Image(ai3) = &pdfcraft_edit::list_added(&cos3)[0].content else { panic!("expected added image") }; + assert!(ai3.flip_v, "PCAdded record must have flip_v = true"); + + // Non-finite transform must be refused with error + let res = s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([f64::NAN, 0.0, 0.0, 1.0, 0.0, 0.0]) }); + assert!(res.is_err(), "NaN transform must be refused"); + + let res_inf = s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([1.0, 0.0, 0.0, f64::INFINITY, 0.0, 0.0]) }); + assert!(res_inf.is_err(), "infinite transform must be refused"); + + let res_move = s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Move([f64::NAN, 20.0, 100.0, 60.0]) }); + assert!(res_move.is_err(), "NaN position must be refused"); +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index b799a12b10..acafb9d987 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -3755,7 +3755,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } } } - ui.ctx().data_mut(|d| d.insert_persisted(resp.id.with("image-menu"), over_image)); + ui.ctx().data_mut(|d| d.insert_persisted(crate::edit_text_ui::image_menu_id(), over_image)); } let mut calc_hover = None; for &i in &visible_pages { @@ -4629,7 +4629,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { return; } let is_image_menu = tool == QuickTool::EditText - && (ui.ctx().data_mut(|d| d.get_persisted::(resp.id.with("image-menu"))).unwrap_or(false) + && (ui.ctx().data_mut(|d| d.get_persisted::(crate::edit_text_ui::image_menu_id())).unwrap_or(false) || view.image_selection.as_ref().is_some_and(|sel| { let pointer_pos = ui.input(|inp| inp.pointer.hover_pos().or(inp.pointer.interact_pos())); let Some(p) = pointer_pos else { return false }; @@ -4685,6 +4685,9 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { let can_paste = can_paste_image; default_page_context_menu(ui, page, at, can_copy_items, can_paste, &mut page_action); }); + if !resp.context_menu_opened() { + ui.ctx().data_mut(|d| d.insert_persisted(crate::edit_text_ui::image_menu_id(), false)); + } (wanted, visible_now) }); view.single_edges = visible_pages.first().copied().filter(|_| !view.continuous).map(|page| { diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index e054d210ce..e2c72ea241 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -1372,6 +1372,10 @@ fn user_box(xf: &PageXform, info: &DocInfo, page: usize, r: Rect) -> [f64; 4] { [u[0].min(v[0]) as f64, u[1].min(v[1]) as f64, u[0].max(v[0]) as f64, u[1].max(v[1]) as f64] } +pub(crate) fn image_menu_id() -> egui::Id { + egui::Id::new("image-menu") +} + /// Images on a page: select, move, resize, right-click. Returns `true` when the pointer was used. #[allow(clippy::too_many_arguments)] pub(crate) fn image_input( @@ -1895,13 +1899,14 @@ pub(crate) fn image_input( // (a logo is smaller than its menu), and a menu that is not drawn for a frame cannot be clicked. // Only a menu opened by a right-click on the picture: one opened over text or empty page while // a picture stays selected is the page's own menu. - let menu_key = egui::Id::new("image-menu"); + let menu_key = image_menu_id(); let Some(p) = hover_pos.filter(|p| xf.rect.contains(*p)) else { return false }; let Some(hit) = boxes.iter().enumerate().rposition(|(i, b)| { b.is_positive() && (b.contains(p) || quad_contains(ImageObb::from_image(&images[i], xf, info, page).corners(), p)) && image_contains(i, p) }) else { if resp.secondary_clicked() || resp.clicked() { view.image_selection = None; + ui.ctx().data_mut(|d| d.insert_persisted(menu_key, false)); } return false; }; @@ -1930,7 +1935,7 @@ pub(crate) fn image_input( Some(ImageSelection { page, index: hit, drag: None, crop: None, crop_mode: false, crop_rect: None, turning: false, settled: None }); } if resp.secondary_clicked() { - ui.ctx().data_mut(|d| d.insert_temp(menu_key, true)); + ui.ctx().data_mut(|d| d.insert_persisted(menu_key, true)); } true } @@ -1951,12 +1956,18 @@ pub(crate) fn image_context_menu( use pdfcraft_engine::ImageEdit as E; let raster = !img.is_form; + let close = |ui: &mut egui::Ui| { + ui.ctx().data_mut(|d| d.insert_persisted(image_menu_id(), false)); + egui::Popup::close_all(ui.ctx()); + ui.close(); + }; + // Top horizontal icon bar: Comment, Crop, Rotate CCW, Rotate CW ui.horizontal(|ui| { ui.spacing_mut().item_spacing.x = 4.0; if crate::icons::button(ui, "message-square", 24.0, false, tl!("Add a sticky note here")).clicked() { *action = Some(ImageAction::AddComment(page, hit)); - ui.close(); + close(ui); } if raster && crate::icons::button(ui, "crop", 24.0, false, tl!("Crop")).clicked() { *action = Some(ImageAction::Crop); @@ -1966,15 +1977,15 @@ pub(crate) fn image_context_menu( s.crop_mode = true; s.crop_rect = None; } - ui.close(); + close(ui); } if crate::icons::button(ui, "rotate-ccw", 24.0, false, tl!("Rotate Counterclockwise")).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Rotate(3) }); - ui.close(); + close(ui); } if crate::icons::button(ui, "rotate-cw", 24.0, false, tl!("Rotate Clockwise")).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Rotate(1) }); - ui.close(); + close(ui); } }); @@ -1982,15 +1993,15 @@ pub(crate) fn image_context_menu( if ui.button(tl!("Cut Image")).clicked() { *action = Some(ImageAction::Cut(page, hit)); - ui.close(); + close(ui); } if ui.button(tl!("Copy Image")).clicked() { *action = Some(ImageAction::Copy(page, hit)); - ui.close(); + close(ui); } if ui.add_enabled(can_paste, egui::Button::new(tl!("Paste"))).clicked() { *action = Some(ImageAction::Paste(page, at)); - ui.close(); + close(ui); } ui.separator(); @@ -1998,66 +2009,66 @@ pub(crate) fn image_context_menu( let is_cropped = img.visible_rect.is_some() && img.visible_rect != Some(img.rect); 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(); + close(ui); } if ui.button(tl!("Flip horizontal")).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Flip { horizontal: true } }); - ui.close(); + close(ui); } if ui.button(tl!("Flip vertical")).clicked() { view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Flip { horizontal: false } }); - ui.close(); + close(ui); } 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(); + close(ui); } 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(); + close(ui); } 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(); + close(ui); } 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(); + close(ui); } }); if ui.add_enabled(raster, egui::Button::new(tl!("Replace image"))).clicked() { *action = Some(ImageAction::Replace(page, hit)); - ui.close(); + close(ui); } if ui.add_enabled(raster, egui::Button::new(tl!("Save Image As…"))).clicked() { *action = Some(ImageAction::Save(page, hit)); - ui.close(); + close(ui); } ui.separator(); if ui.button(tl!("Draw freehand")).clicked() { *action = Some(ImageAction::DrawFreehand); - ui.close(); + close(ui); } if ui.button(tl!("Redact Image")).clicked() { *action = Some(ImageAction::Redact(page, hit)); - ui.close(); + close(ui); } if ui.add_enabled(raster, egui::Button::new("Export image")).clicked() { *action = Some(ImageAction::Save(page, hit)); - ui.close(); + close(ui); } ui.separator(); if ui.button(tl!("Create Link…")).clicked() { *action = Some(ImageAction::Link(page, hit)); - ui.close(); + close(ui); } ui.menu_button("Edit image using", |ui| { @@ -2066,7 +2077,7 @@ pub(crate) fn image_context_menu( if ui.button(tl!("Recognize Text…")).clicked() { *action = Some(ImageAction::Recognize(page)); - ui.close(); + close(ui); } ui.separator(); @@ -2074,7 +2085,7 @@ pub(crate) fn image_context_menu( if ui.button(tl!("Delete")).clicked() { view.image_selection = None; view.pending_edit = Some(Edit::EditPageImage { page, index: hit, change: E::Delete }); - ui.close(); + close(ui); } } diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 03a620ae35..1a618fb8a9 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -7151,3 +7151,120 @@ fn copy_pasted_image_discards_coordinates_and_moves_without_offset() { let images_after_move = h.state().session.get(id).unwrap().page_images(0); assert_eq!(images_after_move[1].rect, target, "moved image rect matches target exactly without offset"); } + +#[test] +fn clicking_after_image_context_menu_action_resets_menu_state_and_updates_selection() { + let mut png = Vec::new(); + image::RgbImage::from_pixel(80, 40, image::Rgb([200, 40, 40])).write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + 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.open_bytes("picture.png", None, png.clone()).expect("opens"); + app + }); + h.run_steps(4); + let id = h.state().views[0].id; + // Move the first image to left side: [10.0, 10.0, 30.0, 30.0] + h.state_mut().apply_edit(pdfcraft_engine::Edit::EditPageImage { + page: 0, + index: 0, + change: pdfcraft_engine::ImageEdit::Move([10.0, 10.0, 30.0, 30.0]), + }); + // Add second image to right side: [50.0, 10.0, 70.0, 30.0] + let mut png2 = Vec::new(); + image::RgbImage::from_pixel(20, 20, image::Rgb([40, 200, 40])).write_to(&mut std::io::Cursor::new(&mut png2), image::ImageFormat::Png).unwrap(); + h.state_mut().apply_edit(pdfcraft_engine::Edit::AddImage { + page: 0, + rect: Some([50.0, 10.0, 70.0, 30.0]), + name: "img2.png".into(), + bytes: std::sync::Arc::new(png2), + }); + h.run_steps(4); + + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + + let r = h.state().views[0].page_screen_rect(0).expect("on screen"); + let user_to_screen = |u: [f64; 2]| { + let info = &h.state().session.get(id).unwrap().info.pages[0]; + let v = info.user_to_view(u[0] as f32, u[1] as f32); + let scale = r.width() / info.width; + egui::pos2(r.left() + v[0] * scale, r.top() + v[1] * scale) + }; + + let p0 = user_to_screen([20.0, 20.0]); + let p1 = user_to_screen([60.0, 20.0]); + let p_empty = user_to_screen([40.0, 35.0]); + + // 1. Select image 0 + h.hover_at(p0); + h.run_steps(1); + h.drag_at(p0); + h.run_steps(1); + h.drop_at(p0); + h.run_steps(2); + assert_eq!(h.state().views[0].image_selection.as_ref().map(|s| s.index), Some(0), "image 0 selected"); + + // 2. Select image 1 (switches selection) + h.hover_at(p1); + h.run_steps(1); + h.drag_at(p1); + h.run_steps(1); + h.drop_at(p1); + h.run_steps(2); + assert_eq!(h.state().views[0].image_selection.as_ref().map(|s| s.index), Some(1), "image 1 selected"); + + // 3. Click empty space (clears selection) + h.hover_at(p_empty); + h.run_steps(1); + h.drag_at(p_empty); + h.run_steps(1); + h.drop_at(p_empty); + h.run_steps(2); + assert!(h.state().views[0].image_selection.is_none(), "empty page clears selection"); + + // 4. Select image 0 again + h.hover_at(p0); + h.run_steps(1); + h.drag_at(p0); + h.run_steps(1); + h.drop_at(p0); + h.run_steps(2); + assert_eq!(h.state().views[0].image_selection.as_ref().map(|s| s.index), Some(0), "image 0 selected again"); + + // 5. Right-click image 0 to open context menu + h.hover_at(p0); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: p0, button: egui::PointerButton::Secondary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: p0, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + + // 6. Click "Flip horizontal" + h.get_by_label("Flip horizontal").click(); + h.run_steps(3); + + let flag_persisted = h.ctx.data_mut(|d| d.get_persisted::(egui::Id::new("image-menu"))); + assert_eq!(flag_persisted, Some(false), "image-menu flag should be reset to false"); + + // 7. Click image 1: should switch selection to image 1 + h.hover_at(p1); + h.run_steps(1); + h.drag_at(p1); + h.run_steps(1); + h.drop_at(p1); + h.run_steps(2); + assert_eq!( + h.state().views[0].image_selection.as_ref().map(|s| s.index), + Some(1), + "clicking other image switches selection after context menu action" + ); + + // 8. Click empty space: should clear selection + h.hover_at(p_empty); + h.run_steps(1); + h.drag_at(p_empty); + h.run_steps(1); + h.drop_at(p_empty); + h.run_steps(2); + assert!(h.state().views[0].image_selection.is_none(), "clicking empty page clears selection after context menu action"); +}