Merge pull request #222 from storytold/fix/212-puppet-input-cache

Puppet: dragging pins keeps the layer under the Puppet cached (#212)

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<Buf>, 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))
 }

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<u64> {
-    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<u64> {
-    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<u64> {
     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<u64> {
+/// `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<usize>) -> Option<u64> {
     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 {

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);
+}