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.
This commit is contained in:
SBaksa
2026-10-21 13:57:18 +11:00
parent 1f182ff0bf
commit 3c6be18fb7
4 changed files with 52 additions and 6 deletions
+39
View File
@@ -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]
+2
View File
@@ -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;
+2
View File
@@ -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;
+9 -6
View File
@@ -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