From 77640a030f3142b11c71777952d1a9b989deff65 Mon Sep 17 00:00:00 2001 From: Valentin Klein Date: Thu, 15 Oct 2026 08:37:18 +0200 Subject: [PATCH] Draw: a released pen stroke stays on screen until the page raster has it (#1858) (#1987) Co-authored-by: Claude Sonnet 5.5 --- crates/ui-egui/src/canvas.rs | 4 +++ crates/ui-egui/src/comments.rs | 56 ++++++++++++++++++++++++++++++++ crates/ui-egui/src/editing.rs | 4 +++ crates/ui-egui/tests/comments.rs | 36 ++++++++++++++++++++ 4 files changed, 100 insertions(+) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 33e1c25c59..53925c008c 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -836,6 +836,8 @@ impl DocView { /// was toggled). Out-of-date rasters stay on screen until their replacements arrive, so the /// view never flashes blank. pub fn invalidate_content(&mut self) { + // Undo, redo and other document-wide changes: a released drawing's preview would be a ghost. + self.comments.settling.clear(); for p in self.pages.values_mut() { p.tag = STALE_TAG; } @@ -1704,6 +1706,7 @@ impl DocView { self.stale_tiles.remove(&key); if !self.stale_tiles.iter().any(|k| k.0 == page) { self.forms.unbaked.retain(|(_, p)| *p != page); + self.comments.page_rendered(page); } } else if r.request.tag & PRINT_TAG != 0 { let tex = ctx.load_texture(format!("print-{:?}-{page}", self.id), img, TextureOptions::LINEAR); @@ -1720,6 +1723,7 @@ impl DocView { self.pages.insert(page, PageTex { tag: r.request.tag, tex }); self.signature_drag.page_received(page); self.move_background.page_received(page); + self.comments.page_rendered(page); self.waiting_since.remove(&page); if !self.stale_tiles.iter().any(|k| k.0 == page) { self.forms.unbaked.retain(|(_, p)| *p != page); diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index 67d2c1ce91..e0a6e23eb7 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -409,12 +409,53 @@ pub struct Composer { pub focus: bool, } +/// A released drawing gesture waiting for the page raster to show its annotation. +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct Settling { + pub page: usize, + pub tool: CommentTool, + pub points: Vec<[f64; 2]>, + /// Frame time (seconds) at release; a raster that never arrives must not leave a ghost. + pub since: f64, +} + +/// Longest a released drawing stays painted without its page raster, and how many are kept. +const SETTLE_SECS: f64 = 20.0; +const SETTLE_MAX: usize = 16; + +impl CommentView { + /// Remember a released drag drawing so it stays on screen until its page is re-rendered. + fn settle(&mut self, now: f64, page: usize, tool: CommentTool, points: &[[f64; 2]]) { + if !matches!(tool, CommentTool::Ink | CommentTool::Line | CommentTool::Arrow | CommentTool::Rectangle | CommentTool::Oval) { + return; + } + self.settling.retain(|s| now - s.since < SETTLE_SECS); + if self.settling.len() >= SETTLE_MAX { + self.settling.remove(0); + } + self.settling.push(Settling { page, tool, points: points.to_vec(), since: now }); + } + + /// How many released drawings are still painted from their points (waiting for a raster). + pub fn settling_len(&self) -> usize { + self.settling.len() + } + + /// A fresh raster of `page` is on screen: it draws the committed annotation itself. + pub(crate) fn page_rendered(&mut self, page: usize) { + self.settling.retain(|s| s.page != page); + } +} + /// Per-document commenting state. #[derive(Clone, Debug, Default)] pub struct CommentView { /// The selected comment: (page, index in `/Annots`). pub selected: Option<(usize, usize)>, pub gesture: Option, + /// Drawings just released: shown from their points until the page raster that includes the + /// committed annotation arrives, so the stroke never vanishes in between (#1858). + pub(crate) settling: Vec, /// Escape ends egui's drag too; ignore its synthetic release until the mouse is up. cancelled_drag: bool, pub composer: Option, @@ -671,6 +712,8 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, if tool == CommentTool::Eraser { view.pending_edit = erase(cx, &points); } else if let Some(shape) = drawn_shape(tool, &points) { + let now = ui.input(|i| i.time); + cv.settle(now, page, tool, &points); view.pending_edit = Some(new_comment(cx, tool, shape, String::new())); } } @@ -684,6 +727,8 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, && let Some(shape) = drawn_shape(tool, &[cx.to_user(p)]) { cv.selected = None; + let now = ui.input(|i| i.time); + cv.settle(now, cx.page, tool, &[cx.to_user(p)]); view.pending_edit = Some(new_comment(cx, tool, shape, String::new())); } true @@ -1062,8 +1107,19 @@ pub(crate) fn paint_page(ui: &egui::Ui, painter: &egui::Painter, cx: &PageCx<'_> } fn paint_gesture(painter: &egui::Painter, cx: &PageCx<'_>, view: &DocView) { + let now = painter.ctx().input(|i| i.time); + for s in view.comments.settling.iter().filter(|s| s.page == cx.page && now - s.since < SETTLE_SECS) { + paint_draw(painter, cx, &s.tool, &s.points); + } if let Some(Gesture::Draw { page, tool, points }) = &view.comments.gesture && *page == cx.page + { + paint_draw(painter, cx, tool, points); + } +} + +/// A drawing tool's shape from its points (a live drag, or a released one still settling). +fn paint_draw(painter: &egui::Painter, cx: &PageCx<'_>, tool: &CommentTool, points: &[[f64; 2]]) { { let style = cx.prefs.style(*tool); let zoom = cx.xf.rect.width() / cx.xf.pw.max(1.0); diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 4d2e09168b..8b8cd62f6c 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -42,6 +42,10 @@ impl PdfCraftApp { let Some(doc) = self.session.get(id) else { return true }; let info = &doc.info; let view = &mut self.views[i]; + 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(); + } view.signature_drag.committed(&edit, doc.edit_generation()); view.move_background.committed(&edit, doc.edit_generation()); if let Some(page) = comment_page(&edit).or_else(|| content_page(&edit)) { diff --git a/crates/ui-egui/tests/comments.rs b/crates/ui-egui/tests/comments.rs index 503ae7a04b..46c6c216e1 100644 --- a/crates/ui-egui/tests/comments.rs +++ b/crates/ui-egui/tests/comments.rs @@ -406,6 +406,42 @@ fn shapes_and_ink_are_drawn_by_dragging() { assert!((r[0] - 40.0).abs() < 3.0 && (r[1] - 40.0).abs() < 3.0 && (r[2] - 140.0).abs() < 3.0 && (r[3] - 100.0).abs() < 3.0, "{r:?}"); } +/// Press, drag and release a stroke, running only one frame after the release. +fn stroke_without_settling(h: &mut Harness<'static, PdfCraftApp>, a: (f32, f32), b: (f32, f32)) { + let (from, to) = (at(h, a.0, a.1), at(h, b.0, b.1)); + h.hover_at(from); + h.run_steps(1); + h.drag_at(from); + h.run_steps(1); + for k in 1..=4 { + h.hover_at(from + (to - from) * (k as f32 / 4.0)); + h.run_steps(1); + } + h.drop_at(to); + h.run_steps(1); +} + +/// #1858: on release the live stroke is replaced by the committed annotation, but the page +/// raster that draws it arrives later. The stroke stays painted from its points meanwhile. +#[test] +fn a_released_pen_stroke_stays_drawn_until_the_page_raster_has_it() { + let mut h = harness(|app| app.set_option("quick", "ink").unwrap()); + stroke_without_settling(&mut h, (40.0, 100.0), (140.0, 60.0)); + assert_eq!(comments(&h).len(), 1, "the stroke is committed"); + assert_eq!(h.state().views[0].comments.settling_len(), 1, "still drawn from its points"); + settle(&mut h); + assert_eq!(h.state().views[0].comments.settling_len(), 0, "the raster draws it now"); +} + +#[test] +fn undoing_a_pen_stroke_before_its_raster_leaves_no_ghost() { + let mut h = harness(|app| app.set_option("quick", "ink").unwrap()); + stroke_without_settling(&mut h, (40.0, 100.0), (140.0, 60.0)); + assert_eq!(h.state().views[0].comments.settling_len(), 1); + h.state_mut().undo(); + assert_eq!(h.state().views[0].comments.settling_len(), 0); +} + #[test] fn pen_strokes_stay_unselected_so_writing_continues() { let mut h = harness(|app| app.set_option("quick", "ink").unwrap());