address review: effect detection by breadth-first search so depth cannot hide a comp

This commit is contained in:
Dylan M. Taylor
2026-11-04 01:04:35 +00:00
parent 319ea0f8bb
commit 49c632379d
2 changed files with 50 additions and 14 deletions
+22 -14
View File
@@ -438,22 +438,30 @@ pub trait RemoteFrames: Send + Sync {
/// Whether a comp (or a comp it nests) has a layer with enabled effects.
pub fn comp_has_effects(project: &Project, comp: ItemId) -> bool {
// Each comp is answered once: a comp nested by many layers at each of several levels was
// walked again for every path to it.
fn walk(p: &Project, comp: ItemId, depth: usize, seen: &mut std::collections::HashMap<ItemId, bool>) -> bool {
if let Some(&r) = seen.get(&comp) {
return r;
// Breadth first with a visited set: each comp is looked at once, at its shallowest level, so a
// comp first met deep in one chain is still found by a shorter route (a result memoised by
// comp alone would depend on the depth it was first reached at).
let mut seen = std::collections::HashSet::from([comp]);
let mut level = vec![comp];
// Comps nested up to 16 levels below the first are searched.
for _ in 0..=16 {
let mut next = vec![];
for id in level {
let Some(c) = project.comp(id) else { continue };
for l in &c.layers {
if l.switches.effects && l.effects().is_some_and(|fx| fx.groups().any(|g| g.enabled)) {
return true;
}
if let effectcraft_engine::project::LayerSource::Comp { item } = l.source
&& seen.insert(item)
{
next.push(item);
}
}
}
seen.insert(comp, false);
let Some(c) = p.comp(comp) else { return false };
let r = c.layers.iter().any(|l| {
l.switches.effects && l.effects().is_some_and(|fx| fx.groups().any(|g| g.enabled))
|| depth < 16 && matches!(l.source, effectcraft_engine::project::LayerSource::Comp { item } if walk(p, item, depth + 1, seen))
});
seen.insert(comp, r);
r
level = next;
}
walk(project, comp, 0, &mut std::collections::HashMap::new())
false
}
pub struct Frames {
+28
View File
@@ -1104,3 +1104,31 @@ fn effect_detection_answers_each_comp_once() {
assert!(!comp_has_effects(&p, below));
assert!(start.elapsed() < std::time::Duration::from_secs(5));
}
#[test]
fn an_effect_found_by_a_short_route_is_not_hidden_by_a_deep_one() {
use effectcraft_engine::Session;
use serde_json::json;
let mut s = Session::default();
let id = |v: serde_json::Value, k: &str| v[k].as_u64().unwrap();
let comp = |s: &mut Session| ItemId(id(s.execute("comp.new", json!({"name": "C", "width": 8, "height": 8, "duration": 1})).unwrap(), "comp"));
let nest = |s: &mut Session, outer: ItemId, inner: ItemId| {
s.execute("layer.addItem", json!({"comp": outer.0, "item": inner.0, "time": 0.0})).unwrap();
};
// `fx` holds a layer with an effect; seventeen comps stack above it, one level too many to be found.
let fx = comp(&mut s);
let l = id(s.execute("layer.newSolid", json!({"comp": fx.0, "color": "#ff0000"})).unwrap(), "layer");
s.execute("effect.apply", json!({"comp": fx.0, "layers": [l], "effect": "ec.vr.converter"})).unwrap();
let mut chain = fx;
for _ in 0..17 {
let up = comp(&mut s);
nest(&mut s, up, chain);
chain = up;
}
assert!(!comp_has_effects(&s.project, chain), "too deep to be found");
// A root that nests the chain first and `fx` directly after it.
let root = comp(&mut s);
nest(&mut s, root, chain);
nest(&mut s, root, fx);
assert!(comp_has_effects(&s.project, root));
}