From 3c6be18fb7c2bf68969b54cbbf99f47aeeac0f94 Mon Sep 17 00:00:00 2001 From: SBaksa Date: Wed, 21 Oct 2026 13:57:18 +1100 Subject: [PATCH] Select All of the document: a later copy cancels one still reading; build the job in one pass A whole-document copy reads its pages off the UI thread, and a copy made meanwhile (a page selection, a text field's, a comment's, an image) went to the clipboard first and was then replaced by the document's text. Any later copy now drops the pending one: copy_selected_text and the image copies drop it directly, and finish_document_copy, now run at the end of the frame, drops it when anything else copied that frame. DocumentCopyJob::of classifies each page from text_failed (a set) instead of looking it up in pages_without_text's Vec, which was quadratic in the page count. --- crates/ui-egui/src/canvas.rs | 39 ++++++++++++++++++++++++++++++++ crates/ui-egui/src/content_ui.rs | 2 ++ crates/ui-egui/src/image_pick.rs | 2 ++ crates/ui-egui/src/lib.rs | 15 +++++++----- 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 36552ddd1a..79e21538e2 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -5797,6 +5797,45 @@ trailer << /Root 1 0 R >> assert!(!app.views[0].all_text, "not past an edit of the document"); } + /// A whole-document copy reads the pages without text, keeps those already read and skips + /// those that failed before. + #[test] + fn a_document_copy_reads_only_the_pages_it_needs() { + let mut app = PdfCraftApp::new(); + app.open_bytes("three.pdf", None, text_pages_pdf(&["Alpha one", "Bravo two", "Charlie three"])).unwrap(); + app.views[0].text_failed.insert(2); + let job = crate::DocumentCopyJob::of(&app.views[0], app.session.get(app.views[0].id).unwrap()); + let kinds: Vec<_> = job.pages.iter().map(|p| matches!(p, crate::CopyPage::ToRead)).collect(); + assert_eq!(kinds, vec![true, true, false], "pages 1 and 2 to read, page 3 failed before"); + assert!(matches!(job.pages[2], crate::CopyPage::Skip)); + } + + /// A later copy wins: a whole-document copy still reading its pages never replaces what was + /// copied after it started, from the page or anywhere else (a text field, a comment). + #[test] + fn a_later_copy_cancels_a_document_copy_still_reading() { + let mut app = PdfCraftApp::new(); + app.open_bytes("three.pdf", None, text_pages_pdf(&["Alpha one", "Bravo two", "Charlie three"])).unwrap(); + let ctx = egui::Context::default(); + // Another copy in the same frame (here a text field's) drops it. + app.views[0].select_all(); + assert!(app.copy_selected_text(0, &ctx) && app.document_copy.is_some()); + let mut out = ctx.run_ui(egui::RawInput::default(), |ui| { + ui.ctx().copy_text("a word".to_owned()); + app.finish_document_copy(ui.ctx()); + }); + out.textures_delta.clear(); + assert!(app.document_copy.is_none(), "cancelled"); + let copies: Vec<_> = out.platform_output.commands.iter().filter(|c| matches!(c, egui::OutputCommand::CopyText(_))).collect(); + assert_eq!(copies, vec![&egui::OutputCommand::CopyText("a word".to_owned())], "the later copy stays"); + // So does copying a page selection. + app.views[0].select_all(); + assert!(app.copy_selected_text(0, &ctx) && app.document_copy.is_some()); + app.views[0].select_text(0, 0, 5); + app.copy_selected_text(0, &ctx); + assert!(app.document_copy.is_none(), "a page selection's copy cancels it"); + } + /// A press anywhere on a page ends Select All of the document, whatever the tool (here the /// Hand, which selects no text) and whether the page's text is read yet. #[test] diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index 5f5ef32f77..b1ba7e4f99 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -731,6 +731,8 @@ impl crate::PdfCraftApp { /// Copy Image: image `index` on `page`, at its own resolution, to the clipboard. pub(crate) fn copy_page_image(&mut self, page: usize, index: usize) -> bool { + // A newer copy wins over a whole-document copy still reading its pages. + self.document_copy = None; let Some((_, id)) = self.active_ids() else { return false }; if !self.may_copy(id) { return false; diff --git a/crates/ui-egui/src/image_pick.rs b/crates/ui-egui/src/image_pick.rs index 53e4bdf158..403e9836c0 100644 --- a/crates/ui-egui/src/image_pick.rs +++ b/crates/ui-egui/src/image_pick.rs @@ -183,6 +183,8 @@ impl PdfCraftApp { /// Copy Image in Original Format: image `index` on `page` as a file on the clipboard (JPEG /// as stored in the PDF, otherwise PNG). pub(crate) fn copy_page_image_file(&mut self, page: usize, index: usize) { + // A newer copy wins over a whole-document copy still reading its pages. + self.document_copy = None; let Some((_, id)) = self.active_ids() else { return }; if !self.may_copy(id) { return; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6d1f7e70a6..f2dae65a81 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -402,12 +402,11 @@ struct DocumentCopyJob { impl DocumentCopyJob { fn of(v: &DocView, doc: &pdfcraft_engine::Document) -> DocumentCopyJob { - let missing = v.pages_without_text(); let pages = (0..v.page_count()) .map(|p| match v.texts.get(&p) { Some(t) => CopyPage::Read(t.clone()), - None if missing.contains(&p) => CopyPage::ToRead, - None => CopyPage::Skip, + None if v.text_failed.contains(&p) => CopyPage::Skip, + None => CopyPage::ToRead, }) .collect(); DocumentCopyJob { pages, bytes: doc.bytes.clone(), password: doc.password.as_deref().map(std::sync::Arc::from) } @@ -2970,7 +2969,6 @@ impl PdfCraftApp { object_ui::nudge_keys(ctx, &mut self.views[i], doc); } canvas::shortcuts(&mut self.views[i], ctx, self.scroll_speed); - self.finish_document_copy(ctx); self.copy_key(i, ctx); self.image_pick_keys(i, ctx); } @@ -3009,6 +3007,8 @@ impl PdfCraftApp { /// After Select All of the document (#2639), pages whose text wasn't read for display are read /// off the UI thread ([`DocumentCopy`]), and the clipboard gets the text when they're done. pub(crate) fn copy_selected_text(&mut self, i: usize, ctx: &egui::Context) -> bool { + // A newer copy wins: a whole-document copy still reading must not replace it later. + self.document_copy = None; let Some(v) = self.views.get(i) else { return false }; let id = v.id; if v.all_text && !v.pages_without_text().is_empty() { @@ -3029,10 +3029,12 @@ impl PdfCraftApp { } /// The clipboard text of a whole-document copy, once its pages are read; dropped with the - /// document it was copied from. + /// document it was copied from, and by any later copy (a text field's, a comment's), which + /// it must not replace. Run at the end of the frame, so it sees this frame's copies. fn finish_document_copy(&mut self, ctx: &egui::Context) { let Some(c) = &self.document_copy else { return }; - if !self.views.iter().any(|v| v.id == c.doc) { + let copied = ctx.output(|o| o.commands.iter().any(|c| matches!(c, egui::OutputCommand::CopyText(_) | egui::OutputCommand::CopyImage(_)))); + if copied || !self.views.iter().any(|v| v.id == c.doc) { self.document_copy = None; return; } @@ -3355,6 +3357,7 @@ impl eframe::App for PdfCraftApp { impl PdfCraftApp { fn finish_render_frame(&mut self, ctx: &egui::Context) { + self.finish_document_copy(ctx); // The Combine grid's thumbnails, whichever tab shows (renders under way still finish). self.combine_thumbs_frame(ctx); // Retire hidden documents before admitting the visible document's textures, so the