Merge pull request #3047 from MarekSpodBiedry/issue-3028

Preview: render at most two frames on the GPU at once so video memory stays flat (#3028)

crates/ui-egui/src/frames.rs

@@ -33,7 +33,7 @@
 
 use std::collections::{BTreeSet, HashMap, HashSet, VecDeque};
 use std::sync::atomic::{AtomicBool, Ordering};
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, Condvar, Mutex, PoisonError};
 
 use effectcraft_engine::project::{Comp, Heard, ItemId, ItemKind, Node, Project, PropGroup};
 use effectcraft_engine::render::disk_cache::{self, DiskCache};
@@ -54,6 +54,84 @@ const GPU_BUDGET: usize = 1 << 30;
 const MIN_GPU_BUDGET: usize = 64 << 20;
 const GPU_PRESSURE_COOLDOWN: std::time::Duration = std::time::Duration::from_secs(5);
 const GPU_RECOVERY_INTERVAL: std::time::Duration = std::time::Duration::from_secs(30);
+/// GPU frames that render at once, however many frame threads there are. Each one holds its own
+/// working textures, so 16 at once ran an RTX 3090 out of video memory (#3028).
+const MAX_GPU_RENDERS: usize = 2;
+
+/// Counts the GPU renders in flight; the rest of the frame threads wait their turn.
+struct GpuRenderSlots {
+    #[cfg(test)]
+    entered: std::sync::atomic::AtomicUsize,
+    busy: Mutex<usize>,
+    freed: Condvar,
+}
+
+/// One GPU render slot, given back on drop.
+struct GpuRenderSlot<'a>(&'a GpuRenderSlots);
+
+static GPU_RENDER_SLOTS: GpuRenderSlots = GpuRenderSlots::new();
+
+impl GpuRenderSlots {
+    const fn new() -> Self {
+        Self {
+            #[cfg(test)]
+            entered: std::sync::atomic::AtomicUsize::new(0),
+            busy: Mutex::new(0),
+            freed: Condvar::new(),
+        }
+    }
+
+    /// The UI-thread display path must hand a frame to the remote worker instead of waiting
+    /// behind preview lanes. Never block the window to enforce the native memory cap.
+    fn try_acquire(&self, max: usize) -> Option<GpuRenderSlot<'_>> {
+        let mut busy = match self.busy.try_lock() {
+            Ok(busy) => busy,
+            Err(std::sync::TryLockError::Poisoned(error)) => error.into_inner(),
+            Err(std::sync::TryLockError::WouldBlock) => return None,
+        };
+        if !cfg!(target_arch = "wasm32") && *busy >= max {
+            return None;
+        }
+        *busy += 1;
+        #[cfg(test)]
+        self.entered.fetch_add(1, Ordering::SeqCst);
+        Some(GpuRenderSlot(self))
+    }
+
+    /// A Rayon helper may be executing a nested frame while an admitted render waits for
+    /// its work. Blocking that helper on a permit would deadlock the render (#3028).
+    fn for_render(&self, max: usize) -> Option<GpuRenderSlot<'_>> {
+        // Dedicated preview pools have one thread, protected from nested drains. They cannot
+        // be helpers of another admitted frame, so waiting preserves GPU preview throughput.
+        #[cfg(not(target_arch = "wasm32"))]
+        if DRAINING_LANE.get() {
+            return Some(self.acquire(max));
+        }
+        if rayon::current_thread_index().is_some() { self.try_acquire(max) } else { Some(self.acquire(max)) }
+    }
+
+    /// Wait until fewer than `max` renders are in flight, then take a slot. The browser has no
+    /// threads to wait on: it never blocks.
+    fn acquire(&self, max: usize) -> GpuRenderSlot<'_> {
+        let mut busy = self.busy.lock().unwrap_or_else(PoisonError::into_inner);
+        while !cfg!(target_arch = "wasm32") && *busy >= max {
+            busy = self.freed.wait(busy).unwrap_or_else(PoisonError::into_inner);
+        }
+        *busy += 1;
+        #[cfg(test)]
+        self.entered.fetch_add(1, Ordering::SeqCst);
+        GpuRenderSlot(self)
+    }
+}
+
+impl Drop for GpuRenderSlot<'_> {
+    fn drop(&mut self) {
+        let mut busy = self.0.busy.lock().unwrap_or_else(PoisonError::into_inner);
+        *busy = busy.saturating_sub(1);
+        self.0.freed.notify_one();
+    }
+}
+
 /// The window's frame gaps that adapt [`Queue::gpu_limit`] while a preview plays: a longer one
 /// is a stall, shorter ones for a while are smooth.
 const UI_STALL: std::time::Duration = std::time::Duration::from_millis(100);
