diff --git a/crates/aep/src/container/effect_map.rs b/crates/aep/src/container/effect_map.rs index 68a905be0..605e0b6f5 100644 --- a/crates/aep/src/container/effect_map.rs +++ b/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}"); + } } diff --git a/crates/aep/src/import/build.rs b/crates/aep/src/import/build.rs index 6f7e4f78f..4113c6c94 100644 --- a/crates/aep/src/import/build.rs +++ b/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 { + 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, pending: &[PendingComp<'_, '_>]) -> HashMap> { + 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::>())) + }), + ) +} + +/// [`precomp_candidates`] from each comp's precomp-eligible layer source ids. +fn candidate_links(sources: &HashMap, comps: impl IntoIterator)>) -> HashMap> { let mut links: HashMap> = 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> = 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; diff --git a/crates/aep/tests/it/import/bounds.rs b/crates/aep/tests/it/import/bounds.rs index 2fa0c3003..ef06696c7 100644 --- a/crates/aep/tests/it/import/bounds.rs +++ b/crates/aep/tests/it/import/bounds.rs @@ -89,6 +89,12 @@ fn comp_with<'a>(c: &Chunk<'a>, id: u32, name: &str, layers: Vec>) -> /// 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 { + 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 { 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 { 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.