aleleba/pdfcraft
1
0
Fork 0
mirror of https://github.com/storytold/pdfcraft.git synced 2026-11-11 16:36:13 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(ui): Update draws the page shown without its marks at once, and a page that fails alone

Browse Source 
The preview showed the page's thumbnail, old marks and all, until the
unmarked page rendered, and one page that failed to render switched the
unmarked preview off for every page. The page shown is now drawn from the
unmarked document straight away at the preview's size; the pages a step
goes to render ahead off the UI thread; a page that fails falls back to
its own thumbnail while the others keep theirs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
alehel
2026-10-28 12:40:45 +01:00
co-authored by Claude Opus 5.5
parent 1e01497649
commit 098a1f8321
3 changed files with 74 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/ui-egui/src/app.rs
+1 -1
View File
@@ -307,7 +307,7 @@ impl PdfCraftApp {
/// `true` while any open document still waits for page renders (used by headless capture).
pub fn render_pending(&self) -> bool {
self.views.iter().any(|v| v.render_pending()) || self.unmarked_page.pending()
self.views.iter().any(|v| v.render_pending())
}
}
crates/ui-egui/src/dialogs/marks_ui.rs
+63 -33
View File
@@ -3,6 +3,8 @@
//! page the range selects, with the marks laid out on it. "Update" opens the same dialog in
//! replace mode.
use std::collections::{HashMap, HashSet};
use egui::{Align, Color32, CornerRadius, Layout, Rect, Stroke, pos2, vec2};
use pdfcraft_engine::{Background, Edit, FontFamily, HeaderFooter, MarkKind, Watermark};
@@ -186,31 +188,35 @@ fn behind(color: [f64; 3], opacity: f64) -> Color32 {
/// In Update, the pages previewed without the marks the dialog replaces. Their thumbnails still
/// have them, so the new marks would be drawn over the old ones, and a new background colour
/// would tint the old one. The document without them is made once per dialog (it costs about a
/// save), and each page shown is rendered from it off the UI thread, like the thumbnails.
/// save). The page shown is drawn from it straight away, at the preview's small size, so neither
/// a blank sheet nor the old marks show while it renders; the pages a step goes to next render
/// ahead, off the UI thread, so stepping rarely waits.
#[derive(Default)]
pub(crate) struct UnmarkedPage {
/// The document, its edit generation and the kind of mark taken away.
key: Option<(pdfcraft_engine::DocId, u64, MarkKind)>,
pages: Option<pdfcraft_engine::UnmarkedPages>,
renderer: Option<pdfcraft_render::RenderPool>,
/// The page and scale last asked for.
asked: Option<(usize, u64)>,
/// The page and scale the texture shows.
texture: Option<(usize, u64, egui::TextureHandle)>,
/// The pages rendered, with the scale (×1000) they were rendered at: the page shown and the
/// pages next to it.
textures: HashMap<usize, (u64, egui::TextureHandle)>,
/// Pages that failed to render: their thumbnails show instead, the other pages still don't.
failed: HashSet<usize>,
/// The pages asked of the renderer ahead, and not back yet. Nothing waits for them: a step
/// is a frame, which takes what has come back first and draws the rest itself.
ahead: Vec<(usize, u64)>,
}
impl UnmarkedPage {
pub(crate) fn pending(&self) -> bool {
self.renderer.is_some() && self.asked.is_some() && self.texture.as_ref().map(|(p, t, _)| (*p, *t)) != self.asked
}
/// The picture to preview `page` on in place of its thumbnail, or `None` for the thumbnail:
/// not Update, no marks of `kind` in the document, the pages can't be prepared, or `page` is
/// still rendering (its thumbnail shows until then, rather than a blank sheet).
/// not Update, no marks of `kind` in the document, the pages can't be prepared, or `page`
/// fails to render. `near` are the pages a step goes to from it, rendered ahead.
fn picture(
&mut self,
ctx: &egui::Context,
(doc, id): (&pdfcraft_engine::Document, pdfcraft_engine::DocId),
page: usize,
near: &[usize],
kind: MarkKind,
replace: bool,
) -> Option<egui::TextureId> {
@@ -218,34 +224,57 @@ impl UnmarkedPage {
if self.key != key {
*self = Self { key, ..Default::default() };
if replace {
self.renderer = doc.without_marks(kind).ok().flatten().map(|p| p.renderer());
self.pages = doc.without_marks(kind).ok().flatten();
self.renderer = self.pages.as_ref().map(|p| p.renderer());
}
}
let renderer = self.renderer.as_ref()?;
let (pages, renderer) = (self.pages.as_ref()?, self.renderer.as_ref()?);
// At the preview's size in pixels.
let p = doc.info.pages.get(page)?;
let scale = PREVIEW_SIDE * ctx.pixels_per_point() / p.width.max(p.height).max(1.0);
let asked = (page, (scale * 1000.0) as u64);
if self.asked != Some(asked) {
renderer.set_queue(vec![pdfcraft_render::RenderRequest { page, scale, tag: asked.1, ..Default::default() }]);
self.asked = Some(asked);
}
if let Some(out) = renderer.try_recv() {
let scale = |page: usize| {
let p = doc.info.pages.get(page)?;
let s = PREVIEW_SIDE * ctx.pixels_per_point() / p.width.max(p.height).max(1.0);
Some((s, (s * 1000.0) as u64))
};
let load = |ctx: &egui::Context, out: pdfcraft_render::RenderedPage| {
let image = crate::canvas::texture_image([out.width as usize, out.height as usize], out.rgba);
ctx.load_texture("marks-preview-unmarked", image, egui::TextureOptions::LINEAR)
};
while let Some(out) = renderer.try_recv() {
let at = (out.request.page, out.request.tag);
self.ahead.retain(|a| *a != at);
if out.error.is_some() {
// The thumbnails, as before, rather than no page at all.
self.renderer = None;
self.texture = None;
return None;
}
if (out.request.page, out.request.tag) == asked {
let image = crate::canvas::texture_image([out.width as usize, out.height as usize], out.rgba);
self.texture = Some((page, asked.1, ctx.load_texture("marks-preview-unmarked", image, egui::TextureOptions::LINEAR)));
self.failed.insert(at.0);
} else {
self.textures.insert(at.0, (at.1, load(ctx, out)));
}
}
if self.pending() {
ctx.request_repaint_after(std::time::Duration::from_millis(16));
let (s, tag) = scale(page)?;
if !self.failed.contains(&page) && self.textures.get(&page).is_none_or(|(t, _)| *t != tag) {
match pages.render_page(page, s) {
Ok(out) => {
self.textures.insert(page, (tag, load(ctx, out)));
}
Err(_) => {
self.failed.insert(page);
}
}
}
self.texture.as_ref().filter(|(p, ..)| *p == page).map(|(.., t)| t.id())
// Ahead: the pages a step goes to, not rendered yet at their scale.
let wanted: Vec<(usize, u64)> = near
.iter()
.filter(|p| !self.failed.contains(p))
.filter_map(|&p| scale(p).map(|(_, t)| (p, t)))
.filter(|(p, t)| self.textures.get(p).is_none_or(|(have, _)| have != t))
.collect();
if !wanted.is_empty() && wanted != self.ahead {
let requests = wanted
.iter()
.filter_map(|&(p, _)| scale(p).map(|(s, tag)| pdfcraft_render::RenderRequest { page: p, scale: s, tag, ..Default::default() }));
renderer.set_queue(requests.collect());
self.ahead = wanted;
}
self.textures.retain(|p, _| *p == page || near.contains(p));
self.textures.get(&page).filter(|_| !self.failed.contains(&page)).map(|(_, t)| t.id())
}
}
@@ -450,7 +479,8 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M
// frame the engine lays the marks out in. Update draws it without the marks it replaces.
let target = shown_preview(&d.range, &mut d.preview, current, count);
let page = doc.info.pages.get(target).map(|p| (p.width as f64, p.height as f64)).unwrap_or((612.0, 792.0));
let unmarked = app.unmarked_page.picture(ui.ctx(), (doc, id), target, kind, d.replace);
let near: Vec<usize> = [false, true].into_iter().filter_map(|forward| step_preview(&d.range, target, count, forward)).collect();
let unmarked = app.unmarked_page.picture(ui.ctx(), (doc, id), target, &near, kind, d.replace);
let picture = unmarked.or_else(|| {
app.views.get_mut(i).and_then(|view| {
view.need_thumbnail(target, true);
crates/ui-egui/tests/it/marks.rs
+10 -16
View File
@@ -386,8 +386,6 @@ fn update_previews_the_page_without_the_marks_it_replaces() {
let mut app = PdfCraftApp::new();
app.set_option("language", "en").unwrap();
app.open_bytes("doc.pdf", None, pdf.clone()).unwrap();
// The Pages panel keeps both thumbnails, for stepping to page 2 in Update below.
app.set_option("panel", "pages").unwrap();
app
});
h.run_steps(4);
@@ -427,30 +425,26 @@ fn update_previews_the_page_without_the_marks_it_replaces() {
assert!(h.state_mut().execute("edit.background.update"));
h.run_steps(2);
h.state_mut().marks_draft.bg.color = [0.6, 0.8, 1.0];
let white = |i: &egui::ColorImage| at(i, 0.5, 0.75) == egui::Color32::WHITE;
let (tint, page) = wait_for_preview(&mut h, &uploads, "the page without its background", white);
h.step();
// Update shows the page without the marks from the frame it opens: neither a blank sheet nor
// the old marks while it renders.
let shown = |h: &Harness<'static, PdfCraftApp>| preview_picture(h, &uploads).expect("a page is previewed");
let (tint, page) = shown(&h);
assert_eq!(tint, egui::Color32::from_rgb(153, 204, 255), "the new colour tints the page");
assert_eq!(at(&page, 0.5, 0.75), egui::Color32::WHITE, "the page is previewed without the yellow background");
assert!(count(&page, red) > 200 && count(&page, blue) > 40 && count(&page, dark) > 1000, "the watermark, header and page content stay");
// Stepping to page 2 shows its thumbnail until it renders without the background, not a
// blank sheet: the pages are prepared once, and each renders on its own.
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
while h.state().views[0].thumb(1).is_none() {
assert!(std::time::Instant::now() < deadline, "page 2's thumbnail never loaded");
h.step();
std::thread::sleep(std::time::Duration::from_millis(5));
}
// So does a step to page 2: the pages are prepared once, and each is drawn as it is shown.
h.state_mut().marks_draft.preview = Some(1);
h.step();
assert!(preview_picture(&h, &uploads).is_some(), "a page shows while page 2 renders");
wait_for_preview(&mut h, &uploads, "page 2 without its background", white);
assert_eq!(at(&shown(&h).1, 0.5, 0.75), egui::Color32::WHITE, "page 2 is previewed without its background at once");
h.get_by_label("Cancel").click();
h.run_steps(2);
assert!(h.state_mut().execute("edit.watermark.update"));
h.run_steps(2);
h.state_mut().marks_draft.wm.text = "NEW".into();
let (_, page) = wait_for_preview(&mut h, &uploads, "the page without its watermark", |i| count(i, red) == 0);
h.step();
let (_, page) = shown(&h);
assert_eq!(count(&page, red), 0, "the new watermark isn't previewed over the old one");
assert!(at(&page, 0.5, 0.75) == yellow && count(&page, blue) > 40, "the background and header stay");
h.get_by_label("Cancel").click();
@@ -458,7 +452,7 @@ fn update_previews_the_page_without_the_marks_it_replaces() {
assert!(h.state_mut().execute("edit.header_footer.update"));
h.run_steps(2);
let (_, page) = wait_for_preview(&mut h, &uploads, "the page without its header", |i| count(i, blue) == 0);
let (_, page) = shown(&h);
assert_eq!(count(&page, blue), 0, "the new header isn't previewed over the old one");
assert!(at(&page, 0.5, 0.75) == yellow && count(&page, red) > 200, "the background and watermark stay");
}