From 9994665436cb10eecfa39928753aecd8daf31875 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 18:43:13 -0800 Subject: [PATCH] Pages: an edit no longer renders every page thumbnail again While the Pages panel was shown, every edit made all page thumbnails out of date, and the next frame rendered one for every page of the document on the interface thread, including pages out of view. Each move step, drag step and typed character paid for it. - Only thumbnails of rows in view are rendered. - After an edit, the previous thumbnail stays on screen until the document has been unchanged for 0.4 s. Then one out-of-date thumbnail is rendered again per frame. - Thumbnails of deleted pages and parents are dropped. Part of #1411 --- crates/ui-egui/src/panels/pages.rs | 84 ++++++++++++++++++++---- crates/ui-egui/src/panels/pages/tests.rs | 56 ++++++++++++++-- 2 files changed, 123 insertions(+), 17 deletions(-) diff --git a/crates/ui-egui/src/panels/pages.rs b/crates/ui-egui/src/panels/pages.rs index f46868a35..ff8e2f3c7 100644 --- a/crates/ui-egui/src/panels/pages.rs +++ b/crates/ui-egui/src/panels/pages.rs @@ -17,22 +17,85 @@ enum ThumbOf { Parent(SpreadId), } -/// (document uid, revision, page or parent, thumbnail height). -type ThumbKey = (u64, u64, ThumbOf, u32); +/// (document uid, page or parent, thumbnail height). +type ThumbKey = (u64, ThumbOf, u32); -static THUMBS: Mutex>> = Mutex::new(None); +/// The newest thumbnail of each page and parent, with the document revision it shows. +static THUMBS: Mutex>> = Mutex::new(None); /// Widest parent thumbnail, in pixels (a spread of many pages stays a small texture). const MAX_THUMB_W: f64 = 1024.0; +/// Seconds the document stays unchanged before thumbnails of an older revision are drawn again. +const THUMB_QUIET_SECS: f64 = 0.4; + +/// Thumbnails of an older revision drawn again in one frame. +const THUMB_REFRESHES_PER_FRAME: usize = 1; + +/// When the active document's revision last changed, and how many thumbnails were drawn again in +/// the current frame. +#[derive(Clone, Copy, Debug, Default)] +struct ThumbClock { + doc: (u64, u64), + changed_at: f64, + frame: u64, + refreshed: usize, +} + +#[cfg(test)] +thread_local! { + /// Thumbnails rendered on this thread. + static THUMB_RENDERS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// The thumbnail of `of`, `h` points high. A thumbnail of an older revision stays on screen until +/// the document has been unchanged for [`THUMB_QUIET_SECS`]; after that, at most +/// [`THUMB_REFRESHES_PER_FRAME`] of them are drawn again per frame. fn thumb(app: &mut DesignApp, ctx: &egui::Context, of: ThumbOf, h: f32) -> Option { let st = app.session.active()?; - let key = (st.uid, st.revision, of, h.round() as u32); + let key = (st.uid, of, h.round() as u32); + let clock_id = egui::Id::unique("pages_thumb_clock"); + let (now, frame) = (ctx.input(|i| i.time), ctx.cumulative_frame_nr()); + let mut clock = ctx.data(|d| d.get_temp::(clock_id)).unwrap_or_default(); + let changed = clock.doc != (st.uid, st.revision); + if changed { + clock = ThumbClock { doc: (st.uid, st.revision), changed_at: now, frame, refreshed: 0 }; + } + if clock.frame != frame { + clock.frame = frame; + clock.refreshed = 0; + } + if now < clock.changed_at { + clock.changed_at = now; + } let mut g = THUMBS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); let map = g.get_or_insert_with(HashMap::new); - if let Some(t) = map.get(&key) { - return Some(t.clone()); + if changed { + // Drop the thumbnails of pages and parents the document no longer has. + let pages = st.doc.page_count(); + map.retain(|k, _| { + k.0 != st.uid + || match k.1 { + ThumbOf::Page(abs) => abs < pages, + ThumbOf::Parent(id) => st.doc.parent_index(id).is_some(), + } + }); } + if let Some((rev, t)) = map.get(&key) { + if *rev == st.revision { + return Some(t.clone()); + } + let wait = THUMB_QUIET_SECS - (now - clock.changed_at); + if wait > 0.0 || clock.refreshed >= THUMB_REFRESHES_PER_FRAME { + ctx.request_repaint_after(std::time::Duration::from_secs_f64(wait.max(0.0))); + ctx.data_mut(|d| d.insert_temp(clock_id, clock)); + return Some(t.clone()); + } + clock.refreshed += 1; + } + ctx.data_mut(|d| d.insert_temp(clock_id, clock)); + #[cfg(test)] + THUMB_RENDERS.with(|n| n.set(n.get() + 1)); let px = h as f64 * ctx.pixels_per_point() as f64; let mut r = designcraft_render::Renderer::new(); r.threads = 0; @@ -59,10 +122,7 @@ fn thumb(app: &mut DesignApp, ctx: &egui::Context, of: ThumbOf, h: f32) -> Optio }; let ci = egui::ColorImage::from_rgba_premultiplied([img.width as usize, img.height as usize], &img.pixels); let tex = crate::widgets::load_texture(ctx, format!("{name}_{}", h.round()), ci, egui::TextureOptions::LINEAR); - // Drop thumbnails of older revisions. - let (uid, rev) = (st.uid, st.revision); - map.retain(|k, _| !(k.0 == uid && k.1 != rev)); - map.insert(key, tex.clone()); + map.insert(key, (st.revision, tex.clone())); Some(tex) } @@ -268,7 +328,7 @@ impl ParentRow { /// `thumbnails`), or blank pages for [None] and when the spread can't be rendered. fn parent_icon(app: &mut DesignApp, ui: &egui::Ui, right: egui::Pos2, row: &ParentRow, h: f32, thumbnails: bool, t: &Tokens) { let area = egui::Rect::from_min_size(egui::pos2(right.x - row.unit_w * h, right.y - h / 2.0), vec2(row.unit_w * h, h)); - let tex = if thumbnails { row.parent.and_then(|(_, id)| thumb(app, ui.ctx(), ThumbOf::Parent(id), h)) } else { None }; + let tex = if thumbnails && ui.is_rect_visible(area) { row.parent.and_then(|(_, id)| thumb(app, ui.ctx(), ThumbOf::Parent(id), h)) } else { None }; let p = ui.painter(); match tex { Some(tex) => { @@ -488,7 +548,7 @@ pub fn show(app: &mut DesignApp, ui: &mut egui::Ui) { egui::WidgetInfo::selected(egui::Role::Button, true, selected, format!("Page {}", app.session.page_label(abs))) }); let tint = if selected { SELECTED_TINT } else { Color32::WHITE }; - match thumbnails.then(|| thumb(app, ui.ctx(), ThumbOf::Page(abs), th)).flatten() { + match (thumbnails && ui.is_rect_visible(pr)).then(|| thumb(app, ui.ctx(), ThumbOf::Page(abs), th)).flatten() { Some(tex) => { ui.painter().image(tex.id(), pr, egui::Rect::from_min_max(egui::pos2(0.0, 0.0), egui::pos2(1.0, 1.0)), tint); } diff --git a/crates/ui-egui/src/panels/pages/tests.rs b/crates/ui-egui/src/panels/pages/tests.rs index 7e97a262d..62644227f 100644 --- a/crates/ui-egui/src/panels/pages/tests.rs +++ b/crates/ui-egui/src/panels/pages/tests.rs @@ -61,11 +61,11 @@ fn double_clicking_a_parent_shows_that_parent() { for name in names { assert_eq!(h.get_by_label(name).accesskit_node().toggled() == Some(egui::accesskit::Toggled::True), name == "C-Parent", "{name}"); } - // Parent rows show a rendered thumbnail. + // Parent rows show a rendered thumbnail (of the revision before the double click until the + // document has been unchanged for THUMB_QUIET_SECS). let id = st.doc.parents[2].id; - let rev = (st.uid, st.revision); - let thumbs = super::THUMBS.lock().unwrap(); - assert!(thumbs.as_ref().unwrap().keys().any(|k| (k.0, k.1) == rev && k.2 == super::ThumbOf::Parent(id))); + let thumbs = super::THUMBS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + assert!(thumbs.as_ref().unwrap().keys().any(|k| k.0 == st.uid && k.1 == super::ThumbOf::Parent(id))); } fn harness(app: DesignApp) -> Harness<'static, DesignApp> { @@ -321,12 +321,58 @@ fn parent_thumbnails_follow_panel_options() { let id = st.doc.parents[0].id; let thumbs = super::THUMBS.lock().unwrap(); let keys = thumbs.as_ref().map(|m| m.keys().copied().collect::>()).unwrap_or_default(); - keys.into_iter().filter(|k| k.0 == st.uid && k.2 == super::ThumbOf::Parent(id)).map(|k| k.3).collect::>() + keys.into_iter().filter(|k| k.0 == st.uid && k.1 == super::ThumbOf::Parent(id)).map(|k| k.2).collect::>() }; assert_eq!(parent_thumb_heights(true), vec![IconSize::Large.row_h() as u32 - 5]); assert_eq!(parent_thumb_heights(false), Vec::::new()); } +/// Page thumbnails are rendered only for rows in view. After an edit the previous thumbnails stay +/// until the document has been unchanged for `THUMB_QUIET_SECS`, and then one is drawn again per +/// frame. Thumbnails of deleted pages are dropped. +#[test] +fn an_edit_redraws_only_thumbnails_in_view_one_per_frame() { + let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"pages": 60, "facingPages": false})).unwrap(); + let ctx = egui::Context::default(); + let frame = |app: &mut DesignApp, time: f64| { + let before = THUMB_RENDERS.with(std::cell::Cell::get); + let raw = egui::RawInput { + screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, vec2(280.0, 520.0))), + time: Some(time), + ..Default::default() + }; + ctx.run_ui(raw, |ui| show(app, ui)).textures_delta.clear(); + THUMB_RENDERS.with(std::cell::Cell::get) - before + }; + let shown: usize = (0..3).map(|_| frame(&mut app, 0.0)).sum(); + assert!(shown > 0 && shown < 60, "only the rows in view are rendered: {shown}"); + assert_eq!(frame(&mut app, 0.0), 0, "an unchanged document renders nothing"); + + app.run("frame.create", json!({"rect": [36, 36, 200, 200]})).unwrap(); + assert_eq!(frame(&mut app, 1.0), 0, "the edit's frame keeps the previous thumbnails"); + assert_eq!(frame(&mut app, 1.0 + THUMB_QUIET_SECS / 2.0), 0, "still editing"); + let quiet = 1.0 + THUMB_QUIET_SECS + 0.01; + let mut redrawn = 0; + for _ in 0..shown { + let n = frame(&mut app, quiet); + assert_eq!(n, THUMB_REFRESHES_PER_FRAME, "one thumbnail per frame"); + redrawn += n; + } + assert_eq!(redrawn, shown); + assert_eq!(frame(&mut app, quiet), 0, "every thumbnail in view is current"); + + // Deleted pages lose their thumbnails. + let uid = app.session.active().unwrap().uid; + let cached = |abs: usize| { + THUMBS.lock().unwrap_or_else(std::sync::PoisonError::into_inner).as_ref().unwrap().keys().any(|k| k.0 == uid && k.1 == ThumbOf::Page(abs)) + }; + assert!(cached(2)); + app.run("layout.pages.delete", json!({"pages": (2..60).collect::>()})).unwrap(); + frame(&mut app, quiet + 1.0); + assert!((2..60).all(|abs| !cached(abs)), "no thumbnail of a deleted page is kept"); +} + #[test] fn parents_are_added_duplicated_and_renamed_from_the_panel() { let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());