Merge pull request #423 from cdinhoo/codex/effect-input-control-fixes

crates/render/src/cache.rs

@@ -1,8 +1,9 @@
 //! Layer render cache.
 //!
 //! The processed pixels of a layer (source → masks → effects, in layer space) depend only on the
-//! layer's own non-transform properties, its source item and the render options — not on its
-//! transform, opacity, blend mode or anything below it. [`LayerCache`] keeps those buffers keyed
+//! layer's non-transform properties, its source item, selected-layer dependencies and the render
+//! options. Its own transform is normally applied later; referenced layers' transforms can
+//! affect their sampled pixels. [`LayerCache`] keeps those buffers keyed
 //! by a content hash of exactly those inputs, *evaluated at the frame time*, so:
 //!
 //! * a static layer (or one whose only animation is in Transform) renders once and is reused on
@@ -25,6 +26,8 @@ use effectcraft_project::{ItemKind, Layer, LayerSource, Node, PropGroup};
 
 use crate::eval::EvalCtx;
 
+mod dependencies;
+
 /// Thread-safe, memory-budgeted cache of processed layer buffers, optionally backed by the
 /// persistent [`DiskCache`](crate::disk_cache::DiskCache): a memory miss reads the disk, and
 /// buffers that were slow to render are written there (keys salted per project, see
@@ -534,6 +537,9 @@ fn key_with(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, f
 #[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);
+    ctx.comp.frame_rate.as_f64().to_bits().hash(&mut h);
+    ctx.comp.enable_frame_blending.hash(&mut h);
+    dependencies::hash(&mut h, ctx, layer, effects)?;
     match &layer.source {
         LayerSource::Solid { item } => {
             let it = ctx.project.item(*item)?;

crates/render/src/cache/dependencies.rs

@@ -0,0 +1,109 @@
+//! Content selected by layer parameters participates in the owner's cache key. Keep the
+//! ordinary layer cache (including static self reads), and conservatively decline only
+//! cyclic/over-budget graphs or expressions sampled at unknown times.
+use super::{KeyHasher, hash_debug, hash_values};
+use crate::EvalCtx;
+use effectcraft_project::{ItemId, ItemKind, Layer, LayerId, LayerSource, Node, PropGroup};
+use std::hash::Hash;
+
+pub(super) fn hash(h: &mut KeyHasher, ctx: &EvalCtx, layer: &Layer, effects: Option<usize>) -> Option<()> {
+    let mut deps = Dependencies { h, path: vec![(ctx.comp_id, layer.id)], remaining: 1024, other_times: super::reads_other_times(layer) };
+    deps.effects(ctx, layer, effects)
+}
+
+struct Dependencies<'a> {
+    h: &'a mut KeyHasher,
+    path: Vec<(ItemId, LayerId)>,
+    remaining: usize,
+    other_times: bool,
+}
+
+impl Dependencies<'_> {
+    fn effects(&mut self, ctx: &EvalCtx, layer: &Layer, limit: Option<usize>) -> Option<()> {
+        let previous = self.other_times;
+        self.other_times |= super::reads_other_times(layer);
+        if layer.switches.effects
+            && let Some(fx) = layer.effects()
+        {
+            for g in fx.groups().take(limit.unwrap_or(usize::MAX)).filter(|g| g.enabled) {
+                self.group(ctx, layer, g, 0)?;
+            }
+        }
+        self.other_times = previous;
+        Some(())
+    }
+
+    fn group(&mut self, ctx: &EvalCtx, layer: &Layer, g: &PropGroup, depth: usize) -> Option<()> {
+        if depth >= 32 {
+            return None;
+        }
+        for node in &g.children {
+            match node {
+                Node::Prop(p) => {
+                    if let Some(id) = ctx.value(layer, p).as_layer() {
+                        p.uid.hash(self.h);
+                        id.hash(self.h);
+                        let other = ctx.layer(LayerId(id));
+                        other.is_some().hash(self.h);
+                        if let Some(other) = other {
+                            if other.id == layer.id {
+                                // Source/Masks never enter the same stack again. The owner
+                                // key already covers its source and mask properties.
+                                hash_debug(self.h, &ctx.world_matrix(other));
+                            } else {
+                                self.layer(ctx, other)?;
+                            }
+                        }
+                    }
+                }
+                Node::Group(sub) if sub.enabled => self.group(ctx, layer, sub, depth + 1)?,
+                _ => {}
+            }
+        }
+        Some(())
+    }
+
+    fn layer(&mut self, ctx: &EvalCtx, layer: &Layer) -> Option<()> {
+        let id = (ctx.comp_id, layer.id);
+        if self.path.len() > crate::MAX_FX_DEPTH || self.path.contains(&id) {
+            return None;
+        }
+        self.remaining = self.remaining.checked_sub(1)?;
+        if (self.other_times || super::reads_other_times(layer)) && super::has_expression(&layer.props) {
+            return None;
+        }
+        self.path.push(id);
+        // Include transforms/parents as well as pixels: selected-layer anchors and
+        // continuously rasterized sources can depend on them.
+        hash_debug(self.h, layer);
+        hash_values(self.h, ctx, layer, &layer.props);
+        hash_debug(self.h, &ctx.world_matrix(layer));
+        if super::time_dependent(layer) {
+            layer.layer_time(ctx.time).0.hash(self.h);
+        }
+        match layer.source {
+            LayerSource::Solid { item } | LayerSource::Footage { item } | LayerSource::Comp { item } => {
+                let source = ctx.project.item(item);
+                hash_debug(self.h, &source);
+                if let Some(source) = source {
+                    match &source.kind {
+                        ItemKind::Footage(_) => ctx.source_time(layer).0.hash(self.h),
+                        ItemKind::Comp(comp) => {
+                            let time = ctx.source_time(layer);
+                            time.0.hash(self.h);
+                            let nested = EvalCtx::new(ctx.project, item, comp, time);
+                            for child in &comp.layers {
+                                self.layer(&nested, child)?;
+                            }
+                        }
+                        _ => {}
+                    }
+                }
+            }
+            _ => {}
+        }
+        self.effects(ctx, layer, None)?;
+        self.path.pop();
+        Some(())
+    }
+}

crates/render/src/lib.rs

@@ -100,6 +100,7 @@ struct FxHost<'r, 'a, 'c> {
     origin: [f64; 2],
     /// Index of the effect being rendered (bounds `self_at` to effects before it).
     index: std::sync::atomic::AtomicUsize,
+    original: Option<&'c Buf>,
 }
 
 impl FxHost<'_, '_, '_> {