@@ -1614,15 +1692,29 @@ struct Worker {
 thread_local! {
     /// This pool thread is inside [`Worker::drain`] (rendering a frame).
     static DRAINING: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
+    /// Only the single thread of a dedicated preview lane may wait for GPU admission.
+    static DRAINING_LANE: std::cell::Cell<bool> = 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 Draining {
+    fn enter(lane: Option<usize>) -> Option<Self> {
+        if DRAINING.replace(true) {
+            return None;
+        }
+        DRAINING_LANE.set(lane.is_some());
+        Some(Self)
+    }
+}
+
 #[cfg(not(target_arch = "wasm32"))]
 impl Drop for Draining {
     fn drop(&mut self) {
+        DRAINING_LANE.set(false);
         DRAINING.set(false);
     }
 }
@@ -1651,11 +1743,7 @@ impl Worker {
     /// `lane`: the lane this runs on (see [`Queue::idle_lanes`]), `None` on the pool.
     #[cfg(not(target_arch = "wasm32"))]
     fn drain(&self, lane: Option<usize>) {
-        if DRAINING.get() {
-            return;
-        }
-        DRAINING.set(true);
-        let _reset = Draining;
+        let Some(_reset) = Draining::enter(lane) else { return };
         while self.run_next(lane) {}
     }
 
@@ -1852,7 +1940,10 @@ impl Worker {
         r.reads = Some(&files);
         r.accel = Some(g as &dyn effectcraft_engine::render::Accelerator);
         r.active_accel()?;
-        let f = g.within_memory(|| g.render_display(&r, job.comp, job.t));
+        let f = {
+            let _slot = GPU_RENDER_SLOTS.try_acquire(MAX_GPU_RENDERS)?;
+            g.within_memory(|| g.render_display(&r, job.comp, job.t))
+        };
         effectcraft_engine::render::Accelerator::frame_end(g);
         match f {
             Ok(f) => {
@@ -1893,7 +1984,7 @@ impl Worker {
             let auto = if cfg!(target_arch = "wasm32") { None } else { r.frame_auto(a, job.comp, true) };
             if auto.is_none_or(|(p, key)| p.choose(key)) {
                 let t0 = web_time::Instant::now();
-                match g.within_memory(|| g.render_display(&r, job.comp, job.t)) {
+                match gpu_attempt(g, || g.render_display(&r, job.comp, job.t)) {
                     Ok(Some(f)) => {
                         self.gpu_succeeded();
                         if let Some((p, key)) = auto {
@@ -1912,13 +2003,21 @@ impl Worker {
                         }
                     }
                     Err(e) => {
-                        self.gpu_attempt_failed(g, &e);
+                        self.gpu_attempt_rejected(g, &e);
                         return self.render_job(job, false, files);
                     }
                 }
             }
             let t0 = web_time::Instant::now();
-            let img = cpu_frame(&r.comp_frame_cpu(job.comp, job.t), &job.src.project, &job.opts);
+            // CPU compositing in Auto can still run GPU layer effects. Keep those inside the
+            // same admission and error scope as a GPU-composited frame (#3028).
+            let img = match gpu_attempt(g, || r.comp_frame_cpu(job.comp, job.t)) {
+                Ok(img) => cpu_frame(&img, &job.src.project, &job.opts),
+                Err(e) => {
+                    self.gpu_attempt_rejected(g, &e);
+                    return self.render_job(job, false, files);
+                }
+            };
             if let Some((p, key)) = auto {
                 p.record(key, false, t0.elapsed().as_secs_f64() * 1000.0);
             }
@@ -1926,19 +2025,37 @@ impl Worker {
         }
         if let Some(g) = gpu {
             // The GPU composites and the frame is read back.
-            return match r.comp_frame_scoped(job.comp, job.t, |a| g.within_memory(|| a.comp_frame(&r, job.comp, job.t))) {
+            return match r.comp_frame_scoped(job.comp, job.t, |a| gpu_attempt(g, || a.comp_frame(&r, job.comp, job.t))) {
                 Ok(img) => {
                     self.gpu_succeeded();
                     cpu_frame(&img, &job.src.project, &job.opts)
                 }
                 Err(e) => {
-                    self.gpu_attempt_failed(g, &e);
+                    self.gpu_attempt_rejected(g, &e);
                     self.render_job(job, false, files)
                 }
             };
         }
         cpu_frame(&r.comp_frame(job.comp, job.t), &job.src.project, &job.opts)
     }
+    /// Busy shared-pool workers retry on the CPU without treating contention as a GPU fault.
+    fn gpu_attempt_rejected(&self, gpu: &Gpu, error: &GpuAttemptError) {
+        if let GpuAttemptError::Failed(error) = error {
+            self.gpu_attempt_failed(gpu, error);
+        }
+    }
+}
+
+enum GpuAttemptError {
+    Busy,
+    Failed(FrameError),
+}
+
+/// Bound all preview GPU attempts, including effects in an Auto CPU composite. The permit
+/// ends before the caller retries on the CPU, so failed frames cannot block GPU admissions.
+fn gpu_attempt<T>(gpu: &Gpu, render: impl FnOnce() -> T) -> Result<T, GpuAttemptError> {
+    let _slot = GPU_RENDER_SLOTS.for_render(MAX_GPU_RENDERS).ok_or(GpuAttemptError::Busy)?;
+    gpu.within_memory(render).map_err(GpuAttemptError::Failed)
 }
 
 /// Who asked for a frame (see [`Frames::request_with`]).

crates/ui-egui/src/frames/tests.rs

@@ -3123,3 +3123,246 @@ mod frame_files {
         assert_eq!(healed.hits(), (1, 0), "the rewritten entry is used");
     }
 }
+
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn gpu_renders_in_flight_stay_within_the_cap() {
+    use std::sync::atomic::AtomicUsize;
+    let slots = GpuRenderSlots::new();
+    let now = AtomicUsize::new(0);
+    let peak = AtomicUsize::new(0);
+    std::thread::scope(|s| {
+        for _ in 0..16 {
+            s.spawn(|| {
+                let _slot = slots.acquire(2);
+                let n = now.fetch_add(1, Ordering::SeqCst) + 1;
+                peak.fetch_max(n, Ordering::SeqCst);
+                std::thread::sleep(std::time::Duration::from_millis(5));
+                now.fetch_sub(1, Ordering::SeqCst);
+            });
+        }
+    });
+    assert!(peak.load(Ordering::SeqCst) <= 2);
+    assert_eq!(*slots.busy.lock().unwrap_or_else(PoisonError::into_inner), 0);
+}
+
+/// #3028: exercise the actual worker routes, not only the admission counter. The Auto CPU
+/// compositor still has a live accelerator for its layer effects.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn gpu_worker_routes_admit_display_readback_and_auto_effects() {
+    use effectcraft_engine::render::{Accelerator, Backend};
+    let Some(gpu) = Gpu::headless() else {
+        eprintln!("GPU worker route test skipped: no adapter");
+        return;
+    };
+    let (frames, mut src, key, _, _, _) = disk_fixture(&[]);
+    let mut session = effectcraft_engine::Session::default();
+    session.execute("comp.new", serde_json::json!({"width": 16, "height": 16, "duration": 1})).unwrap();
+    let comp = session.active_comp_id().unwrap();
+    let solid = session.execute("layer.newSolid", serde_json::json!({"color": "#ff0000"})).unwrap()["layer"].clone();
+    session.execute("effect.apply", serde_json::json!({"layers": [solid], "effect": "Gaussian Blur"})).unwrap();
+    src.project = session.project;
+    src.gpu = Some(gpu.clone());
+    src.gpu_display = true;
+    let mut job = Job {
+        key,
+        src,
+        comp,
+        t: Tick::ZERO,
+        opts: RenderOpts { backend: Backend::Gpu, ..Default::default() },
+        urgent: true,
+        background: false,
+        seq: 0,
+        cancel: Arc::default(),
+    };
+    let worker = frames.worker();
+    let admissions = || GPU_RENDER_SLOTS.entered.load(Ordering::SeqCst);
+    // This path is called on the UI thread by dispatch_remote: a full GPU must decline
+    // immediately so the frame can go to the remote worker and the window keeps painting.
+    let first = GPU_RENDER_SLOTS.acquire(MAX_GPU_RENDERS);
+    let second = GPU_RENDER_SLOTS.acquire(MAX_GPU_RENDERS);
+    let (tx, rx) = std::sync::mpsc::channel();
+    let declined = std::thread::scope(|scope| {
+        scope.spawn(|| tx.send(worker.render_gpu(&job).is_none()).unwrap());
+        let result = rx.recv_timeout(std::time::Duration::from_secs(2));
+        drop((first, second));
+        result
+    });
+    assert_eq!(declined, Ok(true), "UI display must not wait for GPU admission");
+    let before = admissions();
+    assert!(worker.render_gpu(&job).is_some(), "direct display path renders");
+    assert!(admissions() > before, "direct display bypassed GPU admission");
+
+    let before = admissions();
+    assert!(worker.render_job(&job, true, &ReadLog::new()).is_gpu());
+    assert!(admissions() > before, "worker display bypassed GPU admission");
+
+    job.src.gpu_display = false;
+    job.src.layer_cache.clear();
+    let before = admissions();
+    assert!(!worker.render_job(&job, true, &ReadLog::new()).is_gpu());
+    assert!(admissions() > before, "readback bypassed GPU admission");
+
+    job.src.gpu_display = true;
+    job.src.layer_cache.clear();
+    job.opts.backend = Backend::Auto;
+    let renderer = Renderer::new(&job.src.project, job.src.footage.as_ref(), job.opts);
+    let (pick, key) = renderer.frame_auto(&gpu, comp, true).unwrap();
+    pick.declined(key); // Select CPU compositing while retaining GPU-capable layer effects.
+    let before = admissions();
+    assert!(!worker.render_job(&job, true, &ReadLog::new()).is_gpu());
+    assert!(admissions() > before, "Auto CPU effects bypassed GPU admission");
+
+    let before = admissions();
+    assert!(!worker.render_job(&job, false, &ReadLog::new()).is_gpu());
+    assert_eq!(admissions(), before, "CPU retry must not hold a GPU slot");
+    assert_eq!(*GPU_RENDER_SLOTS.busy.lock().unwrap(), 0);
+
+    // Saturation in the shared Rayon pool is contention, not a GPU fault. It must finish
+    // through the fresh CPU route without admitting GPU effects or retiring the device.
+    let first = GPU_RENDER_SLOTS.acquire(MAX_GPU_RENDERS);
+    let second = GPU_RENDER_SLOTS.acquire(MAX_GPU_RENDERS);
+    let before = admissions();
+    let pool = rayon::ThreadPoolBuilder::new().num_threads(2).build().unwrap();
+    let (tx, rx) = std::sync::mpsc::channel();
+    let cpu = std::thread::scope(|scope| {
+        scope.spawn(|| pool.install(|| tx.send(!worker.render_job(&job, true, &ReadLog::new()).is_gpu()).unwrap()));
+        let result = rx.recv_timeout(std::time::Duration::from_secs(2));
+        drop((first, second));
+        result
+    });
+    assert_eq!(cpu, Ok(true), "busy pool worker must complete on the CPU");
+    assert_eq!(admissions(), before);
+    assert!(worker.gpu_ready());
+    assert!(gpu.context().check_health().is_ok());
+
+    // The real dedicated-lane drain waits, then caches a GPU frame instead of silently
+    // switching a healthy preview to CPU rendering while the other lanes are busy.
+    frames.queue.lock().unwrap().jobs.push(Job {
+        key: job.key,
+        src: job.src.clone(),
+        comp,
+        t: job.t,
+        opts: RenderOpts { backend: Backend::Gpu, ..job.opts },
+        urgent: false,
+        background: true,
+        seq: 1,
+        cancel: Arc::default(),
+    });
+    let first = GPU_RENDER_SLOTS.acquire(MAX_GPU_RENDERS);
+    let second = GPU_RENDER_SLOTS.acquire(MAX_GPU_RENDERS);
+    let pool = rayon::ThreadPoolBuilder::new().num_threads(1).build().unwrap();
+    let (started, ready) = std::sync::mpsc::channel();
+    let (tx, rx) = std::sync::mpsc::channel();
+    let (early, finished) = std::thread::scope(|scope| {
+        scope.spawn(|| {
+            pool.install(|| {
+                started.send(()).unwrap();
+                worker.drain(Some(0));
+                tx.send(()).unwrap();
+            })
+        });
+        ready.recv_timeout(std::time::Duration::from_secs(2)).unwrap();
+        let early = rx.recv_timeout(std::time::Duration::from_millis(100));
+        drop((first, second));
+        let finished = early.or_else(|_| rx.recv_timeout(std::time::Duration::from_secs(2)));
+        (early, finished)
+    });
+    assert!(early.is_err(), "preview lane must wait for an available GPU permit");
+    assert_eq!(finished, Ok(()));
+    assert!(frames.cache.lock().unwrap().map.get(&job.key).unwrap().img.is_gpu());
+
+    // A retirement found inside the scope releases the permit before the fresh CPU retry.
+    gpu.context().device_failure_notifier()("synthetic worker retirement".into());
+    let before = admissions();
+    assert!(!worker.render_job(&job, true, &ReadLog::new()).is_gpu());
+    assert_eq!(admissions(), before + 1, "failed GPU attempt must retry without a second admission");
+    assert_eq!(*GPU_RENDER_SLOTS.busy.lock().unwrap(), 0);
+    gpu.frame_end();
+}
+
+/// Early returns and unwinding release admission even before another render starts.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn gpu_render_slots_return_after_failure_and_unwind() {
+    let slots = GpuRenderSlots::new();
+    let failed = || -> Result<(), ()> {
+        let _slot = slots.acquire(2);
+        Err(())
+    };
+    assert!(failed().is_err());
+    assert_eq!(*slots.busy.lock().unwrap(), 0);
+    let result = std::panic::catch_unwind(|| {
+        let _slot = slots.acquire(2);
+        panic!("synthetic render panic");
+    });
+    assert!(result.is_err());
+    assert_eq!(*slots.busy.lock().unwrap(), 0);
+}
+
+/// A Rayon helper must decline a full gate rather than wait for a render that can itself be
+/// waiting for that helper. Release the permits on timeout so a regression fails, not hangs.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn gpu_render_slots_never_block_rayon_helpers() {
+    let slots = GpuRenderSlots::new();
+    let first = slots.acquire(2);
+    let second = slots.acquire(2);
+    let pool = rayon::ThreadPoolBuilder::new().num_threads(2).build().unwrap();
+    let (tx, rx) = std::sync::mpsc::channel();
+    let declined = std::thread::scope(|scope| {
+        scope.spawn(|| {
+            pool.install(|| {
+                rayon::join(|| {}, || tx.send(slots.for_render(2).is_none()).unwrap());
+            });
+        });
+        let result = rx.recv_timeout(std::time::Duration::from_secs(2));
+        drop((first, second));
+        result
+    });
+    assert_eq!(declined, Ok(true), "Rayon helpers must not wait on GPU permits");
+    assert_eq!(*slots.busy.lock().unwrap(), 0);
+}
+
+/// Dedicated preview lanes have one protected thread and can wait without creating the
+/// shared-pool helper dependency cycle. They must keep rendering on a healthy GPU.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn gpu_preview_lane_waits_for_admission_and_resets_its_marker() {
+    let slots = GpuRenderSlots::new();
+    let first = slots.acquire(2);
+    let second = slots.acquire(2);
+    let pool = rayon::ThreadPoolBuilder::new().num_threads(1).build().unwrap();
+    let (started, ready) = std::sync::mpsc::channel();
+    let (tx, rx) = std::sync::mpsc::channel();
+    let (early, admitted) = std::thread::scope(|scope| {
+        scope.spawn(|| {
+            pool.install(|| {
+                let _drain = Draining::enter(Some(0)).unwrap();
+                assert!(Draining::enter(None).is_none(), "a lane cannot start a nested drain");
+                assert!(DRAINING_LANE.get(), "nested rejection must not clear the lane marker");
+                started.send(()).unwrap();
+                tx.send(slots.for_render(2).is_some()).unwrap();
+            });
+        });
+        ready.recv_timeout(std::time::Duration::from_secs(2)).unwrap();
+        let early = rx.recv_timeout(std::time::Duration::from_millis(100));
+        drop((first, second));
+        let admitted = early.or_else(|_| rx.recv_timeout(std::time::Duration::from_secs(2)));
+        (early, admitted)
+    });
+    assert!(early.is_err(), "busy dedicated lane must wait instead of falling back to CPU");
+    assert_eq!(admitted, Ok(true));
+    pool.install(|| {
+        assert!(!DRAINING.get());
+        assert!(!DRAINING_LANE.get());
+        let panic = std::panic::catch_unwind(|| {
+            let _drain = Draining::enter(Some(0)).unwrap();
+            panic!("synthetic lane failure");
+        });
+        assert!(panic.is_err());
+        assert!(!DRAINING.get());
+        assert!(!DRAINING_LANE.get());
+    });
+}

docs/architecture.md

@@ -412,7 +412,12 @@ as NaNs (a kernel that skips pixels fails the oracle) and validation errors pani
 (`ui-egui::frames`); headless renders, the CLI (unless `--gpu`) and CI use the CPU. Outside
 tests, device errors are logged rather than fatal. A viewer frame that runs out of video memory
 (`Gpu::within_memory`) renders on the CPU, the uploaded layers and pooled textures are freed, and
-the RAM preview keeps half as many GPU frames from then on. The same guard catches a frame's
+the RAM preview keeps half as many GPU frames from then on. However many frame threads the viewer
+runs, at most two of them start GPU render attempts at once. Shared-pool workers use the CPU
+when both slots are busy rather than blocking Rayon helpers; dedicated single-thread preview
+lanes wait for admission and remain on the GPU, so
+working-texture pressure is bounded by concurrent attempts rather than the thread count (#3028).
+A single large frame or retained frames can still exceed the available video memory. The same guard catches a frame's
 validation errors (an invalid texture, often left by a failed allocation): that frame renders on
 the CPU and the kept textures are freed, but the compositor stays on and, with no allocation
 failure before it, the RAM preview keeps its GPU frames (#1319). A frame whose render panics is