AEP import: precomp loop search counts only same-component users; only eligible layers are candidate links; popup keys lower whole strings (final sigma)

crates/aep/src/container/effect_map.rs

@@ -1160,7 +1160,9 @@ pub const GLOW_MIDPOINT_MAX: f64 = 0.990_005_493_164_062_5;
 /// language, so an item can start with a capital outside ASCII (French "Écran"), and its key must
 /// be the same as for the same word in lower case.
 pub fn popup_key(item: &str) -> String {
-    popup_stem(item).chars().flat_map(char::to_lowercase).collect()
+    // The whole string is lowered at once: Unicode's lower case depends on the context (a Greek
+    // capital sigma ends a word as final sigma), which a per-character mapping loses.
+    popup_stem(item).to_lowercase()
 }
 
 /// [`popup_key`] before the lower-casing, borrowed: `item` trimmed, without a final ` channel`
@@ -1178,7 +1180,9 @@ const SUFFIX: &[u8] = b" channel";
 
 /// Whether two popup items have the same [`popup_key`], without building either key.
 pub(crate) fn popup_keys_match(a: &str, b: &str) -> bool {
-    popup_stem(a).chars().flat_map(char::to_lowercase).eq(popup_stem(b).chars().flat_map(char::to_lowercase))
+    let (a, b) = (popup_stem(a), popup_stem(b));
+    // ASCII lowers per byte without a key; anything else lowers as a whole string (see `popup_key`).
+    if a.is_ascii() && b.is_ascii() { a.eq_ignore_ascii_case(b) } else { a.to_lowercase() == b.to_lowercase() }
 }
 
 /// The English [`popup_key`] a non-English After Effects popup item name stands for, when this
@@ -1253,6 +1257,12 @@ mod tests {
             "Luminance",
             "\u{c9}CRAN",
             "\u{e9}cran",
+            // Greek: a capital sigma at the end of a word lowers to the final form.
+            "\u{39f}\u{3a3}",
+            "\u{3bf}\u{3c2}",
+            "\u{3bf}\u{3c3}",
+            "\u{39f}\u{3a3} Channel",
+            "\u{3a3}\u{39f}\u{3a3}",
         ];
         for a in items {
             assert_eq!(popup_key(a), old_key(a), "{a:?}");
@@ -1270,4 +1280,13 @@ mod tests {
         assert_eq!(popup_key("Off for 32 bpc Color"), "off for 32 bpc color");
         assert_eq!(popup_key("Red Channel"), "red");
     }
+
+    /// Lower case is context sensitive: a capital sigma at the end of a word is a final sigma.
+    #[test]
+    fn popup_key_keeps_the_final_sigma() {
+        assert_eq!(popup_key("\u{39f}\u{3a3}"), "\u{3bf}\u{3c2}");
+        assert!(popup_keys_match("\u{39f}\u{3a3}", "\u{3bf}\u{3c2}"));
+        assert!(!popup_keys_match("\u{39f}\u{3a3}", "\u{3bf}\u{3c3}"));
+        assert_eq!(popup_key("\u{3a3}\u{39f}\u{3a3} Channel"), "\u{3c3}\u{3bf}\u{3c2}");
+    }
 }

crates/aep/src/import/build.rs

@@ -905,7 +905,12 @@ impl Precomps {
         if closes_loop(from, to, |to| inside && found.is_some_and(|u| u.contains(&to))) {
             return Link::Loop;
         }
-        self.used_by.entry(to).or_default().insert(from);
+        // Only a link inside a component can lie on a path the loop search follows (it never
+        // leaves the component of the layer's comp), so only those are kept: the searches never
+        // visit, or pay budget for, a link between components.
+        if self.same_component(from, to) {
+            self.used_by.entry(to).or_default().insert(from);
+        }
         Link::Accepted
     }
 
