From d247b98b95634a82614413496d715de8f8e97384 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 23:29:44 +0000 Subject: [PATCH] Viewer: dragging a layer no longer piles up frame renders until memory runs out (#647) Every viewer request spawns a job on the frame pool. A pool thread blocked inside a render (waiting on a parallel loop's join) runs other pool work meanwhile, rayon's injected jobs included: the spawn of the next request started a whole frame render on top of the waiting one. Dragging makes a new revision, and a new viewer frame, every UI frame, so renders nested without bound on every thread, each holding its buffers (a large image's full-resolution copies), until all memory was used. Holding the layer still makes no new revisions, which is why only moving it did this. A spawn now drains the queue, and one that lands inside a render on its thread returns at once; the render below takes the queued jobs when it is done. Each pool thread renders one frame at a time. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QiAJeFttZp83Fh7YuzAR5f --- crates/ui-egui/src/frames.rs | 115 +++++++++++++++++++++++++++++++++-- 1 file changed, 111 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 33fd676d..ccffeb9f 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -768,10 +768,8 @@ impl Frames { #[cfg(not(target_arch = "wasm32"))] if self.remote.is_none() { let w = self.worker(); - // One spawn per job; each spawn renders whichever queued job matters most right now. - let job = move || { - w.run_next(); - }; + // One spawn per job; each spawn renders whichever queued jobs matter most right now. + let job = move || w.drain(); match &self.pool { Some(pool) => pool.spawn(job), None => rayon::spawn(job), @@ -924,7 +922,42 @@ struct Worker { gpu_retired: Arc, } +#[cfg(not(target_arch = "wasm32"))] +thread_local! { + /// This pool thread is inside [`Worker::drain`] (rendering a frame). + static DRAINING: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +/// Clears [`DRAINING`] when the drain ends, even by a panic. +#[cfg(not(target_arch = "wasm32"))] +struct Draining; + +#[cfg(not(target_arch = "wasm32"))] +impl Drop for Draining { + fn drop(&mut self) { + DRAINING.set(false); + } +} + impl Worker { + /// Render queued jobs, most important first, until the queue is empty. + /// + /// A pool thread waiting inside a render (on a parallel loop's join) runs other pool work + /// meanwhile, including the spawns of newer requests: each would start a whole frame render + /// on top of the one waiting, with all of its buffers. Dragging a layer asks for a new frame + /// every UI frame, so renders piled up on every thread until memory ran out (#647). A spawn + /// that lands inside a render returns at once instead; the render below it takes the queued + /// jobs when it is done, so each thread renders one frame at a time. + #[cfg(not(target_arch = "wasm32"))] + fn drain(&self) { + if DRAINING.get() { + return; + } + DRAINING.set(true); + let _reset = Draining; + while self.run_next() {} + } + /// Render the queued job that matters most right now. `false` when the queue was empty. fn run_next(&self) -> bool { let Worker { queue, content_keys, .. } = self; @@ -1234,6 +1267,80 @@ mod tests { assert!(!f.is_cached(&key)); } + /// #647: a pool thread waiting inside a render (on a parallel loop) never starts another + /// frame render on top of it. Dragging a layer queues a frame every UI frame; renders used to + /// nest on every thread, each holding its buffers, until memory ran out. + #[test] + fn a_waiting_render_never_starts_another_on_its_thread() { + use effectcraft_engine::Session; + use effectcraft_engine::project::{Footage, FootageKind}; + use effectcraft_engine::render::Image; + use effectcraft_engine::time::FrameRate; + use serde_json::json; + use std::cell::Cell; + use std::sync::atomic::AtomicUsize; + thread_local! { + static DEPTH: Cell = const { Cell::new(0) }; + } + /// A decoder that works in parallel, as the renderer does: its thread joins the loop and + /// may wait for the parts other threads took. + #[derive(Default)] + struct Parallel { + calls: AtomicUsize, + deepest: AtomicUsize, + } + impl FootageSource for Parallel { + fn frame(&self, _: ItemId, _: &Footage, _: Tick) -> Option> { + let d = DEPTH.get() + 1; + DEPTH.set(d); + self.deepest.fetch_max(d, Ordering::SeqCst); + self.calls.fetch_add(1, Ordering::SeqCst); + (0..64).into_par_iter().for_each(|_| std::thread::sleep(std::time::Duration::from_millis(1))); + DEPTH.set(d - 1); + Some(Arc::new(Image::new(4, 4))) + } + } + let mut s = Session::default(); + let cid = ItemId(s.execute("comp.new", json!({"name": "A", "width": 8, "height": 8, "duration": 10})).unwrap()["comp"].as_u64().unwrap()); + let clip = Footage { + kind: FootageKind::Video, + width: 4, + height: 4, + has_video: true, + frame_rate: FrameRate::FPS_30, + duration: Tick::from_seconds_f64(10.0), + ..Default::default() + }; + let item = Arc::make_mut(&mut s.project).add_item("clip.mov", Default::default(), None, ItemKind::Footage(clip)); + s.execute("layer.addItem", json!({"item": item.0})).unwrap(); + let footage = Arc::new(Parallel::default()); + let src = RenderSource { + project: s.project.clone(), + footage: footage.clone(), + expr: None, + layer_cache: Arc::default(), + gpu: None, + gpu_display: false, + disk: None, + }; + let f = Frames::default(); + let content = f.content_of(&s.project, s.revision, cid); + let frames = 60; + for frame in 0..frames { + let key = FrameKey { revision: s.revision, content, comp: cid.0, frame, scale: 1000, view: 0, opts: 0 }; + f.request(&src, key, cid, FrameRate::FPS_30.tick_of(frame), RenderOpts::default()); + } + for _ in 0..4000 { + if f.inflight() == 0 { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert_eq!(f.inflight(), 0, "every queued frame rendered"); + assert!(footage.calls.load(Ordering::SeqCst) >= frames as usize); + assert_eq!(footage.deepest.load(Ordering::SeqCst), 1, "renders nested on a waiting thread"); + } + /// #284: scrubbing asks for a viewer frame at every step; a newer one drops the comp's viewer /// frames still waiting (they would all render first), but not prefetch frames or another /// comp's viewer frame.