#3087: stop building placeholders once the import's budget is spent

A parameter past the budget was skipped and the next one tried, so the
rest of the effect, and every later effect without a counterpart, still
read its shared definitions and built popup items. The first parameter
that does not fit now ends the budget; a later effect without a
counterpart is reported on its own line without reading its definitions.
The docs now say that a placeholder's layer parameters report their keys
and expression.

crates/aep/src/import/effects.rs

@@ -92,6 +92,12 @@ pub(crate) fn import_effects<'f, 'a>(
             }
             continue;
         }
+        if found.is_none() && col.placeholder_cost >= crate::import::placeholder::MAX_COST {
+            // The import's placeholder budget is spent: the definitions (shared by many instances)
+            // are not read again. Its own line: grouped, it would read as kept.
+            col.lose(LossCategory::Effects, format!("{lost}, not kept (past the import's placeholder parameters)"));
+            continue;
+        }
         let params = defs.params(&fx);
         report_records_without_definition(&fx, &params, cx, &shown, col);
         let Some((map, spec)) = found else {

crates/aep/src/import/placeholder.rs

@@ -8,11 +8,12 @@
 //! The values keep After Effects' units, the ones its expressions read: sliders and angles as
 //! stored, points in layer pixels, colours RGBA 0..1, checkboxes on / off, popups as EffectCraft's
 //! 0-based items over the file's items in order (expressions read them 1-based, as After Effects
-//! does). Layer parameters name their layer. Paths and arbitrary data are not kept (reported when
-//! they hold something).
+//! does). Layer parameters name their layer (their keys and expression are reported). Paths and
+//! arbitrary data are not kept (reported when they hold something).
 //!
 //! The parameters a placeholder holds are bounded per import ([`MAX_COST`]): definitions can be
-//! shared by many instances, so a small file could otherwise ask for very many properties.
+//! shared by many instances, so a small file could otherwise ask for very many properties. Once
+//! the budget is spent, later effects without a counterpart are reported without a placeholder.
 
 use effectcraft_keyframe::Value;
 use effectcraft_project::build::Ids;
@@ -44,8 +45,6 @@ pub(crate) fn placeholder(
     let here = format!("{}: '{}' effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name());
     for p in params.iter().filter(|p| !carries_nothing(p)) {
         if col.placeholder_cost >= MAX_COST {
-            // Once the budget is spent nothing more is built (definitions shared by many instances
-            // would otherwise still be walked item by item).
             col.lose(LossCategory::Effects, format!("{here}: parameters past the import's {MAX_COST} placeholder parameters not kept"));
             break;
         }
@@ -65,8 +64,14 @@ pub(crate) fn placeholder(
         }
         let cost = col.placeholder_cost.saturating_add(items).saturating_add(1);
         if cost > MAX_COST {
-            col.lose(LossCategory::Effects, format!("{here}: parameter {label} not kept (past the import's {MAX_COST} placeholder parameters)"));
-            continue;
+            // The budget is spent: nothing more is built, here or in later effects (definitions
+            // shared by many instances would otherwise still be walked item by item).
+            col.lose(
+                LossCategory::Effects,
+                format!("{here}: parameter {label} and the ones after it not kept (past the import's {MAX_COST} placeholder parameters)"),
+            );
+            col.placeholder_cost = MAX_COST;
+            break;
         }
         col.placeholder_cost = cost;
         let mut prop = Property::new(ids.alloc(), &param_id(p), p.name.trim(), default).with_ui(ui);

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

@@ -153,6 +153,56 @@ fn a_hostile_placeholder_stays_bounded() {
     assert!(losses(&r, LossCategory::Effects, "Big Fx").iter().any(|d| d.contains("placeholder parameters")));
 }
 
+/// Two instances of an effect whose parameters fill the import's budget: the first keeps what fits
+/// and stops at the first parameter past it; the second is reported without a placeholder (its
+/// shared definitions are not read again).
+#[test]
+fn an_effect_past_the_spent_budget_gets_no_placeholder() {
+    let items: &'static [&'static str] = Box::leak((0..1000).map(|i| &*Box::leak(format!("I{i}").into_boxed_str())).collect::<Vec<_>>().into_boxed_slice());
+    let fx = Fx { match_name: "Big Fx", name: "Big", defs: (0..300).map(|_| popup("P", 1, items)).collect(), values: vec![], data: vec![], keys: vec![] };
+    let bytes = twice(&bytes_with(&fx), "Big Fx");
+    let mut project = effectcraft_project::Project::default();
+    let none = |_: &str| false;
+    let r = effectcraft_aep::import::import(
+        &mut project,
+        &bytes,
+        &effectcraft_aep::import::ImportOptions { allow_losses: true, ..effectcraft_aep::import::ImportOptions::new(&none) },
+    )
+    .unwrap();
+    let placeholders: Vec<usize> = project
+        .comps()
+        .flat_map(|(_, c)| c.layers.iter())
+        .filter_map(|l| l.props.sub("effects"))
+        .flat_map(|fx| fx.children.iter().filter_map(|n| n.as_group()).filter(|g| g.match_id == "Big Fx").map(|g| g.props().count()).collect::<Vec<_>>())
+        .collect();
+    // 199 popups of 1000 items (1001 each) fit in 200 000; the 200th stops the first instance.
+    assert_eq!(placeholders, [199]);
+    let found = losses(&r, LossCategory::Effects, "Big Fx");
+    assert_eq!(found.iter().filter(|d| d.contains("and the ones after it not kept")).count(), 1, "{found:?}");
+    assert!(found.iter().any(|d| d.contains("not kept (past the import's placeholder parameters)")), "{found:?}");
+}
+
+/// `bytes` with the layer's instance of `match_name` (its `tdmn` and `sspc`) repeated after it.
+fn twice(bytes: &[u8], match_name: &str) -> Vec<u8> {
+    let mut file = parse(bytes).unwrap();
+    let mut done = 0;
+    edit(&mut file.chunks, false, &mut |c, in_project_copy| {
+        let Body::List { children, .. } = &mut c.body else { return };
+        if in_project_copy {
+            return;
+        }
+        let Some(at) = children.iter().position(|k| k.is(b"tdmn") && k.bytes().is_some_and(|b| b.split(|x| *x == 0).next() == Some(match_name.as_bytes())))
+        else {
+            return;
+        };
+        let pair: Vec<Chunk<'_>> = children[at..at + 2].to_vec();
+        children.splice(at + 2..at + 2, pair);
+        done += 1;
+    });
+    assert_eq!(done, 1, "one instance");
+    write(&file).unwrap()
+}
+
 /// `bytes` with the expression `text` on the parameter record `param` (enabled: `tdb4` byte 120
 /// 1, docs/aep-format.md 3.6).
 fn with_expression(bytes: &[u8], param: &str, text: &str) -> Vec<u8> {

docs/aep-import.md

@@ -325,14 +325,16 @@ parameter gets the file's value:
   in After Effects, each with its value, keys and expression, in the units After Effects'
   expressions read (sliders and angles as stored, points in layer pixels, colours, checkboxes,
   popups over the file's items with separators, read 1-based; layer parameters name their layer,
-  without a record the layer itself as for Layer Control),
+  without a record the layer itself as for Layer Control, and their keys and expression are
+  reported),
   so expressions that read them (`effect("Settings")("Speed")`) keep working. A parameter's id is
   its match name (`CC Cylinder-0003`), or `param<index>` when that name is no path segment (a
   pseudo effect's `Pseudo/0.25-0001`). Paths and arbitrary data are not kept; a popup whose
   definition is borrowed, or whose default slot holds no item, and that has no value record takes
   its first item (reported). One import keeps at
   most 200 000 placeholder parameters and popup items (a popup at most 1024 items); the rest is
-  reported. A placeholder is still reported in `effects` and counted in
+  reported, and once that is spent a later effect without a counterpart arrives without a
+  placeholder (reported). A placeholder is still reported in `effects` and counted in
   `imported.placeholderEffects`; .aep export reports it and leaves it out.
 
 ## What is reported