#3299: aep import/export: linear hostile-input bounds, reported losses, deduplicated export code

Import: precomp loop check by candidate-graph components (no search
without a loop), one LayerId -> index map for mattes, parents and effect
layer references, popup mapping linear in the file's items, lazy loss
details (lose_with), orientation keys at one time reported, mask path
ease speeds under the property limit, Blending Options nested past the
read depth reported.

Export: precomp loops by components, parent loops found once, cached
natural sort keys, one exported-layer set, parameter definitions built
from the effect table (old encode-and-parse path kept as a test
reference), shared key-time loop, key-list rewrite, spatial record,
tdmn/tdsn helpers, unreadable curve and mesh entries reported.

Corpus import and export dumps are byte-identical to the base.

crates/aep/src/container/effect_map.rs

@@ -1143,8 +1143,25 @@ pub const GLOW_MIDPOINT_MAX: f64 = 0.990_005_493_164_062_5;
 /// The key a popup item is matched by between the two programs: trimmed, lower case, and
 /// Set Matte's "Red Channel" ... "Alpha Channel" are EffectCraft's "Red" ... "Alpha" (904 E24).
 pub fn popup_key(item: &str) -> String {
-    let s = item.trim().to_ascii_lowercase();
-    s.strip_suffix(" channel").map(str::to_string).unwrap_or(s)
+    popup_stem(item).to_ascii_lowercase()
+}
+
+/// [`popup_key`] before the lower-casing, borrowed: `item` trimmed, without a final ` channel`
+/// in any case (the suffix is ASCII, so the cut is on a character boundary).
+pub(crate) fn popup_stem(item: &str) -> &str {
+    let t = item.trim();
+    let cut = t.len().saturating_sub(SUFFIX.len());
+    match (t.as_bytes().get(cut..), t.get(..cut)) {
+        (Some(end), Some(stem)) if t.len() >= SUFFIX.len() && end.eq_ignore_ascii_case(SUFFIX) => stem,
+        _ => t,
+    }
+}
+
+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).eq_ignore_ascii_case(popup_stem(b))
 }
 
 /// The table row of an After Effects match name.
@@ -1164,3 +1181,43 @@ pub const HUE_SATURATION_MASTER: [&str; 3] = ["hue", "saturation", "lightness"];
 /// The EffectCraft parameters Levels' RGB channel values (`Extra::Levels`) go to, in the order of
 /// the data (input black, input white, gamma, output black, output white: 512-516).
 pub const LEVELS_RGB: [&str; 5] = ["inBlack", "inWhite", "gamma", "outBlack", "outWhite"];
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// The key as it was built before the borrowed form (the reference for the differential test).
+    fn old_key(item: &str) -> String {
+        let s = item.trim().to_ascii_lowercase();
+        s.strip_suffix(" channel").map(str::to_string).unwrap_or(s)
+    }
+
+    #[test]
+    fn the_borrowed_popup_key_matches_the_owned_one() {
+        let items = [
+            "",
+            " ",
+            "(-",
+            "Red",
+            "red channel",
+            "Red Channel",
+            " Alpha CHANNEL ",
+            "channel",
+            " channel",
+            "x channel",
+            "Xchannel",
+            "Ä channel",
+            "Grün Channel",
+            "é",
+            "Ünit  channel",
+            "a channel channel",
+            "Luminance",
+        ];
+        for a in items {
+            assert_eq!(popup_key(a), old_key(a), "{a:?}");
+            for b in items {
+                assert_eq!(popup_keys_match(a, b), old_key(a) == old_key(b), "{a:?} {b:?}");
+            }
+        }
+    }
+}

crates/aep/src/export/effects.rs

@@ -29,14 +29,17 @@
 use std::collections::HashMap;
 use std::sync::OnceLock;
 
-use crate::effect_map::{Conv, EffectMap, Extra, HUE_SATURATION_MASTER, LEVELS_RGB, by_effect, by_match_name, popup_key};
-use crate::{AepFile, Body, CURVE_IDENTITY, Chunk, CurvesData, MeshData, ParamKind};
-use effectcraft_keyframe::{Interp, Keyframe, Value};
+use crate::effect_map::{Conv, EffectMap, Extra, HUE_SATURATION_MASTER, LEVELS_RGB, by_effect, by_match_name, popup_keys_match};
+use crate::{AepFile, CURVE_IDENTITY, Chunk, CurvesData, MeshData, ParamKind};
+use effectcraft_keyframe::{Interp, Value};
 use effectcraft_project::{GroupKind, Layer, LayerId, Node, ParamUi, PropGroup, Property};
 
+use crate::export::fx_defs::{FX_DEFS, FxDef, PDef};
+use crate::export::keys::{fill_key_list, replace_cdat, spatial_record, timed_keys};
 use crate::export::plan::{CompPlan, EaseSlot, KeyPlan, LIMIT};
 use crate::export::report::{Collector, LossCategory};
