From 45e81e603eb36002cd90363c9987bdb07b98808a Mon Sep 17 00:00:00 2001 From: FredApps Date: Mon, 9 Nov 2026 18:16:57 +0100 Subject: [PATCH] #3335: isolate proxy caches and settle small render batches --- crates/export/src/pipeline.rs | 57 +++++++++++++++++++++++++- crates/render/src/layers.rs | 8 ++++ crates/render/src/tests.rs | 1 + crates/render/src/tests/proxy_cache.rs | 45 ++++++++++++++++++++ 4 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 crates/render/src/tests/proxy_cache.rs diff --git a/crates/export/src/pipeline.rs b/crates/export/src/pipeline.rs index c233435a8..922b1b4ea 100644 --- a/crates/export/src/pipeline.rs +++ b/crates/export/src/pipeline.rs @@ -530,10 +530,21 @@ impl<'a> Cx<'a> { self.decide(Choice { renderer: crate::JobRenderer::Gpu, gpu_ms: None, cpu_ms: None, why: "the project's renderer (too few frames to measure)" }); return vec![]; } - // Small batches (few threads) measure one frame each; a batch too small for even that - // (a resumed sequence skipping frames) renders on the GPU and leaves it to the next. + // Nothing will be emitted for a skipped batch, so a later batch can still measure. + if ks.is_empty() { + return vec![]; + } + // A resumed sequence can have only one or two missing frames in its first batch. + // Settle before emitting them: measuring a later batch could otherwise switch to the + // CPU after GPU pixels have already been written, including across output modules. let n = PROBE_FRAMES.min(ks.len().saturating_sub(1) / 2); if n == 0 { + self.decide(Choice { + renderer: crate::JobRenderer::Gpu, + gpu_ms: None, + cpu_ms: None, + why: "the project's renderer (too few pending frames to measure)", + }); return vec![]; } let (Some(&first), Some(gpu_ks), Some(cpu_ks)) = (ks.first(), ks.get(1..1 + n), ks.get(1 + n..ks.len().min(1 + n + PROBE_CPU_FRAMES))) else { @@ -1198,6 +1209,48 @@ mod tests { assert_eq!(Cx::new(&job).gpu_cap(), None, "no accelerator"); } + /// A resumed sequence can start with one or two missing frames. Once emitted, their + /// compositor must also be used by every later batch and the item's other output modules. + #[test] + fn a_small_first_batch_settles_the_renderer_before_emitting_frames() { + if std::env::var_os(RENDERER_ENV).is_some() { + return; + } + use effectcraft_project::render_queue::{OutputFormat, OutputModule, RenderSettings}; + let (settings, output) = (RenderSettings::default(), OutputModule::for_format(OutputFormat::PngSequence)); + let mut p = Project::default(); + p.settings.gpu_acceleration = true; + let comp = Comp::new(16, 8, effectcraft_time::FrameRate::FPS_25, Tick::from_seconds_f64(1.0)); + let cid = p.add_item("Comp", effectcraft_color::Label::Sandstone, None, effectcraft_project::ItemKind::Comp(comp.into())); + let accel = NoAccel; + for n in [1, 2] { + let shared = Mutex::new(None); + let job = Job { + project: &p, + footage: &effectcraft_render::NoFootage, + expr: None, + accel: Some(&accel), + comp: cid, + settings: &settings, + output: &output, + path: "/r/f_[#####].png", + sink: None, + nested_switches: true, + options: crate::JobOptions { renderer: Some(&shared), ..Default::default() }, + }; + let cx = Cx::new(&job); + let empty = cx.render_frames(Vec::new(), |_, on| on, |_, on| on); + assert!(empty.is_empty()); + assert_eq!(cx.choice(), None, "skipped batches do not choose"); + let first = cx.render_frames((0..n).collect(), |_, on| on, |_, on| on); + assert_eq!(first, vec![crate::JobRenderer::Gpu; n as usize]); + assert_eq!(*shared.lock().unwrap(), Some(Backend::Gpu), "settled before any pixels are emitted"); + let later = cx.render_frames((n..12).collect(), |_, on| on, |_, on| on); + assert!(later.iter().all(|on| *on == crate::JobRenderer::Gpu)); + assert_eq!(Cx::new(&job).backend(), crate::JobRenderer::Gpu, "the next output module follows"); + } + } + #[test] fn a_job_that_chose_the_cpu_is_not_capped() { // #3335: the CPU's frame-parallel compositor is the point of choosing it; a GPU project diff --git a/crates/render/src/layers.rs b/crates/render/src/layers.rs index df976a857..cfae5dbab 100644 --- a/crates/render/src/layers.rs +++ b/crates/render/src/layers.rs @@ -295,7 +295,15 @@ impl<'a> Renderer<'a> { /// in this render ([`Self::mask_blur`]'s gate: the render options, the precomp layers above). /// The key has the layer's own motion blur state, but a mask set to On blurs without it. pub(crate) fn mask_gate_key(&self, ctx: &EvalCtx, layer: &Layer, key: Option) -> Option { + // Queue jobs share this cache but may override Proxy Use differently. The item's + // proxy switch alone does not describe the source a renderer actually reads. let k = key?; + let k = match self.opts.proxy { + effectcraft_project::render_queue::ProxyUse::CurrentSettings => k, + effectcraft_project::render_queue::ProxyUse::UseAll => cache::derive(k, 0x7072_6f78_7901), + effectcraft_project::render_queue::ProxyUse::UseCompOnly => cache::derive(k, 0x7072_6f78_7902), + effectcraft_project::render_queue::ProxyUse::UseNone => cache::derive(k, 0x7072_6f78_7903), + }; let masked = layer.masks().is_some_and(|m| !m.children.is_empty()); Some(if masked && !self.mask_blur(ctx, layer).allowed { cache::derive(k, 0x6d62_6f66) } else { k }) } diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 59fa0fadc..4ff292d9f 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -20,6 +20,7 @@ mod mask_feather; mod motion_tile; mod nested; mod paint; +mod proxy_cache; mod remap; mod roi; mod stretch_limits; diff --git a/crates/render/src/tests/proxy_cache.rs b/crates/render/src/tests/proxy_cache.rs new file mode 100644 index 000000000..85343a25d --- /dev/null +++ b/crates/render/src/tests/proxy_cache.rs @@ -0,0 +1,45 @@ +use super::setup; +use crate::eval::EvalCtx; +use crate::{FootageSource, Image, LayerCache, RenderOpts, Renderer}; +use effectcraft_color::Label; +use effectcraft_project::{Footage, FootageKind, ItemId, ItemKind, LayerSource, Proxy, build, render_queue::ProxyUse}; +use effectcraft_time::Tick; +use std::sync::Arc; + +/// A source and its proxy deliberately have different pixels. +struct Pictures; +impl FootageSource for Pictures { + fn frame(&self, _: ItemId, footage: &Footage, _: Tick) -> Option> { + let color = if footage.path == "proxy" { [0.0, 1.0, 0.0, 1.0] } else { [1.0, 0.0, 0.0, 1.0] }; + Some(Arc::new(Image::filled(16, 16, color))) + } +} + +/// A queue run reuses its layer cache across jobs with different Proxy Use settings. +/// Time effects read layer_input, which must never serve another job's source pixels. +#[test] +fn shared_layer_cache_separates_proxy_use() { + let (mut p, cid, comp) = setup(); + let footage = Footage { path: "original".into(), kind: FootageKind::Still, width: 16, height: 16, has_video: true, ..Default::default() }; + let item = p.add_item("Footage", Label::Red, None, ItemKind::Footage(footage.clone())); + p.item_mut(item).unwrap().proxy = Some(Box::new(Proxy { footage: Footage { path: "proxy".into(), ..footage }, enabled: false })); + let layer = build::layer(&mut p, &comp, "Footage", LayerSource::Footage { item }, (16, 16), None); + p.comp_mut(cid).unwrap().layers.push(layer); + for enabled in [false, true] { + p.item_mut(item).unwrap().proxy.as_mut().unwrap().enabled = enabled; + let cache = LayerCache::default(); + let ctx = EvalCtx::new(&p, cid, p.comp(cid).unwrap(), Tick::ZERO); + let layer = &ctx.comp.layers[0]; + for proxy in [ProxyUse::UseAll, ProxyUse::UseNone, ProxyUse::CurrentSettings, ProxyUse::UseCompOnly, ProxyUse::UseAll] { + let mut r = Renderer::new(&p, &Pictures, RenderOpts { proxy, ..Default::default() }); + let input = r.layer_input(&ctx, layer, 0).unwrap(); + let content = r.content_buf(&ctx, layer).unwrap(); + r.cache = Some(&cache); + assert_eq!(r.layer_input(&ctx, layer, 0).unwrap().img.data, input.img.data, "{proxy:?}, enabled={enabled}"); + assert_eq!(r.content_buf(&ctx, layer).unwrap().img.data, content.img.data, "{proxy:?}, enabled={enabled}"); + let hits = cache.stats().hits; + r.layer_input(&ctx, layer, 0).unwrap(); + assert!(cache.stats().hits > hits, "unchanged proxy settings still reuse inputs"); + } + } +}