@@ -921,8 +926,11 @@ impl Precomps {
         let mut stack = vec![comp];
         while let Some(c) = stack.pop() {
             for u in self.used_by.get(&c).into_iter().flatten() {
+                if !self.same_component(*u, comp) {
+                    continue;
+                }
                 self.steps_left = self.steps_left.checked_sub(1)?;
-                if self.same_component(*u, comp) && found.insert(*u) {
+                if found.insert(*u) {
                     stack.push(*u);
                 }
             }
@@ -931,16 +939,32 @@ impl Precomps {
     }
 }
 
+/// The source id of a layer that `build_layers` decides by [`Precomps::accept`] when the source is
+/// a composition: a camera, light, text, shape or null layer never becomes a precomp layer, whatever
+/// source it stores, so it is no candidate link.
+fn precomp_layer_source(h: &LayerHeader) -> Option<u32> {
+    if h.layer_type != LayerType::Av || h.is_null_object() { None } else { h.source_id }
+}
+
 /// Every link a precomp layer of the file could make: comp -> the comps its layers name as their
 /// source (resolved as `build_layers` resolves them).
 fn precomp_candidates(sources: &HashMap<u32, Source>, pending: &[PendingComp<'_, '_>]) -> HashMap<ItemId, HashSet<ItemId>> {
+    candidate_links(
+        sources,
+        pending.iter().filter_map(|p| {
+            let comp = p.item.comp()?;
+            Some((p.id, comp.layers().filter_map(|l| l.header().and_then(|h| precomp_layer_source(&h))).collect::<Vec<_>>()))
+        }),
+    )
+}
+
+/// [`precomp_candidates`] from each comp's precomp-eligible layer source ids.
+fn candidate_links(sources: &HashMap<u32, Source>, comps: impl IntoIterator<Item = (ItemId, Vec<u32>)>) -> HashMap<ItemId, HashSet<ItemId>> {
     let mut links: HashMap<ItemId, HashSet<ItemId>> = HashMap::new();
-    for p in pending {
-        let Some(comp) = p.item.comp() else { continue };
-        for layer in comp.layers() {
-            let Some(s) = layer.header().and_then(|h| h.source_id).and_then(|s| sources.get(&s)) else { continue };
+    for (from, ids) in comps {
+        for s in ids.iter().filter_map(|id| sources.get(id)) {
             if s.kind == SourceKind::Comp {
-                links.entry(p.id).or_default().insert(s.item);
+                links.entry(from).or_default().insert(s.item);
             }
         }
     }
@@ -1262,6 +1286,41 @@ mod tests {
         refused
     }
 
+    #[test]
+    fn outside_users_do_not_spend_the_loop_search_budget() {
+        // A is in a loop with B (A <-> B). 100 comps outside the loop show A; the budget is nearly
+        // spent. Building A: its layer of B needs A's users, which are only B's side of the loop:
+        // the outside comps cannot close a loop and must cost nothing, so A's layer is decided.
+        let (a, b) = (ItemId(1), ItemId(2));
+        let ext = |i: u64| ItemId(10 + i);
+        let mut candidates: HashMap<ItemId, HashSet<ItemId>> = HashMap::from([(a, HashSet::from([b])), (b, HashSet::from([a]))]);
+        for i in 0..100 {
+            candidates.insert(ext(i), HashSet::from([a]));
+        }
+        let mut p = Precomps::new(&candidates);
+        for i in 0..100 {
+            assert_eq!(p.accept(ext(i), a, &mut Users::NotSearched), Link::Accepted);
+        }
+        p.steps_left = 5;
+        // B shows A first (inside the loop, one user of A is B itself once accepted), then A shows B.
+        assert_eq!(p.accept(b, a, &mut Users::NotSearched), Link::Accepted);
+        assert_eq!(p.accept(a, b, &mut Users::NotSearched), Link::Loop);
+    }
+
+    #[test]
+    fn ineligible_layers_are_no_candidate_links() {
+        let source = |item: u64, kind| Source { item: ItemId(item), name: String::new(), kind, width: 1, height: 1 };
+        let sources = HashMap::from([(1, source(1, SourceKind::Comp)), (2, source(2, SourceKind::Comp)), (3, source(3, SourceKind::Solid))]);
+        // Comp 1 shows 2 (eligible); comp 2 has no eligible layer of comp 1 (a null or text layer
+        // with that source is filtered by `precomp_layer_source` before): no loop component.
+        let links = candidate_links(&sources, [(ItemId(1), vec![2, 3]), (ItemId(2), vec![])]);
+        let p = Precomps::new(&links);
+        assert!(!p.same_component(ItemId(1), ItemId(2)));
+        // With the back-edge as a candidate they share one, which is what ineligible layers used to do.
+        let looped = candidate_links(&sources, [(ItemId(1), vec![2]), (ItemId(2), vec![1])]);
+        assert!(Precomps::new(&looped).same_component(ItemId(1), ItemId(2)));
+    }
+
     #[test]
     fn the_precomp_loop_check_decides_as_the_full_walk_did() {
         let mut refused = 0;

crates/aep/tests/it/import/bounds.rs

@@ -89,6 +89,12 @@ fn comp_with<'a>(c: &Chunk<'a>, id: u32, name: &str, layers: Vec<Chunk<'a>>) ->
 /// Composition `B` (id 900) with `n` layers of `C`, and `D` (id 901) with `n` layers of `B`,
 /// after `C` in the file. With `c_uses_d`, `C` also gets a layer of `D`, which closes a loop.
 fn precomp_chain(n: u32, c_uses_d: bool) -> Vec<u8> {
+    precomp_chain_with(n, c_uses_d, false)
+}
+
+/// [`precomp_chain`]; with `null_back_edge` the layer of `D` in `C` is a null object (which shows
+/// nothing, so no loop).
+fn precomp_chain_with(n: u32, c_uses_d: bool, null_back_edge: bool) -> Vec<u8> {
     let mut file = parse(SYNTHETIC).unwrap();
     let c = comp_c(&file);
     let solid = solid_layer(&c);
@@ -102,7 +108,15 @@ fn precomp_chain(n: u32, c_uses_d: bool) -> Vec<u8> {
         let c = fold.iter_mut().find(|k| k.is_list(b"Item") && k.child(b"idta").is_some_and(|i| be_u32(i, 16) == COMP_C)).expect("C");
         let kids = children(c);
         let at = kids.iter().position(|k| k.is_list(b"Layr")).expect("a layer");
-        kids.insert(at, layer_copy(&solid, 300_000, &|l| set_be_u32(l, 40, 901)));
+        kids.insert(
+            at,
+            layer_copy(&solid, 300_000, &|l| {
+                set_be_u32(l, 40, 901);
+                if null_back_edge && let Body::Data(d) = &mut l.body {
+                    d.to_mut()[38] |= 0x80;
+                }
+            }),
+        );
     }
     write(&file).unwrap()
 }
@@ -139,6 +153,17 @@ fn every_precomp_layer_that_closes_a_loop_is_refused() {
     assert!(losses(&r, LossCategory::UnsupportedLayers).iter().any(|d| d.contains("precomposition loop")));
 }
 
+#[test]
+fn a_null_layer_naming_a_composition_closes_no_loop() {
+    // The same files as above, but C's layer of D is a null object: it shows nothing, so D's
+    // layers of B are no loop and all of them are imported.
+    let n = 300;
+    let (p, r) = run(&precomp_chain_with(n, true, true));
+    assert_eq!(comp_named(&p, "D").layers.len(), n as usize);
+    assert!(comp_named(&p, "C").layers.iter().any(|l| l.source == LayerSource::Null));
+    assert!(!losses(&r, LossCategory::UnsupportedLayers).iter().any(|d| d.contains("loop")), "{:?}", r.report.losses);
+}
+
 #[test]
 fn many_parented_and_matted_layers_resolve() {
     // `n` copies of the solid layer in C, each parented to it and using it as an alpha matte.