-use crate::export::tree::{R, child_data, child_mut, data_mut, kids_mut, list_mut, put, put_f64, put_u8, put_u16, put_u32, tpl};
+use crate::export::synth::{NO_NAME, is_tdmn, tdmn, tdmn_name, tdsn, utf8_in};
+use crate::export::tree::{R, child_data, child_mut, data_mut, kids_mut, list_mut, put, put_f64, put_u8, put_u32, tpl};
 
 /// How a parameter is stored in its value record.
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -154,10 +157,6 @@ pub(crate) struct ParDef {
     pub record: String,
     /// `pard@12`.
     pub code: u32,
-    /// `pard@16`, or a checkbox's `pdnm` label.
-    pub name: String,
-    /// The popup items (`pdnm`, separated by `|`).
-    pub items: Vec<String>,
     /// The default in stored units (`pard@56`).
     pub default: Vec<f64>,
     /// The slider range a value record repeats as `tdum` / `tduM`.
@@ -168,114 +167,76 @@ pub(crate) struct ParDef {
     pub data: Option<Vec<u8>>,
 }
 
-fn be_i32(d: &[u8], at: usize) -> Option<i32> {
-    Some(i32::from_be_bytes(d.get(at..at.checked_add(4)?)?.try_into().ok()?))
-}
-fn be_u32(d: &[u8], at: usize) -> Option<u32> {
-    Some(u32::from_be_bytes(d.get(at..at.checked_add(4)?)?.try_into().ok()?))
-}
-fn be_f32(d: &[u8], at: usize) -> Option<f64> {
-    Some(f64::from(f32::from_be_bytes(d.get(at..at.checked_add(4)?)?.try_into().ok()?)))
-}
-fn be_f64(d: &[u8], at: usize) -> Option<f64> {
-    Some(f64::from_be_bytes(d.get(at..at.checked_add(8)?)?.try_into().ok()?))
-}
-
-fn cname(d: &[u8]) -> String {
-    d.iter().take_while(|b| **b != 0).map(|b| char::from(*b)).collect()
+/// A fixed slider's or angle's value as its 16.16 `pard` copy holds it (`fxsynth::pard`).
+fn fixed(v: f64) -> f64 {
+    f64::from(crate::export::fxsynth::fixed_i32(v)) / 65536.0
 }
 
 impl ParDef {
-    fn from_pard(pard: &[u8], pdnm: Option<&str>, data: Option<&[u8]>) -> ParDef {
-        let code = be_u32(pard, 12).unwrap_or(u32::MAX);
-        let fx = |at: usize| be_i32(pard, at).map(|v| f64::from(v) / 65536.0);
-        let default: Vec<f64> = match code {
-            10 => be_f64(pard, 56).into_iter().collect(),
-            2 | 3 => fx(56).into_iter().collect(),
-            1 => be_i32(pard, 56).map(f64::from).into_iter().collect(),
-            4 | 7 => be_u32(pard, 56).map(f64::from).into_iter().collect(),
+    /// The definition of parameter `p` of the table row `fx`, in the stored units the
+    /// prototype's `pard` holds (`fxsynth::default_sspc` writes it from the same row): fixed
+    /// sliders, angles and points at 16.16, integers, checkboxes and popups rounded, colours as
+    /// bytes, float slider ranges as `f32`. Read straight from the table; the round trip through
+    /// the encoded `pard` that this replaces is the reference in the tests.
+    fn from_table(fx: &FxDef, p: &PDef) -> ParDef {
+        let d = |i: usize| p.default.get(i).copied().unwrap_or(0.0);
+        let int = |v: f64| f64::from(crate::export::fxsynth::int_i32(v));
+        let f32_of = |v: f64| f64::from(v as f32);
+        let default: Vec<f64> = match p.code {
+            10 => vec![d(0)],
+            2 | 3 => vec![fixed(d(0))],
+            1 => vec![int(d(0))],
+            4 => vec![f64::from(d(0).round().max(0.0) as u32)],
+            7 => vec![f64::from(d(0).round().clamp(0.0, f64::from(u16::MAX)) as u32)],
             // Fractions of the layer, or percent after an After Effects re-save (see the reader).
             6 => {
-                let p: Vec<f64> = [fx(56), fx(60)].into_iter().flatten().collect();
-                if p.iter().any(|c| c.abs() > 1.0 + 1e-9) { p.iter().map(|c| c / 100.0).collect() } else { p }
+                let pt = [fixed(d(0)), fixed(d(1))];
+                if pt.iter().any(|c| c.abs() > 1.0 + 1e-9) { pt.iter().map(|c| c / 100.0).collect() } else { pt.to_vec() }
             }
-            5 => pard.get(56..60).map(|c| c.iter().map(|b| f64::from(*b)).collect()).unwrap_or_default(),
+            5 => (0..4).map(|i| f64::from(d(i).round().clamp(0.0, 255.0) as u8)).collect(),
             _ => Vec::new(),
         };
-        let range = match code {
-            10 => be_f32(pard, 112).zip(be_f32(pard, 116)),
-            2 => fx(132).zip(fx(136)),
-            1 => be_i32(pard, 132).map(f64::from).zip(be_i32(pard, 136).map(f64::from)),
-            _ => None,
+        // A slider without a range or valid maximum in the table stores zeros there.
+        let (lo, hi) = p.range.unwrap_or((0.0, 0.0));
+        let max = p.valid_max.unwrap_or(0.0);
+        let (range, valid_max) = match p.code {
+            10 => (Some((f32_of(lo), f32_of(hi))), Some(f32_of(max))),
+            2 => (Some((fixed(lo), fixed(hi))), Some(fixed(max))),
+            1 => (Some((int(lo), int(hi))), Some(int(max))),
+            _ => (None, None),
         };
-        let valid_max = match code {
-            10 => be_f32(pard, 108),
-            2 => fx(128),
-            1 => be_i32(pard, 128).map(f64::from),
-            _ => None,
-        };
-        let pname = pard.get(16..48).map(cname).unwrap_or_default();
-        let (name, items) = match (code, pdnm) {
-            (7, Some(p)) if !p.is_empty() => (pname, p.split('|').map(str::to_string).collect()),
-            (4, Some(p)) if pname.trim().is_empty() => (p.to_string(), Vec::new()),
-            _ => (pname, Vec::new()),
-        };
-        ParDef { record: String::new(), code, name, items, default, range, valid_max, data: data.map(<[u8]>::to_vec) }
+        // The record is the text of its `tdmn` as the exporter reads it back (`synth::tdmn_name`),
+        // so the instance finds its records by it.
+        let record = tdmn_name(&tdmn(&fx.record(p))).unwrap_or_default();
+        // The prototype's `pard` holds no name and its `pdnm` is empty: no name, no popup items.
+        let data = if p.code == 11 { crate::export::fxsynth::default_data(fx.match_name) } else { None };
+        ParDef { record, code: u32::from(p.code), default, range, valid_max, data }
+    }
+
+    /// The definitions of the table row `fx`, by position (the table's index).
+    fn of_effect(fx: &FxDef) -> HashMap<u16, ParDef> {
+        let mut out = HashMap::with_capacity(fx.params.len());
+        let mut position = 0u16;
+        for p in fx.params {
+            out.insert(position, ParDef::from_table(fx, p));
+            position = position.saturating_add(1);
+        }
+        out
     }
 }
 
-/// The parameter definitions of an instance's `parT`, by position (the table's index).
-fn par_defs(sspc: &Chunk<'_>) -> HashMap<u16, ParDef> {
-    let mut out = HashMap::new();
-    let Some(part) = sspc.child_list(b"parT") else { return out };
-    let k = part.children();
-    let mut i = 0usize;
-    let mut position = 0u16;
-    while i < k.len() {
-        let (Some(m), Some(p)) = (k.get(i), k.get(i + 1)) else { break };
-        if !(m.is(b"tdmn") && p.is(b"pard")) {
-            i += 1;
-            continue;
-        }
-        let name = tdmn_name(m).unwrap_or_default();
-        let mut j = i + 2;
-        let mut pdnm = None;
-        let mut data = None;
-        while let Some(c) = k.get(j) {
-            if c.is(b"tdmn") {
-                break;
-            }
-            if c.is(b"pdnm") {
-                pdnm = c.children().iter().find(|u| u.is(b"Utf8")).and_then(|u| u.bytes()).map(|b| String::from_utf8_lossy(b).to_string());
-            }
-            if c.is(b"aRbp") {
-                data = c.bytes();
-            }
-            j += 1;
-        }
-        if let Some(bytes) = p.bytes() {
-            out.insert(position, ParDef { record: name, ..ParDef::from_pard(bytes, pdnm.as_deref(), data) });
-        }
-        position = position.saturating_add(1);
-        i = j;
-    }
-    out
-}
-
-/// The parameter definitions of every effect type, by match name (read from the default
-/// instances built from the effect table).
+/// The parameter definitions of every effect type, by match name (from the effect table the
+/// default instances are built from).
 fn defs() -> &'static HashMap<String, HashMap<u16, ParDef>> {
     static DEFS: OnceLock<HashMap<String, HashMap<u16, ParDef>>> = OnceLock::new();
-    DEFS.get_or_init(|| {
-        crate::export::fx_defs::FX_DEFS.iter().map(|fx| (fx.match_name.to_string(), par_defs(&crate::export::fxsynth::default_sspc(fx)))).collect()
-    })
+    DEFS.get_or_init(|| FX_DEFS.iter().map(|fx| (fx.match_name.to_string(), ParDef::of_effect(fx))).collect())
 }
 
 // ---------------------------------------------------------------------------------------------
 // Planning.
 
 /// The stored components of an EffectCraft value, `None` when it does not fit.
-fn stored(conv: Conv, kind: PKind, v: &Value, size: (f64, f64), ui: &ParamUi, def: &ParDef) -> Option<Vec<f64>> {
+fn stored(conv: Conv, kind: PKind, v: &Value, size: (f64, f64), ui: &ParamUi) -> Option<Vec<f64>> {
     let c = v.components();
     let g = |i: usize| c.get(i).copied().and_then(sane);
     let one = |x: f64| -> Vec<f64> {
@@ -306,8 +267,8 @@ fn stored(conv: Conv, kind: PKind, v: &Value, size: (f64, f64), ui: &ParamUi, de
             };
             let ParamUi::Popup { options } = ui else { return None };
             let name = options.get(i)?;
-            let file: Vec<&str> = if def.items.is_empty() { items.to_vec() } else { def.items.iter().map(String::as_str).collect() };
-            let at = file.iter().position(|f| f.trim() != "(-" && popup_key(f) == popup_key(name))?;
+            // The prototype's `pdnm` names no items: the table's are After Effects'.
+            let at = items.iter().position(|f| f.trim() != "(-" && popup_keys_match(f, name))?;
             vec![f64::from(u32::try_from(at).ok()?.checked_add(1)?)]
         }
         // Not a value record (layer parameters, histograms).
@@ -317,16 +278,15 @@ fn stored(conv: Conv, kind: PKind, v: &Value, size: (f64, f64), ui: &ParamUi, de
 
 /// Whether a popup's items are After Effects' items in the same order (separators included, as
 /// After Effects counts them), so an expression's 1-based result picks the same item in both.
-fn same_popup_order(items: &[&str], ui: &ParamUi, def: &ParDef) -> bool {
+fn same_popup_order(items: &[&str], ui: &ParamUi) -> bool {
     let ParamUi::Popup { options } = ui else { return false };
-    let file: Vec<&str> = if def.items.is_empty() { items.to_vec() } else { def.items.iter().map(String::as_str).collect() };
-    file.len() == options.len() && file.iter().zip(options).all(|(f, o)| f.trim() != "(-" && popup_key(f) == popup_key(o))
+    items.len() == options.len() && items.iter().zip(options).all(|(f, o)| f.trim() != "(-" && popup_keys_match(f, o))
 }
 
 /// [`stored`], reporting what it changes in a value it can store: a colour channel outside 0 to 1
 /// (written as the nearest limit) and a fraction on an integer parameter (rounded).
-fn stored_reported(conv: Conv, kind: PKind, v: &Value, (size, ui, def): ((f64, f64), &ParamUi, &ParDef), what: &str, col: &mut Collector) -> Option<Vec<f64>> {
-    let s = stored(conv, kind, v, size, ui, def)?;
+fn stored_reported(conv: Conv, kind: PKind, v: &Value, (size, ui): ((f64, f64), &ParamUi), what: &str, col: &mut Collector) -> Option<Vec<f64>> {
+    let s = stored(conv, kind, v, size, ui)?;
     if matches!(conv, Conv::Color) {
         let c = v.components();
         if c.iter().take(3).any(|x| !(0.0..=1.0).contains(x)) {
@@ -339,7 +299,7 @@ fn stored_reported(conv: Conv, kind: PKind, v: &Value, (size, ui, def): ((f64, f
         }
     }
     if kind == PKind::Int
-        && let Some(raw) = stored(conv, PKind::Fixed, v, size, ui, def)
+        && let Some(raw) = stored(conv, PKind::Fixed, v, size, ui)
         && raw.iter().zip(&s).any(|(a, b)| a.to_bits() != b.to_bits())
     {
         col.lose(LossCategory::Effects, format!("{what}: {raw:?} is written as the whole number {s:?}"));
@@ -501,16 +461,26 @@ fn plan_extra(spec: &EffectMap, g: &PropGroup, pdefs: &HashMap<u16, ParDef>, her
     (data != before).then_some((index, data))
 }
 
-/// `x,y` pairs (EffectCraft's curve points and mesh offsets; whitespace or `;` between pairs).
-fn pairs_of(s: &str) -> Vec<(f64, f64)> {
-    s.split(|c: char| c.is_whitespace() || c == ';')
+/// `x,y` pairs (EffectCraft's curve points; whitespace or `;` between pairs), and how many
+/// entries are not a pair of finite numbers (left out).
+fn pairs_of(s: &str) -> (Vec<(f64, f64)>, usize) {
+    let mut unreadable = 0usize;
+    let pairs = s
+        .split(|c: char| c.is_whitespace() || c == ';')
         .filter(|t| !t.is_empty())
         .filter_map(|t| {
-            let (x, y) = t.split_once(',')?;
-            let (x, y) = (x.trim().parse::<f64>().ok()?, y.trim().split(',').next()?.trim().parse::<f64>().ok()?);
-            (x.is_finite() && y.is_finite()).then_some((x, y))
+            let pair = (|| {
+                let (x, y) = t.split_once(',')?;
+                let (x, y) = (x.trim().parse::<f64>().ok()?, y.trim().split(',').next()?.trim().parse::<f64>().ok()?);
+                (x.is_finite() && y.is_finite()).then_some((x, y))
+            })();
+            if pair.is_none() {
+                unreadable = unreadable.saturating_add(1);
+            }
+            pair
         })
-        .collect()
+        .collect();
+    (pairs, unreadable)
 }
 
 /// The arbitrary-data parameter's index and default data.
@@ -537,7 +507,10 @@ fn static_str<'p>(g: &'p PropGroup, id: &str, here: &str, col: &mut Collector) -
 fn plan_curves(g: &PropGroup, pdefs: &HashMap<u16, ParDef>, here: &str, col: &mut Collector) -> Option<(u16, Vec<u8>)> {
     let mut curves: [Vec<(u8, u8)>; 5] = Default::default();
     for (slot, id) in curves.iter_mut().zip(["rgb", "red", "green", "blue", "alpha"]) {
-        let pts = static_str(g, id, here, col).map(pairs_of).unwrap_or_default();
+        let (pts, unreadable) = static_str(g, id, here, col).map(pairs_of).unwrap_or_default();
+        if unreadable > 0 {
+            col.lose(LossCategory::InvalidValues, format!("{here}: curve '{id}': {unreadable} entries are not an x,y pair of numbers and are not written"));
+        }
         let mut ints: Vec<(u8, u8)> = Vec::with_capacity(pts.len());
         let (mut rounded, mut clipped) = (false, false);
         for (x, y) in pts {
@@ -584,17 +557,25 @@ fn plan_mesh(g: &PropGroup, pdefs: &HashMap<u16, ParDef>, size: (f64, f64), here
     let (rows, columns) = (int("rows"), int("columns"));
     // Per vertex `dx,dy` (pixels), optionally followed by the eight pixel moves of its handles
     // (top, right, bottom, left) that the importer keeps.
+    let mut unreadable = 0usize;
     let offs: Vec<Vec<f64>> = static_str(g, "mesh", here, col)
         .map(|s| {
             s.split(|c: char| c.is_whitespace() || c == ';')
                 .filter(|t| !t.is_empty())
                 .filter_map(|t| {
-                    let v: Vec<f64> = t.split(',').map(|x| x.trim().parse::<f64>().ok().filter(|x| x.is_finite())).collect::<Option<_>>()?;
-                    (v.len() >= 2).then_some(v)
+                    let v: Option<Vec<f64>> = t.split(',').map(|x| x.trim().parse::<f64>().ok().filter(|x| x.is_finite())).collect();
+                    let v = v.filter(|v| v.len() >= 2);
+                    if v.is_none() {
+                        unreadable = unreadable.saturating_add(1);
+                    }
+                    v
                 })
                 .collect()
         })
         .unwrap_or_default();
+    if unreadable > 0 {
+        col.lose(LossCategory::InvalidValues, format!("{here}: the Distortion Mesh has {unreadable} entries that are not numbers, left out"));
+    }
     let count = usize::try_from((rows + 1) * (columns + 1)).ok()?;
     let moved = offs.iter().any(|v| v.iter().any(|x| *x != 0.0));
     if !moved && rows == 7 && columns == 7 {
@@ -714,7 +695,7 @@ fn plan_param(pin: ParamIn<'_>, index: u16, cp: &CompPlan, here: &str, col: &mut
         // factor parameter has other units) or a popup's items are in another order, its result
         // means something else in After Effects. Popups count from 1 in both programs' expressions.
         let other_units = match conv {
-            Conv::Popup { items, .. } => !same_popup_order(items, &prop.ui, def),
+            Conv::Popup { items, .. } => !same_popup_order(items, &prop.ui),
             Conv::Percent { valid_max } => (valid_max - 100.0).abs() > 1e-9,
             Conv::Factor { factor, .. } => (factor - 1.0).abs() > 1e-9,
             _ => false,
@@ -724,7 +705,7 @@ fn plan_param(pin: ParamIn<'_>, index: u16, cp: &CompPlan, here: &str, col: &mut
         }
     }
     if prop.keys.is_empty() {
-        let Some(s) = stored_reported(conv, kind, &prop.value, (size, &prop.ui, def), &what, col) else {
+        let Some(s) = stored_reported(conv, kind, &prop.value, (size, &prop.ui), &what, col) else {
             if matches!(conv, Conv::Popup { .. }) {
                 col.lose(LossCategory::Effects, format!("{what}: the popup item has no After Effects counterpart, the default was written"));
             } else {
@@ -744,7 +725,7 @@ fn plan_param(pin: ParamIn<'_>, index: u16, cp: &CompPlan, here: &str, col: &mut
             format!("{what}: keys on a popup, checkbox, angle or integer slider are not written (not sampled), the first key's value was written"),
         );
         let first = prop.keys.first().map(|k| &k.value).unwrap_or(&prop.value);
-        let s = stored_reported(conv, kind, first, (size, &prop.ui, def), &what, col)?;
+        let s = stored_reported(conv, kind, first, (size, &prop.ui), &what, col)?;
         return (!is_default(kind, &s, def) || expr.is_some()).then_some(ParamPlan { index, kind, value: ParamValue::Static(s), expr });
     }
     let keys = plan_keys(pin, cp, &what, col);
@@ -756,22 +737,8 @@ fn plan_param(pin: ParamIn<'_>, index: u16, cp: &CompPlan, here: &str, col: &mut
 }
 
 fn plan_keys(pin: ParamIn<'_>, cp: &CompPlan, what: &str, col: &mut Collector) -> Vec<KeyPlan> {
-    let ParamIn { conv, kind, def, prop, size } = pin;
-    let keys: &[Keyframe] = &prop.keys;
-    let mut timed: Vec<(i64, &Keyframe)> = Vec::with_capacity(keys.len());
-    for k in keys {
-        let (u, inexact) = cp.tb.units(k.time);
-        if inexact {
-            col.lose(LossCategory::RoundedTimes, format!("{what}: key at {:.4} s moved to the time base", k.time.seconds()));
-        }
-        timed.push((u, k));
-    }
-    timed.sort_by_key(|(u, _)| *u);
-    let before = timed.len();
-    timed.dedup_by_key(|(u, _)| *u);
-    if timed.len() != before {
-        col.lose(LossCategory::ApproximatedKeyframes, format!("{what}: keys at the same time were merged"));
-    }
+    let ParamIn { conv, kind, prop, size, .. } = pin;
+    let timed = timed_keys(&prop.keys, &cp.tb, what, "key", col);
     // The ease of colour keys is sampled only as speed 0 / influence 0, which After Effects
     // interpolates linearly (598): a colour key is written linear.
     let ease_ok = matches!(kind, PKind::Fixed | PKind::Float | PKind::Point);
@@ -790,7 +757,7 @@ fn plan_keys(pin: ParamIn<'_>, cp: &CompPlan, what: &str, col: &mut Collector) -
             col.lose(LossCategory::InvalidValues, format!("{what}: key time out of range, key dropped"));
             continue;
         };
-        let Some(value) = stored_reported(conv, kind, &k.value, (size, &prop.ui, def), what, col) else {
+        let Some(value) = stored_reported(conv, kind, &k.value, (size, &prop.ui), what, col) else {
             col.lose(LossCategory::InvalidValues, format!("{what}: a key value that cannot be stored, key dropped"));
             continue;
         };
@@ -884,11 +851,6 @@ fn keyed_kind(k: PKind) -> Option<KeyedKind> {
     }
 }
 
-pub(crate) fn tdmn_name(c: &Chunk<'_>) -> Option<String> {
-    let d = c.bytes()?;
-    Some(d.iter().take_while(|b| **b != 0).map(|b| char::from(*b)).collect())
-}
-
 impl FxProtos {
     /// The prototypes built in code from the effect table (`fx_defs`, `fxsynth`).
     pub(crate) fn build() -> FxProtos {
@@ -919,27 +881,10 @@ pub(crate) struct FxUse {
     pub types: Vec<&'static str>,
 }
 
-/// A `tdsn` chunk holding `text`.
-fn tdsn(text: &str) -> Chunk<'static> {
-    nested(*b"tdsn", text)
-}
-
-/// A chunk that holds one `Utf8` chunk (`tdsn`, `fnam`).
-fn nested(id: [u8; 4], text: &str) -> Chunk<'static> {
-    Chunk { id, body: Body::Chunks(vec![Chunk::data(*b"Utf8", text.as_bytes().to_vec())]), pad: None }
-}
-
 /// The value record of `p`, built from the record of its parameter type.
 pub(crate) fn value_record(protos: &FxProtos, p: &ParamPlan, def: &ParDef, source_size: (u32, u32)) -> R<(Chunk<'static>, Chunk<'static>)> {
     let mut rec = protos.kinds.get(&p.kind.code()).cloned().ok_or_else(|| tpl("effect value prototype missing"))?;
-    {
-        let kids = kids_mut(&mut rec)?;
-        if let Some(slot) = kids.iter_mut().find(|c| c.is(b"tdsn"))
-            && !def.name.is_empty()
-        {
-            *slot = tdsn(&def.name);
-        }
-    }
+    // The record keeps the prototype's `tdsn`: the prototypes name no parameter.
     if p.kind == PKind::Point {
         // The layer's aspect, as on the Anchor Point (369: 16 / 9, 383: 1 on 100 x 100).
         put_f64(child_data(&mut rec, b"tdb4")?, 24, f64::from(source_size.0) / f64::from(source_size.1.max(1)))?;
@@ -970,35 +915,22 @@ pub(crate) fn value_record(protos: &FxProtos, p: &ParamPlan, def: &ParDef, sourc
                     put_u8(tdb4, at, *ktdb4.get(at).ok_or_else(|| tpl("short tdb4"))?)?;
                 }
             }
-            let n = u32::try_from(keys.len()).map_err(|_| tpl("too many keys"))?;
             let dims = p.kind.dims();
             let size = match kk {
                 KeyedKind::Fixed | KeyedKind::Float => 8 + 40 * dims,
                 KeyedKind::Color | KeyedKind::Point => 56 + 24 * dims,
             };
-            {
-                let lhd3 = child_data(&mut list, b"lhd3")?;
-                crate::export::synth::put_key_counts(lhd3, n, u32::try_from(size).map_err(|_| tpl("record size"))?)?;
-            }
-            let mut ldat = Vec::with_capacity(size * keys.len());
+            let mut ldat = Vec::with_capacity(size.saturating_mul(keys.len()));
             for k in keys {
                 ldat.extend(match kk {
                     KeyedKind::Fixed | KeyedKind::Float => crate::export::emit::plain_record(1, k, size)?,
-                    KeyedKind::Color | KeyedKind::Point => spatial_record(kk, k, dims, size)?,
+                    // A colour (flags `0x0001`, 210, 364) or a point (flags `0x0007`, the
+                    // temporal ease in layer heights per second, 382, 598).
+                    KeyedKind::Color | KeyedKind::Point => spatial_record(k, dims, kk == KeyedKind::Point, size)?,
                 });
             }
-            {
-                let lk = kids_mut(&mut list)?;
-                match lk.iter_mut().find(|c| c.is(b"ldat")) {
-                    Some(c) => *c = Chunk::data(*b"ldat", ldat),
-                    None => lk.push(Chunk::data(*b"ldat", ldat)),
-                }
-            }
-            let kids = kids_mut(&mut rec)?;
-            let at = kids.iter().position(|c| c.is(b"cdat")).ok_or_else(|| tpl("value record without cdat"))?;
-            if let Some(slot) = kids.get_mut(at) {
-                *slot = list;
-            }
+            fill_key_list(&mut list, ldat, keys.len(), size)?;
+            replace_cdat(&mut rec, list)?;
         }
     }
     if let Some((text, enabled)) = &p.expr {
@@ -1013,46 +945,7 @@ pub(crate) fn value_record(protos: &FxProtos, p: &ParamPlan, def: &ParDef, sourc
         let at = kids.iter().position(|c| c.is(b"tdum")).unwrap_or(kids.len());
         kids.insert(at, Chunk::data(*b"Utf8", text.as_bytes().to_vec()));
     }
-    Ok((Chunk::data(*b"tdmn", tdmn_bytes(&def.record)), rec))
-}
-
-/// A key of a colour (flags `0x0001`, 210, 364) or point (flags `0x0007`, `u32@8` the automatic
-/// tangent switch, 382) parameter: the spatial layout with the value at 56.
-fn spatial_record(kk: KeyedKind, k: &KeyPlan, n: usize, size: usize) -> R<Vec<u8>> {
-    let mut r = vec![0u8; size];
-    put(&mut r, 0, &k.time.to_be_bytes())?;
-    put_u8(&mut r, 4, k.in_interp)?;
-    put_u8(&mut r, 5, k.out_interp)?;
-    let base: u16 = if kk == KeyedKind::Point { 0x0007 } else { 0x0001 };
-    put_u16(&mut r, 6, base | (u16::from(k.label) << 6))?;
-    if kk == KeyedKind::Point {
-        put_u32(&mut r, 8, u32::from(k.spatial_auto))?;
-        // The temporal ease (598): speed in layer heights per second, influence a fraction.
-        let (i, o) = (k.in_ease.first().copied().unwrap_or_default(), k.out_ease.first().copied().unwrap_or_default());
-        put_f64(&mut r, 24, i.speed)?;
-        put_f64(&mut r, 32, i.influence)?;
-        put_f64(&mut r, 40, o.speed)?;
-        put_f64(&mut r, 48, o.influence)?;
-    }
-    for j in 0..n {
-        put_f64(&mut r, 56 + 8 * j, k.value.get(j).copied().unwrap_or(0.0))?;
-        if kk == KeyedKind::Point {
-            put_f64(&mut r, 56 + 8 * n + 8 * j, k.spatial_in.get(j).copied().unwrap_or(0.0))?;
-            put_f64(&mut r, 56 + 16 * n + 8 * j, k.spatial_out.get(j).copied().unwrap_or(0.0))?;
-        }
-    }
-    Ok(r)
-}
-
-/// A match name padded to the 40 bytes of `tdmn`.
-fn tdmn_bytes(name: &str) -> Vec<u8> {
-    let mut v = vec![0u8; 40];
-    for (i, b) in name.bytes().take(39).enumerate() {
-        if let Some(slot) = v.get_mut(i) {
-            *slot = b;
-        }
-    }
-    v
+    Ok((tdmn(&def.record), rec))
 }
 
 /// The `tdmn` + `sspc` of one instance on the layer with id `layer_id`.
@@ -1066,14 +959,15 @@ fn instance(
 ) -> R<Vec<Chunk<'static>>> {
     let (tdmn, sspc, _) = protos.defaults.get(fx.match_name).ok_or_else(|| tpl("effect prototype missing"))?;
     let mut sspc = sspc.clone();
-    let pdefs = par_defs(&sspc);
+    // The prototype's definitions are the table's (the prototype is built from it).
+    let pdefs = defs().get(fx.match_name).ok_or_else(|| tpl("effect definitions missing"))?;
     // `fnam` keeps the effect's own name; a name of the user's goes into the value group's
     // `tdsn`, which holds the placeholder `-_0_/-` otherwise (610 against 609).
     let own = child_mut(&mut sspc, b"fnam")?.children().iter().find(|c| c.is(b"Utf8")).and_then(|c| c.bytes()).map(|b| String::from_utf8_lossy(b).to_string());
     if own.as_deref() != Some(fx.name.as_str()) && !fx.name.is_empty() {
         let values = list_mut(&mut sspc, b"tdgp")?;
         let slot = child_mut(values, b"tdsn")?;
-        *slot = nested(*b"tdsn", &fx.name);
+        *slot = utf8_in(*b"tdsn", &fx.name);
     }
     // The fx switch: the value group's `tdsb` (`fx-switch` cases).
     put_u32(child_data(list_mut(&mut sspc, b"tdgp")?, b"tdsb")?, 0, u32::from(fx.enabled))?;
@@ -1086,9 +980,7 @@ fn instance(
         let values = list_mut(&mut sspc, b"tdgp")?;
         let kids = kids_mut(values)?;
         // The input layer (`-0000`) names the layer that owns the effect (320).
-        let tdpi_of = |kids: &mut Vec<Chunk<'static>>, record: &str| -> Option<usize> {
-            kids.iter().position(|c| c.is(b"tdmn") && tdmn_name(c).as_deref() == Some(record)).map(|i| i + 1)
-        };
+        let tdpi_of = |kids: &mut Vec<Chunk<'static>>, record: &str| -> Option<usize> { kids.iter().position(|c| is_tdmn(c, record)).map(|i| i + 1) };
         let input = pdefs.get(&0).map(|d| d.record.clone()).unwrap_or_default();
         // The prototype's own layer id (its input layer), to tell "the owning layer" from None.
         let mut proto_owner = None;
@@ -1108,7 +1000,7 @@ fn instance(
         let mapped: Vec<u16> = by_match_name(fx.match_name)
             .map(|s| s.params.iter().filter(|p| matches!(p.conv, Conv::LayerRef { .. })).filter_map(|p| u16::try_from(p.index).ok()).collect())
             .unwrap_or_default();
-        for (index, def) in &pdefs {
+        for (index, def) in pdefs {
             if *index == 0 || def.code != 0 {
                 continue;
             }
@@ -1130,10 +1022,7 @@ fn instance(
         // Arbitrary data: the `aRbp` of the `LIST aRbs` after the parameter's record.
         if let Some((index, data)) = &fx.data {
             let want = pdefs.get(index).map(|d| d.record.clone()).unwrap_or_default();
-            let i = kids
-                .iter()
-                .position(|c| c.is(b"tdmn") && tdmn_name(c).as_deref() == Some(want.as_str()))
-                .ok_or_else(|| tpl("arbitrary data record missing"))?;
+            let i = kids.iter().position(|c| is_tdmn(c, &want)).ok_or_else(|| tpl("arbitrary data record missing"))?;
             let list = kids.iter_mut().skip(i + 1).take(3).find(|c| c.is_list(b"aRbs")).ok_or_else(|| tpl("arbitrary data missing"))?;
             let arbp = child_mut(list, b"aRbp")?;
             *data_mut(arbp)? = data.clone();
@@ -1161,7 +1050,7 @@ fn instance(
             let Some(sel) = v.first().map(|x| x.round()).filter(|x| *x >= 1.0 && *x <= 1000.0) else { continue };
             let want = pdefs.get(&p.index).map(|d| d.record.clone()).unwrap_or_default();
             let kids = kids_mut(part)?;
-            if let Some(i) = kids.iter().position(|c| c.is(b"tdmn") && tdmn_name(c).as_deref() == Some(want.as_str()))
+            if let Some(i) = kids.iter().position(|c| is_tdmn(c, &want))
                 && let Some(pard) = kids.get_mut(i + 1)
             {
                 put_u32(data_mut(pard)?, 56, sel as u32)?;
@@ -1187,12 +1076,12 @@ pub(crate) fn build_parade(
     source_size: (u32, u32),
     uses: &mut FxUse,
 ) -> R<Vec<Chunk<'static>>> {
-    let mut kids = vec![Chunk::data(*b"tdsb", vec![0, 0, 0, 1]), tdsn("-_0_/-")];
+    let mut kids = vec![Chunk::data(*b"tdsb", vec![0, 0, 0, 1]), tdsn(NO_NAME)];
     for fx in fxs {
         kids.extend(instance(protos, fx, layer_id, layer_ids, source_size, uses)?);
     }
-    kids.push(Chunk::data(*b"tdmn", tdmn_bytes("ADBE Group End")));
-    Ok(vec![Chunk::data(*b"tdmn", tdmn_bytes("ADBE Effect Parade")), Chunk::list(*b"tdgp", kids)])
+    kids.push(tdmn(crate::export::synth::GROUP_END));
+    Ok(vec![tdmn("ADBE Effect Parade"), Chunk::list(*b"tdgp", kids)])
 }
 
 /// Writes the project-level effect lists: `Pefl` (one `pjef` per type, sorted by match name, 365)
@@ -1219,3 +1108,221 @@ pub(crate) fn put_project_lists(protos: &FxProtos, file: &mut AepFile<'static>,
     file.chunks.insert(at + 1, Chunk::list(*b"EfdG", efdg));
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// The definitions as they were found before: the prototype's `parT` encoded from the table
+    /// and parsed back (the reference for [`ParDef::from_table`]).
+    mod reference {
+        use super::super::*;
+
+        fn be_i32(d: &[u8], at: usize) -> Option<i32> {
+            Some(i32::from_be_bytes(d.get(at..at.checked_add(4)?)?.try_into().ok()?))
+        }
+        fn be_u32(d: &[u8], at: usize) -> Option<u32> {
+            Some(u32::from_be_bytes(d.get(at..at.checked_add(4)?)?.try_into().ok()?))
+        }
+        fn be_f32(d: &[u8], at: usize) -> Option<f64> {
+            Some(f64::from(f32::from_be_bytes(d.get(at..at.checked_add(4)?)?.try_into().ok()?)))
+        }
+        fn be_f64(d: &[u8], at: usize) -> Option<f64> {
+            Some(f64::from_be_bytes(d.get(at..at.checked_add(8)?)?.try_into().ok()?))
+        }
+
+        fn cname(d: &[u8]) -> String {
+            d.iter().take_while(|b| **b != 0).map(|b| char::from(*b)).collect()
+        }
+
+        /// A definition as the decoder read it, with the name and popup items it also read.
+        pub(super) struct Old {
+            pub def: ParDef,
+            pub name: String,
+            pub items: Vec<String>,
+        }
+
+        /// The decoder of an encoded `pard` that `ParDef::from_table` replaces.
+        pub(super) fn from_pard(pard: &[u8], pdnm: Option<&str>, data: Option<&[u8]>) -> Old {
+            let code = be_u32(pard, 12).unwrap_or(u32::MAX);
+            let fx = |at: usize| be_i32(pard, at).map(|v| f64::from(v) / 65536.0);
+            let default: Vec<f64> = match code {
+                10 => be_f64(pard, 56).into_iter().collect(),
+                2 | 3 => fx(56).into_iter().collect(),
+                1 => be_i32(pard, 56).map(f64::from).into_iter().collect(),
+                4 | 7 => be_u32(pard, 56).map(f64::from).into_iter().collect(),
+                // Fractions of the layer, or percent after an After Effects re-save (see the reader).
+                6 => {
+                    let p: Vec<f64> = [fx(56), fx(60)].into_iter().flatten().collect();
+                    if p.iter().any(|c| c.abs() > 1.0 + 1e-9) { p.iter().map(|c| c / 100.0).collect() } else { p }
+                }
+                5 => pard.get(56..60).map(|c| c.iter().map(|b| f64::from(*b)).collect()).unwrap_or_default(),
+                _ => Vec::new(),
+            };
+            let range = match code {
+                10 => be_f32(pard, 112).zip(be_f32(pard, 116)),
+                2 => fx(132).zip(fx(136)),
+                1 => be_i32(pard, 132).map(f64::from).zip(be_i32(pard, 136).map(f64::from)),
+                _ => None,
+            };
+            let valid_max = match code {
+                10 => be_f32(pard, 108),
+                2 => fx(128),
+                1 => be_i32(pard, 128).map(f64::from),
+                _ => None,
+            };
+            let pname = pard.get(16..48).map(cname).unwrap_or_default();
+            let (name, items) = match (code, pdnm) {
+                (7, Some(p)) if !p.is_empty() => (pname, p.split('|').map(str::to_string).collect()),
+                (4, Some(p)) if pname.trim().is_empty() => (p.to_string(), Vec::new()),
+                _ => (pname, Vec::new()),
+            };
+            Old { def: ParDef { record: String::new(), code, default, range, valid_max, data: data.map(<[u8]>::to_vec) }, name, items }
+        }
+
+        /// The parameter definitions of an instance's `parT`, by position (the table's index).
+        pub(super) fn par_defs(sspc: &Chunk<'_>) -> HashMap<u16, Old> {
+            let mut out = HashMap::new();
+            let Some(part) = sspc.child_list(b"parT") else { return out };
+            let k = part.children();
+            let mut i = 0usize;
+            let mut position = 0u16;
+            while i < k.len() {
+                let (Some(m), Some(p)) = (k.get(i), k.get(i + 1)) else { break };
+                if !(m.is(b"tdmn") && p.is(b"pard")) {
+                    i += 1;
+                    continue;
+                }
+                let name = tdmn_name(m).unwrap_or_default();
+                let mut j = i + 2;
+                let mut pdnm = None;
+                let mut data = None;
+                while let Some(c) = k.get(j) {
+                    if c.is(b"tdmn") {
+                        break;
+                    }
+                    if c.is(b"pdnm") {
+                        pdnm = c.children().iter().find(|u| u.is(b"Utf8")).and_then(|u| u.bytes()).map(|b| String::from_utf8_lossy(b).to_string());
+                    }
+                    if c.is(b"aRbp") {
+                        data = c.bytes();
+                    }
+                    j += 1;
+                }
+                if let Some(bytes) = p.bytes() {
+                    let mut old = from_pard(bytes, pdnm.as_deref(), data);
+                    old.def.record = name;
+                    out.insert(position, old);
+                }
+                position = position.saturating_add(1);
+                i = j;
+            }
+            out
+        }
+    }
+
+    fn bits(v: &[f64]) -> Vec<u64> {
+        v.iter().map(|x| x.to_bits()).collect()
+    }
+
+    /// Every field equal, numbers bit for bit.
+    fn assert_same(a: &HashMap<u16, reference::Old>, b: &HashMap<u16, ParDef>, what: &str) {
+        let mut keys: Vec<&u16> = a.keys().collect();
+        keys.sort_unstable();
+        assert_eq!(keys.len(), b.len(), "{what}");
+        for k in keys {
+            let (old, y) = (&a[k], b.get(k).unwrap_or_else(|| panic!("{what}: {k}")));
+            // The prototypes name no parameter and no popup item.
+            assert!(old.name.is_empty() && old.items.is_empty(), "{what}: parameter {k}");
+            let x = &old.def;
+            let range = |r: Option<(f64, f64)>| r.map(|(lo, hi)| (lo.to_bits(), hi.to_bits()));
+            assert_eq!(
+                (&x.record, x.code, bits(&x.default), range(x.range), x.valid_max.map(f64::to_bits), &x.data),
+                (&y.record, y.code, bits(&y.default), range(y.range), y.valid_max.map(f64::to_bits), &y.data),
+                "{what}: parameter {k}"
+            );
+        }
+    }
+
+    #[test]
+    fn the_table_definitions_equal_the_encoded_and_parsed_ones() {
+        for fx in FX_DEFS {
+            let parsed = reference::par_defs(&crate::export::fxsynth::default_sspc(fx));
+            assert_same(&parsed, &ParDef::of_effect(fx), fx.match_name);
+        }
+    }
+
+    #[test]
+    fn a_record_name_that_is_not_ascii_is_found_in_its_prototype() {
+        static PARAMS: [PDef; 1] = [PDef { id: 1, code: 2, default: &[0.0], range: None, valid_max: None, valid_min: None, display: 0 }];
+        let fx = FxDef { match_name: "Effekt Größe", name: "x", params: &PARAMS };
+        let record = ParDef::of_effect(&fx).get(&0).map(|d| d.record.clone()).expect("the parameter");
+        assert_eq!(record, "Effekt Größe-0001");
+        let sspc = crate::export::fxsynth::default_sspc(&fx);
+        let part = sspc.child_list(b"parT").expect("parT");
+        assert!(part.children().iter().any(|c| is_tdmn(c, &record)));
+    }
+
+    #[test]
+    fn generated_definitions_equal_the_encoded_and_parsed_ones() {
+        // Every parameter type with defaults, ranges and maxima that are fractional, negative,
+        // huge, beyond the 16.16 and f32 ranges, and not numbers.
+        let values = [0.0, -0.0, 1.0, -1.0, 0.5, 1.0 / 3.0, 1.01, 100.0, 255.4, 255.6, 256.0, -3.7, 65535.6, 70000.0, 1e9, -1e9, 1e40, f64::NAN, f64::INFINITY];
+        let mut state = 7u64;
+        let mut pick = move |n: usize| {
+            state = state.wrapping_mul(6_364_136_223_846_793_005).wrapping_add(1_442_695_040_888_963_407);
+            usize::try_from(state >> 33).unwrap_or(0) % n
+        };
+        for round in 0..400 {
+            let params: Vec<PDef> = (0..16u16)
+                .map(|id| {
+                    let code = [0u8, 1, 2, 3, 4, 5, 6, 7, 9, 10, 11, 12, 13, 14, 15][pick(15)];
+                    let default: &'static [f64] = Box::leak((0..pick(5)).map(|_| values[pick(values.len())]).collect::<Vec<_>>().into_boxed_slice());
+                    let opt = |pick: &mut dyn FnMut(usize) -> usize| (pick(3) > 0).then(|| values[pick(values.len())]);
+                    let range = opt(&mut pick).map(|lo| (lo, values[pick(values.len())]));
+                    PDef { id, code, default, range, valid_max: opt(&mut pick), valid_min: opt(&mut pick), display: u32::try_from(pick(40)).unwrap_or(0) }
+                })
+                .collect();
+            let fx =
+                FxDef { match_name: Box::leak(format!("Generated {round}").into_boxed_str()), name: "Generated", params: Box::leak(params.into_boxed_slice()) };
+            let parsed = reference::par_defs(&crate::export::fxsynth::default_sspc(&fx));
+            assert_same(&parsed, &ParDef::of_effect(&fx), fx.match_name);
+        }
+    }
+
+    fn group(effect: &str, text: (&str, &str)) -> PropGroup {
+        let spec = effectcraft_effects::find(effect).expect("the effect");
+        let mut next = 1;
+        let mut g = effectcraft_effects::instantiate(spec, &mut effectcraft_project::build::Ids(&mut next), "fx", [100.0, 100.0]);
+        g.prop_mut(text.0).expect("the text parameter").value = Value::Str(text.1.to_string());
+        g
+    }
+
+    fn invalid(col: Collector) -> Vec<String> {
+        col.finish().losses.into_iter().filter(|l| l.category == LossCategory::InvalidValues).flat_map(|l| l.details).collect()
+    }
+
+    #[test]
+    fn unreadable_curve_points_are_reported() {
+        let pdefs = defs().get("ADBE CurvesCustom").expect("the Curves prototype");
+        let mut col = Collector::default();
+        plan_curves(&group("ec.color.curves", ("rgb", "0,0 junk 0.5,0.7 nan,1 1,1")), pdefs, "fx", &mut col);
+        let found = invalid(col);
+        assert!(found.iter().any(|d| d.contains("curve 'rgb': 2 entries")), "{found:?}");
+        let mut col = Collector::default();
+        plan_curves(&group("ec.color.curves", ("rgb", "0,0 0.5,0.7 1,1")), pdefs, "fx", &mut col);
+        assert_eq!(invalid(col), Vec::<String>::new());
+    }
+
+    #[test]
+    fn unreadable_mesh_entries_are_reported() {
+        let pdefs = defs().get("ADBE MESH WARP").expect("the Mesh Warp prototype");
+        let mut col = Collector::default();
+        plan_mesh(&group("ec.distort.meshwarp", ("mesh", "0,0 x,1 2")), pdefs, (100.0, 100.0), "fx", &mut col);
+        let found = invalid(col);
+        assert!(found.iter().any(|d| d.contains("2 entries that are not numbers")), "{found:?}");
+        let mut col = Collector::default();
+        plan_mesh(&group("ec.distort.meshwarp", ("mesh", "0,0 1,1")), pdefs, (100.0, 100.0), "fx", &mut col);
+        assert_eq!(invalid(col), Vec::<String>::new());
+    }
+}

crates/aep/src/export/emit.rs

@@ -7,6 +7,7 @@ use crate::{AepFile, Chunk};
 use effectcraft_project::{ItemId, LayerId};
 
 use crate::export::plan::{CompPlan, Entry, ItemPlan, KeyPlan, LayerPlan, LayerSrc, ProjectPlan, PropKind, PropPlan, SolidPlan, natural_key};
+use crate::export::synth::tdmn_name;
 use crate::export::template::Template;
 use crate::export::tree::{
     R, child_data, data_mut, kids_mut, list_mut, put, put_bits, put_cstr, put_f32, put_f64, put_i32, put_u8, put_u16, put_u32, set_utf8, tpl,
@@ -659,11 +660,6 @@ const GROUP_ORDER: [&str; 13] = [
     "ADBE Group End",
 ];
 
-fn tdmn_name(c: &Chunk<'_>) -> Option<String> {
-    let d = c.bytes()?;
-    Some(d.iter().take_while(|b| **b != 0).map(|b| char::from(*b)).collect())
-}
-
 /// Inserts the properties of `lp` into the layer's Transform group, in the order the file uses.
 fn put_properties(cx: &Cx<'_>, layr: &mut Chunk<'static>, lp: &LayerPlan, cp: &CompPlan) -> R<()> {
     if lp.props.is_empty() {

crates/aep/src/export/fxsynth.rs

@@ -39,7 +39,17 @@ pub(crate) fn value_spec(code: u8, range: Option<(f64, f64)>) -> Option<RecSpec>
 }
 
 fn fixed(v: f64) -> [u8; 4] {
-    ((v * 65536.0).round().clamp(f64::from(i32::MIN), f64::from(i32::MAX)) as i32).to_be_bytes()
+    fixed_i32(v).to_be_bytes()
+}
+
+/// A number as the 16.16 `i32` a `pard` stores it in (rounded, within the `i32` range).
+pub(crate) fn fixed_i32(v: f64) -> i32 {
+    (v * 65536.0).round().clamp(f64::from(i32::MIN), f64::from(i32::MAX)) as i32
+}
+
+/// A number as the `i32` of an integer slider's `pard` (rounded).
+pub(crate) fn int_i32(v: f64) -> i32 {
+    v.round() as i32
 }
 
 /// The 148-byte `pard` of a parameter: the type at 12, the default at 56 and the copies After
@@ -70,7 +80,7 @@ fn pard_layout(p: &PDef, popup_items: Option<u16>) -> Fixed {
             b.put(56, &fixed(d(0)));
             b.put(60, &fixed(d(0)));
         }
-        1 => b.put(56, &(d(0).round() as i32).to_be_bytes()),
+        1 => b.put(56, &int_i32(d(0)).to_be_bytes()),
         // A checkbox: the default again as a byte at 60.
         4 => {
             b.put(56, &(d(0).round().max(0.0) as u32).to_be_bytes());
@@ -115,7 +125,7 @@ fn pard_layout(p: &PDef, popup_items: Option<u16>) -> Fixed {
             }
         }
         1 | 2 => {
-            let enc = |v: f64| if p.code == 2 { fixed(v) } else { (v.round() as i32).to_be_bytes() };
+            let enc = |v: f64| if p.code == 2 { fixed(v) } else { int_i32(v).to_be_bytes() };
             if let Some(m) = p.valid_min {
                 b.put(124, &enc(m));
             }

crates/aep/src/export/plan.rs

@@ -2,7 +2,7 @@
 //! file's units, with everything that cannot be written already reported. Pure data; the chunks
 //! are built from it in `emit`.
 
-use std::collections::{BTreeMap, HashSet};
+use std::collections::{BTreeMap, HashMap, HashSet};
 
 use effectcraft_color::{BlendMode, Label};
 use effectcraft_keyframe::{Ease, Interp, Keyframe};
@@ -11,6 +11,7 @@ use effectcraft_time::{FrameRate, Tick};
 
 use crate::export::report::{Collector, LossCategory};
 use crate::export::timebase::{TimeBase, time_base};
+use crate::import::build::{closes_loop, components};
 
 /// No sensible coordinate, angle or percentage comes near this; larger numbers (and NaN
 /// or infinity) are written as the default.
@@ -352,24 +353,40 @@ pub(crate) fn natural_key(name: &str) -> Vec<(bool, String, u128)> {
     out
 }
 
-fn sort_entries(v: &mut [Entry]) {
-    v.sort_by(|a, b| {
-        let ka = natural_key(&a.name);
-        let kb = natural_key(&b.name);
-        let cmp = |x: &(bool, String, u128), y: &(bool, String, u128)| match (x.0, y.0) {
-            (true, true) => x.2.cmp(&y.2).then_with(|| x.1.cmp(&y.1)),
-            (false, false) => x.1.cmp(&y.1),
-            (true, false) => std::cmp::Ordering::Less,
-            (false, true) => std::cmp::Ordering::Greater,
-        };
-        for (x, y) in ka.iter().zip(kb.iter()) {
-            let c = cmp(x, y);
+/// A [`natural_key`] ordered segment by segment: a number before text, numbers by value and then
+/// by their digits, text by its lowercase string, and a key that is a prefix of the other first.
+/// (The derived order of the tuples is not this one: it puts text first and compares the digits
+/// before the value.)
+#[derive(PartialEq, Eq)]
+struct Natural(Vec<(bool, String, u128)>);
+
+impl Ord for Natural {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        for (x, y) in self.0.iter().zip(other.0.iter()) {
+            let c = match (x.0, y.0) {
+                (true, true) => x.2.cmp(&y.2).then_with(|| x.1.cmp(&y.1)),
+                (false, false) => x.1.cmp(&y.1),
+                (true, false) => std::cmp::Ordering::Less,
+                (false, true) => std::cmp::Ordering::Greater,
+            };
             if c != std::cmp::Ordering::Equal {
                 return c;
             }
         }
-        ka.len().cmp(&kb.len()).then_with(|| a.ec.0.cmp(&b.ec.0))
-    });
+        self.0.len().cmp(&other.0.len())
+    }
+}
+
+impl PartialOrd for Natural {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+/// Sorts by name in natural order, then by project id. Each name's key is built once, not on
+/// every comparison (the sort is stable, as `sort_by` is).
+fn sort_entries(v: &mut [Entry]) {
+    v.sort_by_cached_key(|e| (Natural(natural_key(&e.name)), e.ec.0));
 }
 
 /// Plans the whole project.
@@ -408,7 +425,8 @@ pub(crate) fn plan(project: &Project, col: &mut Collector) -> ProjectPlan {
             by_parent.entry(item.parent.filter(|p| project.items.contains_key(p))).or_default().push(*id);
         }
     }
-    let cx = PlanCx { project, by_parent: &by_parent, ok: &ok };
+    let component = components(&precomp_links(project));
+    let cx = PlanCx { project, by_parent: &by_parent, ok: &ok, component: &component };
     let root = plan_children(&cx, None, col);
     // The base writes an empty render queue.
     for job in &project.render_queue {
@@ -422,6 +440,8 @@ struct PlanCx<'a> {
     project: &'a Project,
     by_parent: &'a BTreeMap<Option<ItemId>, Vec<ItemId>>,
     ok: &'a HashSet<ItemId>,
+    /// The component of each composition in the precomposition links ([`precomp_loop`]).
+    component: &'a HashMap<ItemId, usize>,
 }
 
 fn plan_children(cx: &PlanCx<'_>, parent: Option<ItemId>, col: &mut Collector) -> Vec<Entry> {
@@ -627,16 +647,17 @@ fn plan_comp(cx: &PlanCx<'_>, comp_id: ItemId, name: &str, comp: &Comp, col: &mu
             cp.layers.push(lp);
         }
     }
-    resolve_parents(&mut cp, name, col);
-    resolve_mattes(&mut cp, name, col);
-    resolve_effect_layers(&mut cp, name, col);
+    // The layers that are written; none of the three steps below adds or removes one.
+    let exported: HashSet<LayerId> = cp.layers.iter().map(|l| l.ec_id).collect();
+    resolve_parents(&mut cp, &exported, name, col);
+    resolve_mattes(&mut cp, &exported, name, col);
+    resolve_effect_layers(&mut cp, &exported, name, col);
     cp
 }
 
 /// An effect's layer parameter that names a layer that is not exported falls back to the owning
 /// layer (After Effects' default) and is reported.
-fn resolve_effect_layers(cp: &mut CompPlan, comp_name: &str, col: &mut Collector) {
-    let exported: HashSet<LayerId> = cp.layers.iter().map(|l| l.ec_id).collect();
+fn resolve_effect_layers(cp: &mut CompPlan, exported: &HashSet<LayerId>, comp_name: &str, col: &mut Collector) {
     for l in &mut cp.layers {
         for fx in &mut l.fx {
             let before = fx.layers.len();
@@ -655,8 +676,7 @@ fn resolve_effect_layers(cp: &mut CompPlan, comp_name: &str, col: &mut Collector
 }
 
 /// A track matte whose layer is not exported, or is the layer itself, is dropped and reported.
-fn resolve_mattes(cp: &mut CompPlan, comp_name: &str, col: &mut Collector) {
-    let exported: HashSet<LayerId> = cp.layers.iter().map(|l| l.ec_id).collect();
+fn resolve_mattes(cp: &mut CompPlan, exported: &HashSet<LayerId>, comp_name: &str, col: &mut Collector) {
     for l in &mut cp.layers {
         if let Some((m, _)) = l.matte
             && (m == l.ec_id || !exported.contains(&m))
@@ -668,17 +688,95 @@ fn resolve_mattes(cp: &mut CompPlan, comp_name: &str, col: &mut Collector) {
 }
 
 /// Parent links to a layer that was not exported, and links that lead into a loop, are cut.
-fn resolve_parents(cp: &mut CompPlan, comp_name: &str, col: &mut Collector) {
-    let exported: HashSet<LayerId> = cp.layers.iter().map(|l| l.ec_id).collect();
-    let mut links: std::collections::HashMap<LayerId, LayerId> = cp.layers.iter().filter_map(|l| l.parent.map(|p| (l.ec_id, p))).collect();
-    for l in &mut cp.layers {
-        let Some(p) = l.parent else { continue };
+fn resolve_parents(cp: &mut CompPlan, exported: &HashSet<LayerId>, comp_name: &str, col: &mut Collector) {
+    let links: Vec<(LayerId, Option<LayerId>)> = cp.layers.iter().map(|l| (l.ec_id, l.parent)).collect();
+    let cut = parents_to_cut(&links, exported);
+    for (l, cut) in cp.layers.iter_mut().zip(cut) {
+        if cut {
+            col.lose(LossCategory::ParentLinks, format!("{comp_name}: '{}' has a parent that was not exported or forms a loop", l.name));
+            l.parent = None;
+        }
+    }
+}
+
+/// Which parent links (`(layer, parent)` in stack order) are cut: one to a layer that is not
+/// exported, and, of each loop, the link of its first layer in stack order (cutting it opens
+/// the loop, so the others stay).
+///
+/// Each parent chain used to be walked from every layer (a chain of N layers took N² steps).
+/// With unique layer ids each layer has one link, the links form chains and disjoint loops, and
+/// the layers on a loop are found once ([`loop_of`]). Repeated ids (not made by EffectCraft)
+/// keep the walk ([`parents_to_cut_by_walk`]).
+fn parents_to_cut(links: &[(LayerId, Option<LayerId>)], exported: &HashSet<LayerId>) -> Vec<bool> {
+    if exported.len() != links.len() {
+        return parents_to_cut_by_walk(links, exported);
+    }
+    let on_loop = loop_of(&links.iter().filter_map(|(l, p)| p.map(|p| (*l, p))).collect());
+    let mut opened: HashSet<usize> = HashSet::new();
+    links
+        .iter()
+        .map(|(l, p)| match p {
+            None => false,
+            Some(p) if !exported.contains(p) => true,
+            Some(_) => on_loop.get(l).is_some_and(|lp| opened.insert(*lp)),
+        })
+        .collect()
+}
+
+/// The loop each layer is on (a number per loop), for the links `layer -> parent`. Each layer is
+/// visited once.
+fn loop_of(links: &HashMap<LayerId, LayerId>) -> HashMap<LayerId, usize> {
+    // Absent: not seen; `false`: on the chain being walked; `true`: finished.
+    let mut done: HashMap<LayerId, bool> = HashMap::with_capacity(links.len());
+    let mut on_loop: HashMap<LayerId, usize> = HashMap::new();
+    let mut loops = 0usize;
+    for &start in links.keys() {
+        let mut chain = Vec::new();
+        let mut at = start;
+        loop {
+            match done.get(&at) {
+                Some(false) => {
+                    // Back on the chain: the part from `at` on is a loop.
+                    if let Some(i) = chain.iter().position(|l| *l == at) {
+                        for l in chain.get(i..).unwrap_or_default() {
+                            on_loop.insert(*l, loops);
+                        }
+                        loops = loops.saturating_add(1);
+                    }
+                    break;
+                }
+                Some(true) => break,
+                None => {}
+            }
+            done.insert(at, false);
+            chain.push(at);
+            match links.get(&at) {
+                Some(next) => at = *next,
+                None => break,
+            }
+        }
+        for l in chain {
+            done.insert(l, true);
+        }
+    }
+    on_loop
+}
+
+/// [`parents_to_cut`] by following each chain from its layer (for repeated layer ids): a chain
+/// longer than the number of links is a loop.
+fn parents_to_cut_by_walk(layers: &[(LayerId, Option<LayerId>)], exported: &HashSet<LayerId>) -> Vec<bool> {
+    let mut links: HashMap<LayerId, LayerId> = layers.iter().filter_map(|(l, p)| p.map(|p| (*l, p))).collect();
+    let mut out = Vec::with_capacity(layers.len());
+    for (l, p) in layers {
+        let Some(p) = *p else {
+            out.push(false);
+            continue;
+        };
         let mut cut = !exported.contains(&p);
         if !cut {
-            // Follow the chain; a chain longer than the number of links is a loop.
             let mut at = p;
             for _ in 0..=links.len() {
-                if at == l.ec_id {
+                if at == *l {
                     cut = true;
                     break;
                 }
@@ -687,16 +785,16 @@ fn resolve_parents(cp: &mut CompPlan, comp_name: &str, col: &mut Collector) {
                     None => break,
                 }
             }
-            if !cut && at == l.ec_id {
+            if !cut && at == *l {
                 cut = true;
             }
         }
         if cut {
-            col.lose(LossCategory::ParentLinks, format!("{comp_name}: '{}' has a parent that was not exported or forms a loop", l.name));
-            links.remove(&l.ec_id);
-            l.parent = None;
+            links.remove(l);
         }
+        out.push(cut);
     }
+    out
 }
 
 /// The source of a layer and its size; `None` (with a loss) for a layer that is not written.
@@ -761,7 +859,7 @@ fn plan_source(
             }
         },
         LayerSource::Comp { item } => match cx.project.item(*item).filter(|_| cx.ok.contains(item)).map(|i| &i.kind) {
-            Some(ItemKind::Comp(c)) if !precomp_loop(cx.project, comp_id, *item) => {
+            Some(ItemKind::Comp(c)) if !precomp_loop(cx.component, comp_id, *item) => {
                 (LayerSrc::Comp(*item), (c.width.clamp(1, MAX_SIDE), c.height.clamp(1, MAX_SIDE)))
             }
             Some(ItemKind::Comp(_)) => {
@@ -1115,27 +1213,28 @@ fn has_bezier_keys(g: &effectcraft_project::PropGroup, depth: usize) -> bool {
     })
 }
 
-/// Whether showing the composition `to` inside `from` closes a loop (`to` is `from`, or uses it
-/// through its own precomposition layers). The walk is iterative and visits each item once.
-fn precomp_loop(project: &Project, from: ItemId, to: ItemId) -> bool {
-    let mut seen: HashSet<ItemId> = HashSet::new();
-    let mut stack = vec![to];
-    while let Some(at) = stack.pop() {
-        if at == from {
-            return true;
-        }
-        if !seen.insert(at) {
-            continue;
-        }
-        if let Some(ItemKind::Comp(c)) = project.item(at).map(|i| &i.kind) {
-            for l in &c.layers {
+/// Every precomposition link of the project once: composition -> the items its layers show.
+fn precomp_links(project: &Project) -> HashMap<ItemId, HashSet<ItemId>> {
+    let mut links: HashMap<ItemId, HashSet<ItemId>> = HashMap::new();
+    for (id, item) in &project.items {
+        if let ItemKind::Comp(c) = &item.kind {
+            for l in c.layers.iter() {
                 if let LayerSource::Comp { item } = &l.source {
-                    stack.push(*item);
+                    links.entry(*id).or_default().insert(*item);
                 }
             }
         }
     }
-    false
+    links
+}
+
+/// Whether showing the composition `to` in `from` closes a loop: `to` is `from` or uses it
+/// through its own precomposition layers. The layer itself links `from` to `to`, so `to` uses
+/// `from` exactly when both are in one component of the project's precomposition links
+/// (`component`, found once per export). Before, every precomposition layer walked the layers of
+/// every composition below its source (N layers of a precomposition with N layers took N² steps).
+fn precomp_loop(component: &HashMap<ItemId, usize>, from: ItemId, to: ItemId) -> bool {
+    closes_loop(from, to, |to| matches!((component.get(&from), component.get(&to)), (Some(a), Some(b)) if a == b))
 }
 
 /// The names of a layer's Material Options that differ from a new layer's (value, keys or an
@@ -1341,3 +1440,212 @@ fn plan_remap(p: &Property, comp: &Comp, cp: &CompPlan, layer: &str, comp_name:
     }
     plan
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// The comparator as it was written before the keys were cached (the reference).
+    fn old_cmp(a: &str, b: &str) -> std::cmp::Ordering {
+        let (ka, kb) = (natural_key(a), natural_key(b));
+        let cmp = |x: &(bool, String, u128), y: &(bool, String, u128)| match (x.0, y.0) {
+            (true, true) => x.2.cmp(&y.2).then_with(|| x.1.cmp(&y.1)),
+            (false, false) => x.1.cmp(&y.1),
+            (true, false) => std::cmp::Ordering::Less,
+            (false, true) => std::cmp::Ordering::Greater,
+        };
+        for (x, y) in ka.iter().zip(kb.iter()) {
+            let c = cmp(x, y);
+            if c != std::cmp::Ordering::Equal {
+                return c;
+            }
+        }
+        ka.len().cmp(&kb.len())
+    }
+
+    /// A pseudo-random generator for the differential tests.
+    fn lcg(seed: u64) -> impl FnMut(u64) -> u64 {
+        let mut state = seed;
+        move |n| {
+            state = state.wrapping_mul(6_364_136_223_846_793_005).wrapping_add(1_442_695_040_888_963_407);
+            (state >> 33) % n.max(1)
+        }
+    }
+
+    #[test]
+    fn parent_links_are_cut_as_the_walk_cut_them() {
+        for seed in 0..500 {
+            let mut next = lcg(seed);
+            let n = 1 + next(12);
+            // Unique ids, or (one seed in five) some repeated; parents among the layers, missing
+            // ones (not exported) and none.
+            let ids: Vec<LayerId> = (0..n).map(|i| LayerId(if seed % 5 == 0 { next(n) } else { i })).collect();
+            let links: Vec<(LayerId, Option<LayerId>)> = ids
+                .iter()
+                .map(|l| {
+                    let p = match next(4) {
+                        0 => None,
+                        1 => Some(LayerId(100 + next(3))),
+                        _ => Some(LayerId(next(n))),
+                    };
+                    (*l, p)
+                })
+                .collect();
+            let exported: HashSet<LayerId> = ids.iter().copied().collect();
+            assert_eq!(parents_to_cut(&links, &exported), parents_to_cut_by_walk(&links, &exported), "{links:?}");
+        }
+    }
+
+    #[test]
+    fn a_long_parent_chain_is_resolved_in_linear_time() {
+        // 200 000 layers each parented to the next and the last to the first: one link is cut.
+        // Walking the chain from every layer (as before) is 4e10 steps.
+        let n = 200_000u64;
+        let links: Vec<(LayerId, Option<LayerId>)> = (0..n).map(|i| (LayerId(i), Some(LayerId((i + 1) % n)))).collect();
+        let exported: HashSet<LayerId> = (0..n).map(LayerId).collect();
+        let cut = parents_to_cut(&links, &exported);
+        assert_eq!(cut.iter().filter(|c| **c).count(), 1);
+        assert!(cut[0]);
+    }
+
+    /// The loop check as it was (the reference): a walk from `to` over the layers of every
+    /// composition below it.
+    fn old_precomp_loop(project: &Project, from: ItemId, to: ItemId) -> bool {
+        let mut seen: HashSet<ItemId> = HashSet::new();
+        let mut stack = vec![to];
+        while let Some(at) = stack.pop() {
+            if at == from {
+                return true;
+            }
+            if !seen.insert(at) {
+                continue;
+            }
+            if let Some(ItemKind::Comp(c)) = project.item(at).map(|i| &i.kind) {
+                for l in c.layers.iter() {
+                    if let LayerSource::Comp { item } = &l.source {
+                        stack.push(*item);
+                    }
+                }
+            }
+        }
+        false
+    }
+
+    /// A project with `comps` compositions; composition `i` shows the items `shows(i)` (repeats
+    /// and non-compositions allowed), as layers.
+    fn project_of(comps: usize, shows: &mut dyn FnMut(usize) -> Vec<usize>) -> (Project, Vec<ItemId>) {
+        use effectcraft_project::build;
+        let mut p = Project::default();
+        let solid = p.add_item(
+            "S",
+            effectcraft_color::Label::None,
+            None,
+            ItemKind::Solid(effectcraft_project::Solid { color: [0.0; 3], width: 4, height: 4, pixel_aspect: 1.0 }),
+        );
+        let ids: Vec<ItemId> = (0..comps)
+            .map(|i| {
+                let c = Comp::new(4, 4, effectcraft_time::FrameRate::new(25, 1), Tick(effectcraft_time::TICKS_PER_SECOND));
+                p.add_item(&format!("C{i}"), effectcraft_color::Label::None, None, ItemKind::Comp(std::sync::Arc::new(c)))
+            })
+            .collect();
+        for (i, id) in ids.iter().enumerate() {
+            let Some(mut c) = p.comp(*id).cloned() else { continue };
+            for s in shows(i) {
+                let source = match ids.get(s) {
+                    Some(item) => LayerSource::Comp { item: *item },
+                    None => LayerSource::Solid { item: solid },
+                };
+                let l = build::layer(&mut p, &c, "L", source, (4, 4), None);
+                c.layers.push(l);
+            }
+            if let Some(item) = p.item_mut(*id) {
+                item.kind = ItemKind::Comp(std::sync::Arc::new(c));
+            }
+        }
+        (p, ids)
+    }
+
+    #[test]
+    fn precomp_loops_are_found_as_the_walk_found_them() {
+        for seed in 0..300 {
+            let mut next = lcg(seed);
+            let comps = 1 + usize::try_from(next(10)).unwrap_or(0);
+            let (p, ids) = project_of(comps, &mut |_| (0..next(6)).map(|_| usize::try_from(next(comps as u64 + 2)).unwrap_or(0)).collect());
+            let component = components(&precomp_links(&p));
+            for from in &ids {
+                let Some(c) = p.comp(*from) else { continue };
+                for l in c.layers.iter() {
+                    if let LayerSource::Comp { item } = &l.source {
+                        assert_eq!(precomp_loop(&component, *from, *item), old_precomp_loop(&p, *from, *item), "{from:?} -> {item:?}");
+                    }
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn layered_precomps_are_checked_in_linear_time() {
+        // K top compositions each show K middle ones, each of which shows K leaves: the old walk
+        // from every layer took K^3 steps; the components are found once.
+        let k = 60;
+        let (p, ids) = project_of(3 * k, &mut |i| if i < 2 * k { (0..k).map(|j| if i < k { k + j } else { 2 * k + j }).collect() } else { Vec::new() });
+        let component = components(&precomp_links(&p));
+        let mut checked = 0;
+        for from in &ids {
+            let Some(c) = p.comp(*from) else { continue };
+            for l in c.layers.iter() {
+                if let LayerSource::Comp { item } = &l.source {
+                    assert!(!precomp_loop(&component, *from, *item));
+                    checked += 1;
+                }
+            }
+        }
+        assert_eq!(checked, 2 * k * k);
+        // One more layer, a leaf showing a top composition, closes a loop through all three.
+        let (p, ids) = project_of(3 * k, &mut |i| match i {
+            _ if i < k => (k..2 * k).collect(),
+            _ if i < 2 * k => (2 * k..3 * k).collect(),
+            _ if i == 2 * k => vec![0],
+            _ => Vec::new(),
+        });
+        let component = components(&precomp_links(&p));
+        assert!(precomp_loop(&component, ids[2 * k], ids[0]));
+        assert!(precomp_loop(&component, ids[0], ids[k]));
+        assert!(!precomp_loop(&component, ids[k], ids[2 * k + 1]));
+    }
+
+    #[test]
+    fn the_cached_natural_key_orders_as_the_comparator_did() {
+        let names = [
+            "",
+            "a",
+            "A",
+            "a1",
+            "a01",
+            "a001",
+            "a2",
+            "a10",
+            "1",
+            "01",
+            "001",
+            "10",
+            "1a",
+            "a b",
+            "a-b",
+            "Comp 2",
+            "comp 10",
+            "Comp 1 copy",
+            "ß",
+            "Ä 3",
+            "x99999999999999999999999999999999999999999999",
+            "x100000000000000000000000000000000000000000000",
+            "Solids",
+            "solids 0",
+        ];
+        for a in names {
+            for b in names {
+                assert_eq!(Natural(natural_key(a)).cmp(&Natural(natural_key(b))), old_cmp(a, b), "{a:?} {b:?}");
+            }
+        }
+    }
+}

crates/aep/src/import.rs

@@ -24,7 +24,7 @@
 //! for wasm32; it reads no files itself: the caller passes the bytes, a way to ask whether a
 //! footage path exists and one to ask whether a font is installed.
 
-mod build;
+pub(crate) mod build;
 mod camera;
 mod effects;
 mod lumetri;

crates/aep/src/import/build.rs

@@ -56,7 +56,7 @@ struct PendingComp<'f, 'a> {
 struct Ctx<'o> {
     sources: HashMap<u32, Source>,
     /// Which comps use which comps as precomps (to refuse loops).
-    uses: HashMap<ItemId, Vec<ItemId>>,
+    precomps: Precomps,
     created: Vec<ItemId>,
     col: Collector,
     options: &'o ImportOptions<'o>,
@@ -69,7 +69,7 @@ struct Ctx<'o> {
 pub(crate) fn run(project: &mut Project, view: &AepProject<'_, '_>, options: &ImportOptions<'_>) -> (ItemId, Vec<ItemId>, Vec<ItemId>, ImportReport) {
     let mut cx = Ctx {
         sources: HashMap::new(),
-        uses: HashMap::new(),
+        precomps: Precomps::default(),
         created: Vec::new(),
         col: Collector::default(),
         options,
@@ -101,9 +101,9 @@ pub(crate) fn run(project: &mut Project, view: &AepProject<'_, '_>, options: &Im
                 // After Effects writes an `Sfdr` even for an empty folder (and other chunks next to the
                 // items in it); without one the contents are lost.
                 if !item.has_folder_contents() {
-                    cx.col.lose(LossCategory::DamagedData, format!("folder '{name}': its contents list is missing, imported empty"));
+                    cx.col.lose_with(LossCategory::DamagedData, || format!("folder '{name}': its contents list is missing, imported empty"));
                 } else if item.folder_contents_unreadable() {
-                    cx.col.lose(LossCategory::DamagedData, format!("folder '{name}': its contents list is damaged, imported empty"));
+                    cx.col.lose_with(LossCategory::DamagedData, || format!("folder '{name}': its contents list is damaged, imported empty"));
                 }
                 let mut kids: Vec<_> = item.children().map(|k| (k, id)).collect();
                 kids.reverse();
@@ -112,16 +112,16 @@ pub(crate) fn run(project: &mut Project, view: &AepProject<'_, '_>, options: &Im
             Some(AepKind::Composition) => {
                 let name = if name.is_empty() { "Composition".to_string() } else { name };
                 let Some(comp_view) = item.comp() else {
-                    cx.col.lose(LossCategory::DamagedData, format!("composition '{name}': composition data missing, not imported"));
+                    cx.col.lose_with(LossCategory::DamagedData, || format!("composition '{name}': composition data missing, not imported"));
                     continue;
                 };
                 let Some(header) = comp_view.header() else {
-                    cx.col.lose(LossCategory::DamagedData, format!("composition '{name}': header unreadable, not imported"));
+                    cx.col.lose_with(LossCategory::DamagedData, || format!("composition '{name}': header unreadable, not imported"));
                     continue;
                 };
                 let tb = header.time_base;
                 let Some(rate) = frame_rate(tb.timescale, tb.frame_duration) else {
-                    cx.col.lose(LossCategory::DamagedData, format!("composition '{name}': time base unreadable, not imported"));
+                    cx.col.lose_with(LossCategory::DamagedData, || format!("composition '{name}': time base unreadable, not imported"));
                     continue;
                 };
                 let map = TimeMap { timescale: tb.timescale, frame_duration: tb.frame_duration, rate };
@@ -137,12 +137,13 @@ pub(crate) fn run(project: &mut Project, view: &AepProject<'_, '_>, options: &Im
                 pending.push(PendingComp { id, item, map });
             }
             Some(AepKind::Footage) => import_footage(project, &mut cx, &item, name, label, parent, &null_only),
-            Some(AepKind::Other(code)) => cx.col.lose(LossCategory::UnsupportedItems, format!("'{name}': item type {code}")),
+            Some(AepKind::Other(code)) => cx.col.lose_with(LossCategory::UnsupportedItems, || format!("'{name}': item type {code}")),
             None => cx.col.lose(LossCategory::DamagedData, "a project item without a readable type".to_string()),
         }
     }
 
     // Pass 2: the layers. The effect definitions are indexed once for every layer of every comp.
+    cx.precomps = Precomps::new(&precomp_candidates(&cx.sources, &pending));
     let defs = view.effect_definition_index();
     for p in &pending {
         build_layers(project, &mut cx, &defs, p);
@@ -185,12 +186,12 @@ fn new_comp(h: &CompHeader, renderer: Option<AepRenderer>, map: &TimeMap, name:
     comp.work_area = if start >= Tick::ZERO && end > start {
         (start, end)
     } else {
-        col.lose(LossCategory::DamagedData, format!("composition '{name}': the work area is empty or negative, the whole composition is used"));
+        col.lose_with(LossCategory::DamagedData, || format!("composition '{name}': the work area is empty or negative, the whole composition is used"));
         (Tick::ZERO, duration)
     };
     comp.background = h.background.map(|c| f32::from(c) / 255.0);
     comp.pixel_aspect = h.pixel_aspect.to_f64().filter(|v| v.is_finite() && *v > 0.0).unwrap_or_else(|| {
-        col.lose(LossCategory::DamagedData, format!("composition '{name}': the pixel aspect ratio is not valid, 1 is used"));
+        col.lose_with(LossCategory::DamagedData, || format!("composition '{name}': the pixel aspect ratio is not valid, 1 is used"));
         1.0
     });
     comp.shutter_angle = f64::from(h.shutter_angle);
@@ -210,7 +211,7 @@ fn new_comp(h: &CompHeader, renderer: Option<AepRenderer>, map: &TimeMap, name:
 /// Unreadable footage is recorded as a loss, solids and null placeholders are handled separately, and file and sequence footage is linked to its files.
 fn import_footage(project: &mut Project, cx: &mut Ctx<'_>, item: &AepItem<'_, '_>, name: String, label: Label, parent: ItemId, null_only: &HashSet<u32>) {
     let Some(f) = item.footage() else {
-        cx.col.lose(LossCategory::DamagedData, format!("footage '{name}': data unreadable, not imported"));
+        cx.col.lose_with(LossCategory::DamagedData, || format!("footage '{name}': data unreadable, not imported"));
         return;
     };
     let aep_id = item.id();
@@ -229,7 +230,7 @@ fn import_footage(project: &mut Project, cx: &mut Ctx<'_>, item: &AepItem<'_, '_
         let mut rgb = f.solid_rgb().unwrap_or([0.0; 3]);
         if rgb.iter().any(|c| !c.is_finite()) {
             rgb = rgb.map(|c| if c.is_finite() { c } else { 0.0 });
-            cx.col.lose(LossCategory::DamagedData, format!("solid '{name}': a color channel is not a number, replaced by 0"));
+            cx.col.lose_with(LossCategory::DamagedData, || format!("solid '{name}': a color channel is not a number, replaced by 0"));
         }
         let solid = Solid { color: rgb, width: f.width, height: f.height, pixel_aspect: 1.0 };
         let id = project.add_item(&name, label, Some(parent), ItemKind::Solid(solid));
@@ -262,10 +263,9 @@ fn import_footage(project: &mut Project, cx: &mut Ctx<'_>, item: &AepItem<'_, '_
         let bytes = len(folder.len().saturating_add(1)).saturating_mul(u64::from(frames)).saturating_add(file_bytes);
         if frames > MAX_SEQUENCE_FRAMES || frames > cx.sequence_budget || bytes > cx.sequence_bytes {
             // Not expanded: the item keeps its first file (a still standing for the sequence).
-            cx.col.lose(
-                LossCategory::DamagedData,
-                format!("footage '{name}': a sequence of {frames} frames is longer than the import expands; only its first file is linked"),
-            );
+            cx.col.lose_with(LossCategory::DamagedData, || {
+                format!("footage '{name}': a sequence of {frames} frames is longer than the import expands; only its first file is linked")
+            });
             path = format!("{folder}{sep}{}", seq.nth_file_name(0).unwrap_or_default());
         } else {
             cx.sequence_budget -= frames;
@@ -277,7 +277,7 @@ fn import_footage(project: &mut Project, cx: &mut Ctx<'_>, item: &AepItem<'_, '_
         }
     } else if f.is_sequence && !path.is_empty() {
         // The alias is a folder, but the file names could not be read: the item links the folder.
-        cx.col.lose(LossCategory::DamagedData, format!("footage '{name}': image sequence naming not read, linked to its folder"));
+        cx.col.lose_with(LossCategory::DamagedData, || format!("footage '{name}': image sequence naming not read, linked to its folder"));
     }
     let present = !path.is_empty() && (cx.options.exists)(&path);
     if !present {
@@ -478,10 +478,13 @@ fn build_layers(project: &mut Project, cx: &mut Ctx<'_>, defs: &EffectDefinition
     let mut layer_refs: Vec<PendingLayerRef> = Vec::new();
     // The id of the layer above in the file (the matte of the older "layer above" form).
     let mut above: Option<u32> = None;
+    // The comps that use this one, directly or through others: a precomp layer of one of them
+    // would close a loop. Found once, at the first precomp layer (see `Precomps`).
+    let mut users: Option<HashSet<ItemId>> = None;
 
     for layer in comp_view.layers() {
         let Some(h) = layer.header() else {
-            cx.col.lose(LossCategory::DamagedData, format!("{comp_name}: a layer header is unreadable, layer not imported"));
+            cx.col.lose_with(LossCategory::DamagedData, || format!("{comp_name}: a layer header is unreadable, layer not imported"));
             above = None;
             continue;
         };
@@ -545,11 +548,10 @@ fn build_layers(project: &mut Project, cx: &mut Ctx<'_>, defs: &EffectDefinition
                 SourceKind::Solid => (LayerSource::Solid { item: s.item }, (s.width.max(1), s.height.max(1))),
                 SourceKind::Footage => (LayerSource::Footage { item: s.item }, (s.width.max(1), s.height.max(1))),
                 SourceKind::Comp => {
-                    if would_loop(&cx.uses, p.id, s.item) {
+                    if !cx.precomps.accept(p.id, s.item, &mut users) {
                         skip(cx, &mut skipped, LossCategory::UnsupportedLayers, "precomposition loop, layer");
                         continue;
                     }
-                    cx.uses.entry(p.id).or_default().push(s.item);
                     (LayerSource::Comp { item: s.item }, (s.width.max(1), s.height.max(1)))
                 }
             },
@@ -571,7 +573,7 @@ fn build_layers(project: &mut Project, cx: &mut Ctx<'_>, defs: &EffectDefinition
             Some(b) => l.blend_mode = b,
             // Cameras and lights have no blend mode (their byte is 0, 251).
             None if is_camera || light.is_some() => {}
-            None => cx.col.lose(LossCategory::BlendModes, format!("{comp_name}: '{name}' uses blend mode code {}", h.blend_mode.code())),
+            None => cx.col.lose_with(LossCategory::BlendModes, || format!("{comp_name}: '{name}' uses blend mode code {}", h.blend_mode.code())),
         }
         let what = format!("{comp_name}: '{name}'");
         l.markers = import_markers(layer.markers(), layer.chunk(), &what, &tm, &mut cx.col);
@@ -640,9 +642,13 @@ fn build_layers(project: &mut Project, cx: &mut Ctx<'_>, defs: &EffectDefinition
         comp.layers.push(l);
     }
 
-    resolve_parents(&mut comp, &parents, &ids, &skipped, &comp_name, &mut cx.col);
-    resolve_mattes(&mut comp, &mattes, &ids, &skipped, &comp_name, &mut cx.col);
-    resolve_layer_refs(&mut comp, &layer_refs, &ids, &mut cx.col);
+    // Project layer id -> position in `comp.layers` (the first, as `Comp::layer_mut` finds it):
+    // one lookup per link instead of a scan of the layers, which made a composition of many
+    // parented, matted or referenced layers quadratic.
+    let at = layer_positions(&comp);
+    resolve_parents(&mut comp, &at, &parents, &ids, &skipped, &comp_name, &mut cx.col);
+    resolve_mattes(&mut comp, &at, &mattes, &ids, &skipped, &comp_name, &mut cx.col);
+    resolve_layer_refs(&mut comp, &at, &layer_refs, &ids, &mut cx.col);
     // Global Light is one setting per composition here: the layers' stored values are adopted.
     effectcraft_project::styles::sync_comp(&mut comp);
 
@@ -662,10 +668,10 @@ fn import_source_text(l: &mut Layer, layer: &AepLayer<'_, '_>, what: &str, cx: &
             let value = text_doc(&doc, what, cx.options.has_font, &mut cx.col);
             match l.props.prop_mut("text/sourceText") {
                 Some(p) => p.value = Value::Text(Box::new(value)),
-                None => cx.col.lose(LossCategory::DamagedData, format!("{what}: the text layer has no Source Text property, the text is not imported")),
+                None => cx.col.lose_with(LossCategory::DamagedData, || format!("{what}: the text layer has no Source Text property, the text is not imported")),
             }
         }
-        None => cx.col.lose(LossCategory::DamagedData, format!("{what}: the text document is unreadable, the layer is imported with empty text")),
+        None => cx.col.lose_with(LossCategory::DamagedData, || format!("{what}: the text document is unreadable, the layer is imported with empty text")),
     }
     report_text_extras(layer, what, &mut cx.col);
 }
@@ -679,33 +685,176 @@ fn import_text_animators(l: &mut Layer, layer: &AepLayer<'_, '_>, what: &str, tm
     }
     match l.props.group_mut("text/animators") {
         Some(target) => import_animators(&animators, what, tm, ids, target, col),
-        None => col.lose(LossCategory::TextLayers, format!("{what}: the text layer has no animator group, its text animators are not imported")),
+        None => col.lose_with(LossCategory::TextLayers, || format!("{what}: the text layer has no animator group, its text animators are not imported")),
     }
 }
 
-/// Whether adding `to` as a precomp of `from` would close a loop.
-fn would_loop(uses: &HashMap<ItemId, Vec<ItemId>>, from: ItemId, to: ItemId) -> bool {
-    if from == to {
-        return true;
+/// Whether a precomp layer of `to` in `from` closes a loop: `to` is `from`, or `to` already uses
+/// `from` (`uses_from`, directly or through others). Shared by the import and the export.
+pub(crate) fn closes_loop(from: ItemId, to: ItemId, uses_from: impl FnOnce(ItemId) -> bool) -> bool {
+    to == from || uses_from(to)
+}
+
+/// The walk's own stack in [`components`]: a node, the nodes it links to and how many are done.
+type Walk = Vec<(ItemId, Vec<ItemId>, usize)>;
+
+/// The strongly connected components of the graph `edges` (node -> the nodes it links to): each
+/// node's component number (every node of a loop has the same one). Iterative (Tarjan), linear in
+/// nodes and links.
+pub(crate) fn components(edges: &HashMap<ItemId, HashSet<ItemId>>) -> HashMap<ItemId, usize> {
+    struct Tarjan<'e> {
+        edges: &'e HashMap<ItemId, HashSet<ItemId>>,
+        index: HashMap<ItemId, usize>,
+        low: HashMap<ItemId, usize>,
+        on_stack: HashSet<ItemId>,
+        stack: Vec<ItemId>,
+        counter: usize,
     }
-    let mut seen = HashSet::new();
-    let mut stack = vec![to];
-    while let Some(c) = stack.pop() {
-        if c == from {
-            return true;
-        }
-        if seen.insert(c) {
-            stack.extend(uses.get(&c).into_iter().flatten().copied());
+    impl Tarjan<'_> {
+        fn enter(&mut self, n: ItemId, walk: &mut Walk) {
+            self.index.insert(n, self.counter);
+            self.low.insert(n, self.counter);
+            self.counter = self.counter.saturating_add(1);
+            self.stack.push(n);
+            self.on_stack.insert(n);
+            walk.push((n, self.edges.get(&n).map(|s| s.iter().copied().collect()).unwrap_or_default(), 0));
         }
     }
-    false
+    let mut t = Tarjan { edges, index: HashMap::new(), low: HashMap::new(), on_stack: HashSet::new(), stack: Vec::new(), counter: 0 };
+    let mut component: HashMap<ItemId, usize> = HashMap::new();
+    let mut found = 0usize;
+    for &root in edges.keys() {
+        if t.index.contains_key(&root) {
+            continue;
+        }
+        let mut walk: Walk = Vec::new();
+        t.enter(root, &mut walk);
+        while let Some((v, next, done)) = walk.last_mut() {
+            let v = *v;
+            if let Some(w) = next.get(*done).copied() {
+                *done += 1;
+                if !t.index.contains_key(&w) {
+                    t.enter(w, &mut walk);
+                } else if t.on_stack.contains(&w) {
+                    let iw = t.index.get(&w).copied().unwrap_or(usize::MAX);
+                    if let Some(lv) = t.low.get_mut(&v) {
+                        *lv = (*lv).min(iw);
+                    }
+                }
+                continue;
+            }
+            walk.pop();
+            let lv = t.low.get(&v).copied().unwrap_or(usize::MAX);
+            if let Some((u, _, _)) = walk.last()
+                && let Some(lu) = t.low.get_mut(u)
+            {
+                *lu = (*lu).min(lv);
+            }
+            if Some(lv) == t.index.get(&v).copied() {
+                while let Some(x) = t.stack.pop() {
+                    t.on_stack.remove(&x);
+                    component.insert(x, found);
+                    if x == v {
+                        break;
+                    }
+                }
+                found = found.saturating_add(1);
+            }
+        }
+    }
+    component
+}
+
+/// The precomp links of the import: the links every precomp layer of the file could make (the
+/// candidates, known before any layer is built), and the ones accepted so far, stored as precomp
+/// -> the comps that use it.
+///
+/// A precomp layer of `to` in `from` closes a loop when `to` already uses `from` over accepted
+/// links ([`closes_loop`]). That path and the layer's own link form a loop of candidate links, so
+/// `to` and `from` share a component of the candidate graph: between components the answer is no
+/// without a search (a file without a precomp loop never searches). Within a component, the comps
+/// that use `from` are found once per composition, among that component's comps only: while
+/// `from`'s layers are built the only links added start at `from`, and a path that reaches `from`
+/// never needs one of them. (Before, every precomp layer walked the whole graph, which held one
+/// link per layer.)
+#[derive(Default)]
+struct Precomps {
+    used_by: HashMap<ItemId, HashSet<ItemId>>,
+    component: HashMap<ItemId, usize>,
+}
+
+impl Precomps {
+    /// `candidates`: comp -> the comps its layers could show.
+    fn new(candidates: &HashMap<ItemId, HashSet<ItemId>>) -> Self {
+        Precomps { used_by: HashMap::new(), component: components(candidates) }
+    }
+
+    /// Decides a precomp layer of `to` in `from`: recorded and `true`, or `false` when it closes a
+    /// loop. `users` holds the comps that use `from` once they are needed (one per composition).
+    fn accept(&mut self, from: ItemId, to: ItemId, users: &mut Option<HashSet<ItemId>>) -> bool {
+        let looped = closes_loop(from, to, |to| self.same_component(from, to) && users.get_or_insert_with(|| self.users_of(from)).contains(&to));
+        if !looped {
+            self.used_by.entry(to).or_default().insert(from);
+        }
+        !looped
+    }
+
+    /// Whether `a` and `b` are in one component of the candidate graph.
+    fn same_component(&self, a: ItemId, b: ItemId) -> bool {
+        matches!((self.component.get(&a), self.component.get(&b)), (Some(x), Some(y)) if x == y)
+    }
+
+    /// The comps of `comp`'s component that use it over accepted links, directly or through
+    /// others (each visited once).
+    fn users_of(&self, comp: ItemId) -> HashSet<ItemId> {
+        let mut found = HashSet::new();
+        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) && found.insert(*u) {
+                    stack.push(*u);
+                }
+            }
+        }
+        found
+    }
+}
+
+/// 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>> {
+    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 };
+            if s.kind == SourceKind::Comp {
+                links.entry(p.id).or_default().insert(s.item);
+            }
+        }
+    }
+    links
+}
+
+/// Each layer's position in `comp.layers`, the first one for an id that repeats.
+fn layer_positions(comp: &Comp) -> HashMap<LayerId, usize> {
+    let mut at = HashMap::with_capacity(comp.layers.len());
+    for (i, l) in comp.layers.iter().enumerate() {
+        at.entry(l.id).or_insert(i);
+    }
+    at
+}
+
+/// The layer with project id `id`, through [`layer_positions`].
+pub(crate) fn layer_at<'c>(comp: &'c mut Comp, at: &HashMap<LayerId, usize>, id: LayerId) -> Option<&'c mut Layer> {
+    comp.layers.get_mut(*at.get(&id)?)
 }
 
 fn apply_timing(l: &mut Layer, h: &LayerHeader, tm: &TimeMap, name: &str, col: &mut Collector) {
     // The exact ratio `stretch / stretch_den` (50 % is 1/2, -100 % is -1/1); a zero or unreadable
     // one is 100 %.
     let stretch = h.stretch_ratio().filter(|r| r.num != 0).unwrap_or_else(|| {
-        col.lose(LossCategory::DamagedData, format!("layer '{name}': the time stretch is zero or unreadable, 100 % is used"));
+        col.lose_with(LossCategory::DamagedData, || format!("layer '{name}': the time stretch is zero or unreadable, 100 % is used"));
         Rational { num: 1, den: 1 }
     });
     let stored = stretch.to_f64().map_or(100.0, |r| r * 100.0);
@@ -714,7 +863,9 @@ fn apply_timing(l: &mut Layer, h: &LayerHeader, tm: &TimeMap, name: &str, col: &
     // out points keep the comp times the file stores.
     l.stretch = effectcraft_project::limits::stretch(stored);
     if l.stretch.to_bits() != stored.to_bits() {
-        col.lose(LossCategory::DamagedData, format!("layer '{name}': the time stretch of {stored:e} % is outside ±1 % to ±9900 %, {} % is used", l.stretch));
+        col.lose_with(LossCategory::DamagedData, || {
+            format!("layer '{name}': the time stretch of {stored:e} % is outside ±1 % to ±9900 %, {} % is used", l.stretch)
+        });
     }
     // A stored time with a zero denominator (or a sum that overflows) has no value; it is read as 0
     // and reported, so the layer's timing never changes silently.
@@ -726,9 +877,11 @@ fn apply_timing(l: &mut Layer, h: &LayerHeader, tm: &TimeMap, name: &str, col: &
     l.start_time = tm.ticks(h.start.num, h.start.den);
     let points = [(h.in_relative, h.in_point()), (h.out_relative, h.out_point())].map(|(offset, reduced)| absolute(h.start, offset, stretch, reduced));
     if !unreadable.is_empty() {
-        col.lose(LossCategory::DamagedData, format!("layer '{name}': the stored {} time is unreadable (zero denominator), 0 is used", unreadable.join(", ")));
+        col.lose_with(LossCategory::DamagedData, || {
+            format!("layer '{name}': the stored {} time is unreadable (zero denominator), 0 is used", unreadable.join(", "))
+        });
     } else if points.iter().any(Option::is_none) {
-        col.lose(LossCategory::DamagedData, format!("layer '{name}': the in or out point is out of range, 0 is used"));
+        col.lose_with(LossCategory::DamagedData, || format!("layer '{name}': the in or out point is out of range, 0 is used"));
     }
     let [a, b] = points.map(|p| p.map_or(Tick::ZERO, |(n, d)| tm.ticks(n, d)));
     // A negative stretch reverses the layer (981-983): the stored in point is then the later
@@ -799,7 +952,15 @@ struct PendingMatte {
 /// above: that layer is used. The matte layer's video switch is imported as stored (After Effects
 /// switches it off when the matte is set, 403; EffectCraft, like After Effects, still uses a
 /// hidden layer as a matte).
-fn resolve_mattes(comp: &mut Comp, mattes: &[PendingMatte], ids: &HashMap<u32, LayerId>, skipped: &HashMap<u32, String>, comp_name: &str, col: &mut Collector) {
+fn resolve_mattes(
+    comp: &mut Comp,
+    at: &HashMap<LayerId, usize>,
+    mattes: &[PendingMatte],
+    ids: &HashMap<u32, LayerId>,
+    skipped: &HashMap<u32, String>,
+    comp_name: &str,
+    col: &mut Collector,
+) {
     for m in mattes {
         let kind = match m.matte.mode {
             MatteMode::Alpha => MatteKind::Alpha,
@@ -807,24 +968,24 @@ fn resolve_mattes(comp: &mut Comp, mattes: &[PendingMatte], ids: &HashMap<u32, L
             MatteMode::Luma => MatteKind::Luma,
             MatteMode::LumaInverted => MatteKind::LumaInverted,
             MatteMode::Other(code) => {
-                col.lose(LossCategory::TrackMattes, format!("{comp_name}: '{}' has a track matte of unknown mode {code}", m.name));
+                col.lose_with(LossCategory::TrackMattes, || format!("{comp_name}: '{}' has a track matte of unknown mode {code}", m.name));
                 continue;
             }
         };
         let Some(source) = m.matte.layer.or(m.above) else {
-            col.lose(LossCategory::TrackMattes, format!("{comp_name}: '{}' has a track matte but no layer above it", m.name));
+            col.lose_with(LossCategory::TrackMattes, || format!("{comp_name}: '{}' has a track matte but no layer above it", m.name));
             continue;
         };
         let Some(target) = ids.get(&source).copied() else {
             let why = skipped.get(&source).map_or("a layer that does not exist", |s| s.as_str());
-            col.lose(LossCategory::TrackMattes, format!("{comp_name}: '{}' has {why} as its track matte", m.name));
+            col.lose_with(LossCategory::TrackMattes, || format!("{comp_name}: '{}' has {why} as its track matte", m.name));
             continue;
         };
         if target == m.layer {
-            col.lose(LossCategory::TrackMattes, format!("{comp_name}: '{}' is its own track matte", m.name));
+            col.lose_with(LossCategory::TrackMattes, || format!("{comp_name}: '{}' is its own track matte", m.name));
             continue;
         }
-        if let Some(l) = comp.layer_mut(m.layer) {
+        if let Some(l) = layer_at(comp, at, m.layer) {
             l.track_matte = Some(TrackMatte { layer: target, kind });
             col.imported.track_mattes += 1;
         }
@@ -869,6 +1030,7 @@ fn parent_cycles(links: &HashMap<LayerId, LayerId>) -> HashSet<LayerId> {
 /// Sets the parent links that point at an imported layer, without loops.
 fn resolve_parents(
     comp: &mut Comp,
+    at: &HashMap<LayerId, usize>,
     parents: &[(u32, LayerId, String, u32)],
     ids: &HashMap<u32, LayerId>,
     skipped: &HashMap<u32, String>,
@@ -883,7 +1045,7 @@ fn resolve_parents(
             }
             None => {
                 let why = skipped.get(parent).map_or("a layer that does not exist", |s| s.as_str());
-                col.lose(LossCategory::ParentLinks, format!("{comp_name}: '{name}' has {why} as its parent"));
+                col.lose_with(LossCategory::ParentLinks, || format!("{comp_name}: '{name}' has {why} as its parent"));
             }
         }
     }
@@ -892,11 +1054,11 @@ fn resolve_parents(
     let looped: Vec<LayerId> = links.keys().copied().filter(|l| on_loop.contains(l)).collect();
     for l in &looped {
         links.remove(l);
-        let name = comp.layer(*l).map(|x| x.name.clone()).unwrap_or_default();
-        col.lose(LossCategory::ParentLinks, format!("{comp_name}: '{name}' has a parent loop"));
+        let name = at.get(l).and_then(|i| comp.layers.get(*i)).map(|x| x.name.clone()).unwrap_or_default();
+        col.lose_with(LossCategory::ParentLinks, || format!("{comp_name}: '{name}' has a parent loop"));
     }
     for (layer, parent) in links {
-        if let Some(l) = comp.layer_mut(layer) {
+        if let Some(l) = layer_at(comp, at, layer) {
             l.parent = Some(parent);
         }
     }
@@ -928,6 +1090,137 @@ mod tests {
         assert_eq!(found, [1, 2, 3, 5]);
     }
 
+    /// The loop check as it was (a walk over every link per precomp layer), the reference.
+    fn old_would_loop(uses: &HashMap<ItemId, Vec<ItemId>>, from: ItemId, to: ItemId) -> bool {
+        if from == to {
+            return true;
+        }
+        let mut seen = HashSet::new();
+        let mut stack = vec![to];
+        while let Some(c) = stack.pop() {
+            if c == from {
+                return true;
+            }
+            if seen.insert(c) {
+                stack.extend(uses.get(&c).into_iter().flatten().copied());
+            }
+        }
+        false
+    }
+
+    /// Builds the compositions `0..comps` in order, each with `layers` precomp layers of
+    /// pseudo-random compositions, deciding every layer with the old and the new check.
+    fn decide_both(comps: u64, layers: u64, seed: u64) -> usize {
+        let mut state = seed;
+        let mut next = || {
+            state = state.wrapping_mul(6_364_136_223_846_793_005).wrapping_add(1_442_695_040_888_963_407);
+            state >> 33
+        };
+        let plan: Vec<(ItemId, Vec<ItemId>)> = (0..comps).map(|c| (ItemId(c), (0..layers).map(|_| ItemId(next() % comps)).collect())).collect();
+        let mut candidates: HashMap<ItemId, HashSet<ItemId>> = HashMap::new();
+        for (from, tos) in &plan {
+            candidates.entry(*from).or_default().extend(tos.iter().copied());
+        }
+        let (mut uses, mut new) = (HashMap::<ItemId, Vec<ItemId>>::new(), Precomps::new(&candidates));
+        let mut refused = 0;
+        for (from, tos) in &plan {
+            let mut users: Option<HashSet<ItemId>> = None;
+            for to in tos {
+                let accepted = new.accept(*from, *to, &mut users);
+                assert_eq!(!accepted, old_would_loop(&uses, *from, *to), "{from:?} -> {to:?}");
+                if accepted {
+                    uses.entry(*from).or_default().push(*to);
+                } else {
+                    refused += 1;
+                }
+            }
+        }
+        refused
+    }
+
+    #[test]
+    fn the_precomp_loop_check_decides_as_the_full_walk_did() {
+        let mut refused = 0;
+        for seed in 0..200 {
+            refused += decide_both(1 + seed % 12, 1 + seed % 5, seed);
+        }
+        // Both kinds of answer occur.
+        assert!(refused > 0);
+    }
+
+    #[test]
+    fn many_layers_of_one_precomp_are_checked_in_linear_time() {
+        // C holds 200 000 layers of B, B 200 000 of A, built B first: before, every layer of C
+        // walked B's 200 000 links (4e10 steps); now no layer searches (no loop in the file).
+        let (a, b, c) = (ItemId(1), ItemId(2), ItemId(3));
+        let candidates = HashMap::from([(b, HashSet::from([a])), (c, HashSet::from([b]))]);
+        let mut p = Precomps::new(&candidates);
+        for (from, to) in [(b, a), (c, b)] {
+            let mut users = None;
+            for _ in 0..200_000 {
+                assert!(p.accept(from, to, &mut users));
+            }
+            assert!(users.is_none(), "no search");
+        }
+    }
+
+    #[test]
+    fn layered_precomps_are_checked_without_a_search() {
+        // K top comps each show K middle comps, each of which shows K leaf comps, built leaves
+        // last (the order that made each leaf's users the whole graph): K^3 steps before the
+        // components, now none. K = 300: 180 000 links.
+        let k = 300u64;
+        let (top, mid, leaf) = (|i: u64| ItemId(i), |i: u64| ItemId(1000 + i), |i: u64| ItemId(2000 + i));
+        let mut candidates: HashMap<ItemId, HashSet<ItemId>> = HashMap::new();
+        for i in 0..k {
+            candidates.insert(top(i), (0..k).map(mid).collect());
+            candidates.insert(mid(i), (0..k).map(leaf).collect());
+        }
+        let mut p = Precomps::new(&candidates);
+        for from in (0..k).map(top).chain((0..k).map(mid)) {
+            let mut users = None;
+            for to in candidates.get(&from).into_iter().flatten() {
+                assert!(p.accept(from, *to, &mut users));
+            }
+            assert!(users.is_none(), "no search");
+        }
+    }
+
+    #[test]
+    fn components_are_the_loops() {
+        let n = |i: u64| ItemId(i);
+        // 1 -> 2 -> 3 -> 1 is a loop, 4 leads into it, 5 -> 5 links to itself, 6 -> 7 ends.
+        let edges: HashMap<ItemId, HashSet<ItemId>> = [(1, 2), (2, 3), (3, 1), (4, 1), (5, 5), (6, 7)].into_iter().fold(HashMap::new(), |mut m, (a, b)| {
+            m.entry(n(a)).or_default().insert(n(b));
+            m
+        });
+        let c = components(&edges);
+        assert!(c[&n(1)] == c[&n(2)] && c[&n(2)] == c[&n(3)]);
+        let others = [4, 5, 6, 7].map(|i| c[&n(i)]);
+        assert!(others.iter().all(|x| *x != c[&n(1)]));
+        assert_eq!(others.iter().collect::<HashSet<_>>().len(), 4);
+        // A chain of 200 000 links closed into one loop, walked without recursion.
+        let big: HashMap<ItemId, HashSet<ItemId>> = (0..200_000u64).map(|i| (n(i), HashSet::from([n((i + 1) % 200_000)]))).collect();
+        let c = components(&big);
+        assert_eq!(c.values().collect::<HashSet<_>>().len(), 1);
+    }
+
+    #[test]
+    fn layer_positions_find_the_first_layer_of_an_id() {
+        let mut project = Project::default();
+        let mut comp = Comp::new(10, 10, effectcraft_time::FrameRate::new(25, 1), Tick::ZERO);
+        for (id, name) in [(5, "a"), (6, "b"), (5, "c")] {
+            let mut l = build::layer(&mut project, &comp, name, LayerSource::Null, (10, 10), None);
+            l.id = LayerId(id);
+            l.name = name.into();
+            comp.layers.push(l);
+        }
+        let at = layer_positions(&comp);
+        assert_eq!(layer_at(&mut comp, &at, LayerId(5)).map(|l| l.name.clone()).as_deref(), Some("a"));
+        assert_eq!(layer_at(&mut comp, &at, LayerId(6)).map(|l| l.name.clone()).as_deref(), Some("b"));
+        assert!(layer_at(&mut comp, &at, LayerId(7)).is_none());
+    }
+
     #[test]
     fn a_long_parent_chain_is_checked_in_linear_time() {
         // 200 000 layers each parented to the next, the last to the first: walking the chain from

crates/aep/src/import/effects.rs

@@ -34,7 +34,7 @@
 //! import. Hue/Saturation's master values and Levels' values are arbitrary data, decoded by the
 //! reader (444-446, 512-516).
 
-use crate::effect_map::{CRYPTOMATTE_OUTPUT, Conv, EFFECTS, EffectMap, Extra, HUE_SATURATION_MASTER, LEVELS_RGB, ParamMap, popup_key};
+use crate::effect_map::{CRYPTOMATTE_OUTPUT, Conv, EFFECTS, EffectMap, Extra, HUE_SATURATION_MASTER, LEVELS_RGB, ParamMap, popup_key, popup_stem};
 use crate::{
     CryptomatteData, CurvesData, DefinitionSource, Effect as AepEffect, EffectDefinitions, EffectParam, ExtractorChannels, HueSaturationData,
     Layer as AepLayer, LayerRef, LevelsData, MeshData, ParamKind, ParamValue,
@@ -83,25 +83,26 @@ pub(crate) fn import_effects<'f, 'a>(
         }
         let shown = fx.display_name().unwrap_or_else(|| fx.match_name().to_string());
         let Some(map) = EFFECTS.iter().find(|m| m.match_name == fx.match_name()) else {
-            col.lose(LossCategory::Effects, format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name()));
+            col.lose_with(LossCategory::Effects, || format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name()));
             continue;
         };
         let Some(spec) = effectcraft_effects::find(map.effect) else {
-            col.lose(LossCategory::Effects, format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name()));
+            col.lose_with(LossCategory::Effects, || format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name()));
             continue;
         };
         if defs.source(&fx) == DefinitionSource::Missing {
-            col.lose(
-                LossCategory::Effects,
-                format!("{}: '{}' has the effect '{shown}' ({}) without readable parameter definitions, not imported", cx.comp, cx.layer, fx.match_name()),
-            );
+            col.lose_with(LossCategory::Effects, || {
+                format!("{}: '{}' has the effect '{shown}' ({}) without readable parameter definitions, not imported", cx.comp, cx.layer, fx.match_name())
+            });
             continue;
         }
         let size = [f64::from(cx.size.0), f64::from(cx.size.1)];
         let mut group = effectcraft_effects::instantiate(spec, ids, &shown, size);
         match fx.enabled() {
             Some(on) => group.enabled = on,
-            None => col.lose(LossCategory::DamagedData, format!("{}: '{}': the fx switch of '{shown}' is unreadable, imported switched on", cx.comp, cx.layer)),
+            None => col.lose_with(LossCategory::DamagedData, || {
+                format!("{}: '{}': the fx switch of '{shown}' is unreadable, imported switched on", cx.comp, cx.layer)
+            }),
         }
         let params = defs.params(&fx);
         // Borrowed definitions (another instance's or the project copy) may not name every value
@@ -109,15 +110,14 @@ pub(crate) fn import_effects<'f, 'a>(
         if params.first().is_some_and(|p| p.borrowed_definition) {
             let defined: std::collections::HashSet<&str> = params.iter().map(|p| p.match_name.as_str()).collect();
             for name in fx.value_record_names().iter().filter(|n| !defined.contains(n.as_str())) {
-                col.lose(
-                    LossCategory::Effects,
+                col.lose_with(LossCategory::Effects, || {
                     format!(
                         "{}: '{}' effect '{shown}' ({}): the value record {name} has no parameter definition, not imported",
                         cx.comp,
                         cx.layer,
                         fx.match_name()
-                    ),
-                );
+                    )
+                });
             }
         }
         if map.extra == Extra::Lumetri {
@@ -182,7 +182,7 @@ fn import_param(fx: &AepEffect<'_, '_>, map: &EffectMap, p: &EffectParam<'_, '_>
         // Not in the table: lost only when the file stores something for it. A layer parameter
         // set to no layer (its record always exists, Timewarp's Matte Layer 529) holds nothing.
         if p.stored().is_some() && !matches!(p.stored_value(), Some(ParamValue::Layer(0))) {
-            col.lose(LossCategory::Effects, format!("{here}: parameter {label} not imported"));
+            col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label} not imported"));
         }
         return;
     };
@@ -195,7 +195,7 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
     let (here, label) = names;
     let ParamOut { group, refs, col } = out;
     if p.kind != m.kind {
-        col.lose(LossCategory::Effects, format!("{here}: parameter {label} has type {:?}, expected {:?}; not imported", p.kind, m.kind));
+        col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label} has type {:?}, expected {:?}; not imported", p.kind, m.kind));
         return;
     }
     match m.conv {
@@ -215,7 +215,7 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
         // After Effects 2026 writes 0.990005 (the 16.16 rounding differs); the scale is the same.
         let ok = found.is_some_and(|v| (v - valid_max).abs() <= valid_max.abs() * 1e-4);
         if !ok {
-            col.lose(LossCategory::Effects, format!("{here}: parameter {label} has the valid maximum {found:?}, expected {valid_max}; not imported"));
+            col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label} has the valid maximum {found:?}, expected {valid_max}; not imported"));
         }
         ok
     };
@@ -224,10 +224,9 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
         (Conv::Popup { .. }, Some(items)) => items.clone(),
         (Conv::Popup { items, .. }, None) => {
             if p.popup_item_count().map(usize::from) != Some(items.len()) {
-                col.lose(
-                    LossCategory::Effects,
-                    format!("{here}: parameter {label}: the file names no popup items and has {:?} of them, not imported", p.popup_item_count()),
-                );
+                col.lose_with(LossCategory::Effects, || {
+                    format!("{here}: parameter {label}: the file names no popup items and has {:?} of them, not imported", p.popup_item_count())
+                });
                 return;
             }
             items.iter().map(|s| s.to_string()).collect()
@@ -263,7 +262,7 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
     };
     // Grouped parameters are addressed by path (`master/exposure`).
     let Some(prop) = group.prop_mut(m.target) else {
-        col.lose(LossCategory::Effects, format!("{here}: parameter {label} has no counterpart '{}'", m.target));
+        col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label} has no counterpart '{}'", m.target));
         return;
     };
     // For a popup: file item (0-based over all items) -> EffectCraft item, by name.
@@ -271,7 +270,7 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
         Conv::Popup { .. } => match popup_mapping(&items, &prop.ui) {
             Some(map) => Some(map),
             None => {
-                col.lose(LossCategory::Effects, format!("{here}: parameter {label}: the popup items do not match EffectCraft's, not imported"));
+                col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label}: the popup items do not match EffectCraft's, not imported"));
                 return;
             }
         },
@@ -288,7 +287,7 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
             _ => false,
         };
         if converted && stored.expression().is_some() {
-            col.lose(LossCategory::Effects, format!("{here}: the expression on parameter {label} is not imported (the value is converted here)"));
+            col.lose_with(LossCategory::Effects, || format!("{here}: the expression on parameter {label} is not imported (the value is converted here)"));
         } else {
             import_expression(&stored, prop, col);
         }
@@ -296,14 +295,14 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
     if p.is_animated() {
         if kind == Kind::EffectBool {
             // Keys on a checkbox are not sampled by the corpus.
-            col.lose(LossCategory::Effects, format!("{here}: the keyframes of parameter {label} are not imported (not decoded), default used"));
+            col.lose_with(LossCategory::Effects, || format!("{here}: the keyframes of parameter {label} are not imported (not decoded), default used"));
             return;
         }
         if let Some(stored) = p.stored() {
             let before = prop.keys.len();
             import_keys(&stored, kind, cx.layer, &p.match_name, cx.tm, prop, col);
             if prop.keys.len() == before {
-                col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} has no readable keyframe, default used"));
+                col.lose_with(LossCategory::DamagedData, || format!("{here}: parameter {label} has no readable keyframe, default used"));
             }
             if let Some(map) = &popup_map {
                 remap_popup_keys(prop, map, here, label, col);
@@ -317,7 +316,7 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
     let value = match (p.stored(), p.recorded_value()) {
         (Some(_), Some(v)) => Some(v),
         (Some(_), None) => {
-            col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds an unreadable value, default used"));
+            col.lose_with(LossCategory::DamagedData, || format!("{here}: parameter {label} holds an unreadable value, default used"));
             return;
         }
         // Absent = default. A borrowed popup's default slot is not reliable (a selection writes
@@ -335,7 +334,7 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
             Some(Some(ec)) => Some(Value::Enum(ec)),
             Some(None) => {
                 let item = usize::try_from(i).ok().and_then(|i| i.checked_sub(1)).and_then(|i| items.get(i)).cloned().unwrap_or_default();
-                col.lose(LossCategory::Effects, format!("{here}: parameter {label}: item '{item}' has no counterpart, default used"));
+                col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label}: item '{item}' has no counterpart, default used"));
                 return;
             }
             None => None,
@@ -347,7 +346,7 @@ pub(crate) fn import_mapped(p: &EffectParam<'_, '_>, m: &ParamMap, cx: &LayerCtx
             report_beyond_limit(m.limit, std::iter::once(&v), here, label, col);
             prop.value = v;
         }
-        None => col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds a value that is not valid ({value:?}), default used")),
+        None => col.lose_with(LossCategory::DamagedData, || format!("{here}: parameter {label} holds a value that is not valid ({value:?}), default used")),
     }
 }
 
@@ -357,7 +356,9 @@ fn report_beyond_limit<'v>(limit: Option<(f64, f64)>, values: impl Iterator<Item
     let Some((lo, hi)) = limit else { return };
     let beyond: Vec<f64> = values.filter_map(|v| if let Value::Scalar(x) = v { Some(*x) } else { None }).filter(|x| *x < lo || *x > hi).collect();
     if let Some(x) = beyond.first() {
-        col.lose(LossCategory::Effects, format!("{here}: parameter {label} is {x}, EffectCraft's effect uses {lo} to {hi} (rendered as the nearest of them)"));
+        col.lose_with(LossCategory::Effects, || {
+            format!("{here}: parameter {label} is {x}, EffectCraft's effect uses {lo} to {hi} (rendered as the nearest of them)")
+        });
     }
 }
 
@@ -375,11 +376,11 @@ fn import_layer_ref(p: &EffectParam<'_, '_>, target: &str, own: bool, cx: &Layer
         src.value = Value::Enum(0);
     }
     let Some(prop) = group.prop_mut(target) else {
-        col.lose(LossCategory::Effects, format!("{here}: parameter {label} has no counterpart '{target}'"));
+        col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label} has no counterpart '{target}'"));
         return;
     };
     if p.is_animated() || p.stored().is_some_and(|s| s.expression().is_some()) {
-        col.lose(LossCategory::Effects, format!("{here}: keyframes or an expression on the layer parameter {label} are not imported"));
+        col.lose_with(LossCategory::Effects, || format!("{here}: keyframes or an expression on the layer parameter {label} are not imported"));
     }
     // No record: the default, the owning layer (320).
     match p.layer_ref(cx.aep_id).unwrap_or(LayerRef::This) {
@@ -395,19 +396,21 @@ fn import_layer_ref(p: &EffectParam<'_, '_>, target: &str, own: bool, cx: &Layer
 }
 
 /// Sets the layer parameters that name another layer of the composition (`ids`: file id ->
-/// project layer); one whose layer was not imported stays unset and is reported.
+/// project layer; `at`: project layer -> position in the composition); one whose layer was not
+/// imported stays unset and is reported.
 pub(crate) fn resolve_layer_refs(
     comp: &mut effectcraft_project::Comp,
+    at: &std::collections::HashMap<effectcraft_project::LayerId, usize>,
     refs: &[PendingLayerRef],
     ids: &std::collections::HashMap<u32, effectcraft_project::LayerId>,
     col: &mut Collector,
 ) {
     for r in refs {
         let Some(target) = ids.get(&r.target) else {
-            col.lose(LossCategory::Effects, format!("{} names a layer that is not imported (id {}), left unset", r.what, r.target));
+            col.lose_with(LossCategory::Effects, || format!("{} names a layer that is not imported (id {}), left unset", r.what, r.target));
             continue;
         };
-        if let Some(prop) = comp.layers.iter_mut().find(|l| l.id == r.layer).and_then(|l| l.props.find_mut(r.prop)) {
+        if let Some(prop) = crate::import::build::layer_at(comp, at, r.layer).and_then(|l| l.props.find_mut(r.prop)) {
             prop.value = Value::Layer(Some(target.0));
         }
     }
@@ -415,9 +418,12 @@ pub(crate) fn resolve_layer_refs(
 
 /// The EffectCraft item of each file item (by name, ignoring case and separators); `None` when
 /// the parameter here is not a popup.
+///
+/// The file's item count is not bounded (its `pdnm`), EffectCraft's options are: each side's key
+/// is taken once and compared without a copy, so the cost is linear in the file's items.
 fn popup_mapping(file_items: &[String], ui: &ParamUi) -> Option<Vec<Option<u32>>> {
     let ParamUi::Popup { options } = ui else { return None };
-    let norm = popup_key;
+    let options: Vec<&str> = options.iter().map(|o| popup_stem(o)).collect();
     Some(
         file_items
             .iter()
@@ -425,7 +431,8 @@ fn popup_mapping(file_items: &[String], ui: &ParamUi) -> Option<Vec<Option<u32>>
                 if item.trim() == "(-" {
                     return None;
                 }
-                options.iter().position(|o| norm(o) == norm(item)).and_then(|i| u32::try_from(i).ok())
+                let item = popup_stem(item);
+                options.iter().position(|o| o.eq_ignore_ascii_case(item)).and_then(|i| u32::try_from(i).ok())
             })
             .collect(),
     )
@@ -445,7 +452,7 @@ fn remap_popup_keys(prop: &mut effectcraft_project::Property, map: &[Option<u32>
         _ => true,
     });
     if prop.keys.len() != before {
-        col.lose(LossCategory::Effects, format!("{here}: a keyframe of parameter {label} has an item without a counterpart, dropped"));
+        col.lose_with(LossCategory::Effects, || format!("{here}: a keyframe of parameter {label} has an item without a counterpart, dropped"));
     }
     if let Some(first) = prop.keys.first() {
         prop.value = first.value.clone();
@@ -455,17 +462,16 @@ fn remap_popup_keys(prop: &mut effectcraft_project::Property, map: &[Option<u32>
 /// Hue/Saturation's master values, Levels' RGB values: from the arbitrary data of `p`.
 fn import_extra(fx: &AepEffect<'_, '_>, extra: Extra, p: &EffectParam<'_, '_>, here: &str, size: (u32, u32), group: &mut PropGroup, col: &mut Collector) {
     if p.is_animated() || p.stored().is_some_and(|s| s.expression().is_some()) {
-        col.lose(LossCategory::Effects, format!("{here}: keyframes or an expression on {} are not imported", p.name));
+        col.lose_with(LossCategory::Effects, || format!("{here}: keyframes or an expression on {} are not imported", p.name));
     }
     let Some(data) = fx.arbitrary_data(&p.match_name) else {
         // No record: every value is at its default. A Channel Info value record without its
         // aRbs/aRbp payload (every sampled record has it) is damaged: the payload may have chosen
         // other channels (`Z`), so the defaults are not what After Effects renders.
         if p.stored().is_some() && matches!(extra, Extra::Extractor | Extra::Identifier) {
-            col.lose(
-                LossCategory::DamagedData,
-                format!("{here}: the Channel Info data is missing (a value record without its data), the default channels used"),
-            );
+            col.lose_with(LossCategory::DamagedData, || {
+                format!("{here}: the Channel Info data is missing (a value record without its data), the default channels used")
+            });
         }
         return;
     };
@@ -483,10 +489,10 @@ fn import_extra(fx: &AepEffect<'_, '_>, extra: Extra, p: &EffectParam<'_, '_>, h
                     set(id, f64::from(v));
                 }
                 if !d.ranges_are_default() {
-                    col.lose(LossCategory::Effects, format!("{here}: the colour ranges (Reds ... Magentas) are not imported (not sampled)"));
+                    col.lose_with(LossCategory::Effects, || format!("{here}: the colour ranges (Reds ... Magentas) are not imported (not sampled)"));
                 }
             }
-            None => col.lose(LossCategory::DamagedData, format!("{here}: the Hue/Saturation data is unreadable, defaults used")),
+            None => col.lose_with(LossCategory::DamagedData, || format!("{here}: the Hue/Saturation data is unreadable, defaults used")),
         },
         Extra::Levels => match LevelsData::parse(data) {
             Some(d) => {
@@ -494,10 +500,10 @@ fn import_extra(fx: &AepEffect<'_, '_>, extra: Extra, p: &EffectParam<'_, '_>, h
                     set(id, f64::from(v));
                 }
                 if d.channels[1..].iter().any(|c| *c != LevelsData::DEFAULT_CHANNEL) {
-                    col.lose(LossCategory::Effects, format!("{here}: the per-channel values (Red, Green, Blue, Alpha) are not imported (not sampled)"));
+                    col.lose_with(LossCategory::Effects, || format!("{here}: the per-channel values (Red, Green, Blue, Alpha) are not imported (not sampled)"));
                 }
             }
-            None => col.lose(LossCategory::DamagedData, format!("{here}: the Levels data is unreadable, defaults used")),
+            None => col.lose_with(LossCategory::DamagedData, || format!("{here}: the Levels data is unreadable, defaults used")),
         },
         Extra::Curves => match CurvesData::parse(data) {
             Some(d) => {
@@ -511,11 +517,11 @@ fn import_extra(fx: &AepEffect<'_, '_>, extra: Extra, p: &EffectParam<'_, '_>, h
                     }
                 }
             }
-            None => col.lose(LossCategory::DamagedData, format!("{here}: the Curves data is unreadable, defaults used")),
+            None => col.lose_with(LossCategory::DamagedData, || format!("{here}: the Curves data is unreadable, defaults used")),
         },
         Extra::MeshWarp => match MeshData::parse(data) {
             Some(m) => import_mesh(&m, here, size, group, col),
-            None => col.lose(LossCategory::DamagedData, format!("{here}: the Distortion Mesh is unreadable, defaults used")),
+            None => col.lose_with(LossCategory::DamagedData, || format!("{here}: the Distortion Mesh is unreadable, defaults used")),
         },
         Extra::Extractor => match ExtractorChannels::parse(data).map(|c| (c.sources(), c)) {
             // Each output channel reads the footage's channel of the stored name, which
@@ -531,16 +537,16 @@ fn import_extra(fx: &AepEffect<'_, '_>, extra: Extra, p: &EffectParam<'_, '_>, h
             }
             Some((Err(k), c)) => {
                 let names: Vec<&str> = c.channels.iter().map(|(_, n)| n.as_str()).collect();
-                col.lose(
+                col.lose_with(
                     LossCategory::Effects,
-                    format!(
+                    || format!(
                         "{here}: the channels {} are not imported (record {} is of an unknown kind or names no channel), R, G, B, A used",
                         names.join(", "),
                         k.saturating_add(1)
                     ),
                 );
             }
-            None => col.lose(LossCategory::Effects, format!("{here}: the Channel Info data is not in the sampled layout, R, G, B, A used")),
+            None => col.lose_with(LossCategory::Effects, || format!("{here}: the Channel Info data is not in the sampled layout, R, G, B, A used")),
         },
         Extra::Identifier => match crate::identifier_channel_type(data) {
             // No channel chosen (the type's label): After Effects lists only the footage's integer
@@ -550,9 +556,9 @@ fn import_extra(fx: &AepEffect<'_, '_>, extra: Extra, p: &EffectParam<'_, '_>, h
                 if let Some(prop) = group.prop_mut("channelType") {
                     prop.value = Value::Enum(0);
                 }
-                col.lose(
+                col.lose_with(
                     LossCategory::Effects,
-                    format!("{here}: no ID channel is chosen (Channel Info), so After Effects leaves the layer unchanged; EffectCraft draws the footage's object ID channel"),
+                    || format!("{here}: no ID channel is chosen (Channel Info), so After Effects leaves the layer unchanged; EffectCraft draws the footage's object ID channel"),
                 );
             }
             // A chosen channel, by name. EffectCraft's IDentifier has no channel of its own: it
@@ -564,12 +570,12 @@ fn import_extra(fx: &AepEffect<'_, '_>, extra: Extra, p: &EffectParam<'_, '_>, h
                         prop.value = Value::Enum(u32::try_from(kind).unwrap_or(0));
                     }
                 }
-                None => col.lose(
+                None => col.lose_with(
                     LossCategory::Effects,
-                    format!("{here}: the ID channel '{name}' (Channel Info) is not imported; EffectCraft reads the footage's object ID channel (ObjectID, object_id, objectId, id)"),
+                    || format!("{here}: the ID channel '{name}' (Channel Info) is not imported; EffectCraft reads the footage's object ID channel (ObjectID, object_id, objectId, id)"),
                 ),
             },
-            None => col.lose(LossCategory::Effects, format!("{here}: the Channel Info data is not in the sampled layout, the object ID channel used")),
+            None => col.lose_with(LossCategory::Effects, || format!("{here}: the Channel Info data is not in the sampled layout, the object ID channel used")),
         },
         Extra::None | Extra::Lumetri | Extra::Cryptomatte => {}
     }
@@ -599,13 +605,13 @@ fn import_cryptomatte(fx: &AepEffect<'_, '_>, params: &[EffectParam<'_, '_>], cx
         match (p.index, p.kind) {
             (1, ParamKind::ArbitraryData) => {
                 if moving {
-                    col.lose(LossCategory::Effects, format!("{here}: keyframes or an expression on {label} are not imported"));
+                    col.lose_with(LossCategory::Effects, || format!("{here}: keyframes or an expression on {label} are not imported"));
                 }
                 // No record: the definition's default, no layer and no selection. A record
                 // without its data (every sampled record has it) is damaged.
                 let Some(data) = fx.arbitrary_data(&p.match_name) else {
                     if p.stored().is_some() {
-                        col.lose(LossCategory::DamagedData, format!("{here}: the Cryptomatte settings (layer, selection) have no data, none used"));
+                        col.lose_with(LossCategory::DamagedData, || format!("{here}: the Cryptomatte settings (layer, selection) have no data, none used"));
                     }
                     continue;
                 };
@@ -614,12 +620,14 @@ fn import_cryptomatte(fx: &AepEffect<'_, '_>, params: &[EffectParam<'_, '_>], cx
                         set(group, "layerName", Value::Str(d.layer));
                         set(group, "selection", Value::Str(d.selection));
                     }
-                    None => col.lose(LossCategory::DamagedData, format!("{here}: the Cryptomatte settings (layer, selection) are unreadable, none used")),
+                    None => {
+                        col.lose_with(LossCategory::DamagedData, || format!("{here}: the Cryptomatte settings (layer, selection) are unreadable, none used"))
+                    }
                 }
             }
             (2, ParamKind::Popup) => {
                 if moving {
-                    col.lose(LossCategory::Effects, format!("{here}: keyframes or an expression on {label} are not imported, Colors used"));
+                    col.lose_with(LossCategory::Effects, || format!("{here}: keyframes or an expression on {label} are not imported, Colors used"));
                     continue;
                 }
                 let (display, matte_only) = crypto_output(p, &here, &label, col);
@@ -628,19 +636,19 @@ fn import_cryptomatte(fx: &AepEffect<'_, '_>, params: &[EffectParam<'_, '_>], cx
             }
             (3, ParamKind::Checkbox) => {
                 if moving {
-                    col.lose(LossCategory::Effects, format!("{here}: keyframes or an expression on {label} are not imported"));
+                    col.lose_with(LossCategory::Effects, || format!("{here}: keyframes or an expression on {label} are not imported"));
                 } else {
                     match selection_mode_on(p) {
                         // A picking switch of the plug-in; EffectCraft picks with its own tool.
-                        Some(true) => col.lose(LossCategory::Effects, format!("{here}: parameter {label} is on and has no counterpart")),
+                        Some(true) => col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label} is on and has no counterpart")),
                         Some(false) => {}
-                        None => col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds an unreadable value, off used")),
+                        None => col.lose_with(LossCategory::DamagedData, || format!("{here}: parameter {label} holds an unreadable value, off used")),
                     }
                 }
             }
             _ => {
                 if p.stored().is_some() && !matches!(p.stored_value(), Some(ParamValue::Layer(0))) {
-                    col.lose(LossCategory::Effects, format!("{here}: parameter {label} ({:?}) not imported", p.kind));
+                    col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label} ({:?}) not imported", p.kind));
                 }
             }
         }
@@ -671,17 +679,16 @@ fn crypto_output(p: &EffectParam<'_, '_>, here: &str, label: &str, col: &mut Col
         Some(items) => items.clone(),
         None if p.popup_item_count().map(usize::from) == Some(CRYPTOMATTE_OUTPUT.len()) => CRYPTOMATTE_OUTPUT.iter().map(|s| s.to_string()).collect(),
         None => {
-            col.lose(
-                LossCategory::Effects,
-                format!("{here}: parameter {label}: the file names no popup items and has {:?} of them, not imported, Colors used", p.popup_item_count()),
-            );
+            col.lose_with(LossCategory::Effects, || {
+                format!("{here}: parameter {label}: the file names no popup items and has {:?} of them, not imported, Colors used", p.popup_item_count())
+            });
             return CRYPTO_COLORS;
         }
     };
     let index = match (p.stored(), p.stored_value()) {
         (Some(_), Some(ParamValue::Index(i))) => i,
         (Some(_), _) => {
-            col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds an unreadable value, Colors used"));
+            col.lose_with(LossCategory::DamagedData, || format!("{here}: parameter {label} holds an unreadable value, Colors used"));
             return CRYPTO_COLORS;
         }
         // No record: the default. The plug-in's is Colors (1): the `pard` default of the
@@ -699,14 +706,14 @@ fn crypto_output(p: &EffectParam<'_, '_>, here: &str, label: &str, col: &mut Col
         "matted rgba" => (1, false),
         "matte only" => (1, true),
         "matted colors" => {
-            col.lose(
+            col.lose_with(
                 LossCategory::Effects,
-                format!("{here}: parameter {label}: item '{item}' ({index}) has no counterpart, imported as Matte (the layer's colours keyed by the selection, not the ID colours)"),
+                || format!("{here}: parameter {label}: item '{item}' ({index}) has no counterpart, imported as Matte (the layer's colours keyed by the selection, not the ID colours)"),
             );
             (1, false)
         }
         _ => {
-            col.lose(LossCategory::Effects, format!("{here}: parameter {label}: item '{item}' ({index}) has no counterpart, Colors used"));
+            col.lose_with(LossCategory::Effects, || format!("{here}: parameter {label}: item '{item}' ({index}) has no counterpart, Colors used"));
             CRYPTO_COLORS
         }
     }
@@ -734,7 +741,9 @@ fn import_mesh(m: &MeshData, here: &str, size: (u32, u32), group: &mut PropGroup
     let rows_stored = group.prop("rows").map(|p| p.value.as_f64().round());
     let cols_stored = group.prop("columns").map(|p| p.value.as_f64().round());
     if rows_stored != Some(f64::from(m.rows)) || cols_stored != Some(f64::from(m.columns)) {
-        col.lose(LossCategory::Effects, format!("{here}: the Distortion Mesh is {} x {} cells, Rows / Columns say otherwise; not imported", m.columns, m.rows));
+        col.lose_with(LossCategory::Effects, || {
+            format!("{here}: the Distortion Mesh is {} x {} cells, Rows / Columns say otherwise; not imported", m.columns, m.rows)
+        });
         return;
     }
     let mut offs = Vec::with_capacity(m.vertices.len());
@@ -760,7 +769,7 @@ fn import_mesh(m: &MeshData, here: &str, size: (u32, u32), group: &mut PropGroup
             }
         }
         if !(dx.is_finite() && dy.is_finite() && own.iter().all(|x| x.is_finite())) {
-            col.lose(LossCategory::DamagedData, format!("{here}: the Distortion Mesh holds a number that is not valid, not imported"));
+            col.lose_with(LossCategory::DamagedData, || format!("{here}: the Distortion Mesh holds a number that is not valid, not imported"));
             return;
         }
         // EffectCraft's mesh reads the first two numbers of a vertex; moved handles follow as
@@ -775,10 +784,9 @@ fn import_mesh(m: &MeshData, here: &str, size: (u32, u32), group: &mut PropGroup
         offs.push(token);
     }
     if handles_moved {
-        col.lose(
-            LossCategory::Effects,
-            format!("{here}: Distortion Mesh Bezier handles moved on their own are not drawn (EffectCraft's mesh is bilinear; they are kept for the export)"),
-        );
+        col.lose_with(LossCategory::Effects, || {
+            format!("{here}: Distortion Mesh Bezier handles moved on their own are not drawn (EffectCraft's mesh is bilinear; they are kept for the export)")
+        });
     }
     if let Some(p) = group.prop_mut("rows") {
         p.value = Value::Scalar(f64::from(m.rows));
@@ -811,3 +819,34 @@ fn convert(value: ParamValue, kind: Kind) -> Option<Value> {
     };
     kind.value(&raw)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn ui(options: &[&str]) -> ParamUi {
+        ParamUi::Popup { options: options.iter().map(|s| s.to_string()).collect() }
+    }
+
+    #[test]
+    fn popup_items_map_by_name_ignoring_case_separators_and_channel() {
+        let file: Vec<String> = ["Red Channel", "(-", " GREEN ", "Alpha channel", "Other", "red"].iter().map(|s| s.to_string()).collect();
+        let map = popup_mapping(&file, &ui(&["Red", "Green", "Blue", "Alpha", "red"])).expect("a popup");
+        // The first option of a name wins, as `position` finds it.
+        assert_eq!(map, [Some(0), None, Some(1), Some(3), None, Some(0)]);
+        assert_eq!(popup_mapping(&file, &ParamUi::Checkbox), None);
+    }
+
+    #[test]
+    fn a_popup_with_a_huge_item_list_maps_in_linear_time() {
+        // A `pdnm` of 200 000 items (a hostile file: the count is not bounded there) against an
+        // effect's options. Each comparison used to build two keys (two allocations), 4e7 of
+        // them here at 100 options; now each key is borrowed once and compared in place.
+        let options: Vec<String> = (0..100).map(|i| format!("Option {i}")).collect();
+        let file: Vec<String> = (0..200_000).map(|i| format!("Item {}", i % 1000)).collect();
+        let ui = ParamUi::Popup { options };
+        let map = popup_mapping(&file, &ui).expect("a popup");
+        assert_eq!(map.len(), file.len());
+        assert!(map.iter().all(Option::is_none));
+    }
+}

crates/aep/src/import/masks.rs

@@ -11,11 +11,11 @@
 //! progress per second and reported.
 
 use crate::{Layer as AepLayer, MaskAtom, MaskMode as AepMode, PropNode, ShapePath as AepPath, ValueSource};
-use effectcraft_keyframe::{Ease, Interp, Keyframe, ShapePath, Value};
+use effectcraft_keyframe::{Interp, Keyframe, ShapePath, Value};
 use effectcraft_project::build::{self, Ids};
 use effectcraft_project::{GroupKind, MaskMode, Node, PropGroup as EcGroup};
 
-use crate::import::props::{Kind, LIMIT, import_expression, import_keys, interp};
+use crate::import::props::{Kind, LIMIT, ease, import_expression, import_keys, interp, sort_unique};
 use crate::import::report::{Collector, LossCategory};
 use crate::import::time::TimeMap;
 
@@ -129,7 +129,7 @@ fn import_mask(atom: &MaskAtom<'_, '_>, index: usize, name: &str, what: &str, ml
     let first = match first {
         Ok(p) => p,
         Err(why) => {
-            col.lose(LossCategory::Masks, format!("{what}: not imported, {why}"));
+            col.lose_with(LossCategory::Masks, || format!("{what}: not imported, {why}"));
             return None;
         }
     };
@@ -139,11 +139,11 @@ fn import_mask(atom: &MaskAtom<'_, '_>, index: usize, name: &str, what: &str, ml
                 AepMode::Other(c) => c,
                 _ => 0,
             };
-            col.lose(LossCategory::Masks, format!("{what}: mode code {shown} is not known, imported as Add"));
+            col.lose_with(LossCategory::Masks, || format!("{what}: mode code {shown} is not known, imported as Add"));
             MaskMode::Add
         }),
         None => {
-            col.lose(LossCategory::DamagedData, format!("{what}: mode and invert switch unreadable, Add used"));
+            col.lose_with(LossCategory::DamagedData, || format!("{what}: mode and invert switch unreadable, Add used"));
             MaskMode::Add
         }
     };
@@ -185,14 +185,14 @@ fn import_mask(atom: &MaskAtom<'_, '_>, index: usize, name: &str, what: &str, ml
 fn set_static(g: &mut EcGroup, id: &str, value: Value, source: ValueSource, valid: impl Fn(&Value) -> bool, what: &str, col: &mut Collector) {
     match source {
         ValueSource::Default | ValueSource::Keyframed => {}
-        ValueSource::Unreadable => col.lose(LossCategory::DamagedData, format!("{what}: {id} unreadable, default used")),
+        ValueSource::Unreadable => col.lose_with(LossCategory::DamagedData, || format!("{what}: {id} unreadable, default used")),
         ValueSource::Stored => {
             if crate::import::props::sane(&value) && valid(&value) {
                 if let Some(p) = g.get_mut(id) {
                     p.value = value;
                 }
             } else {
-                col.lose(LossCategory::DamagedData, format!("{what}: {id} holds a number that is not valid, default used"));
+                col.lose_with(LossCategory::DamagedData, || format!("{what}: {id} holds a number that is not valid, default used"));
             }
         }
     }
@@ -210,25 +210,25 @@ pub(crate) fn import_path_keys(
     col: &mut Collector,
 ) {
     let Some(list) = path.keys() else {
-        col.lose(LossCategory::DamagedData, format!("{what}: path keyframes unreadable, the first shape is static"));
+        col.lose_with(LossCategory::DamagedData, || format!("{what}: path keyframes unreadable, the first shape is static"));
         return;
     };
     let records: Vec<_> = list.keyframes().collect();
     if records.len() != shapes.len() {
-        col.lose(LossCategory::DamagedData, format!("{what}: {} path keyframes but {} shapes, the first shape is static", records.len(), shapes.len()));
+        col.lose_with(LossCategory::DamagedData, || format!("{what}: {} path keyframes but {} shapes, the first shape is static", records.len(), shapes.len()));
         return;
     }
     let scale = path.time_scale().filter(|s| *s != 0).unwrap_or(tm.timescale);
     let mut keys: Vec<Keyframe> = Vec::new();
     for (rec, shape) in records.iter().zip(shapes) {
         let Some(units) = rec.time_units() else {
-            col.lose(LossCategory::DamagedData, format!("{what}: unreadable path keyframe"));
+            col.lose_with(LossCategory::DamagedData, || format!("{what}: unreadable path keyframe"));
             continue;
         };
         let path = match shape.as_ref().ok_or("its shape is unreadable").and_then(|s| to_path(s, size)) {
             Ok(p) => p,
             Err(why) => {
-                col.lose(LossCategory::DamagedData, format!("{what}: a path keyframe was dropped, {why}"));
+                col.lose_with(LossCategory::DamagedData, || format!("{what}: a path keyframe was dropped, {why}"));
                 continue;
             }
         };
@@ -238,20 +238,25 @@ pub(crate) fn import_path_keys(
         if key.in_interp == Interp::Bezier || key.out_interp == Interp::Bezier {
             // The ease at byte 24 (599, 700): influences as fractions; a speed of 0 is exact, the
             // unit of another speed is not verified (taken as progress per second).
-            match rec.ease().filter(|e| [e.in_speed, e.in_influence, e.out_speed, e.out_influence].iter().all(|v| v.is_finite())) {
-                Some(e) => {
+            // The same rule as every other key's ease (`props::ease`): a speed beyond its limit is
+            // unreadable, not imported as it is.
+            match rec.ease().and_then(path_eases) {
+                Some((in_e, out_e)) => {
+                    let unverified = (key.in_interp == Interp::Bezier && in_e.speed != 0.0) || (key.out_interp == Interp::Bezier && out_e.speed != 0.0);
                     if key.in_interp == Interp::Bezier {
-                        key.in_ease = vec![Ease { speed: e.in_speed, influence: e.in_influence.clamp(0.001, 1.0) }];
+                        key.in_ease = vec![in_e];
                     }
                     if key.out_interp == Interp::Bezier {
-                        key.out_ease = vec![Ease { speed: e.out_speed, influence: e.out_influence.clamp(0.001, 1.0) }];
+                        key.out_ease = vec![out_e];
                     }
-                    if (key.in_interp == Interp::Bezier && e.in_speed != 0.0) || (key.out_interp == Interp::Bezier && e.out_speed != 0.0) {
-                        col.lose(LossCategory::ApproximatedKeyframes, format!("{what}: path key ease speed imported unverified (only speed 0 is sampled)"));
+                    if unverified {
+                        col.lose_with(LossCategory::ApproximatedKeyframes, || {
+                            format!("{what}: path key ease speed imported unverified (only speed 0 is sampled)")
+                        });
                     }
                 }
                 None => {
-                    col.lose(LossCategory::ApproximatedKeyframes, format!("{what}: bezier path key read as linear (its ease is unreadable)"));
+                    col.lose_with(LossCategory::ApproximatedKeyframes, || format!("{what}: bezier path key read as linear (its ease is unreadable)"));
                     key.in_interp = if key.in_interp == Interp::Bezier { Interp::Linear } else { key.in_interp };
                     key.out_interp = if key.out_interp == Interp::Bezier { Interp::Linear } else { key.out_interp };
                 }
@@ -264,12 +269,7 @@ pub(crate) fn import_path_keys(
     if keys.is_empty() {
         return;
     }
-    keys.sort_by_key(|k| k.time);
-    let before = keys.len();
-    keys.dedup_by_key(|k| k.time);
-    if keys.len() != before {
-        col.lose(LossCategory::DamagedData, format!("{what}: path keyframes at the same time"));
-    }
+    sort_unique(&mut keys, what, "path keyframes", col);
     // The first key's incoming and the last key's outgoing ease mean nothing.
     if let Some(first) = keys.first_mut() {
         first.in_ease.clear();
@@ -283,3 +283,26 @@ pub(crate) fn import_path_keys(
     }
     prop.keys = keys;
 }
+
+/// The in and out ease of a mask path key, under the rule of every other key's ease
+/// (`props::ease`); `None` when either side cannot be used.
+fn path_eases(e: crate::KeyEase) -> Option<(effectcraft_keyframe::Ease, effectcraft_keyframe::Ease)> {
+    Some((ease(e.in_speed, e.in_influence, 1.0)?, ease(e.out_speed, e.out_influence, 1.0)?))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn a_path_ease_speed_beyond_the_property_limit_is_unreadable() {
+        let e = |speed: f64| crate::KeyEase { in_speed: speed, in_influence: 0.5, out_speed: 0.0, out_influence: 2.0 };
+        let (i, o) = path_eases(e(3.0)).expect("usable");
+        assert_eq!((i.speed, i.influence, o.speed, o.influence), (3.0, 0.5, 0.0, 1.0));
+        // Finite but far beyond what a property key accepts (before, only finiteness was checked).
+        assert!(path_eases(e(1e12)).is_none());
+        assert!(path_eases(e(-1e12)).is_none());
+        assert!(path_eases(e(f64::NAN)).is_none());
+        assert!(path_eases(crate::KeyEase { out_speed: 1e12, ..e(0.0) }).is_none());
+    }
+}

crates/aep/src/import/props.rs

@@ -188,12 +188,12 @@ pub(crate) fn import_transform(
         }
         if source_of == ValueSource::Unreadable {
             // Present in the file but not decodable: the reader gave the default in its place.
-            col.lose(LossCategory::DamagedData, format!("{layer_name}: {match_name} unreadable, default used"));
+            col.lose_with(LossCategory::DamagedData, || format!("{layer_name}: {match_name} unreadable, default used"));
         }
         if sane(&base) {
             prop.value = base;
         } else {
-            col.lose(LossCategory::DamagedData, format!("{layer_name}: {match_name} holds a number that is not valid, default used"));
+            col.lose_with(LossCategory::DamagedData, || format!("{layer_name}: {match_name} holds a number that is not valid, default used"));
         }
         let Some(PropNode::Property(node)) = group.get(match_name) else { continue };
         if source_of == ValueSource::Keyframed {
@@ -204,7 +204,7 @@ pub(crate) fn import_transform(
     for (match_name, id) in ROTATIONS {
         let Some(PropNode::Property(node)) = group.get(match_name) else { continue };
         let Some(prop) = target.get_mut(id) else {
-            col.lose(LossCategory::TransformProperties, format!("{layer_name}: {match_name}"));
+            col.lose_with(LossCategory::TransformProperties, || format!("{layer_name}: {match_name}"));
             continue;
         };
         if node.is_animated() {
@@ -214,7 +214,7 @@ pub(crate) fn import_transform(
             if sane(&value) {
                 prop.value = value;
             } else {
-                col.lose(LossCategory::DamagedData, format!("{layer_name}: {match_name} holds a number that is not valid, default used"));
+                col.lose_with(LossCategory::DamagedData, || format!("{layer_name}: {match_name} holds a number that is not valid, default used"));
             }
         }
         import_expression(&node, prop, col);
@@ -226,9 +226,9 @@ pub(crate) fn import_transform(
             (Some(o), None) if !o.animated && !o.has_expression && o.degrees == [0.0; 3] => {}
             (Some(o), Some(prop)) if o.animated && !o.has_expression => import_orientation_keys(layer, layer_name, tm, prop, col),
             (Some(o), Some(_)) if o.has_expression => {
-                col.lose(LossCategory::TransformProperties, format!("{layer_name}: ADBE Orientation expression (not decoded)"));
+                col.lose_with(LossCategory::TransformProperties, || format!("{layer_name}: ADBE Orientation expression (not decoded)"));
             }
-            _ => col.lose(LossCategory::TransformProperties, format!("{layer_name}: ADBE Orientation")),
+            _ => col.lose_with(LossCategory::TransformProperties, || format!("{layer_name}: ADBE Orientation")),
         }
     }
     report_other_properties(&group, layer_name, col);
@@ -241,7 +241,7 @@ fn import_orientation_keys(layer: &AepLayer<'_, '_>, layer_name: &str, tm: &Time
     let what = format!("{layer_name}: ADBE Orientation");
     let stored = layer.orientation_keys();
     if stored.is_empty() {
-        col.lose(LossCategory::TransformProperties, format!("{what} keys unreadable, the static value is used"));
+        col.lose_with(LossCategory::TransformProperties, || format!("{what} keys unreadable, the static value is used"));
         return;
     }
     // Key times count in the property's time scale (a stretched layer's is scaled), as in
@@ -252,7 +252,7 @@ fn import_orientation_keys(layer: &AepLayer<'_, '_>, layer_name: &str, tm: &Time
     for k in &stored {
         let value = Value::Vec3(k.degrees);
         if !sane(&value) {
-            col.lose(LossCategory::DamagedData, format!("{what}: keyframe value is not a number"));
+            col.lose_with(LossCategory::DamagedData, || format!("{what}: keyframe value is not a number"));
             continue;
         }
         let mut key = Keyframe::new(tm.units(i64::from(k.time_units), scale), value);
@@ -268,10 +268,10 @@ fn import_orientation_keys(layer: &AepLayer<'_, '_>, layer_name: &str, tm: &Time
         keys.push(key);
     }
     if approximated {
-        col.lose(LossCategory::ApproximatedKeyframes, format!("{what}: bezier key read as linear (the Orientation ease is not sampled)"));
+        col.lose_with(LossCategory::ApproximatedKeyframes, || format!("{what}: bezier key read as linear (the Orientation ease is not sampled)"));
     }
-    keys.sort_by_key(|k| k.time);
-    keys.dedup_by_key(|k| k.time);
+    // Reported as on every other property (`import_keys`, mask paths).
+    sort_unique(&mut keys, &what, "keyframes", col);
     col.imported.keyframes = col.imported.keyframes.saturating_add(keys.len());
     if let Some(first) = keys.first() {
         prop.value = first.value.clone();
@@ -290,7 +290,7 @@ fn report_other_properties(group: &PropGroup<'_, '_>, layer_name: &str, col: &mu
         // A property in its default state (`tdsb` 3) holds nothing to lose.
         let stored = p.is_animated() || p.flags() == Some(1) || p.expression().is_some();
         if stored && !(IGNORED.contains(&name) && p.flags() != Some(1) && !p.is_animated()) {
-            col.lose(LossCategory::TransformProperties, format!("{layer_name}: {name}"));
+            col.lose_with(LossCategory::TransformProperties, || format!("{layer_name}: {name}"));
         }
     }
 }
@@ -309,14 +309,26 @@ pub(crate) fn interp(i: Option<Interpolation>, col: &mut Collector, what: &str)
         Some(Interpolation::Bezier) => Interp::Bezier,
         Some(Interpolation::Hold) => Interp::Hold,
         Some(Interpolation::Other(code)) => {
-            col.lose(LossCategory::ApproximatedKeyframes, format!("{what}: unknown interpolation {code} read as linear"));
+            col.lose_with(LossCategory::ApproximatedKeyframes, || format!("{what}: unknown interpolation {code} read as linear"));
             Interp::Linear
         }
     }
 }
 
-/// An ease that can be evaluated: finite speed, influence within 0.1 % and 100 %.
-fn ease(speed: f64, influence: f64, factor: f64) -> Option<Ease> {
+/// Puts `keys` in time order and keeps the first of keys at the same time, reporting that some
+/// were dropped (`noun` names them: "keyframes", "path keyframes").
+pub(crate) fn sort_unique(keys: &mut Vec<Keyframe>, what: &str, noun: &str, col: &mut Collector) {
+    keys.sort_by_key(|k| k.time);
+    let before = keys.len();
+    keys.dedup_by_key(|k| k.time);
+    if keys.len() != before {
+        col.lose_with(LossCategory::DamagedData, || format!("{what}: {noun} at the same time"));
+    }
+}
+
+/// An ease that can be evaluated: finite speed within `LIMIT * 100`, influence within 0.1 % and
+/// 100 % (also the rule for mask path keys, `import/masks.rs`).
+pub(crate) fn ease(speed: f64, influence: f64, factor: f64) -> Option<Ease> {
     let speed = speed * factor;
     (speed.is_finite() && speed.abs() <= LIMIT * 100.0 && influence.is_finite()).then(|| Ease { speed, influence: influence.clamp(0.001, 1.0) })
 }
@@ -358,11 +370,11 @@ pub(crate) fn import_keys(node: &AepProp<'_, '_>, kind: Kind, layer_name: &str,
     let mut keys: Vec<Keyframe> = Vec::new();
     for rec in list.keyframes() {
         let (Some(units), Some(raw)) = (rec.time_units(), rec.value()) else {
-            col.lose(LossCategory::DamagedData, format!("{what}: unreadable keyframe"));
+            col.lose_with(LossCategory::DamagedData, || format!("{what}: unreadable keyframe"));
             continue;
         };
         let Some(value) = kind.value(&raw) else {
-            col.lose(LossCategory::DamagedData, format!("{what}: keyframe value is missing or not a number"));
+            col.lose_with(LossCategory::DamagedData, || format!("{what}: keyframe value is missing or not a number"));
             continue;
         };
         let mut key = Keyframe::new(tm.units(i64::from(units), scale), value);
@@ -404,7 +416,7 @@ pub(crate) fn import_keys(node: &AepProp<'_, '_>, kind: Kind, layer_name: &str,
                 None => {
                     // The ease cannot be used, so the bezier sides become linear.
                     let why = if undecoded { "the ease of this effect parameter is not decoded" } else { "ease data not decodable" };
-                    col.lose(LossCategory::ApproximatedKeyframes, format!("{what}: bezier key read as linear ({why})"));
+                    col.lose_with(LossCategory::ApproximatedKeyframes, || format!("{what}: bezier key read as linear ({why})"));
                     if key.in_interp == Interp::Bezier {
                         key.in_interp = Interp::Linear;
                     }
@@ -419,12 +431,7 @@ pub(crate) fn import_keys(node: &AepProp<'_, '_>, kind: Kind, layer_name: &str,
     if keys.is_empty() {
         return;
     }
-    keys.sort_by_key(|k| k.time);
-    let before = keys.len();
-    keys.dedup_by_key(|k| k.time);
-    if keys.len() != before {
-        col.lose(LossCategory::DamagedData, format!("{what}: keyframes at the same time"));
-    }
+    sort_unique(&mut keys, &what, "keyframes", col);
     // The first key's incoming and the last key's outgoing ease mean nothing.
     if let Some(first) = keys.first_mut() {
         first.in_ease.clear();
@@ -438,3 +445,25 @@ pub(crate) fn import_keys(node: &AepProp<'_, '_>, kind: Kind, layer_name: &str,
     }
     prop.keys = keys;
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use effectcraft_time::Tick;
+
+    #[test]
+    fn keys_at_the_same_time_are_merged_and_reported() {
+        let key = |t: i64, v: f64| Keyframe::new(Tick(t), Value::Scalar(v));
+        let mut keys = vec![key(5, 1.0), key(0, 2.0), key(5, 3.0)];
+        let mut col = Collector::default();
+        sort_unique(&mut keys, "L: ADBE Orientation", "keyframes", &mut col);
+        assert_eq!(keys.iter().map(|k| (k.time.0, k.value.clone())).collect::<Vec<_>>(), [(0, Value::Scalar(2.0)), (5, Value::Scalar(1.0))]);
+        let report = col.finish();
+        assert_eq!(report.count(LossCategory::DamagedData), 1);
+        assert_eq!(report.losses[0].details, ["L: ADBE Orientation: keyframes at the same time"]);
+        // Distinct times: nothing reported.
+        let mut col = Collector::default();
+        sort_unique(&mut vec![key(1, 0.0), key(0, 0.0)], "L", "keyframes", &mut col);
+        assert!(col.finish().losses.is_empty());
+    }
+}

crates/aep/src/import/styles.rs

@@ -54,7 +54,7 @@ fn key_kind(conv: StyleConv) -> Option<Kind> {
 fn import_param(spec: &StyleParam, full: &str, node: Option<PropNode<'_, '_>>, prop: &mut EcProp, tm: &TimeMap, what: &str, col: &mut Collector) {
     if spec.conv == StyleConv::Gradient {
         if node.is_some() {
-            col.lose(LossCategory::LayerStyles, format!("{what}: {full} (gradient colours are not decoded, EffectCraft's default gradient kept)"));
+            col.lose_with(LossCategory::LayerStyles, || format!("{what}: {full} (gradient colours are not decoded, EffectCraft's default gradient kept)"));
         }
         return;
     }
@@ -63,7 +63,7 @@ fn import_param(spec: &StyleParam, full: &str, node: Option<PropNode<'_, '_>>, p
     }
     let Some(node) = node else { return };
     let PropNode::Property(p) = node else {
-        col.lose(LossCategory::LayerStyles, format!("{what}: {full} is not a property, default used"));
+        col.lose_with(LossCategory::LayerStyles, || format!("{what}: {full} is not a property, default used"));
         return;
     };
     import_record(spec.conv, &p, full, prop, tm, what, col);
@@ -73,12 +73,12 @@ fn import_record(conv: StyleConv, p: &AepProp<'_, '_>, full: &str, prop: &mut Ec
     if p.is_animated() {
         match key_kind(conv) {
             Some(kind) => import_keys(p, kind, what, full, tm, prop, col),
-            None => col.lose(LossCategory::LayerStyles, format!("{what}: keys on {full} are not imported (not sampled), default used")),
+            None => col.lose_with(LossCategory::LayerStyles, || format!("{what}: keys on {full} are not imported (not sampled), default used")),
         }
     } else if let Some(stored) = p.static_value() {
         match value(conv, &stored) {
             Some(v) => prop.value = v,
-            None => col.lose(LossCategory::LayerStyles, format!("{what}: {full} holds a value that is not valid ({stored:?}), default used")),
+            None => col.lose_with(LossCategory::LayerStyles, || format!("{what}: {full} holds a value that is not valid ({stored:?}), default used")),
         }
     }
     import_expression(p, prop, col);
@@ -102,33 +102,37 @@ pub(crate) fn import_layer_styles(layer: &AepLayer<'_, '_>, l: &mut Layer, gl: &
             Some(sm::FLAGS_ON) => true,
             Some(sm::FLAGS_OFF) => false,
             other => {
-                col.lose(LossCategory::LayerStyles, format!("{what}: style {prefix} in state {} (not sampled), not imported", other.unwrap_or(0)));
+                col.lose_with(LossCategory::LayerStyles, || format!("{what}: style {prefix} in state {} (not sampled), not imported", other.unwrap_or(0)));
                 continue;
             }
         };
         match sm::by_ae(prefix) {
             Some(spec) => applied.push((spec, g, on)),
-            None if prefix == sm::PATTERN_OVERLAY => col.lose(LossCategory::LayerStyles, format!("{what}: Pattern Overlay (no counterpart)")),
-            None => col.lose(LossCategory::LayerStyles, format!("{what}: unknown layer style {prefix}")),
+            None if prefix == sm::PATTERN_OVERLAY => col.lose_with(LossCategory::LayerStyles, || format!("{what}: Pattern Overlay (no counterpart)")),
+            None => col.lose_with(LossCategory::LayerStyles, || format!("{what}: unknown layer style {prefix}")),
         }
     }
     if applied.is_empty() {
         // Without a style the Blending Options group holds only an empty Advanced Blending group
         // (every case); stored values there have nowhere to go.
-        if let Some(PropNode::Group(bo)) = ls.get(sm::BLEND_OPTIONS)
-            && holds_values(&bo, 0)
-        {
-            col.lose(LossCategory::LayerStyles, format!("{what}: Blending Options set without a layer style, not imported"));
+        if let Some(PropNode::Group(bo)) = ls.get(sm::BLEND_OPTIONS) {
+            match holds_values(&bo, 0) {
+                Held::Values => col.lose_with(LossCategory::LayerStyles, || format!("{what}: Blending Options set without a layer style, not imported")),
+                Held::TooDeep => col.lose_with(LossCategory::LayerStyles, || {
+                    format!("{what}: Blending Options nest groups deeper than {MAX_HELD_DEPTH} levels, not read (they may hold values)")
+                }),
+                Held::Nothing => {}
+            }
         }
         return;
     }
     if !l.can_have_styles() {
-        col.lose(LossCategory::LayerStyles, format!("{what}: this kind of layer takes no layer styles here ({} not imported)", applied.len()));
+        col.lose_with(LossCategory::LayerStyles, || format!("{what}: this kind of layer takes no layer styles here ({} not imported)", applied.len()));
         return;
     }
     for (spec, g, on) in &applied {
         let Some(uid) = st::add_style(l, ids, spec.ec, gl, *on) else {
-            col.lose(LossCategory::LayerStyles, format!("{what}: style {} could not be added", spec.ae));
+            col.lose_with(LossCategory::LayerStyles, || format!("{what}: style {} could not be added", spec.ae));
             continue;
         };
         let Some(group) = l.props.find_group_mut(uid) else { continue };
@@ -138,12 +142,12 @@ pub(crate) fn import_layer_styles(layer: &AepLayer<'_, '_>, l: &mut Layer, gl: &
             let node = records.iter().find(|r| r.match_name == full).map(|r| r.node);
             match group.get_mut(prm.ec) {
                 Some(prop) => import_param(prm, &full, node, prop, tm, what, col),
-                None => col.lose(LossCategory::LayerStyles, format!("{what}: {full} has no counterpart '{}'", prm.ec)),
+                None => col.lose_with(LossCategory::LayerStyles, || format!("{what}: {full} has no counterpart '{}'", prm.ec)),
             }
         }
         for r in &records {
             if r.match_name != "ADBE Group End" && !spec.params.iter().any(|prm| spec.param_name(prm) == r.match_name) {
-                col.lose(LossCategory::LayerStyles, format!("{what}: {} is not imported (unknown parameter)", r.match_name));
+                col.lose_with(LossCategory::LayerStyles, || format!("{what}: {} is not imported (unknown parameter)", r.match_name));
             }
         }
         col.imported.layer_styles += 1;
@@ -151,14 +155,40 @@ pub(crate) fn import_layer_styles(layer: &AepLayer<'_, '_>, l: &mut Layer, gl: &
     import_blending(&ls, l, tm, what, col);
 }
 
-/// True when `g` or a group below it (to a bounded depth) stores a property.
-fn holds_values(g: &AepGroup<'_, '_>, depth: usize) -> bool {
-    depth < 8
-        && g.entries().iter().any(|e| match e.node {
-            PropNode::Property(_) => true,
-            PropNode::Group(sub) => holds_values(&sub, depth.saturating_add(1)),
-            PropNode::Other(_) => false,
-        })
+/// The deepest group below Blending Options that is looked into for stored values.
+const MAX_HELD_DEPTH: usize = 8;
+
+/// What a group stores, looked at to [`MAX_HELD_DEPTH`].
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+enum Held {
+    Nothing,
+    /// A property, in the group or a group below it.
+    Values,
+    /// No property within the depth, but a group at the depth limit that is not empty (it may
+    /// hold values; deeper is not read).
+    TooDeep,
+}
+
+/// Whether `g` or a group below it (to a bounded depth) stores a property. A non-empty group at
+/// the depth limit is not looked into: it counts as [`Held::TooDeep`] (reported), not as empty;
+/// After Effects nests Blending Options two levels deep.
+fn holds_values(g: &AepGroup<'_, '_>, depth: usize) -> Held {
+    if depth >= MAX_HELD_DEPTH {
+        return if g.entries().is_empty() { Held::Nothing } else { Held::TooDeep };
+    }
+    let mut held = Held::Nothing;
+    for e in g.entries() {
+        match e.node {
+            PropNode::Property(_) => return Held::Values,
+            PropNode::Group(sub) => match holds_values(&sub, depth.saturating_add(1)) {
+                Held::Values => return Held::Values,
+                Held::TooDeep => held = Held::TooDeep,
+                Held::Nothing => {}
+            },
+            PropNode::Other(_) => {}
+        }
+    }
+    held
 }
 
 /// Blending Options: Global Light (adopted by the composition later) and Advanced Blending.

crates/aep/tests/it/export.rs

@@ -3,6 +3,7 @@
 #[macro_use]
 mod helpers;
 
+mod bounds;
 mod built;
 mod ease;
 mod effects;

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

@@ -0,0 +1,72 @@
+//! Projects that used to cost quadratic time to export (#3299): many layers of one
+//! precomposition in a composition that is itself shown many times, and a long parent chain. The
+//! sizes are moderate, so the tests check what is written; the bounds are pinned by the unit tests
+//! in `export::plan`.
+
+use std::sync::Arc;
+
+use crate::export::helpers::*;
+use effectcraft_aep::export::LossCategory;
+use effectcraft_project::{ItemId, ItemKind, LayerSource, Project, build};
+use effectcraft_time::FrameRate;
+
+/// Puts `n` layers of `source` into `comp` at once; `edit` changes layer `i`.
+fn fill(p: &mut Project, comp: ItemId, n: usize, source: LayerSource, edit: &dyn Fn(usize, &mut effectcraft_project::Layer)) {
+    let mut c = p.comp(comp).cloned().expect("comp");
+    for i in 0..n {
+        let mut l = build::layer(p, &c, &format!("L{i}"), source.clone(), (100, 100), None);
+        l.name = format!("L{i}");
+        edit(i, &mut l);
+        c.layers.push(l);
+    }
+    if let Some(item) = p.item_mut(comp) {
+        item.kind = ItemKind::Comp(Arc::new(c));
+    }
+}
+
+#[test]
+fn many_layers_of_one_precomp_in_a_shown_composition_are_written() {
+    let n = 1500;
+    let mut p = Project::default();
+    let a = add_comp(&mut p, "A", 100, 100, FrameRate::new(25, 1), 1.0, None);
+    let s = add_solid(&mut p, "S", 100, 100, [1.0, 0.0, 0.0], None);
+    add_layer(&mut p, a, "S", LayerSource::Solid { item: s }, (100, 100));
+    let b = add_comp(&mut p, "B", 100, 100, FrameRate::new(25, 1), 1.0, None);
+    fill(&mut p, b, n, LayerSource::Comp { item: a }, &|_, _| {});
+    let c = add_comp(&mut p, "C", 100, 100, FrameRate::new(25, 1), 1.0, None);
+    fill(&mut p, c, n, LayerSource::Comp { item: b }, &|_, _| {});
+    let r = export_all(&p);
+    assert_eq!(r.report.exported.precomp_layers, 2 * n);
+    assert_eq!(r.report.count(LossCategory::UnsupportedLayers), 0, "{:?}", r.report.losses);
+}
+
+#[test]
+fn a_long_parent_chain_is_written_and_its_loop_cut_once() {
+    let n = 1500;
+    let mut p = Project::default();
+    let s = add_solid(&mut p, "S", 100, 100, [1.0, 0.0, 0.0], None);
+    let c = add_comp(&mut p, "C", 100, 100, FrameRate::new(25, 1), 1.0, None);
+    fill(&mut p, c, n, LayerSource::Solid { item: s }, &|_, _| {});
+    // Each layer parented to the next; with `closed`, the last to the first (a loop).
+    for closed in [false, true] {
+        let mut q = p.clone();
+        with_comp(&mut q, c, |cc| {
+            let ids: Vec<_> = cc.layers.iter().map(|l| l.id).collect();
+            for i in 0..n {
+                let next = if i + 1 < n {
+                    Some(ids[i + 1])
+                } else if closed {
+                    Some(ids[0])
+                } else {
+                    None
+                };
+                if let Some(l) = cc.layers.get_mut(i) {
+                    l.parent = next;
+                }
+            }
+        });
+        let r = export_all(&q);
+        assert_eq!(r.report.exported.layers, n);
+        assert_eq!(r.report.count(LossCategory::ParentLinks), usize::from(closed), "{:?}", r.report.losses);
+    }
+}

crates/aep/tests/it/import.rs

@@ -5,6 +5,7 @@ mod helpers;
 
 mod aepx;
 mod blend_modes;
+mod bounds;
 mod cameras;
 mod cases;
 mod combined;

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

@@ -0,0 +1,316 @@
+//! Hostile inputs that used to cost quadratic time, and losses that used to be silent (#3299).
+//!
+//! The quadratic cases are built from the committed `tests/fixtures/synthetic.aep` (composition
+//! `C`, id 2, with a sequence layer, id 17, and a solid layer, id 16): its composition item and
+//! its solid layer are copied with new ids. The sizes here are moderate, so the tests check what
+//! arrives; the bounds themselves are pinned by the unit tests next to the code
+//! (`import::build`, `import::effects`).
+
+use effectcraft_aep::import::{ImportOptions, ImportResult, LossCategory, import};
+use effectcraft_aep::{AepFile, Body, Chunk, parse, write};
+use effectcraft_project::{ItemKind, LayerId, LayerSource, Project};
+
+const SYNTHETIC: &[u8] = include_bytes!("../../fixtures/synthetic.aep");
+/// The item id of composition `C` and the layer id of its solid layer.
+const COMP_C: u32 = 2;
+const SOLID_LAYER: u32 = 16;
+
+fn run(bytes: &[u8]) -> (Project, ImportResult) {
+    let mut project = Project::default();
+    let none = |_: &str| false;
+    let r = import(&mut project, bytes, &ImportOptions { allow_losses: true, ..ImportOptions::new(&none) }).expect("imports");
+    (project, r)
+}
+
+fn losses(r: &ImportResult, category: LossCategory) -> Vec<String> {
+    r.report.losses.iter().filter(|l| l.category == category).flat_map(|l| l.details.clone()).collect()
+}
+
+fn set_be_u32(c: &mut Chunk<'_>, at: usize, v: u32) {
+    if let Body::Data(d) = &mut c.body {
+        d.to_mut()[at..at + 4].copy_from_slice(&v.to_be_bytes());
+    }
+}
+
+fn be_u32(c: &Chunk<'_>, at: usize) -> u32 {
+    u32::from_be_bytes(c.bytes().expect("data")[at..at + 4].try_into().unwrap())
+}
+
+fn children<'c, 'a>(c: &'c mut Chunk<'a>) -> &'c mut Vec<Chunk<'a>> {
+    match &mut c.body {
+        Body::List { children, .. } | Body::Chunks(children) => children,
+        _ => panic!("not a list"),
+    }
+}
+
+/// The project's folder list (`LIST Fold`, top level).
+fn fold<'f, 'a>(file: &'f mut AepFile<'a>) -> &'f mut Vec<Chunk<'a>> {
+    children(file.chunks.iter_mut().find(|c| c.is_list(b"Fold")).expect("Fold"))
+}
+
+/// The `Item` list of composition `C`.
+fn comp_c<'a>(file: &AepFile<'a>) -> Chunk<'a> {
+    let fold = file.list(b"Fold").expect("Fold");
+    fold.children().iter().find(|c| c.is_list(b"Item") && c.child(b"idta").is_some_and(|i| be_u32(i, 16) == COMP_C)).expect("composition C").clone()
+}
+
+/// The solid layer of `C` (`LIST Layr` with id 16).
+fn solid_layer<'a>(comp: &Chunk<'a>) -> Chunk<'a> {
+    comp.children().iter().find(|c| c.is_list(b"Layr") && c.child(b"ldta").is_some_and(|l| be_u32(l, 0) == SOLID_LAYER)).expect("the solid layer").clone()
+}
+
+/// A copy of `layer` with layer id `id`; `edit` changes its `ldta`.
+fn layer_copy<'a>(layer: &Chunk<'a>, id: u32, edit: &dyn Fn(&mut Chunk<'a>)) -> Chunk<'a> {
+    let mut l = layer.clone();
+    let ldta = children(&mut l).iter_mut().find(|c| c.is(b"ldta")).expect("ldta");
+    set_be_u32(ldta, 0, id);
+    edit(ldta);
+    l
+}
+
+/// A composition like `C` with item id `id`, name `name` and only the layers `layers`.
+fn comp_with<'a>(c: &Chunk<'a>, id: u32, name: &str, layers: Vec<Chunk<'a>>) -> Chunk<'a> {
+    let mut item = c.clone();
+    let kids = children(&mut item);
+    kids.retain(|k| !k.is_list(b"Layr"));
+    for k in kids.iter_mut() {
+        if k.is(b"idta") {
+            set_be_u32(k, 16, id);
+        }
+        if k.is(b"Utf8") {
+            *k = Chunk::data(*b"Utf8", name.as_bytes().to_vec());
+        }
+    }
+    let at = kids.iter().position(|k| k.is_list(b"Ewst")).unwrap_or(kids.len());
+    kids.splice(at..at, layers);
+    item
+}
+
+/// 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> {
+    let mut file = parse(SYNTHETIC).unwrap();
+    let c = comp_c(&file);
+    let solid = solid_layer(&c);
+    let layers = |first: u32, source: u32| -> Vec<Chunk<'_>> { (0..n).map(|i| layer_copy(&solid, first + i, &|l| set_be_u32(l, 40, source))).collect() };
+    let b = comp_with(&c, 900, "B", layers(100_000, COMP_C));
+    let d = comp_with(&c, 901, "D", layers(200_000, 900));
+    let fold = fold(&mut file);
+    let at = fold.iter().position(|k| k.is_list(b"Item") && k.child(b"idta").is_some_and(|i| be_u32(i, 16) == COMP_C)).expect("C") + 1;
+    fold.splice(at..at, [b, d]);
+    if c_uses_d {
+        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)));
+    }
+    write(&file).unwrap()
+}
+
+fn comp_named<'p>(p: &'p Project, name: &str) -> &'p effectcraft_project::Comp {
+    p.items.values().find_map(|i| if let ItemKind::Comp(c) = &i.kind { (i.name == name).then_some(&**c) } else { None }).unwrap_or_else(|| panic!("no {name}"))
+}
+
+#[test]
+fn many_layers_of_one_precomp_in_a_used_composition_import() {
+    let n = 1500;
+    let (p, r) = run(&precomp_chain(n, false));
+    let (b, d) = (comp_named(&p, "B"), comp_named(&p, "D"));
+    let item = |name: &str| p.items.values().find(|i| i.name == name).map(|i| i.id).expect("item");
+    assert_eq!(b.layers.len(), n as usize);
+    assert!(b.layers.iter().all(|l| l.source == LayerSource::Comp { item: item("C") }));
+    assert!(d.layers.iter().all(|l| l.source == LayerSource::Comp { item: item("B") }));
+    assert_eq!(d.layers.len(), n as usize);
+    assert_eq!(r.report.imported.precomp_layers, 2 * n as usize);
+    let found = losses(&r, LossCategory::UnsupportedLayers);
+    assert!(!found.iter().any(|d| d.contains("loop")), "{found:?}");
+}
+
+#[test]
+fn every_precomp_layer_that_closes_a_loop_is_refused() {
+    // C (built first) uses D; B uses C; so D's layers of B would close C -> D -> B -> C.
+    let n = 300;
+    let (_, base) = run(SYNTHETIC);
+    let (p, r) = run(&precomp_chain(n, true));
+    assert_eq!(comp_named(&p, "B").layers.len(), n as usize);
+    assert!(comp_named(&p, "D").layers.is_empty());
+    let refused = r.report.count(LossCategory::UnsupportedLayers) - base.report.count(LossCategory::UnsupportedLayers);
+    assert_eq!(refused, n as usize, "{:?}", r.report.losses);
+    assert!(losses(&r, LossCategory::UnsupportedLayers).iter().any(|d| d.contains("precomposition loop")));
+}
+
+#[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.
+    let n = 1500;
+    let mut file = parse(SYNTHETIC).unwrap();
+    let c = comp_c(&file);
+    let solid = solid_layer(&c);
+    let fold = fold(&mut file);
+    let item = 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(item);
+    let at = kids.iter().position(|k| k.is_list(b"Layr")).expect("a layer");
+    let copies: Vec<Chunk<'_>> = (0..n)
+        .map(|i| {
+            layer_copy(&solid, 100_000 + i, &|l| {
+                set_be_u32(l, 132, SOLID_LAYER);
+                if let Body::Data(d) = &mut l.body {
+                    d.to_mut()[107] = 1;
+                }
+                set_be_u32(l, 160, SOLID_LAYER);
+            })
+        })
+        .collect();
+    kids.splice(at..at, copies);
+    let (p, r) = run(&write(&file).unwrap());
+    let c = comp_named(&p, "C");
+    // The original solid layer: the last layer whose source is the solid and that has no parent.
+    let target: LayerId = c.layers.iter().rev().find(|l| l.parent.is_none() && matches!(l.source, LayerSource::Solid { .. })).map(|l| l.id).expect("the solid");
+    let linked: Vec<_> = c.layers.iter().filter(|l| l.parent.is_some()).collect();
+    assert_eq!(linked.len(), n as usize);
+    assert!(linked.iter().all(|l| l.parent == Some(target) && l.track_matte.map(|m| m.layer) == Some(target)));
+    assert_eq!(r.report.imported.track_mattes, n as usize);
+    assert_eq!(r.report.count(LossCategory::ParentLinks), 0, "{:?}", r.report.losses);
+}
+
+/// `count` nested groups (`tdmn` + `LIST tdgp`), the innermost empty.
+fn nested(count: usize) -> Vec<Chunk<'static>> {
+    let end = || {
+        let mut name = b"ADBE Group End".to_vec();
+        name.resize(40, 0);
+        Chunk::data(*b"tdmn", name)
+    };
+    let mut inner = vec![end()];
+    for _ in 0..count {
+        let mut name = b"ADBE Deep Group".to_vec();
+        name.resize(40, 0);
+        inner = vec![Chunk::data(*b"tdmn", name), Chunk::list(*b"tdgp", inner), end()];
+    }
+    inner
+}
+
+#[test]
+fn blending_options_nested_past_the_read_depth_are_reported() {
+    let mut file = parse(SYNTHETIC).unwrap();
+    let mut patched = 0;
+    // Every layer's Advanced Blending group gets groups nested twelve deep (no property in them).
+    fn visit(chunks: &mut [Chunk<'_>], patched: &mut usize) {
+        let mut next_is_adv = false;
+        for c in chunks.iter_mut() {
+            if next_is_adv && c.is_list(b"tdgp") {
+                *children(c) = nested(12);
+                *patched += 1;
+            }
+            next_is_adv = c.is(b"tdmn") && c.bytes().is_some_and(|b| b.starts_with(b"ADBE Adv Blend Group\0"));
+            visit(c.children_mut(), patched);
+        }
+    }
+    visit(&mut file.chunks, &mut patched);
+    assert!(patched >= 2, "{patched}");
+    let (_, r) = run(&write(&file).unwrap());
+    let found = losses(&r, LossCategory::LayerStyles);
+    assert!(found.iter().any(|d| d.contains("deeper than 8 levels")), "{found:?}");
+    // Nested within the depth, an empty group still holds nothing.
+    let mut file = parse(SYNTHETIC).unwrap();
+    fn shallow(chunks: &mut [Chunk<'_>]) {
+        let mut next_is_adv = false;
+        for c in chunks.iter_mut() {
+            if next_is_adv && c.is_list(b"tdgp") {
+                *children(c) = nested(3);
+            }
+            next_is_adv = c.is(b"tdmn") && c.bytes().is_some_and(|b| b.starts_with(b"ADBE Adv Blend Group\0"));
+            shallow(c.children_mut());
+        }
+    }
+    shallow(&mut file.chunks);
+    let (_, r) = run(&write(&file).unwrap());
+    assert_eq!(losses(&r, LossCategory::LayerStyles), Vec::<String>::new());
+}
+
+/// Every key record of the property `match_name` (the `LIST list` of its `LIST tdbs`; a mask
+/// shape's vertex lists are not touched) gets `patch`.
+fn patch_keys(file: &mut AepFile<'_>, match_name: &[u8], patch: &dyn Fn(usize, &mut [u8])) -> usize {
+    fn lists(c: &mut Chunk<'_>, patch: &dyn Fn(usize, &mut [u8]), n: &mut usize) {
+        if c.is_list(b"tdbs") {
+            for k in c.children_mut() {
+                keys(k, patch, n);
+            }
+            return;
+        }
+        for k in c.children_mut() {
+            lists(k, patch, n);
+        }
+    }
+    fn keys(c: &mut Chunk<'_>, patch: &dyn Fn(usize, &mut [u8]), n: &mut usize) {
+        if c.is_list(b"list") {
+            let size = c.child(b"lhd3").and_then(|h| h.bytes()).map(|h| u32::from_be_bytes(h[16..20].try_into().unwrap()) as usize).unwrap_or(0);
+            if let Some(Body::Data(d)) = children(c).iter_mut().find(|k| k.is(b"ldat")).map(|k| &mut k.body)
+                && size > 0
+            {
+                for (i, rec) in d.to_mut().chunks_exact_mut(size).enumerate() {
+                    patch(i, rec);
+                    *n += 1;
+                }
+            }
+        }
+    }
+    fn visit(chunks: &mut [Chunk<'_>], name: &[u8], patch: &dyn Fn(usize, &mut [u8]), n: &mut usize) {
+        let mut next = false;
+        for c in chunks.iter_mut() {
+            if next {
+                lists(c, patch, n);
+            }
+            next = c.is(b"tdmn") && c.bytes().is_some_and(|b| b.strip_prefix(name).is_some_and(|r| r.first() == Some(&0)));
+            visit(c.children_mut(), name, patch, n);
+        }
+    }
+    let mut n = 0;
+    visit(&mut file.chunks, match_name, patch, &mut n);
+    n
+}
+
+#[test]
+fn orientation_keys_at_the_same_time_are_reported() {
+    let Some(bytes) = crate::common::try_fixture("703-key-orientation-0-to-10-20-30-linear.aep") else { return };
+    let mut file = parse(&bytes).unwrap();
+    // The second key at the first key's time (the record starts with its time).
+    let first = std::cell::Cell::new([0u8; 4]);
+    let n = patch_keys(&mut file, b"ADBE Orientation", &|i, rec| {
+        if i == 0 {
+            first.set(rec[0..4].try_into().unwrap());
+        } else {
+            rec[0..4].copy_from_slice(&first.get());
+        }
+    });
+    assert!(n >= 2, "{n}");
+    let (p, r) = run(&write(&file).unwrap());
+    let found = losses(&r, LossCategory::DamagedData);
+    assert!(found.iter().any(|d| d.contains("ADBE Orientation: keyframes at the same time")), "{found:?}");
+    let l = comp_named(&p, "C").layers.iter().find(|l| l.name == "L").expect("L");
+    assert_eq!(l.props.prop("transform/orientation").expect("orientation").keys.len(), 1);
+}
+
+#[test]
+fn a_mask_path_ease_speed_beyond_the_limit_is_not_imported() {
+    let Some(bytes) = crate::common::try_fixture("700-key-mask-path-ease-out20-in50.aep") else { return };
+    let mut file = parse(&bytes).unwrap();
+    // Both speeds (bytes 24 and 40 of a mask path record) far past what a property key accepts.
+    let n = patch_keys(&mut file, b"ADBE Mask Shape", &|_, rec| {
+        rec[24..32].copy_from_slice(&1e12f64.to_be_bytes());
+        rec[40..48].copy_from_slice(&1e12f64.to_be_bytes());
+    });
+    assert!(n >= 2, "{n}");
+    let (p, r) = run(&write(&file).unwrap());
+    let found = losses(&r, LossCategory::ApproximatedKeyframes);
+    assert!(found.iter().any(|d| d.contains("its ease is unreadable")), "{found:?}");
+    let speeds: Vec<f64> = p
+        .items
+        .values()
+        .filter_map(|i| if let ItemKind::Comp(c) = &i.kind { Some(c.layers.iter().cloned().collect::<Vec<_>>()) } else { None })
+        .flatten()
+        .flat_map(|l| l.props.sub("masks").map(|m| m.groups().filter_map(|g| g.prop("path").cloned()).collect::<Vec<_>>()).unwrap_or_default())
+        .flat_map(|p| p.keys)
+        .flat_map(|k| k.in_ease.into_iter().chain(k.out_ease).map(|e| e.speed))
+        .collect();
+    assert!(speeds.iter().all(|s| s.abs() <= 1e11), "{speeds:?}");
+}