collective
Browse / github.com/storytold/deckcraft / patch
Slide show: render the Presenter View's next-slide preview off the UI thread
Slide show: render the Presenter View's next-slide preview off the UI thread ## What this fixes With Presenter View on, every slide change re-rendered the next-slide preview **synchronously and single-threaded on the UI thread**. Turning the page waited for it. Measured on the sample deck at 1920×1080 (release build, kittest harness, per-frame time after PageDown): | | first frames after a slide change | |---|---| | Presenter View off | 5–36 ms | | Presenter View on (before) | 35–80 ms | Refs #964 (that issue already names this preview as a likely cause of the sluggishness). ## Change (`crates/ui-egui/src/show/presenter.rs`, fields in `show.rs`) - `presenter_wind
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Naseq (imported from pull request #1371, open)
- Base
ee875abffa1a31cb760c843bb29e79391a90a766- Size
- +179 −11 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
c5c11c74a6004182b722f0d2
How it went
No other operator has reported using it yet.
Diff
raw · crates/ui-egui/src/show.rs crates/ui-egui/src/show/presenter.rs crates/ui-egui/src/show/tests.rs
@@ -52,7 +52,13 @@ pub struct Show { tex: Option<(Key, TextureHandle)>, /// The shapes stacked above the playing videos, drawn over the video frames (#1005). overlay: Option<(Key, Vec<ShapeId>, TextureHandle)>,- next_tex: Option<(usize, TextureHandle)>,+ /// The Presenter View's next-slide preview: the presentation and slide it shows.+ next_tex: Option<(std::sync::Arc<Presentation>, usize, TextureHandle)>,+ /// The one next-slide preview being rendered on another thread (#964).+ next_job: Option<(std::sync::Arc<Presentation>, usize, std::sync::mpsc::Receiver<egui::ColorImage>)>,+ /// Preview renders started on another thread.+ #[cfg(test)]+ preview_spawns: usize, /// The next slide's first frame, rendered in the background while this one is shown so /// advancing only uploads it (#240). prefetch: Option<Prefetch>,@@ -214,6 +220,9 @@ impl Show { tex: None, overlay: None, next_tex: None,+ next_job: None,+ #[cfg(test)]+ preview_spawns: 0, prefetch: None, backdrop: None, blank: None,@@ -75,27 +75,67 @@ pub(super) fn wall_clock() -> String { } } +/// The Presenter View's preview of slide `n`, `size` pixels.+fn render_preview(doc: &Presentation, n: usize, size: (u32, u32), threads: u16) -> egui::ColorImage {+ let opts = RenderOpts { scale: size.0 as f64 / doc.slide_size.width.max(1.0), size: Some(size), threads, ..Default::default() };+ crate::textures::to_color_image(&deckcraft_render::render_slide(doc, n, &opts), false)+}+ /// Render the presenter window and set `keep` false when the user ends or closes the show.-pub(super) fn presenter_window(ctx: &egui::Context, show: &mut Show, doc: &Presentation, now: f64, keep: &mut bool) {+pub(super) fn presenter_window(ctx: &egui::Context, show: &mut Show, doc: &std::sync::Arc<Presentation>, now: f64, keep: &mut bool) { let cur = show.state.slide; let next = show.state.peek_next(doc); let cur_tex = show.tex.as_ref().map(|(_, t)| t.clone());+ // The preview wanted: slide `n` of this presentation (an edit replaces it).+ let is = |d: &std::sync::Arc<Presentation>, k: usize, n: usize| std::sync::Arc::ptr_eq(d, doc) && k == n; if let Some(n) = next- && show.next_tex.as_ref().is_none_or(|(k, _)| *k != n)+ && show.next_tex.as_ref().is_none_or(|(d, k, _)| !is(d, *k, n)) { // A very tall slide shape would otherwise ask for a picture taller than the GPU's limit. let next_size = crate::textures::fit_size( (480, (480.0 * doc.slide_size.height / doc.slide_size.width.max(1.0)).clamp(1.0, 1e9) as u32), crate::textures::max_side(ctx), );- let img = deckcraft_render::render_slide(- doc,- n,- &RenderOpts { scale: next_size.0 as f64 / doc.slide_size.width.max(1.0), size: Some(next_size), ..Default::default() },- );- show.next_tex = Some((n, crate::textures::upload(ctx, "presenter-next", crate::textures::to_color_image(&img, false))));+ // Rendered on another thread, so turning the page doesn't wait for the preview (#964). One+ // render at a time: paging faster than it renders asks for the newest preview once it is+ // done, and a finished one no longer wanted is dropped.+ if let Some((d, k, rx)) = show.next_job.take() {+ match rx.try_recv() {+ Ok(ci) if is(&d, k, n) => show.next_tex = Some((d, k, crate::textures::upload(ctx, "presenter-next", ci))),+ Ok(_) => {}+ Err(std::sync::mpsc::TryRecvError::Empty) => show.next_job = Some((d, k, rx)),+ // The worker died: render this one here, as before, rather than start another.+ Err(std::sync::mpsc::TryRecvError::Disconnected) => {+ show.next_tex = Some((doc.clone(), n, crate::textures::upload(ctx, "presenter-next", render_preview(doc, n, next_size, 0))));+ }+ }+ }+ if show.next_job.is_none() && show.next_tex.as_ref().is_none_or(|(d, k, _)| !is(d, *k, n)) {+ let (tx, rx) = std::sync::mpsc::channel();+ let worker_doc = doc.clone();+ // The web has no threads: render it here.+ let spawned = !cfg!(target_arch = "wasm32")+ && std::thread::Builder::new()+ .name("show-next-preview".into())+ .spawn(move || {+ let _ = tx.send(render_preview(&worker_doc, n, next_size, 1));+ })+ .is_ok();+ if spawned {+ #[cfg(test)]+ {+ show.preview_spawns += 1;+ }+ show.next_job = Some((doc.clone(), n, rx));+ } else {+ show.next_tex = Some((doc.clone(), n, crate::textures::upload(ctx, "presenter-next", render_preview(doc, n, next_size, 0))));+ }+ }+ if show.next_job.is_some() {+ ctx.request_repaint_after(std::time::Duration::from_millis(16));+ } }- let next_tex = show.next_tex.as_ref().filter(|(k, _)| Some(*k) == next).map(|(_, t)| t.clone());+ let next_tex = show.next_tex.as_ref().filter(|(d, k, _)| next.is_some_and(|n| is(d, *k, n))).map(|(_, _, t)| t.clone()); let no_notes = deckcraft_model::text::TextBody::default(); let notes = doc.slides.get(cur).map_or(&no_notes, |s| &s.notes); let clock = wall_clock();@@ -212,7 +252,10 @@ pub(super) fn presenter_window(ctx: &egui::Context, show: &mut Show, doc: &Prese } None => { ui.painter().rect_filled(nr, CornerRadius::ZERO, Color32::from_gray(40));- ui.painter().text(nr.center(), Align2::CENTER_CENTER, "End of slide show", theme::font(13.0), Color32::from_gray(170));+ // A next slide whose preview is still on its way is not the end.+ if next.is_none() {+ ui.painter().text(nr.center(), Align2::CENTER_CENTER, "End of slide show", theme::font(13.0), Color32::from_gray(170));+ } } } let notes_head = Rect::from_min_size(pos2(right.min.x, nr.max.y + 12.0), vec2(right.width(), 28.0));@@ -993,3 +993,119 @@ fn input_state_tracks_the_modes_that_move_show_click_targets() { show.blank = Some(Color32::BLACK); assert_ne!(show.input_state(), base, "a blank screen has no slide click targets"); }++/// #964: the next-slide preview renders on another thread; the presenter frame that asks for+/// it doesn't wait, and the preview arrives on a later frame.+#[test]+fn the_presenter_next_preview_arrives_without_blocking_the_frame() {+ let mut session = deckcraft_engine::Session::with_new();+ session.execute("slide.new", &json!({})).unwrap();+ let doc = session.active().unwrap().doc.clone();+ // One frame on its own (a harness runs frames until the preview lands): it asks for the+ // preview and returns without it.+ let mut show = Show::new(&doc, 0, false, true);+ let mut keep = true;+ let ctx = egui::Context::default();+ let _ = ctx.run_ui(egui::RawInput::default(), |ui| presenter_window(ui.ctx(), &mut show, &doc, 0.0, &mut keep));+ assert!(show.next_job.is_some() && show.next_tex.is_none(), "the first frame leaves the preview pending");+ assert_eq!(show.preview_spawns, 1);+ let mut harness = egui_kittest::Harness::builder().with_size(vec2(1100.0, 700.0)).build_ui_state(+ |ui, (doc, show, keep): &mut (std::sync::Arc<Presentation>, Show, bool)| {+ let now = ui.input(|i| i.time);+ presenter_window(ui.ctx(), show, doc, now, keep);+ },+ (doc, show, true),+ );+ for _ in 0..200 {+ if harness.state().1.next_tex.is_some() {+ break;+ }+ std::thread::sleep(std::time::Duration::from_millis(5));+ harness.step();+ }+ assert_eq!(harness.state().1.next_tex.as_ref().map(|(_, n, _)| *n), Some(1));+ assert!(harness.state().1.next_job.is_none());+ assert_eq!(harness.state().1.preview_spawns, 1, "the pending render landed; none started over");+}++/// #964: paging quickly keeps one preview render running, and the preview that lands is the+/// newest one asked for.+#[test]+fn paging_quickly_renders_one_preview_at_a_time_and_lands_on_the_newest() {+ let mut session = deckcraft_engine::Session::with_new();+ for _ in 0..7 {+ session.execute("slide.new", &json!({})).unwrap();+ }+ let doc = session.active().unwrap().doc.clone();+ let show = Show::new(&doc, 0, false, true);+ let mut harness = egui_kittest::Harness::builder().with_size(vec2(1100.0, 700.0)).build_ui_state(+ |ui, (doc, show, keep): &mut (std::sync::Arc<Presentation>, Show, bool)| {+ let now = ui.input(|i| i.time);+ presenter_window(ui.ctx(), show, doc, now, keep);+ },+ (doc, show, true),+ );+ let mut turns = 0;+ for _ in 0..6 {+ let doc = harness.state().0.clone();+ let a = harness.state_mut().1.state.next(&doc);+ harness.state_mut().1.apply(a, &doc, 0.0);+ harness.step();+ turns += 1;+ }+ let want = harness.state().1.state.peek_next(&harness.state().0);+ for _ in 0..400 {+ if harness.state().1.next_tex.as_ref().map(|(_, k, _)| Some(*k)) == Some(want) {+ break;+ }+ std::thread::sleep(std::time::Duration::from_millis(5));+ harness.step();+ }+ let st = &harness.state().1;+ assert_eq!(st.next_tex.as_ref().map(|(_, k, _)| *k), want, "the newest preview lands");+ assert!(st.next_job.is_none());+ // A render starts only once the one before it has finished: never more than one per turn.+ assert!(st.preview_spawns <= turns + 1, "{} renders for {turns} turns", st.preview_spawns);++ // While a render is pending, turning the page starts no other.+ let (_tx, rx) = std::sync::mpsc::channel();+ let doc = harness.state().0.clone();+ let spawns = harness.state().1.preview_spawns;+ harness.state_mut().1.next_tex = None;+ harness.state_mut().1.next_job = Some((doc.clone(), 0, rx));+ let a = harness.state_mut().1.state.prev(&doc);+ harness.state_mut().1.apply(a, &doc, 0.0);+ harness.step();+ let st = &harness.state().1;+ assert_eq!(st.preview_spawns, spawns, "no render starts while one is pending");+ assert!(st.next_job.is_some());+}++/// A preview rendered from a presentation that has since been edited (replaced) is dropped,+/// and the preview is rendered again from the new one.+#[test]+fn a_preview_of_an_edited_presentation_is_rendered_again() {+ let mut session = deckcraft_engine::Session::with_new();+ session.execute("slide.new", &json!({})).unwrap();+ let doc = session.active().unwrap().doc.clone();+ let show = Show::new(&doc, 0, false, true);+ let mut harness = egui_kittest::Harness::builder().with_size(vec2(1100.0, 700.0)).build_ui_state(+ |ui, (doc, show, keep): &mut (std::sync::Arc<Presentation>, Show, bool)| {+ let now = ui.input(|i| i.time);+ presenter_window(ui.ctx(), show, doc, now, keep);+ },+ (doc, show, true),+ );+ harness.step();+ let edited = std::sync::Arc::new((*harness.state().0).clone());+ harness.state_mut().0 = edited.clone();+ for _ in 0..400 {+ if harness.state().1.next_tex.as_ref().is_some_and(|(d, _, _)| std::sync::Arc::ptr_eq(d, &edited)) {+ break;+ }+ std::thread::sleep(std::time::Duration::from_millis(5));+ harness.step();+ }+ let tex = harness.state().1.next_tex.as_ref().map(|(d, k, _)| (std::sync::Arc::ptr_eq(d, &edited), *k));+ assert_eq!(tex, Some((true, 1)), "the preview comes from the edited presentation");+}