@@ -146,12 +147,16 @@ pub(crate) fn comp_scene(ctx: &EvalCtx, layer: &Layer) -> Option<effectcraft_eff
 const MAX_FX_DEPTH: usize = 8;
 
 impl EffectHost for FxHost<'_, '_, '_> {
+    fn original(&self) -> Option<&Buf> {
+        self.original
+    }
+
     fn particles(&self) -> Option<&dyn effectcraft_effects::psim::ParticleSim> {
         self.r.active_accel().and_then(|a| a.particles())
     }
     fn layer(&self, id: u64, masks_and_effects: bool) -> Option<LayerPixels> {
         let other = self.ctx.layer(effectcraft_project::LayerId(id))?;
-        if other.id == self.layer.id || self.r.depth > MAX_FX_DEPTH {
+        if (other.id == self.layer.id && masks_and_effects) || self.r.depth > MAX_FX_DEPTH {
             return None;
         }
         let sub = self.r.nested();
@@ -162,7 +167,7 @@ impl EffectHost for FxHost<'_, '_, '_> {
 
     fn layer_masks(&self, id: u64) -> Option<LayerPixels> {
         let other = self.ctx.layer(effectcraft_project::LayerId(id))?;
-        if other.id == self.layer.id || self.r.depth > MAX_FX_DEPTH {
+        if self.r.depth > MAX_FX_DEPTH {
             return None;
         }
         let sub = self.r.nested();
@@ -227,14 +232,17 @@ impl EffectHost for FxHost<'_, '_, '_> {
 
     fn layer_at(&self, id: u64, comp_time: f64, masks_and_effects: bool) -> Option<LayerPixels> {
         let other = self.ctx.layer(effectcraft_project::LayerId(id))?;
-        if other.id == self.layer.id || self.r.depth > MAX_FX_DEPTH {
+        if (other.id == self.layer.id && masks_and_effects) || !comp_time.is_finite() || self.r.depth > MAX_FX_DEPTH {
             return None;
         }
         let ctx = self.ctx.at(Tick::from_seconds_f64(comp_time));
         let sub = self.r.nested();
         Some(if masks_and_effects {
             self.pixels(other, (*sub.content_buf(&ctx, other)?).clone(), true)
+        } else if other.id == self.layer.id {
+            self.pixels(other, sub.source(&ctx, other)?, true)
         } else {
+            // Preserve other-layer reads: time effects historically see source + masks.
             self.pixels(other, (*sub.layer_input(&ctx, other, 0)?).clone(), false)
         })
     }
@@ -961,7 +969,18 @@ impl<'a> Renderer<'a> {
                 *q = [q[0] - origin[0], q[1] - origin[1]];
             }
         }
-        let host = FxHost { r: self, ctx, layer, origin, index: Default::default() };
+        // Capture once, before preceding effects can expand or replace the input. An
+        // adjustment stack's original is the comp below, not its solid source.
+        let needs_original =
+            fx.groups().take(limit).any(|g| g.enabled && matches!(&g.kind, GroupKind::Effect { effect } if effect == "ec.channel.cccomposite"));
+        let mut original = None;
+        if needs_original {
+            target.cpu(&mut |buf| {
+                original = Some(buf.clone());
+                buf
+            });
+        }
+        let host = FxHost { r: self, ctx, layer, origin, index: Default::default(), original: original.as_ref() };
         let env = EffectEnv {
             masks: &mask_shapes,
             host: Some(&host),

crates/render/src/tests.rs

@@ -586,3 +586,186 @@ fn layer_parameters_see_a_whole_shape_layer() {
         assert!(f.get(x, y)[3] < 0.01, "outside at ({x}, {y}): {:?}", f.get(x, y));
     }
 }
+
+#[test]
+fn set_matte_self_source_and_masks_precede_effects() {
+    for source in [0, 1] {
+        let (mut p, cid, comp) = setup();
+        let mut l = solid(&mut p, &comp, [1.0, 0.0, 0.0], 200, 100);
+        let mut next = p.next_id;
+        let mask = build::mask(&mut Ids(&mut next), "Left", ShapePath::rect([50.0, 50.0], 80.0, 80.0), MaskMode::Add, [255, 255, 0]);
+        p.next_id = next;
+        l.props.sub_mut("masks").unwrap().children.push(mask.into());
+        let id = l.id.0;
+        add_effect_200(&mut p, &mut l, "ec.channel.invert", &[("channel", Value::Enum(effectcraft_effects::INVERT_ALPHA))]);
+        add_effect_200(
+            &mut p,
+            &mut l,
+            "ec.channel.setmatte",
+            &[
+                ("takeMatteFromLayer", Value::Layer(Some(id))),
+                ("takeMatteFromLayerSource", Value::Enum(source)),
+                ("compositeMatteWithOriginal", Value::Bool(false)),
+            ],
+        );
+        p.comp_mut(cid).unwrap().layers.push(l);
+        let img = render_frame(&p, cid, Tick::ZERO, 1.0);
+        assert!(img.get(50, 50)[3] > 0.99, "original alpha must survive preceding alpha inversion");
+        assert!((img.get(150, 50)[3] - if source == 0 { 1.0 } else { 0.0 }).abs() < 0.01, "Source and Masks must remain distinct");
+    }
+}
+
+#[test]
+fn cc_composite_restores_stack_input_on_layers_and_adjustments() {
+    for adjustment in [false, true] {
+        for mode in [0, 1] {
+            let (mut p, cid, comp) = setup();
+            let mut l = solid(&mut p, &comp, [1.0, 0.0, 0.0], 200, 100);
+            l.switches.adjustment = adjustment;
+            add_effect_200(&mut p, &mut l, "ec.channel.invert", &[]);
+            add_effect_200(&mut p, &mut l, "ec.channel.cccomposite", &[("compositeOriginal", Value::Enum(mode))]);
+            p.comp_mut(cid).unwrap().layers.push(l);
+            if adjustment {
+                let bg = solid(&mut p, &comp, [1.0, 0.0, 0.0], 200, 100);
+                p.comp_mut(cid).unwrap().layers.push(bg);
+            }
+            let img = render_frame(&p, cid, Tick::ZERO, 1.0);
+            let want = if mode == 0 { [1.0, 0.0, 0.0, 1.0] } else { [0.0, 1.0, 1.0, 1.0] };
+            assert_eq!(img.get(50, 50), want, "adjustment={adjustment} mode={mode}");
+        }
+    }
+}
+
+#[test]
+fn self_inputs_share_source_semantics_across_effects_and_times() {
+    use crate::{EffectHost, FxHost, NoFootage, RenderOpts, Renderer};
+    let (mut p, cid, comp) = setup();
+    let mut l = solid(&mut p, &comp, [1.0, 0.0, 0.0], 200, 100);
+    let mut next = p.next_id;
+    let mask = build::mask(&mut Ids(&mut next), "Left", ShapePath::rect([50.0, 50.0], 80.0, 80.0), MaskMode::Add, [255, 255, 0]);
+    p.next_id = next;
+    l.props.sub_mut("masks").unwrap().children.push(mask.into());
+    p.comp_mut(cid).unwrap().layers.push(l.clone());
+    let c = p.comp(cid).unwrap();
+    let ctx = crate::eval::EvalCtx::new(&p, cid, c, Tick::ZERO);
+    let renderer = Renderer::new(&p, &NoFootage, RenderOpts::default());
+    let host = FxHost { r: &renderer, ctx: &ctx, layer: &l, origin: [0.0; 2], index: Default::default(), original: None };
+    assert_eq!(host.layer(l.id.0, false).unwrap().buf.img.get(150, 50)[3], 1.0);
+    assert_eq!(host.layer_masks(l.id.0).unwrap().buf.img.get(150, 50)[3], 0.0);
+    assert_eq!(host.layer_at(l.id.0, 0.5, false).unwrap().buf.img.get(150, 50)[3], 1.0);
+    assert!(host.layer(l.id.0, true).is_none(), "full-stack self references must not recurse");
+    assert!(host.layer_at(l.id.0, 0.5, true).is_none());
+    assert!(host.layer_at(l.id.0, f64::NAN, false).is_none());
+    // A second built-in using the shared selector: Invert(red) then Blend(original red).
+    l.props.sub_mut("masks").unwrap().children.clear();
+    add_effect_200(&mut p, &mut l, "ec.channel.invert", &[]);
+    let id = l.id.0;
+    add_effect_200(
+        &mut p,
+        &mut l,
+        "ec.channel.blend",
+        &[("blendWithLayer", Value::Layer(Some(id))), ("blendWithLayerSource", Value::Enum(0)), ("blendWithOriginal", Value::Scalar(0.0))],
+    );
+    p.comp_mut(cid).unwrap().layers = vec![l];
+    let c = p.comp(cid).unwrap();
+    let ctx = crate::eval::EvalCtx::new(&p, cid, c, Tick::ZERO);
+    assert!(crate::cache::input_key(&ctx, &c.layers[0], 1.0, false, false, 1).is_some());
+    assert!(crate::cache::input_key(&ctx, &c.layers[0], 1.0, false, false, 2).is_some());
+    assert_eq!(render_frame(&p, cid, Tick::ZERO, 1.0).get(50, 50), [1.0, 0.0, 0.0, 1.0]);
+}
+
+#[test]
+fn timed_source_reads_preserve_other_layer_masks() {
+    use effectcraft_effects::EffectHost;
+    let (mut p, cid, comp) = setup();
+    let owner = solid(&mut p, &comp, [1.0, 0.0, 0.0], 200, 100);
+    let mut other = solid(&mut p, &comp, [0.0, 1.0, 0.0], 200, 100);
+    let mut next = p.next_id;
+    let mask = build::mask(&mut Ids(&mut next), "Left", ShapePath::rect([50.0, 50.0], 80.0, 80.0), MaskMode::Add, [255, 255, 0]);
+    p.next_id = next;
+    other.props.sub_mut("masks").unwrap().children.push(mask.into());
+    p.comp_mut(cid).unwrap().layers = vec![owner.clone(), other.clone()];
+    let ctx = crate::EvalCtx::new(&p, cid, p.comp(cid).unwrap(), Tick::ZERO);
+    let renderer = crate::Renderer::new(&p, &crate::NoFootage, crate::RenderOpts::default());
+    let host = crate::FxHost { r: &renderer, ctx: &ctx, layer: &owner, origin: [0.0; 2], index: Default::default(), original: None };
+    // Current Source and timed Source had different contracts before this fix. Keep
+    // the existing timed read (used by Clone/Time effects) masked for other layers.
+    assert_eq!(host.layer(other.id.0, false).unwrap().buf.img.get(150, 50)[3], 1.0);
+    let timed = host.layer_at(other.id.0, 0.5, false).unwrap();
+    assert_eq!(timed.buf.img.get(50, 50)[3], 1.0);
+    assert_eq!(timed.buf.img.get(150, 50)[3], 0.0);
+}
+
+#[test]
+fn layer_dependencies_remain_cacheable_and_invalidate_only_related_edits() {
+    let (mut p, cid, comp) = setup();
+    let mut owner = solid(&mut p, &comp, [1.0, 0.0, 0.0], 200, 100);
+    let mut source = solid(&mut p, &comp, [0.0, 1.0, 0.0], 200, 100);
+    let third = solid(&mut p, &comp, [0.0, 0.0, 1.0], 200, 100);
+    let unrelated = solid(&mut p, &comp, [0.2, 0.3, 0.4], 200, 100);
+    add_effect_200(&mut p, &mut source, "ec.channel.blend", &[("blendWithLayer", Value::Layer(Some(third.id.0))), ("blendWithOriginal", Value::Scalar(0.0))]);
+    add_effect_200(&mut p, &mut owner, "ec.channel.blend", &[("blendWithLayer", Value::Layer(Some(source.id.0))), ("blendWithOriginal", Value::Scalar(0.0))]);
+    p.comp_mut(cid).unwrap().layers = vec![owner, source, third, unrelated];
+    let key = |p: &Project, time: f64, limit| {
+        let comp = p.comp(cid).unwrap();
+        let ctx = crate::EvalCtx::new(p, cid, comp, Tick::from_seconds_f64(time));
+        crate::cache::input_key(&ctx, &comp.layers[0], 1.0, false, false, limit).unwrap()
+    };
+    let initial = key(&p, 0.0, 1);
+    let input = key(&p, 0.0, 0);
+    assert_eq!(initial, key(&p, 0.5, 1), "static dependencies reuse across frames");
+    let cache = crate::LayerCache::default();
+    let before = render_cached(&p, cid, Tick::ZERO, Some(&cache));
+    render_cached(&p, cid, Tick::ZERO, Some(&cache));
+    assert!(cache.stats().hits > 0, "layer parameters must not disable caching");
+    let LayerSource::Solid { item: unrelated } = p.comp(cid).unwrap().layers[3].source else { panic!() };
+    let ItemKind::Solid(s) = &mut p.item_mut(unrelated).unwrap().kind else { panic!() };
+    s.color = [0.9, 0.8, 0.7];
+    assert_eq!(initial, key(&p, 0.0, 1), "unrelated edits preserve the owner key");
+    let LayerSource::Solid { item: third } = p.comp(cid).unwrap().layers[2].source else { panic!() };
+    let ItemKind::Solid(s) = &mut p.item_mut(third).unwrap().kind else { panic!() };
+    s.color = [1.0, 0.0, 1.0];
+    assert_ne!(initial, key(&p, 0.0, 1), "transitive source edits invalidate the owner");
+    assert_eq!(input, key(&p, 0.0, 0), "effects after an input prefix do not affect its key");
+    let cached = render_cached(&p, cid, Tick::ZERO, Some(&cache));
+    let fresh = render_cached(&p, cid, Tick::ZERO, None);
+    assert_same(&cached, &fresh, "transitive dependency edit");
+    assert_ne!(before.get(50, 50), fresh.get(50, 50));
+    let owner_id = p.comp(cid).unwrap().layers[0].id;
+    p.comp_mut(cid).unwrap().layers[1].props.prop_mut("effects/#1/blendWithLayer").unwrap().value = Value::Layer(Some(owner_id.0));
+    let comp = p.comp(cid).unwrap();
+    let ctx = crate::EvalCtx::new(&p, cid, comp, Tick::ZERO);
+    assert!(crate::cache::layer_key(&ctx, &comp.layers[0], 1.0, false, false).is_none(), "cycles decline caching without recursion");
+}
+
+#[test]
+fn precomp_dependencies_hash_nested_sources_and_evaluated_animation() {
+    let (mut p, cid, comp) = setup();
+    let mut child = solid(&mut p, &comp, [0.0, 0.0, 1.0], 200, 100);
+    let LayerSource::Solid { item: source } = child.source else { panic!() };
+    child.props.prop_mut("transform/opacity").unwrap().keys =
+        vec![Keyframe::new(Tick::ZERO, Value::Scalar(100.0)), Keyframe::new(Tick::from_seconds_f64(1.0), Value::Scalar(50.0))];
+    let mut nested = comp.clone();
+    nested.layers = vec![child];
+    let nested_id = p.add_item("Nested", Label::Sandstone, None, ItemKind::Comp(nested.into()));
+    let pre = build::layer(&mut p, &comp, "Precomp", LayerSource::Comp { item: nested_id }, (200, 100), None);
+    let mut owner = solid(&mut p, &comp, [1.0, 0.0, 0.0], 200, 100);
+    add_effect_200(&mut p, &mut owner, "ec.channel.blend", &[("blendWithLayer", Value::Layer(Some(pre.id.0))), ("blendWithOriginal", Value::Scalar(0.0))]);
+    p.comp_mut(cid).unwrap().layers = vec![owner, pre];
+    let key = |p: &Project, time| {
+        let c = p.comp(cid).unwrap();
+        crate::cache::layer_key(&crate::EvalCtx::new(p, cid, c, time), &c.layers[0], 1.0, false, false).unwrap()
+    };
+    let cache = crate::LayerCache::default();
+    let before = render_cached(&p, cid, Tick::ZERO, Some(&cache));
+    let initial = key(&p, Tick::ZERO);
+    let later = Tick::from_seconds_f64(0.5);
+    assert_ne!(initial, key(&p, later));
+    assert_same(&render_cached(&p, cid, later, Some(&cache)), &render_cached(&p, cid, later, None), "animated precomp dependency");
+    let ItemKind::Solid(s) = &mut p.item_mut(source).unwrap().kind else { panic!() };
+    s.color = [0.0, 1.0, 0.0];
+    assert_ne!(initial, key(&p, Tick::ZERO), "items used inside the precomp invalidate the owner");
+    let fresh = render_cached(&p, cid, Tick::ZERO, None);
+    assert_same(&render_cached(&p, cid, Tick::ZERO, Some(&cache)), &fresh, "precomp source edit");
+    assert_ne!(before.get(50, 50), fresh.get(50, 50));
+}