From db21c733b489965e8fb56bff0a11b542f35a47ed Mon Sep 17 00:00:00 2001 From: nazar-gavrylyk Date: Tue, 10 Nov 2026 15:07:57 +0100 Subject: [PATCH] Auto: a frame's ticket ends it on every exit AutoPick::ticket chooses a side and reports the frame's time once; dropped without one it ends the frame unmeasured. The viewer's frames, the renderer's top-level frames and frames rendered in passes use it, so an Auto CPU frame turned away by GPU admission (#3028), a cancelled export frame or any later exit no longer leaves its frame counted as running. The frame server keeps its choice across calls and still ends a replaced frame itself. --- crates/render/src/auto.rs | 52 +++++++++++++++++++++++++++++++-- crates/render/src/lib.rs | 2 +- crates/render/src/passes.rs | 12 ++++---- crates/render/src/pipeline.rs | 30 +++++++++---------- crates/render/src/tests/auto.rs | 32 ++++++++++++++++++++ crates/ui-egui/src/frames.rs | 27 ++++++++--------- 6 files changed, 116 insertions(+), 39 deletions(-) diff --git a/crates/render/src/auto.rs b/crates/render/src/auto.rs index fc7becb5d..ce714c1f0 100644 --- a/crates/render/src/auto.rs +++ b/crates/render/src/auto.rs @@ -67,7 +67,8 @@ pub struct AutoPick { struct Entry { stats: AutoStats, measuring: [Option; 2], - // Lowered by `record`, `declined` and `abandoned`: every frame asked for ends in one. + // Lowered by `record`, `declined` and `abandoned`: every frame asked for ends in one (an + // [`AutoTicket`] makes sure of it). running: [u32; 2], } @@ -106,8 +107,16 @@ impl AutoPick { /// A warm-up or re-probe frame not reported back by then no longer holds its side. const MEASURING_FOR: std::time::Duration = std::time::Duration::from_secs(10); + /// [`AutoPick::choose`] for a frame that ends where it started: the ticket reports back, and + /// a ticket dropped without a time (an error, a retry elsewhere, a panic, a cancelled future) + /// ends the frame with [`AutoPick::abandoned`]. + pub fn ticket(&self, key: AutoKey) -> AutoTicket<'_> { + AutoTicket { pick: self, key, gpu: self.choose(key), open: true } + } + /// Render the frame for `key` on the GPU? Every frame asked for reports back with - /// [`AutoPick::record`] (or [`AutoPick::declined`]), or ends with [`AutoPick::abandoned`]. + /// [`AutoPick::record`] (or [`AutoPick::declined`]), or ends with [`AutoPick::abandoned`] + /// ([`AutoPick::ticket`] does that on its own). pub fn choose(&self, key: AutoKey) -> bool { let Ok(mut m) = self.stats.lock() else { return true }; if m.len() > Self::MAX_KEYS && !m.contains_key(&key) { @@ -219,3 +228,42 @@ impl AutoPick { } } } + +/// One frame [`AutoPick`] chose a side for ([`AutoPick::ticket`]): it reports the frame's time +/// once, and ends the frame unmeasured when dropped without one. +#[must_use = "a ticket dropped at once ends its frame unmeasured"] +pub struct AutoTicket<'a> { + pick: &'a AutoPick, + key: AutoKey, + gpu: bool, + open: bool, +} + +impl AutoTicket<'_> { + /// The frame renders on the GPU. + pub fn gpu(&self) -> bool { + self.gpu + } + + /// The frame took `ms` on its side. + pub fn record(mut self, ms: f64) { + self.open = false; + self.pick.record(self.key, self.gpu, ms); + } + + /// The GPU did not take the frame: it renders on the CPU, and the ticket reports from there. + pub fn declined(&mut self) { + if self.gpu { + self.pick.declined(self.key); + self.gpu = false; + } + } +} + +impl Drop for AutoTicket<'_> { + fn drop(&mut self) { + if self.open { + self.pick.abandoned(self.key, self.gpu); + } + } +} diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index d1be87a69..f47443b18 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -26,7 +26,7 @@ pub mod three_d; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -pub use auto::{AutoKey, AutoPick}; +pub use auto::{AutoKey, AutoPick, AutoTicket}; pub use cache::{CacheStats, LayerCache, LayerStore, PrefetchStore}; use effectcraft_color::BlendMode; use effectcraft_effects::{Buf, EffectCtx, EffectEnv, EffectHost, LayerPixels, Params}; diff --git a/crates/render/src/passes.rs b/crates/render/src/passes.rs index 37c94fff4..b8f94fc46 100644 --- a/crates/render/src/passes.rs +++ b/crates/render/src/passes.rs @@ -86,10 +86,10 @@ pub async fn comp_frame(r: &Renderer<'_>, comp: ItemId, t: Tick) -> Image { } else if opts.roi.is_none() && let Some(pick) = a.auto_pick() { - let key = AutoKey::new(comp, opts.scale, false); - let gpu = pick.choose(key); - opts.backend = if gpu { Backend::Gpu } else { Backend::Cpu }; - auto = Some((key, gpu)); + // (a frame cancelled or panicking between passes drops it: ended unmeasured) + let ticket = pick.ticket(AutoKey::new(comp, opts.scale, false)); + opts.backend = if ticket.gpu() { Backend::Gpu } else { Backend::Cpu }; + auto = Some(ticket); } } let t0 = web_time::Instant::now(); @@ -98,8 +98,8 @@ pub async fn comp_frame(r: &Renderer<'_>, comp: ItemId, t: Tick) -> Image { } else { in_passes(Some(a), |acc| Renderer { opts, accel: acc, ..*r }.comp_frame(comp, t)).await.value }; - if let (Some((key, chose)), Some(pick)) = (auto, a.auto_pick()) { - pick.record(key, chose, t0.elapsed().as_secs_f64() * 1e3); + if let Some(t) = auto { + t.record(t0.elapsed().as_secs_f64() * 1e3); } img } diff --git a/crates/render/src/pipeline.rs b/crates/render/src/pipeline.rs index 27103f19a..29ce6d69a 100644 --- a/crates/render/src/pipeline.rs +++ b/crates/render/src/pipeline.rs @@ -5,8 +5,9 @@ use super::fx_host::{FxHost, essential_overrides}; use super::opts::{Collapse3d, LayerTiming, MAX_THROUGH, RenderOpts, Through}; use super::reads::{ReadFiles, ReadLog}; use super::{ - Arc, AutoKey, AutoPick, BlendMode, Buf, Comp, EffectCtx, EffectEnv, EvalCtx, Footage, FootageKind, FrameBlend, FrameRate, GroupKind, Image, ItemId, - ItemKind, Layer, LayerCache, LayerSource, Params, Project, Quality, Region, Renderer, Tick, cache, color, masks, shapes, styles, text, three_d, vec2, + Arc, AutoKey, AutoPick, AutoTicket, BlendMode, Buf, Comp, EffectCtx, EffectEnv, EvalCtx, Footage, FootageKind, FrameBlend, FrameRate, GroupKind, Image, + ItemId, ItemKind, Layer, LayerCache, LayerSource, Params, Project, Quality, Region, Renderer, Tick, cache, color, masks, shapes, styles, text, three_d, + vec2, }; use effectcraft_keyframe::Value; use effectcraft_project::LayerId; @@ -584,29 +585,24 @@ impl<'a> Renderer<'a> { if self.depth == 0 && let Some(a) = self.active_accel() { - let auto = self.frame_auto(a, comp_id, false); - if auto.is_none_or(|(p, key)| p.choose(key)) { + // (an error returns with the ticket, which ends the frame unmeasured) + let mut ticket = self.frame_auto(a, comp_id, false).map(|(p, key)| p.ticket(key)); + if ticket.as_ref().is_none_or(AutoTicket::gpu) { let t0 = web_time::Instant::now(); - let res = scope(a); - if res.is_err() - && let Some((p, key)) = auto - { - p.abandoned(key, true); - } - if let Some(img) = res? { - if let Some((p, key)) = auto { - p.record(key, true, t0.elapsed().as_secs_f64() * 1e3); + if let Some(img) = scope(a)? { + if let Some(t) = ticket { + t.record(t0.elapsed().as_secs_f64() * 1e3); } return Ok(img); } - if let Some((p, key)) = auto { - p.declined(key); + if let Some(t) = &mut ticket { + t.declined(); } } let t0 = web_time::Instant::now(); let img = cpu.comp_frame_cpu(comp_id, t); - if let Some((p, key)) = auto { - p.record(key, false, t0.elapsed().as_secs_f64() * 1e3); + if let Some(t) = ticket { + t.record(t0.elapsed().as_secs_f64() * 1e3); } return Ok(img); } diff --git a/crates/render/src/tests/auto.rs b/crates/render/src/tests/auto.rs index da3d9f348..a53bb718a 100644 --- a/crates/render/src/tests/auto.rs +++ b/crates/render/src/tests/auto.rs @@ -328,3 +328,35 @@ fn auto_pick_abandoned_frames_stop_counting_as_running() { let s = pick.stats(key).unwrap(); assert_eq!((s.cpu_ms, s.gpu_ms), (Some(10.0), Some(40.0)), "{s:?}"); } + +/// A ticket that never reports (dropped on an error, a retry elsewhere, a cancelled export +/// future) ends its frame like [`AutoPick::abandoned`]; one the GPU declined reports from the +/// CPU. (Each exit that skipped `record` left its frame counted as running, and every later +/// frame on that side was timed as one of several side by side.) +#[test] +fn auto_tickets_end_their_frame_on_every_exit() { + let pick = AutoPick::default(); + let key = AutoKey::new(ItemId(6), 1.0, false); + let mut dropped = [false; 2]; + for _ in 0..16 { + let t = pick.ticket(key); + let gpu = t.gpu(); + if !std::mem::replace(&mut dropped[usize::from(gpu)], true) { + drop(t); + continue; + } + t.record(if gpu { 40.0 } else { 10.0 }); + } + let s = pick.stats(key).unwrap(); + assert_eq!((s.cpu_ms, s.gpu_ms), (Some(10.0), Some(40.0)), "{s:?}"); + // Declined: the frame renders on the CPU and reports there, one frame on it. + let key = AutoKey::new(ItemId(7), 1.0, false); + pick.ticket(key).record(10.0); + let mut t = pick.ticket(key); + assert!(t.gpu(), "the GPU's warm-up frame"); + t.declined(); + assert!(!t.gpu()); + t.record(10.0); + pick.ticket(key).record(10.0); + assert_eq!(pick.stats(key).unwrap().cpu_ms, Some(10.0), "timed as one frame on the CPU"); +} diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 01b9824d6..c6f48ce0b 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -38,7 +38,7 @@ 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}; use effectcraft_engine::render::reads::{ReadFiles, ReadLog}; -use effectcraft_engine::render::{CancellableFootage, ExprHost, FootageSource, LayerCache, RenderOpts, Renderer}; +use effectcraft_engine::render::{AutoTicket, CancellableFootage, ExprHost, FootageSource, LayerCache, RenderOpts, Renderer}; use effectcraft_engine::time::Tick; use effectcraft_gpu::{DisplayFrame, FrameError, Gpu}; use rayon::prelude::*; @@ -2009,32 +2009,32 @@ impl Worker { { // The browser cannot wait for the GPU to time it (and its CPU path is slow): always // the GPU there. - 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)) { + // Every exit that doesn't record ends the frame unmeasured: the ticket is dropped + // before a retry (`take`), or on return. + let mut ticket = if cfg!(target_arch = "wasm32") { None } else { r.frame_auto(a, job.comp, true).map(|(p, key)| p.ticket(key)) }; + if ticket.as_ref().is_none_or(AutoTicket::gpu) { let t0 = web_time::Instant::now(); match gpu_attempt(g, || g.render_display(&r, job.comp, job.t)) { Ok(Some(f)) => { self.gpu_succeeded(); - if let Some((p, key)) = auto { + if let Some(t) = ticket.take() { // Time the GPU's work, not just its recording. g.wait(); if g.context().check_health().is_err() { - p.abandoned(key, true); + drop(t); return self.render_job(job, false, files); } - p.record(key, true, t0.elapsed().as_secs_f64() * 1000.0); + t.record(t0.elapsed().as_secs_f64() * 1000.0); } return FrameImage::Gpu(Arc::new(f)); } Ok(None) => { - if let Some((p, key)) = auto { - p.declined(key); + if let Some(t) = &mut ticket { + t.declined(); } } Err(e) => { - if let Some((p, key)) = auto { - p.abandoned(key, true); - } + drop(ticket.take()); self.gpu_attempt_rejected(g, &e); return self.render_job(job, false, files); } @@ -2046,12 +2046,13 @@ impl Worker { 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) => { + drop(ticket.take()); 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); + if let Some(t) = ticket { + t.record(t0.elapsed().as_secs_f64() * 1000.0); } return img; }