From a44b042fce81adfe5520acc693c3328274470367 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Wed, 11 Nov 2026 17:00:44 +0530 Subject: [PATCH] #3087: report the items a placeholder popup cuts A popup of more than 1024 items kept only the first 1024 without a word, and a valid selection past them was reported as damaged data. The cut is now reported, and such a selection takes the first item and says so. Co-Authored-By: Claude Opus 5.5 --- crates/aep/src/import/placeholder.rs | 20 ++++++++++++++++---- crates/aep/tests/it/import/placeholders.rs | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/crates/aep/src/import/placeholder.rs b/crates/aep/src/import/placeholder.rs index 4e9fd8047..01d845f24 100644 --- a/crates/aep/src/import/placeholder.rs +++ b/crates/aep/src/import/placeholder.rs @@ -60,6 +60,9 @@ pub(crate) fn placeholder( ParamUi::Popup { options } => options.len(), _ => 0, }; + if p.kind == ParamKind::Popup && popup_len(p).is_some_and(|n| n > items) { + col.lose(LossCategory::Effects, format!("{here}: parameter {label}: the items past the first {MAX_POPUP_ITEMS} not kept")); + } 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)")); @@ -99,7 +102,7 @@ fn shape(p: &EffectParam<'_, '_>, size: (u32, u32)) -> Option<(Option, Par // The file's items, separators (`(-`) included: After Effects counts them. let options: Vec = match &p.items { Some(items) => items.iter().take(MAX_POPUP_ITEMS).cloned().collect(), - None => (1..=usize::from(p.popup_item_count()?).min(MAX_POPUP_ITEMS)).map(|i| format!("Item {i}")).collect(), + None => (1..=popup_len(p)?.min(MAX_POPUP_ITEMS)).map(|i| format!("Item {i}")).collect(), }; let n = u32::try_from(options.len()).ok().filter(|n| *n > 0)?; (Kind::EffectPopup(n), ParamUi::Popup { options }, Value::Enum(0)) @@ -109,6 +112,11 @@ fn shape(p: &EffectParam<'_, '_>, size: (u32, u32)) -> Option<(Option, Par Some((Some(kind), ui, default)) } +/// How many items a popup has: the file's names, else the count in its definition. +fn popup_len(p: &EffectParam<'_, '_>) -> Option { + p.items.as_ref().map(Vec::len).or_else(|| p.popup_item_count().map(usize::from)) +} + /// The property id of `p`: its match name when that is one segment of a property path (a /// third-party effect's `CC Cylinder-0003`), else `param` (a pseudo effect's /// `Pseudo/0.25-0001`: `/` and `.` separate path segments). Expressions also find it by its name. @@ -159,8 +167,12 @@ fn fill(p: &EffectParam<'_, '_>, prop: &mut Property, kind: Kind, cx: &LayerCtx< None => return, }, }; - match convert(value, kind) { - Some(v) => prop.value = v, - None => col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds a value that is not valid ({value:?})")), + match (convert(value, kind), value) { + (Some(v), _) => prop.value = v, + // An item of the file that is past the ones kept (reported with the cut): the first item. + (None, ParamValue::Index(i)) if usize::try_from(i).is_ok_and(|i| i > MAX_POPUP_ITEMS && popup_len(p).is_some_and(|n| i <= n)) => { + col.lose(LossCategory::Effects, format!("{here}: parameter {label}: item {i} is past the items kept, the first item used")); + } + (None, _) => col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds a value that is not valid ({value:?})")), } } diff --git a/crates/aep/tests/it/import/placeholders.rs b/crates/aep/tests/it/import/placeholders.rs index 847060f93..9afccd6b9 100644 --- a/crates/aep/tests/it/import/placeholders.rs +++ b/crates/aep/tests/it/import/placeholders.rs @@ -125,6 +125,22 @@ fn a_popup_default_of_no_item_takes_the_first_item() { assert!(losses(&r, LossCategory::DamagedData, "Some Fx").is_empty()); } +/// A popup past the 1024 items kept: the cut is reported, and a selection among the cut items +/// takes the first item and is reported as such, not as damaged data. +#[test] +fn a_popup_past_the_items_kept_is_reported() { + let items: &'static [&'static str] = Box::leak((0..1100).map(|i| &*Box::leak(format!("I{i}").into_boxed_str())).collect::>().into_boxed_slice()); + let fx = Fx { match_name: "Long Fx", name: "Long", defs: vec![popup("Pick", 1, items)], values: vec![(1, 1050.0)], data: vec![], keys: vec![] }; + let (g, r) = placeholder_of(&fx); + let pick = g.prop("Long Fx-0001").unwrap(); + assert_eq!(pick.value, Value::Enum(0)); + assert!(matches!(&pick.ui, ParamUi::Popup { options } if options.len() == 1024)); + let found = losses(&r, LossCategory::Effects, "Long Fx"); + assert!(found.iter().any(|d| d.contains("past the first 1024")), "{found:?}"); + assert!(found.iter().any(|d| d.contains("item 1050 is past the items kept")), "{found:?}"); + assert!(losses(&r, LossCategory::DamagedData, "Long Fx").is_empty()); +} + /// Many large popups stay within the import's budget: what is beyond is reported, not kept. #[test] fn a_hostile_placeholder_stays_bounded() {