From da48b84b8cb00de3cb2bd045c8e41694db1fb649 Mon Sep 17 00:00:00 2001 From: hade0ntema <296480886+hade0ntema@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:33:24 +0200 Subject: [PATCH 1/2] fix(select): a picked picture that is edited away no longer leaves its highlight behind With the Select tool a click picks a picture and draws a blue highlight over it. The pick was only cleared by Escape, a drag or another click. Picking a picture, then entering Edit text & images and deleting it left the highlight on the empty spot, with nothing left to click to remove it. The pick now remembers the document's edit generation and is dropped once the document has been edited since (a delete, a move, Undo), the same condition under which the copy shortcut already refused it as stale. Co-Authored-By: Claude Sonnet 5.5 --- crates/ui-egui/src/canvas.rs | 1 + crates/ui-egui/src/image_pick.rs | 16 ++++++++++++++-- crates/ui-egui/tests/viewer.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 0acab64a08..7759accf47 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -3275,6 +3275,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { if tool == QuickTool::Select && !consumed { crate::image_pick::page_input(&resp, &xf, i, info, doc, pointer, view); } + crate::image_pick::drop_stale(doc, view); crate::image_pick::paint(painter, &xf, i, info, view); comments::page_after_text(&resp, &pcx, view); if tool == QuickTool::Redact && can_modify { diff --git a/crates/ui-egui/src/image_pick.rs b/crates/ui-egui/src/image_pick.rs index a05aefe2d2..53e4bdf158 100644 --- a/crates/ui-egui/src/image_pick.rs +++ b/crates/ui-egui/src/image_pick.rs @@ -9,12 +9,14 @@ use crate::PdfCraftApp; use crate::canvas::{DocView, PageXform}; use pdfcraft_render::DocInfo; -/// The selected picture: page, index in `page_images` and its box in user space. +/// The selected picture: page, index in `page_images` and its box in user space, and the +/// document's edit generation when it was picked (any later edit may have removed or renumbered it). #[derive(Clone, Copy, Debug, PartialEq)] pub struct PickedImage { pub page: usize, pub index: usize, pub rect: [f64; 4], + pub generation: u64, } const BLUE: Color32 = Color32::from_rgb(0x3A, 0x7B, 0xF0); @@ -43,8 +45,18 @@ pub(crate) fn page_input( let (vx, vy) = xf.screen_to_view(p); let u = info_page.view_to_user(vx, vy); let at = [f64::from(u[0]), f64::from(u[1])]; + let generation = doc.edit_generation(); view.picked_image = - doc.page_image_at(page, at).and_then(|index| doc.page_images(page).get(index).map(|i| PickedImage { page, index, rect: i.rect })); + doc.page_image_at(page, at).and_then(|index| doc.page_images(page).get(index).map(|i| PickedImage { page, index, rect: i.rect, generation })); +} + +/// Forget the selected picture once the document has been edited since it was picked: Edit text & +/// images (or Undo) may have deleted or moved it, and its highlight would stay behind on an empty +/// spot of the page. +pub(crate) fn drop_stale(doc: &pdfcraft_engine::Document, view: &mut DocView) { + if view.picked_image.is_some_and(|p| p.generation != doc.edit_generation()) { + view.picked_image = None; + } } /// The selected picture's highlight on page `page`. diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index 474360865b..aadf6c56d4 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -1384,6 +1384,35 @@ fn a_clicked_picture_is_selected_and_copied_in_its_original_format() { assert_eq!(h.state().views[i].picked_image, None); } +/// A picture picked with the Select tool, then deleted in Edit text & images, leaves no highlight +/// behind on the empty spot where it was. +#[test] +fn a_picked_picture_that_is_deleted_in_edit_mode_loses_its_highlight() { + let (_, mut h) = picture_harness(); + let i = h.state().active.unwrap(); + let id = h.state().views[i].id; + let at = h.state().views[i].page_screen_rect(0).expect("on screen").center(); + h.hover_at(at); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + assert!(h.state().views[i].picked_image.is_some(), "picked with the Select tool"); + // Edit text & images: click the picture, delete it with the Delete key. + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + h.hover_at(at); + h.run_steps(1); + h.drag_at(at); + h.run_steps(1); + h.drop_at(at); + h.run_steps(2); + h.key_press(egui::Key::Delete); + h.run_steps(4); + assert!(h.state().session.get(id).unwrap().page_images(0).is_empty(), "the picture is gone"); + assert_eq!(h.state().views[i].picked_image, None, "and so is its highlight"); +} + #[test] fn right_click_on_a_picture_copies_it_at_full_size() { let (px, mut h) = picture_harness(); From 37a934662928a289c4f96506118f9d3253e92609 Mon Sep 17 00:00:00 2001 From: hade0ntema <296480886+hade0ntema@users.noreply.github.com> Date: Fri, 16 Oct 2026 11:49:06 +0200 Subject: [PATCH 2/2] fix(select): clear the picked picture in the edit itself, not on the next frame The first fix dropped a stale pick while painting the next frame. The window only redraws on the next input, so after deleting a picture in Edit text & images its highlight stayed on screen until the user clicked. Clear the pick in apply_edit, in the same step as the edit, and ask for a repaint. drop_stale stays for the paths that change the document without apply_edit (Undo, Redo). An edit-level test checks the pick right after apply_edit with no frame in between; it fails without the change. The earlier one-frame test could not tell, the test harness redraws more often than the app does. Co-Authored-By: Claude Sonnet 5.5 --- crates/ui-egui/src/editing.rs | 6 ++++++ crates/ui-egui/tests/viewer.rs | 22 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index be1347d68d..6f78199496 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -52,6 +52,12 @@ impl PdfCraftApp { let Some(doc) = self.session.get(id) else { return true }; let info = &doc.info; let view = &mut self.views[i]; + // A picture picked with the Select tool may be the one this edit removed or moved: + // drop its highlight now, not on the next frame (which only comes with the next input). + view.picked_image = None; + if let Some(ctx) = &self.ctx { + ctx.request_repaint(); + } if !matches!(edit, Edit::AddAnnotation(_)) { // Only a new drawing waits on its raster; any other edit may remove or move it. view.comments.settling.clear(); diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index aadf6c56d4..96defce320 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -1408,11 +1408,31 @@ fn a_picked_picture_that_is_deleted_in_edit_mode_loses_its_highlight() { h.drop_at(at); h.run_steps(2); h.key_press(egui::Key::Delete); - h.run_steps(4); + // One frame, no more input: the highlight goes at once, it does not wait for a click. + h.run_steps(1); assert!(h.state().session.get(id).unwrap().page_images(0).is_empty(), "the picture is gone"); assert_eq!(h.state().views[i].picked_image, None, "and so is its highlight"); } +/// The highlight goes in the very edit that removes the picture, not on a later frame: the window +/// only redraws on the next input, so waiting for one leaves the highlight on screen until a click. +#[test] +fn an_edit_clears_the_picked_picture_at_once() { + let (_, mut h) = picture_harness(); + let i = h.state().active.unwrap(); + let at = h.state().views[i].page_screen_rect(0).expect("on screen").center(); + h.hover_at(at); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); + assert!(h.state().views[i].picked_image.is_some(), "picked with the Select tool"); + let delete = pdfcraft_engine::Edit::EditPageImage { page: 0, index: 0, change: pdfcraft_engine::ImageEdit::Delete }; + assert!(h.state_mut().apply_edit(delete)); + // No frame has run since the edit. + assert_eq!(h.state().views[i].picked_image, None); +} + #[test] fn right_click_on_a_picture_copies_it_at_full_size() { let (px, mut h) = picture_harness();