From a831d129756001b7ee4f4f8634a8db6faca94661 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 22:13:19 +0000 Subject: [PATCH] Puppet: dragging pins keeps the layer under the Puppet cached (#212) Every pin move re-rendered the Puppet's input (source, masks and the effects before it) at full size on the UI thread and deep-copied it for the mesh overlay: the cache key of a layer's input hashed all of its effects, so moving a pin of the Puppet changed the key of its own input. The input key now covers only the effects that run before the Puppet, and the overlay shares the cached buffer. Test: input_keys_ignore_later_effects. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018gnTnH3Jmk5zr4tMq1Mc9f --- crates/engine/src/commands/puppet.rs | 7 +++--- crates/render/src/cache.rs | 33 +++++++++++++++++++++++----- crates/render/src/tests_time.rs | 28 +++++++++++++++++++++++ 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/commands/puppet.rs b/crates/engine/src/commands/puppet.rs index 737cc7cf..8d11fc57 100644 --- a/crates/engine/src/commands/puppet.rs +++ b/crates/engine/src/commands/puppet.rs @@ -26,8 +26,9 @@ fn pt(v: Option<&Value>) -> Option<[f64; 2]> { Some([a.first()?.as_f64()?, a.get(1)?.as_f64()?]) } -/// The Puppet effect's input pixels and evaluated parameters at comp time `t`. -pub fn puppet_eval(s: &Session, cid: ItemId, lid: LayerId, fx_uid: Uid, t: Tick) -> Option<(Buf, Params)> { +/// The Puppet effect's input pixels and evaluated parameters at comp time `t`. The input comes +/// from the layer cache, shared: moving pins doesn't change it (#212). +pub fn puppet_eval(s: &Session, cid: ItemId, lid: LayerId, fx_uid: Uid, t: Tick) -> Option<(std::sync::Arc, Params)> { let comp = s.project.comp(cid)?; let layer = comp.layer(lid)?; let fx = layer.effects()?; @@ -37,7 +38,7 @@ pub fn puppet_eval(s: &Session, cid: ItemId, lid: LayerId, fx_uid: Uid, t: Tick) let mut r = Renderer::new(&s.project, &*s.footage, RenderOpts::default()); r.expr = s.expr.as_deref(); r.cache = Some(&s.layer_cache); - let buf = (*r.layer_input(&ctx, layer, index)?).clone(); + let buf = r.layer_input(&ctx, layer, index)?; let params = effectcraft_effects::flatten_params(g, &mut |p| ctx.value(layer, p)); Some((buf, params)) } diff --git a/crates/render/src/cache.rs b/crates/render/src/cache.rs index b30f67cb..97908480 100644 --- a/crates/render/src/cache.rs +++ b/crates/render/src/cache.rs @@ -492,7 +492,10 @@ pub fn layer_key(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bo /// effects (see `Renderer::layer_input`). Unlike [`layer_key`], footage layers are cached here /// (keyed by item and source time), since Time effects read many neighbouring frames. pub fn input_key(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, effects: usize) -> Option { - let base = key_with(ctx, layer, scale, draft, blur, true)?; + if layer.switches.adjustment { + return None; + } + let base = key_any(ctx, layer, scale, draft, blur, true, Some(effects))?; let mut h = KeyHasher(base ^ 0x5bd1_e995_7a3c_11d3); effects.hash(&mut h); Some(h.finish()) @@ -516,18 +519,20 @@ fn has_expression(g: &PropGroup) -> bool { /// Cache key for an adjustment layer's footprint (its source through its masks, see /// `Renderer::adjustment_footprint`): the GPU compositor then uploads it once, not every frame. pub fn footprint_key(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool) -> Option { - Some(derive(key_any(ctx, layer, scale, draft, blur, false)?, 0xf007_9417)) + Some(derive(key_any(ctx, layer, scale, draft, blur, false, None)?, 0xf007_9417)) } fn key_with(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, footage: bool) -> Option { if layer.switches.adjustment { return None; } - key_any(ctx, layer, scale, draft, blur, footage) + key_any(ctx, layer, scale, draft, blur, footage, None) } -/// `blur`: the layer is motion blurred in this render (`Renderer::mb_on`). -fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, footage: bool) -> Option { +/// `blur`: the layer is motion blurred in this render (`Renderer::mb_on`). `effects`: key only +/// the first that many effects (a layer's input, see [`input_key`]). +#[allow(clippy::too_many_arguments)] +fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, footage: bool, effects: Option) -> Option { let mut h = KeyHasher(0xcbf2_9ce4_8422_2325); match &layer.source { LayerSource::Solid { item } => { @@ -576,6 +581,24 @@ fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, fo { continue; } + // A layer's input runs only its first effects: the later ones (a Puppet whose pins are + // dragged, #212) don't change it, so it stays cached. + if let (Some(n), Node::Group(g)) = (effects, c) + && g.match_id == "effects" + { + let mut first = g.clone(); + let mut seen = 0; + first.children.retain(|e| match e { + Node::Prop(_) => true, + Node::Group(_) => { + seen += 1; + seen <= n + } + }); + hash_debug(&mut h, &first); + hash_values(&mut h, ctx, layer, &first); + continue; + } // Static structure: values, keyframes, expressions, enabled flags, effect ids, modes. hash_debug(&mut h, c); if let Node::Group(g) = c { diff --git a/crates/render/src/tests_time.rs b/crates/render/src/tests_time.rs index b2833fb0..31ed9db3 100644 --- a/crates/render/src/tests_time.rs +++ b/crates/render/src/tests_time.rs @@ -103,3 +103,31 @@ fn cache_correct_while_scrubbing() { assert_eq!(render(&p, cid, t, Some(&cache)).data, render(&p, cid, t, None).data, "edited t={t}"); } } + +/// A layer's input up to an effect keys only the effects before it: editing a later effect (a +/// Puppet pin being dragged, #212) keeps the input cached; editing an earlier one doesn't. +#[test] +fn input_keys_ignore_later_effects() { + let (mut p, cid) = scene(Some(("ec.blur.gaussian", &[("blurriness", Value::Scalar(4.0))]))); + { + let spec = effectcraft_effects::find("ec.stylize.mosaic").unwrap(); + let mut next = p.next_id; + let g = effectcraft_effects::instantiate(spec, &mut Ids(&mut next), spec.name, [200.0, 100.0]); + p.next_id = next; + p.comp_mut(cid).unwrap().layers[0].props.sub_mut("effects").unwrap().children.push(g.into()); + } + let key = |p: &Project| { + let comp = p.comp(cid).unwrap(); + let ctx = crate::EvalCtx { project: p, comp_id: cid, comp, time: Tick::from_seconds_f64(0.5), expr: None, footage: None }; + let l = &comp.layers[0]; + (crate::cache::input_key(&ctx, l, 1.0, false, false, 1), crate::cache::input_key(&ctx, l, 1.0, false, false, 2)) + }; + let (one, two) = key(&p); + assert!(one.is_some() && one != two); + let set = |p: &mut Project, path: &str, v: f64| p.comp_mut(cid).unwrap().layers[0].props.prop_mut(path).unwrap().value = Value::Scalar(v); + set(&mut p, "effects/#2/horizontal", 30.0); + assert_eq!(key(&p).0, one, "the second effect doesn't change the input to it"); + assert_ne!(key(&p).1, two); + set(&mut p, "effects/#1/blurriness", 8.0); + assert_ne!(key(&p).0, one); +}