From 49c632379d42c2753a776fac59ce818339ebc364 Mon Sep 17 00:00:00 2001
From: "Dylan M. Taylor" <dylan@dylanmtaylor.com>
Date: Wed, 4 Nov 2026 01:04:35 +0000
Subject: [PATCH] address review: effect detection by breadth-first search so
 depth cannot hide a comp

---
 crates/ui-egui/src/frames.rs       | 36 ++++++++++++++++++------------
 crates/ui-egui/src/frames/tests.rs | 28 +++++++++++++++++++++++
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs
index 9671e72a2..f31c9595b 100644
--- a/crates/ui-egui/src/frames.rs
+++ b/crates/ui-egui/src/frames.rs
@@ -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 {
diff --git a/crates/ui-egui/src/frames/tests.rs b/crates/ui-egui/src/frames/tests.rs
index a8564d8c6..785309b1a 100644
--- a/crates/ui-egui/src/frames/tests.rs
+++ b/crates/ui-egui/src/frames/tests.rs
@@ -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));
+}