From b60f231fe9153e911a6ad980078a77f697eeb8e9 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Wed, 11 Nov 2026 16:29:25 +0530 Subject: [PATCH 1/9] #3087: keep an effect without a counterpart as a placeholder Third-party plug-ins and pseudo effects (expression controls) were dropped on import, so expressions reading them broke. Such an effect now arrives as an effect group whose effect id is its After Effects match name: it renders nothing, like a missing plug-in, and is listed by Find Missing Effects. It holds the effect's parameters flat, named as in After Effects, with their values, keys and expressions in the units After Effects' expressions read. It is still reported (once per match name, "kept without rendering") and counted in imported.placeholderEffects. The parameters and popup items kept are bounded per import. Co-Authored-By: Claude Opus 5.5 --- crates/aep/src/import.rs | 4 +- crates/aep/src/import/effects.rs | 87 +++++---- crates/aep/src/import/placeholder.rs | 166 +++++++++++++++++ crates/aep/src/import/report.rs | 7 + crates/aep/tests/it/import.rs | 1 + crates/aep/tests/it/import/placeholders.rs | 200 +++++++++++++++++++++ crates/expr/src/tests.rs | 18 ++ docs/aep-import.md | 20 ++- 8 files changed, 461 insertions(+), 42 deletions(-) create mode 100644 crates/aep/src/import/placeholder.rs create mode 100644 crates/aep/tests/it/import/placeholders.rs diff --git a/crates/aep/src/import.rs b/crates/aep/src/import.rs index fae3b3256..20a4b7b1e 100644 --- a/crates/aep/src/import.rs +++ b/crates/aep/src/import.rs @@ -14,7 +14,8 @@ //! missing-footage placeholders). `docs/aep-import.md` has the full list. //! //! What does not arrive is **reported**, never dropped silently: [`ImportReport`] counts other -//! effects (by match name) and unimported effect parameters, unusable masks, layer style +//! effects (by match name; they arrive as placeholders that render nothing, with their parameters +//! for expressions) and unimported effect parameters, unusable masks, layer style //! features, text layer features, shape items, unsupported blend modes and transform //! properties, simplified keyframes and unreadable data per [`LossCategory`]. By default the import //! is refused when anything would be lost ([`ImportOptions::allow_losses`] lifts that); a refusal @@ -30,6 +31,7 @@ mod effects; mod lumetri; mod markers; mod masks; +mod placeholder; mod props; mod puppet; mod report; diff --git a/crates/aep/src/import/effects.rs b/crates/aep/src/import/effects.rs index 1ab4b208a..86faac76a 100644 --- a/crates/aep/src/import/effects.rs +++ b/crates/aep/src/import/effects.rs @@ -1,7 +1,8 @@ //! Effects: a table maps an After Effects effect (its match name) and its parameters (by their -//! index in `parT`) to an EffectCraft effect, a parameter id and a unit conversion. Effects and -//! parameters outside the table are reported in [`LossCategory::Effects`] by match name, never -//! dropped silently. +//! index in `parT`) to an EffectCraft effect, a parameter id and a unit conversion. Parameters +//! outside the table are reported in [`LossCategory::Effects`] by match name, never dropped +//! silently. An effect outside it arrives as a placeholder holding its parameters (`placeholder.rs`) +//! and is reported too. //! //! Every conversion is backed by the corpus (`docs/aep-format.md` 3.7, cases 356-450, 511-516): //! @@ -82,44 +83,29 @@ pub(crate) fn import_effects<'f, 'a>( continue; } 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_unmapped_effect(fx.match_name(), format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name())); - continue; - }; - let Some(spec) = effectcraft_effects::find(map.effect) else { - col.lose_unmapped_effect(fx.match_name(), format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name())); - continue; - }; + let found = EFFECTS.iter().find(|m| m.match_name == fx.match_name()).and_then(|m| Some((m, effectcraft_effects::find(m.effect)?))); + let lost = format!("{}: '{}' has the effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name()); 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()), - ); + match found { + Some(_) => col.lose(LossCategory::Effects, format!("{lost} without readable parameter definitions, not imported")), + None => col.lose_unmapped_effect(fx.match_name(), lost), + } 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)), - } let params = defs.params(&fx); - // Borrowed definitions (another instance's or the project copy) may not name every value - // record this instance has: such a record has nothing to import into, so it is reported. - 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, - format!( - "{}: '{}' effect '{shown}' ({}): the value record {name} has no parameter definition, not imported", - cx.comp, - cx.layer, - fx.match_name() - ), - ); - } - } + report_records_without_definition(&fx, ¶ms, cx, &shown, col); + let Some((map, spec)) = found else { + // No counterpart: kept as a placeholder that renders nothing, for expressions (#3087). + let mut group = crate::import::placeholder::placeholder(&fx, ¶ms, cx, &shown, ids, refs, col); + set_enabled(&fx, &mut group, cx, &shown, col); + col.lose_unmapped_effect(fx.match_name(), format!("{lost}, kept without rendering")); + col.imported.placeholder_effects = col.imported.placeholder_effects.saturating_add(1); + target.children.push(Node::Group(group)); + continue; + }; + let size = [f64::from(cx.size.0), f64::from(cx.size.1)]; + let mut group = effectcraft_effects::instantiate(spec, ids, &shown, size); + set_enabled(&fx, &mut group, cx, &shown, col); if map.extra == Extra::Lumetri { crate::import::lumetri::import_lumetri(&fx, ¶ms, cx, &shown, ParamOut { group: &mut group, refs: &mut *refs, col: &mut *col }); } else if map.extra == Extra::Cryptomatte { @@ -134,6 +120,29 @@ pub(crate) fn import_effects<'f, 'a>( } } +/// The effect's fx switch. +fn set_enabled(fx: &AepEffect<'_, '_>, group: &mut PropGroup, cx: &LayerCtx<'_>, shown: &str, col: &mut Collector) { + 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)), + } +} + +/// Borrowed definitions (another instance's or the project copy) may not name every value record +/// this instance has: such a record has nothing to import into, so it is reported. +fn report_records_without_definition(fx: &AepEffect<'_, '_>, params: &[EffectParam<'_, '_>], cx: &LayerCtx<'_>, shown: &str, col: &mut Collector) { + if !params.first().is_some_and(|p| p.borrowed_definition) { + return; + } + 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, + format!("{}: '{}' effect '{shown}' ({}): the value record {name} has no parameter definition, not imported", cx.comp, cx.layer, fx.match_name()), + ); + } +} + /// Parameters that never carry a value to import. pub(crate) fn carries_nothing(p: &EffectParam<'_, '_>) -> bool { matches!(p.kind, ParamKind::Button | ParamKind::NoData | ParamKind::GroupStart | ParamKind::GroupEnd) || (p.index == 0 && p.kind == ParamKind::Layer) @@ -364,7 +373,7 @@ fn report_beyond_limit<'v>(limit: Option<(f64, f64)>, values: impl Iterator, target: &str, own: bool, cx: &LayerCtx<'_>, names: (&str, &str), out: ParamOut<'_>) { +pub(crate) fn import_layer_ref(p: &EffectParam<'_, '_>, target: &str, own: bool, cx: &LayerCtx<'_>, names: (&str, &str), out: ParamOut<'_>) { let (here, label) = names; let ParamOut { group, refs, col } = out; // After Effects hands the named layer's source to the effect, without its masks and effects @@ -799,7 +808,7 @@ fn px(v: f64) -> String { /// A decoded value in EffectCraft's units, `None` when it does not fit the kind or is not a valid /// number. -fn convert(value: ParamValue, kind: Kind) -> Option { +pub(crate) fn convert(value: ParamValue, kind: Kind) -> Option { let raw = match (value, kind) { // The reader gives colours as ARGB fractions; `Kind` takes the stored 0..255. (ParamValue::Argb(c), Kind::EffectColor) => c.map(|x| x * 255.0).to_vec(), diff --git a/crates/aep/src/import/placeholder.rs b/crates/aep/src/import/placeholder.rs new file mode 100644 index 000000000..4e9fd8047 --- /dev/null +++ b/crates/aep/src/import/placeholder.rs @@ -0,0 +1,166 @@ +//! Placeholders: an effect with no EffectCraft counterpart (a third-party plug-in, a pseudo effect +//! of expression controls) arrives as an effect group that renders nothing, as a missing plug-in +//! does. Its effect id is the After Effects match name, so Find Missing Effects lists it. It holds +//! the effect's parameters, flat and named as in After Effects, each with its value, keys and +//! expression, so expressions that read them (`effect("Settings")("Speed")`) keep working. It is +//! still reported, once per match name. +//! +//! 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). +//! +//! 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. + +use effectcraft_keyframe::Value; +use effectcraft_project::build::Ids; +use effectcraft_project::{GroupKind, Node, ParamUi, PropGroup, Property}; + +use crate::import::effects::{LayerCtx, ParamOut, PendingLayerRef, carries_nothing, convert, import_layer_ref, param_label}; +use crate::import::props::{Kind, import_expression, import_keys}; +use crate::import::report::{Collector, LossCategory}; +use crate::{Effect as AepEffect, EffectParam, ParamKind, ParamValue}; + +/// Most placeholder properties and popup items one import creates. +pub(crate) const MAX_COST: usize = 200_000; + +/// Most items read from one placeholder popup. +const MAX_POPUP_ITEMS: usize = 1024; + +/// The placeholder of `fx`, with `params` (its parameter definitions) as properties. +pub(crate) fn placeholder( + fx: &AepEffect<'_, '_>, + params: &[EffectParam<'_, '_>], + cx: &LayerCtx<'_>, + shown: &str, + ids: &mut Ids<'_>, + refs: &mut Vec, + col: &mut Collector, +) -> PropGroup { + let mut group = ids.group(fx.match_name(), shown); + group.kind = GroupKind::Effect { effect: fx.match_name().to_string() }; + 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; + } + let label = param_label(p); + let Some((kind, ui, default)) = shape(p, cx.size) else { + if p.stored().is_some() { + col.lose(LossCategory::Effects, format!("{here}: parameter {label} not kept (type {:?})", p.kind)); + } + continue; + }; + let items = match &ui { + ParamUi::Popup { options } => options.len(), + _ => 0, + }; + 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; + } + col.placeholder_cost = cost; + let mut prop = Property::new(ids.alloc(), ¶m_id(p), p.name.trim(), default).with_ui(ui); + prop.spatial = p.kind == ParamKind::Point; + prop.hold_only = matches!(p.kind, ParamKind::Checkbox | ParamKind::Popup | ParamKind::Layer); + match kind { + Some(kind) => { + fill(p, &mut prop, kind, cx, (&here, &label), col); + group.children.push(Node::Prop(prop)); + } + // A layer parameter: the layer it names (set once every layer exists, by its uid). + None => { + let id = format!("@{}", prop.uid); + group.children.push(Node::Prop(prop)); + import_layer_ref(p, &id, true, cx, (&here, &label), ParamOut { group: &mut group, refs: &mut *refs, col: &mut *col }); + } + } + } + group +} + +/// How parameter `p` is held on a layer `size` big: its unit conversion (`None` for a layer +/// parameter), its control and its value until one is read. `None` for a type that is not kept. +fn shape(p: &EffectParam<'_, '_>, size: (u32, u32)) -> Option<(Option, ParamUi, Value)> { + let (kind, ui, default) = match p.kind { + ParamKind::IntSlider | ParamKind::FixedSlider | ParamKind::FloatSlider => (Kind::EffectNumber(1.0), ParamUi::Number, Value::Scalar(0.0)), + ParamKind::Angle => (Kind::EffectNumber(1.0), ParamUi::Angle, Value::Scalar(0.0)), + ParamKind::Checkbox => (Kind::EffectBool, ParamUi::Checkbox, Value::Bool(false)), + ParamKind::Color => (Kind::EffectColor, ParamUi::Color, Value::Color([0.0, 0.0, 0.0, 1.0])), + ParamKind::Point => (Kind::EffectPoint(f64::from(size.0), f64::from(size.1)), ParamUi::Point, Value::Vec2([0.0, 0.0])), + ParamKind::Layer => return Some((None, ParamUi::Layer, Value::Layer(None))), + ParamKind::Popup => { + // 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(), + }; + let n = u32::try_from(options.len()).ok().filter(|n| *n > 0)?; + (Kind::EffectPopup(n), ParamUi::Popup { options }, Value::Enum(0)) + } + _ => return None, + }; + Some((Some(kind), ui, default)) +} + +/// 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. +fn param_id(p: &EffectParam<'_, '_>) -> String { + let plain = !p.match_name.is_empty() && p.match_name.chars().all(|c| c.is_alphanumeric() || matches!(c, ' ' | '-' | '_')); + if plain { p.match_name.clone() } else { format!("param{}", p.index) } +} + +/// Sets `prop` from `p`: its expression, then its keys or its value (the record, else the `pard` +/// default). `names` is `(here, label)` for the report. +fn fill(p: &EffectParam<'_, '_>, prop: &mut Property, kind: Kind, cx: &LayerCtx<'_>, names: (&str, &str), col: &mut Collector) { + let (here, label) = names; + if let Some(stored) = p.stored() { + import_expression(&stored, prop, col); + } + if p.is_animated() { + let Some(stored) = p.stored() else { return }; + 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 kept (not decoded)")); + return; + } + 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")); + } + return; + } + let value = match (p.stored(), p.recorded_value()) { + (Some(_), Some(v)) => v, + (Some(_), None) => { + col.lose(LossCategory::DamagedData, format!("{here}: parameter {label} holds an unreadable value")); + return; + } + // A borrowed popup's default slot holds another copy's selection (docs/aep-format.md 3.7). + (None, _) if p.kind == ParamKind::Popup && p.borrowed_definition => { + col.lose(LossCategory::Effects, format!("{here}: parameter {label}: the default item is not known (shared definition), the first item used")); + return; + } + // A popup's default slot can hold 0, no item (After Effects' save of a project it opened). + (None, _) => match p.pard_default() { + Some(ParamValue::Index(0)) => { + col.lose(LossCategory::Effects, format!("{here}: parameter {label}: the default item is not stored, the first item used")); + return; + } + Some(v) => v, + 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:?})")), + } +} diff --git a/crates/aep/src/import/report.rs b/crates/aep/src/import/report.rs index 2825b61b3..8b3599266 100644 --- a/crates/aep/src/import/report.rs +++ b/crates/aep/src/import/report.rs @@ -156,6 +156,10 @@ pub struct Imported { /// in [`LossCategory::Effects`]). #[serde(default)] pub effects: usize, + /// Effects with no counterpart, kept as placeholders that render nothing (their parameters, + /// for expressions; each is also in [`LossCategory::Effects`]). + #[serde(default)] + pub placeholder_effects: usize, /// Layer styles (Drop Shadow, Stroke, ...), one per style on a layer. #[serde(default)] pub layer_styles: usize, @@ -273,6 +277,9 @@ pub(crate) struct Collector { /// many instances there were. They get a line each in [`Collector::finish`], so one that /// comes after the example lines are used up is still named (#3365). unmapped_effects: BTreeMap, + /// The properties and popup items the placeholders of this import hold so far (at most + /// `placeholder::MAX_COST`). + pub placeholder_cost: usize, } /// How many different unmapped effects get a line of their own (a file names at most this many). diff --git a/crates/aep/tests/it/import.rs b/crates/aep/tests/it/import.rs index 2535edcff..240c3ea0f 100644 --- a/crates/aep/tests/it/import.rs +++ b/crates/aep/tests/it/import.rs @@ -22,6 +22,7 @@ mod lumetri; mod markers; mod masks; mod mattes; +mod placeholders; mod puppet; mod resaved_defaults; mod robustness; diff --git a/crates/aep/tests/it/import/placeholders.rs b/crates/aep/tests/it/import/placeholders.rs new file mode 100644 index 000000000..847060f93 --- /dev/null +++ b/crates/aep/tests/it/import/placeholders.rs @@ -0,0 +1,200 @@ +//! #3087: an effect with no EffectCraft counterpart (a third-party plug-in, a pseudo effect of +//! expression controls) arrives as a placeholder holding its parameters, so expressions that read +//! them keep working; it renders nothing and is still reported. Each effect is built into +//! `synthetic.aep` (`fx_inject`) in place of its Glow, on the 8 x 8 sequence layer. + +use crate::import::fx_inject::{Default, Fx, bytes_with, import_bytes, losses, other, popup, run, slider, v}; +use effectcraft_aep::import::LossCategory; +use effectcraft_aep::{Body, Chunk, ParamKind as K, parse, write}; +use effectcraft_keyframe::Value; +use effectcraft_project::{GroupKind, ParamUi, PropGroup}; + +/// A third-party effect with one parameter of each kept kind, inside a twirl-down group. +fn third_party(values: Vec<(usize, f64)>, keys: Vec<(usize, Vec<(i32, f64)>)>) -> Fx { + Fx { + match_name: "CC Cylinder", + name: "CC Cylinder", + defs: vec![ + other(K::GroupStart, "Light", Default::None), + slider(K::FloatSlider, "Radius (%)", 30.0, (0.0, 30000.0)), + other(K::GroupEnd, "", Default::None), + other(K::Angle, "Rotation X", Default::Number(12.5)), + other(K::Checkbox, "Wire", Default::Number(1.0)), + other(K::Color, "Light Color", Default::Color([255, 51, 102, 204])), + other(K::Point, "Position", Default::Point(0.25, 0.5)), + popup("Render", 1, &["Full", "(-", "Outside", "Inside"]), + slider(K::IntSlider, "Count", 3.0, (0.0, 50.0)), + ], + values, + data: vec![], + keys, + } +} + +fn placeholder_of(fx: &Fx) -> (PropGroup, effectcraft_aep::import::ImportResult) { + let (g, r) = run(fx, fx.match_name); + (g.expect("the placeholder"), r) +} + +#[test] +fn an_effect_without_a_counterpart_is_kept_with_its_values() { + // Radius 37.5 and Render "Outside" (3: the separator counts) stored, the rest at defaults. + let (g, r) = placeholder_of(&third_party(vec![(2, 37.5), (8, 3.0)], vec![])); + assert_eq!(g.kind, GroupKind::Effect { effect: "CC Cylinder".into() }); + assert_eq!(g.name, "CC Cylinder"); + let names: Vec<(&str, &str)> = g.props().map(|p| (p.match_id.as_str(), p.name.as_str())).collect(); + assert_eq!( + names, + [ + ("CC Cylinder-0002", "Radius (%)"), + ("CC Cylinder-0004", "Rotation X"), + ("CC Cylinder-0005", "Wire"), + ("CC Cylinder-0006", "Light Color"), + ("CC Cylinder-0007", "Position"), + ("CC Cylinder-0008", "Render"), + ("CC Cylinder-0009", "Count"), + ], + "flat, in order, without the group markers" + ); + assert_eq!(v(&g, "CC Cylinder-0002"), Value::Scalar(37.5)); + assert_eq!(v(&g, "CC Cylinder-0004"), Value::Scalar(12.5)); + assert_eq!(v(&g, "CC Cylinder-0005"), Value::Bool(true)); + let Value::Color(c) = v(&g, "CC Cylinder-0006") else { panic!("a colour") }; + assert!(c.iter().zip([0.2, 0.4, 0.8, 1.0]).all(|(a, b)| (a - b).abs() < 1e-6), "{c:?}"); + assert_eq!(v(&g, "CC Cylinder-0007"), Value::Vec2([2.0, 4.0]), "layer pixels"); + assert_eq!(v(&g, "CC Cylinder-0008"), Value::Enum(2), "item 3 of 4, 0-based"); + assert_eq!(v(&g, "CC Cylinder-0009"), Value::Scalar(3.0)); + let render = g.prop("CC Cylinder-0008").unwrap(); + assert_eq!(render.ui, ParamUi::Popup { options: ["Full", "(-", "Outside", "Inside"].map(String::from).to_vec() }); + // Still reported, once, and counted apart from the imported effects. + let found = losses(&r, LossCategory::Effects, "CC Cylinder"); + assert!(found.len() == 1 && found[0].ends_with("has the effect 'CC Cylinder' (CC Cylinder), kept without rendering"), "{found:?}"); + assert_eq!((r.report.imported.placeholder_effects, r.report.imported.effects), (1, 1), "the placeholder and the Levels"); +} + +#[test] +fn keys_and_expressions_of_a_placeholder_are_kept() { + let fx = third_party(vec![(4, 12.5)], vec![(2, vec![(0, 10.0), (25, 20.0)])]); + let bytes = with_expression(&bytes_with(&fx), "CC Cylinder-0004", "time * 90"); + let (g, _) = import_bytes(&bytes, "CC Cylinder"); + let g = g.unwrap(); + let radius = g.prop("CC Cylinder-0002").unwrap(); + assert_eq!(radius.keys.iter().map(|k| k.value.clone()).collect::>(), [Value::Scalar(10.0), Value::Scalar(20.0)]); + let angle = g.prop("CC Cylinder-0004").unwrap(); + assert_eq!(angle.expr.as_ref().map(|e| (e.text.as_str(), e.enabled)), Some(("time * 90", true))); +} + +/// A pseudo effect's parameter match names (`Pseudo/0.25-0001`) are no property path segment: +/// its parameters are `param`, found by their names. +#[test] +fn a_pseudo_effect_is_kept_with_its_parameter_names() { + let fx = Fx { + match_name: "Pseudo/0.25", + name: "Settings", + defs: vec![slider(K::FloatSlider, "Speed", 1.0, (0.0, 100.0)), popup("Mode", 2, &["A", "B"])], + values: vec![(1, 4.0)], + data: vec![], + keys: vec![], + }; + let (g, r) = placeholder_of(&fx); + assert_eq!(g.prop("param1").map(|p| (p.name.as_str(), p.value.clone())), Some(("Speed", Value::Scalar(4.0)))); + assert_eq!(g.prop("param2").map(|p| (p.name.as_str(), p.value.clone())), Some(("Mode", Value::Enum(1)))); + assert_eq!(losses(&r, LossCategory::Effects, "Pseudo/0.25").len(), 1); +} + +/// A popup whose definition is borrowed (the project copy) and has no value record: its default +/// slot holds another copy's selection, so the first item is used and that is reported. +#[test] +fn a_borrowed_popup_without_a_record_takes_the_first_item_and_is_reported() { + let bytes = without_instance_definitions(&bytes_with(&third_party(vec![], vec![]))); + let (g, r) = import_bytes(&bytes, "CC Cylinder"); + assert_eq!(v(&g.unwrap(), "CC Cylinder-0008"), Value::Enum(0)); + let found = losses(&r, LossCategory::Effects, "CC Cylinder"); + assert!(found.iter().any(|d| d.contains("Render") && d.contains("default item is not known")), "{found:?}"); +} + +/// A popup default slot of 0 (no item; After Effects' save of a project it opened) takes the first +/// item and is reported, not read as damaged data. +#[test] +fn a_popup_default_of_no_item_takes_the_first_item() { + let fx = Fx { match_name: "Some Fx", name: "Some", defs: vec![popup("Mode", 0, &["A", "B"])], values: vec![], data: vec![], keys: vec![] }; + let (g, r) = placeholder_of(&fx); + assert_eq!(v(&g, "Some Fx-0001"), Value::Enum(0)); + let found = losses(&r, LossCategory::Effects, "Some Fx"); + assert!(found.iter().any(|d| d.contains("default item is not stored")), "{found:?}"); + assert!(losses(&r, LossCategory::DamagedData, "Some 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() { + let items: &'static [&'static str] = Box::leak((0..1000).map(|i| &*Box::leak(format!("I{i}").into_boxed_str())).collect::>().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 (g, r) = placeholder_of(&fx); + let kept = g.props().count(); + assert!(kept > 0 && kept < 300, "{kept}"); + assert!(g.props().map(|p| if let ParamUi::Popup { options } = &p.ui { options.len() + 1 } else { 1 }).sum::() <= 200_000); + assert!(losses(&r, LossCategory::Effects, "Big Fx").iter().any(|d| d.contains("placeholder parameters"))); +} + +/// `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 { + let mut file = parse(bytes).unwrap(); + let mut done = 0; + edit(&mut file.chunks, false, &mut |c, _| { + let Body::List { children, .. } = &mut c.body else { 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(param.as_bytes()))) else { + return; + }; + let Some(Body::List { children: record, .. }) = children.get_mut(at + 1).map(|k| &mut k.body) else { return }; + for k in record.iter_mut().filter(|k| k.is(b"tdb4")) { + if let Body::Data(d) = &mut k.body + && let Some(b) = d.to_mut().get_mut(120) + { + *b = 1; + } + } + record.push(Chunk::data(*b"Utf8", text.as_bytes().to_vec())); + done += 1; + }); + assert!(done >= 1); + write(&file).unwrap() +} + +/// `bytes` with the layer's instance of `CC Cylinder` stripped of its own definitions (an empty +/// `parT`): it borrows the project copy's (`EfdG`). +fn without_instance_definitions(bytes: &[u8]) -> Vec { + 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(&b"CC Cylinder"[..]))) + else { + return; + }; + let Some(sspc) = children.get_mut(at + 1).filter(|k| k.is_list(b"sspc")) else { return }; + for k in sspc.children_mut() { + if let Body::List { list_type, children } = &mut k.body + && list_type == b"parT" + { + children.clear(); + done += 1; + } + } + }); + assert_eq!(done, 1, "one instance"); + write(&file).unwrap() +} + +/// Calls `f` on every chunk with whether it is inside the project's effect definitions (`EfdG`). +fn edit(chunks: &mut [Chunk<'_>], in_efdg: bool, f: &mut dyn FnMut(&mut Chunk<'_>, bool)) { + for c in chunks { + let inside = in_efdg || c.is_list(b"EfdG"); + f(c, inside); + edit(c.children_mut(), inside, f); + } +} diff --git a/crates/expr/src/tests.rs b/crates/expr/src/tests.rs index d16f1dd54..f6d07bedc 100644 --- a/crates/expr/src/tests.rs +++ b/crates/expr/src/tests.rs @@ -1376,6 +1376,24 @@ fn checkbox_and_dropdown_results() { assert_close!(f.ev(f.b, ROT, "effect('Dropdown Menu Control')('Menu') * 10", 0.0).unwrap().as_f64(), 10.0); } +/// An effect EffectCraft does not have (an imported After Effects placeholder, #3087) is read by +/// name like any other: its parameters by their names, a popup 1-based. +#[test] +fn an_unknown_effects_parameters_are_read_by_name() { + let mut f = fx(); + let g = group_push(&mut f.p, |ids| { + let mut g = ids.group("Pseudo/0.25", "Settings"); + g.kind = effectcraft_project::GroupKind::Effect { effect: "Pseudo/0.25".into() }; + g.children.push(ids.prop("param1", "Speed", Value::Scalar(3.0)).into()); + g.children.push(ids.prop("param2", "Mode", Value::Enum(1)).into()); + g + }); + f.layer_mut(f.b).props.sub_mut("effects").unwrap().children.push(g.into()); + assert_close!(f.ev(f.b, ROT, "effect('Settings')('Speed') * 2", 0.0).unwrap().as_f64(), 6.0); + assert_close!(f.ev(f.b, ROT, "effect('Settings')('Mode') * 10", 0.0).unwrap().as_f64(), 20.0); + assert_close!(f.ev(f.b, ROT, "effect('Settings')(1)", 0.0).unwrap().as_f64(), 3.0); +} + #[test] fn syntax_error_reports_line() { let f = fx(); diff --git a/docs/aep-import.md b/docs/aep-import.md index c704e0a89..b4c37646f 100644 --- a/docs/aep-import.md +++ b/docs/aep-import.md @@ -1,6 +1,6 @@ # After Effects project import -> **Last reviewed:** 2026-11-05 · **Last updated:** 2026-11-10 · **Change:** dropped projects and projects picked in File ▸ Import ▸ File… import as projects, several asked about one at a time; `file.import` hands them to `file.importAep`; the menu entries are named Adobe After Effects Project… (#3103); before that: After Effects' Puppet arrives as EffectCraft's Puppet without its meshes and pins, which are reported mesh by mesh (#3093; the storage is not sampled yet, aep-format 3.7); before that: Lumetri Color's master RGB curve, Hue vs Sat curve and colour wheels are imported (1200 presets made by hand); before that: Lumetri Color is imported (1200: every slider, checkbox and popup with a counterpart, by match name, also from records in the default state; switched-off sections, LUTs, looks, undecoded curves and wheels are reported); before that: the report names the After Effects version a file is written for and the one that wrote it (`aeVersion`); copies saved as 25.x and 24.x import like the 2026 save; before that: Curves and Mesh Warp are imported (961-965, 985; FORMAT S of the corpus); before that: a shape item's eye switch is imported (bit 0 of its group flags, 986 / 987: a switched-off Fill under Trim Paths is no longer drawn, render equal to After Effects), and a layer style's (switch group `tdsb` 0, 988); before that: a negative time stretch (Time Reverse Layer) is imported: in and out point in order, the layer plays backwards (981-983: render equal to After Effects at 7 points); before that: After Effects' None on Compound Blur, Displacement Map and Set Matte arrives as an explicit None (`NO_LAYER`), not as the owning layer; before that: layer styles are imported (the nine styles and Blending Options, 951-979; Global Light becomes the composition's); before that: the 27 effects of round 3 are in the table (`effectcraft_aep::effect_map`); before that: a layer of a clip, sequence or sound ends at its source's end (After Effects ends it there when it opens a project, read-back test f1), unless it is remapped or looped or runs less than a frame past; before that: keys of shape colours are read (the reader took their 152-byte spatial records for plain ones: `tdb4` byte 7 is 0 there, 854); before that: the effect table moved to `effectcraft_aep::effect_map`; point defaults in percent and a popup default slot of 0 (After Effects' save of a project it opened, e1 / e2 re-saves) are read; before that: the justification of right-to-left paragraphs is read relative to the direction (t7); before that: 24 more layer blend modes (906, 907); before that: 27 more effects are imported (batch 6: 903-905), with layer parameters; before that: a Rec. 709 working space also sets the output space (After Effects renders in the working space, 902); before that: a mask whose path was never set arrives as the source rectangle (900, 901); before that: Offset Paths' Copy Offset arrives as EffectCraft's (After Effects' value minus 1, 828); before that: project settings, image sequences, conformed clips and effect instance names are imported, next to the mattes, shapes, text settings and animators, effects, markers, time remapping, cameras and lights); before that: parallel and ambient lights (701, 702) and keyed Orientation (703) are imported; before that: masks on text and shape layers (705, 706: AE vs EffectCraft render 12 and 0 of 5184 points, glyph edges) and Merge Paths, Offset Paths, Repeater (707-710, 731; merge renders equal but for edges; Repeater copy opacity and Offset Paths' outline differ in EffectCraft's renderer) are imported; before that: keyframes: the ease of Anchor Point, effect point, effect colour and mask path keys is imported (597-599, 700, 730: within 0.5 px of After Effects' read-backs); before that: effects: Directional Blur, Radial Blur, CC Radial Blur, Venetian Blinds, Channel Blur, Black & White, Tritone, Luma Key, Simple Choker, Matte Choker and Transform are imported (711-723; render checks: CC Radial Blur and Channel Blur equal, Directional Blur within 5/255; Venetian Blinds, Radial Blur, Black & White (8/255), Tritone (19/255), Luma Key, Matte Choker and Transform differ in EffectCraft's renderer, see STATUS); before that: text: character and paragraph settings (stroke, scales, baseline shift, kerning, ligatures, tsume, indents, spacing, justified text, composer, direction, vertical text; 538-578) and text animators with range selectors (579-594, 114) are imported; render checks 594 (keyed range selector at 0 / 0.5 / 1 / 1.5 / 2.5 s), 595, 596 equal but for glyph edges; before that: cameras, spot and point lights, X / Y Rotation and Orientation, Timewarp and expressions on effect parameters are imported (render checks 486, 535-537 equal but for edges; 488 differs by 5.25 %, cause in EffectCraft's renderer not isolated); before that: text: superscript and subscript are imported (526-528); before that: effects: Invert, Fast Box Blur, Gaussian Blur, Drop Shadow, Glow, Fill, Hue/Saturation, Levels and five expression controls are imported, popups are mapped by item name, checkbox defaults come from the file; before that: shape layers are imported (groups, paths, paints, trim paths, round corners; 451-484, 519 added); before that: markers and Time Remapping are imported (509, 510 added); before that: track mattes are imported (402-408); before that: text: automatic leading is verified (400); effects: Exposure is imported (Channels, all twelve sliders, Bypass, keys), next to Tint and Gradient Ramp; before that: text: U+0003 is a paragraph break (386, 387); before that: a stored text Anchor Point is pixels (385, now V); before that: effects: Tint and Gradient Ramp are imported (all parameters and keys), other effects stay reported; `.aepx` is accepted; text layers are imported (Source Text with style runs, paragraphs and box text; fonts by PostScript name, missing fonts listed); layer styles are reported; before that: layer masks · **Target:** Adobe After Effects 2026 +> **Last reviewed:** 2026-11-05 · **Last updated:** 2026-11-11 · **Change:** an effect without a counterpart arrives as a placeholder with its parameters, values, keys and expressions; it renders nothing and is still reported (#3087); before that: dropped projects and projects picked in File ▸ Import ▸ File… import as projects, several asked about one at a time; `file.import` hands them to `file.importAep`; the menu entries are named Adobe After Effects Project… (#3103); before that: After Effects' Puppet arrives as EffectCraft's Puppet without its meshes and pins, which are reported mesh by mesh (#3093; the storage is not sampled yet, aep-format 3.7); before that: Lumetri Color's master RGB curve, Hue vs Sat curve and colour wheels are imported (1200 presets made by hand); before that: Lumetri Color is imported (1200: every slider, checkbox and popup with a counterpart, by match name, also from records in the default state; switched-off sections, LUTs, looks, undecoded curves and wheels are reported); before that: the report names the After Effects version a file is written for and the one that wrote it (`aeVersion`); copies saved as 25.x and 24.x import like the 2026 save; before that: Curves and Mesh Warp are imported (961-965, 985; FORMAT S of the corpus); before that: a shape item's eye switch is imported (bit 0 of its group flags, 986 / 987: a switched-off Fill under Trim Paths is no longer drawn, render equal to After Effects), and a layer style's (switch group `tdsb` 0, 988); before that: a negative time stretch (Time Reverse Layer) is imported: in and out point in order, the layer plays backwards (981-983: render equal to After Effects at 7 points); before that: After Effects' None on Compound Blur, Displacement Map and Set Matte arrives as an explicit None (`NO_LAYER`), not as the owning layer; before that: layer styles are imported (the nine styles and Blending Options, 951-979; Global Light becomes the composition's); before that: the 27 effects of round 3 are in the table (`effectcraft_aep::effect_map`); before that: a layer of a clip, sequence or sound ends at its source's end (After Effects ends it there when it opens a project, read-back test f1), unless it is remapped or looped or runs less than a frame past; before that: keys of shape colours are read (the reader took their 152-byte spatial records for plain ones: `tdb4` byte 7 is 0 there, 854); before that: the effect table moved to `effectcraft_aep::effect_map`; point defaults in percent and a popup default slot of 0 (After Effects' save of a project it opened, e1 / e2 re-saves) are read; before that: the justification of right-to-left paragraphs is read relative to the direction (t7); before that: 24 more layer blend modes (906, 907); before that: 27 more effects are imported (batch 6: 903-905), with layer parameters; before that: a Rec. 709 working space also sets the output space (After Effects renders in the working space, 902); before that: a mask whose path was never set arrives as the source rectangle (900, 901); before that: Offset Paths' Copy Offset arrives as EffectCraft's (After Effects' value minus 1, 828); before that: project settings, image sequences, conformed clips and effect instance names are imported, next to the mattes, shapes, text settings and animators, effects, markers, time remapping, cameras and lights); before that: parallel and ambient lights (701, 702) and keyed Orientation (703) are imported; before that: masks on text and shape layers (705, 706: AE vs EffectCraft render 12 and 0 of 5184 points, glyph edges) and Merge Paths, Offset Paths, Repeater (707-710, 731; merge renders equal but for edges; Repeater copy opacity and Offset Paths' outline differ in EffectCraft's renderer) are imported; before that: keyframes: the ease of Anchor Point, effect point, effect colour and mask path keys is imported (597-599, 700, 730: within 0.5 px of After Effects' read-backs); before that: effects: Directional Blur, Radial Blur, CC Radial Blur, Venetian Blinds, Channel Blur, Black & White, Tritone, Luma Key, Simple Choker, Matte Choker and Transform are imported (711-723; render checks: CC Radial Blur and Channel Blur equal, Directional Blur within 5/255; Venetian Blinds, Radial Blur, Black & White (8/255), Tritone (19/255), Luma Key, Matte Choker and Transform differ in EffectCraft's renderer, see STATUS); before that: text: character and paragraph settings (stroke, scales, baseline shift, kerning, ligatures, tsume, indents, spacing, justified text, composer, direction, vertical text; 538-578) and text animators with range selectors (579-594, 114) are imported; render checks 594 (keyed range selector at 0 / 0.5 / 1 / 1.5 / 2.5 s), 595, 596 equal but for glyph edges; before that: cameras, spot and point lights, X / Y Rotation and Orientation, Timewarp and expressions on effect parameters are imported (render checks 486, 535-537 equal but for edges; 488 differs by 5.25 %, cause in EffectCraft's renderer not isolated); before that: text: superscript and subscript are imported (526-528); before that: effects: Invert, Fast Box Blur, Gaussian Blur, Drop Shadow, Glow, Fill, Hue/Saturation, Levels and five expression controls are imported, popups are mapped by item name, checkbox defaults come from the file; before that: shape layers are imported (groups, paths, paints, trim paths, round corners; 451-484, 519 added); before that: markers and Time Remapping are imported (509, 510 added); before that: track mattes are imported (402-408); before that: text: automatic leading is verified (400); effects: Exposure is imported (Channels, all twelve sliders, Bypass, keys), next to Tint and Gradient Ramp; before that: text: U+0003 is a paragraph break (386, 387); before that: a stored text Anchor Point is pixels (385, now V); before that: effects: Tint and Gradient Ramp are imported (all parameters and keys), other effects stay reported; `.aepx` is accepted; text layers are imported (Source Text with style runs, paragraphs and box text; fonts by PostScript name, missing fonts listed); layer styles are reported; before that: layer masks · **Target:** Adobe After Effects 2026 File ▸ Import ▸ Adobe After Effects Project… (`file.importAep`) reads an `.aep` or its XML form `.aepx` and adds its items to the open project, in a folder named after the file (as After Effects does when it imports a project). @@ -318,6 +318,22 @@ parameter gets the file's value: `approximated_keyframes`. - **The fx switch** of each instance (the `tdsb` of its value group: 1 on, 0 off; `fx-switch.aep`) is imported and exported, and so is an expression on a stored effect parameter (its text and its switch). +- **Effects without a counterpart** (third-party plug-ins, pseudo effects of expression controls, + `Pseudo/…`) arrive as **placeholders** (#3087): an effect group whose effect id is the After + Effects match name. Like a missing plug-in it renders nothing and is listed by Find Missing + Effects. It holds the effect's parameters flat (without the twirl-down group markers), named as + 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), + so expressions that read them (`effect("Settings")("Speed")`) keep working. A parameter's id is + its match name (`CC Cylinder-0003`), or `param` 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 + `imported.placeholderEffects`; .aep export reports it and leaves it out. ## What is reported @@ -327,7 +343,7 @@ parameter gets the file's value: | Category | Counted | |---|---| -| `effects` | each effect on an imported layer that is not in the table (by name and match name); for an imported effect: each stored parameter outside the table, a parameter whose type or valid maximum differs from the table, keys or an expression on a layer parameter, an effect without readable parameter definitions | +| `effects` | each effect on an imported layer that is not in the table (by name and match name; it arrives as a placeholder, "kept without rendering", unless it has no readable parameter definitions); for an imported effect: each stored parameter outside the table, a parameter whose type or valid maximum differs from the table, keys or an expression on a layer parameter, an effect without readable parameter definitions | | `masks` | a mask that is not imported because its path cannot be used (a Mask Path entry without shapes, a mask on a text or shape layer whose path was never set (its default is not sampled), unreadable, no vertices, a point count that is not a multiple of three, more than 65 536 vertices, numbers that are not finite), a mask mode code outside the seven known ones (imported as Add) | | `layer_styles` | per layer: a style in switch state 3 (not sampled; state 0 is imported as a style switched off, 988), Pattern Overlay, an unknown style or parameter, a gradient (`GCst`, not decoded: EffectCraft's default gradient stays), keys on a popup or checkbox, a value that does not fit, styles on a layer that takes none here (adjustment, camera, light) | | `markers` | a marker with an unknown cue type (imported as an event cue) | From a44b042fce81adfe5520acc693c3328274470367 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Wed, 11 Nov 2026 17:00:44 +0530 Subject: [PATCH 2/9] #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() { From a4d09521156328b0e9a6f53c26a889aa8816596f Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 10:51:32 +0530 Subject: [PATCH 3/9] #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 | 6 +++ crates/aep/src/import/placeholder.rs | 19 +++++--- crates/aep/tests/it/import/placeholders.rs | 50 ++++++++++++++++++++++ docs/aep-import.md | 6 ++- 4 files changed, 72 insertions(+), 9 deletions(-) diff --git a/crates/aep/src/import/effects.rs b/crates/aep/src/import/effects.rs index 01e704b53..b276c39c0 100644 --- a/crates/aep/src/import/effects.rs +++ b/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, ¶ms, cx, &shown, col); let Some((map, spec)) = found else { diff --git a/crates/aep/src/import/placeholder.rs b/crates/aep/src/import/placeholder.rs index 01d845f24..59b0865ac 100644 --- a/crates/aep/src/import/placeholder.rs +++ b/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(), ¶m_id(p), p.name.trim(), default).with_ui(ui); diff --git a/crates/aep/tests/it/import/placeholders.rs b/crates/aep/tests/it/import/placeholders.rs index 9afccd6b9..ee119869f 100644 --- a/crates/aep/tests/it/import/placeholders.rs +++ b/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::>().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 = 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::>()) + .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 { + 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> = 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 { diff --git a/docs/aep-import.md b/docs/aep-import.md index bd7ffa21d..e76468962 100644 --- a/docs/aep-import.md +++ b/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` 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 From 98b8e69f7ff997e1c28d19a4a661d68499b7e41c Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 11:00:10 +0530 Subject: [PATCH 4/9] #3087: name each effect the placeholder budget skips A skipped instance was reported through the capped detail list, so past 20 effect losses it was only counted. It now goes through the per-effect grouping under a key of its own, so it is named like any effect without a counterpart and is not merged into a kept instance's line. --- crates/aep/src/import/effects.rs | 6 ++++-- crates/aep/tests/it/import/placeholders.rs | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/aep/src/import/effects.rs b/crates/aep/src/import/effects.rs index b276c39c0..45ea70840 100644 --- a/crates/aep/src/import/effects.rs +++ b/crates/aep/src/import/effects.rs @@ -94,8 +94,10 @@ pub(crate) fn import_effects<'f, 'a>( } 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)")); + // are not read again. Named like any effect without a counterpart, in a group of its + // own (with the kept instances it would read as kept). + let key = format!("{} (not kept)", fx.match_name()); + col.lose_unmapped_effect(&key, format!("{lost}, not kept (past the import's placeholder parameters)")); continue; } let params = defs.params(&fx); diff --git a/crates/aep/tests/it/import/placeholders.rs b/crates/aep/tests/it/import/placeholders.rs index ee119869f..afa323065 100644 --- a/crates/aep/tests/it/import/placeholders.rs +++ b/crates/aep/tests/it/import/placeholders.rs @@ -180,6 +180,7 @@ fn an_effect_past_the_spent_budget_gets_no_placeholder() { 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:?}"); + assert!(found.iter().any(|d| d.contains("kept without rendering") && !d.contains("more like it")), "the kept instance alone: {found:?}"); } /// `bytes` with the layer's instance of `match_name` (its `tdmn` and `sspc`) repeated after it. From 0da7691d9d367c6d688d0f2602dec64d3284d8df Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 13:06:51 +0530 Subject: [PATCH 5/9] #3087: give a placeholder a match id that is one path segment A pseudo effect's match name (Pseudo/0.25) was its group's match id, so a match path to its parameters (keyframe paste, pick whip) was cut apart at '/' and '.' and found nothing. The effect id stays the match name. --- crates/aep/src/import/placeholder.rs | 18 +++++++++++++++--- crates/aep/tests/it/import/fx_inject.rs | 15 ++++++++++----- crates/aep/tests/it/import/placeholders.rs | 7 +++++++ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/crates/aep/src/import/placeholder.rs b/crates/aep/src/import/placeholder.rs index 59b0865ac..a12ffa6c5 100644 --- a/crates/aep/src/import/placeholder.rs +++ b/crates/aep/src/import/placeholder.rs @@ -40,7 +40,9 @@ pub(crate) fn placeholder( refs: &mut Vec, col: &mut Collector, ) -> PropGroup { - let mut group = ids.group(fx.match_name(), shown); + // The effect id is the match name; the group's match id is one segment of a property path + // (a pseudo effect's `Pseudo/0.25` would be cut apart), so match paths find its parameters. + let mut group = ids.group(&segment(fx.match_name()), shown); group.kind = GroupKind::Effect { effect: fx.match_name().to_string() }; let here = format!("{}: '{}' effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name()); for p in params.iter().filter(|p| !carries_nothing(p)) { @@ -126,8 +128,18 @@ fn popup_len(p: &EffectParam<'_, '_>) -> Option { /// 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. fn param_id(p: &EffectParam<'_, '_>) -> String { - let plain = !p.match_name.is_empty() && p.match_name.chars().all(|c| c.is_alphanumeric() || matches!(c, ' ' | '-' | '_')); - if plain { p.match_name.clone() } else { format!("param{}", p.index) } + if !p.match_name.is_empty() && p.match_name.chars().all(plain) { p.match_name.clone() } else { format!("param{}", p.index) } +} + +/// Whether `c` can stand in one segment of a property path (`/`, `.`, `#` and `@` mean more). +fn plain(c: char) -> bool { + c.is_alphanumeric() || matches!(c, ' ' | '-' | '_') +} + +/// `name` as one segment of a property path: other characters become `_` (`Pseudo/0.25` is +/// `Pseudo_0_25`). +fn segment(name: &str) -> String { + name.chars().map(|c| if plain(c) { c } else { '_' }).collect() } /// Sets `prop` from `p`: its expression, then its keys or its value (the record, else the `pard` diff --git a/crates/aep/tests/it/import/fx_inject.rs b/crates/aep/tests/it/import/fx_inject.rs index ae61794d3..535e82483 100644 --- a/crates/aep/tests/it/import/fx_inject.rs +++ b/crates/aep/tests/it/import/fx_inject.rs @@ -10,7 +10,7 @@ use effectcraft_aep::import::{ImportOptions, ImportResult, LossCategory, import}; use effectcraft_aep::{Body, Chunk, ParamKind, ParamValue, Project as AepProject, parse, write}; use effectcraft_keyframe::Value; -use effectcraft_project::{Project, PropGroup}; +use effectcraft_project::{GroupKind, Project, PropGroup}; const SYNTHETIC: &[u8] = include_bytes!("../../fixtures/synthetic.aep"); @@ -274,10 +274,15 @@ pub fn import_bytes(bytes: &[u8], effect: &str) -> (Option, ImportRes let mut project = Project::default(); let none = |_: &str| false; let r = import(&mut project, bytes, &ImportOptions { allow_losses: true, ..ImportOptions::new(&none) }).unwrap(); - let g = project - .comps() - .flat_map(|(_, c)| c.layers.iter()) - .find_map(|l| l.props.sub("effects").and_then(|fx| fx.children.iter().filter_map(|n| n.as_group()).find(|g| g.match_id == effect).cloned())); + let g = project.comps().flat_map(|(_, c)| c.layers.iter()).find_map(|l| { + l.props.sub("effects").and_then(|fx| { + fx.children + .iter() + .filter_map(|n| n.as_group()) + .find(|g| g.match_id == effect || matches!(&g.kind, GroupKind::Effect { effect: e } if e == effect)) + .cloned() + }) + }); (g, r) } diff --git a/crates/aep/tests/it/import/placeholders.rs b/crates/aep/tests/it/import/placeholders.rs index afa323065..679d508af 100644 --- a/crates/aep/tests/it/import/placeholders.rs +++ b/crates/aep/tests/it/import/placeholders.rs @@ -100,6 +100,13 @@ fn a_pseudo_effect_is_kept_with_its_parameter_names() { assert_eq!(g.prop("param1").map(|p| (p.name.as_str(), p.value.clone())), Some(("Speed", Value::Scalar(4.0)))); assert_eq!(g.prop("param2").map(|p| (p.name.as_str(), p.value.clone())), Some(("Mode", Value::Enum(1)))); assert_eq!(losses(&r, LossCategory::Effects, "Pseudo/0.25").len(), 1); + // The effect id stays the match name (Find Missing Effects, expressions); the group's match id + // is one path segment, so a match path (keyframe paste, pick whip) finds its parameters again. + assert_eq!(g.kind, GroupKind::Effect { effect: "Pseudo/0.25".into() }); + let speed = g.prop("param1").map(|p| p.uid); + let layer = PropGroup::new(0, "layer", "Layer").with(PropGroup::new(0, "effects", "Effects").with(g)); + let path = speed.and_then(|u| layer.match_path_of(u)); + assert_eq!(path.as_deref().and_then(|p| layer.prop(p)).map(|p| p.name.as_str()), Some("Speed"), "{path:?}"); } /// A popup whose definition is borrowed (the project copy) and has no value record: its default From 6f8c39d5f40e4d599ddefacab17855e178e97f2b Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 13:06:51 +0530 Subject: [PATCH 6/9] #3087: expressions find an effect by its After Effects match name effect("Pseudo/0.25") and thisLayer("Effects")("Pseudo/0.25") now match a placeholder's effect id, now that its match id is one path segment. --- crates/expr/src/host.rs | 5 ++++- crates/expr/src/tests.rs | 7 +++++-- docs/aep-import.md | 3 ++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/expr/src/host.rs b/crates/expr/src/host.rs index 43400ea1d..de2afd2ac 100644 --- a/crates/expr/src/host.rs +++ b/crates/expr/src/host.rs @@ -9,7 +9,7 @@ use effectcraft_geom::{Mat3, Mat4}; use effectcraft_keyframe::Value; -use effectcraft_project::{Comp, ItemId, ItemKind, Layer, LayerId, LayerSource, Node, PropGroup, Property}; +use effectcraft_project::{Comp, GroupKind, ItemId, ItemKind, Layer, LayerId, LayerSource, Node, PropGroup, Property}; use effectcraft_render::EvalCtx; use effectcraft_time::Tick; @@ -283,6 +283,9 @@ fn find_child<'g>(g: &'g PropGroup, key: &Key) -> Option<&'g Node> { .find(|n| n.match_id() == m) .or_else(|| g.children.iter().find(|n| n.name().eq_ignore_ascii_case(name))) .or_else(|| g.children.iter().find(|n| compact(n.name()) == c || compact(n.match_id()) == c)) + // An effect by its After Effects match name, an imported placeholder's effect id + // (`effect("Pseudo/0.25")`; its match id is one path segment, `Pseudo_0_25`). + .or_else(|| g.children.iter().find(|n| matches!(n.as_group().map(|g| &g.kind), Some(GroupKind::Effect { effect }) if effect == name))) } } } diff --git a/crates/expr/src/tests.rs b/crates/expr/src/tests.rs index f6d07bedc..96db71a6b 100644 --- a/crates/expr/src/tests.rs +++ b/crates/expr/src/tests.rs @@ -1377,12 +1377,13 @@ fn checkbox_and_dropdown_results() { } /// An effect EffectCraft does not have (an imported After Effects placeholder, #3087) is read by -/// name like any other: its parameters by their names, a popup 1-based. +/// name like any other: its parameters by their names, a popup 1-based. It is also found by its +/// After Effects match name, its effect id (the group's match id is one path segment). #[test] fn an_unknown_effects_parameters_are_read_by_name() { let mut f = fx(); let g = group_push(&mut f.p, |ids| { - let mut g = ids.group("Pseudo/0.25", "Settings"); + let mut g = ids.group("Pseudo_0_25", "Settings"); g.kind = effectcraft_project::GroupKind::Effect { effect: "Pseudo/0.25".into() }; g.children.push(ids.prop("param1", "Speed", Value::Scalar(3.0)).into()); g.children.push(ids.prop("param2", "Mode", Value::Enum(1)).into()); @@ -1392,6 +1393,8 @@ fn an_unknown_effects_parameters_are_read_by_name() { assert_close!(f.ev(f.b, ROT, "effect('Settings')('Speed') * 2", 0.0).unwrap().as_f64(), 6.0); assert_close!(f.ev(f.b, ROT, "effect('Settings')('Mode') * 10", 0.0).unwrap().as_f64(), 20.0); assert_close!(f.ev(f.b, ROT, "effect('Settings')(1)", 0.0).unwrap().as_f64(), 3.0); + assert_close!(f.ev(f.b, ROT, "effect('Pseudo/0.25')('Speed')", 0.0).unwrap().as_f64(), 3.0); + assert_close!(f.ev(f.b, ROT, "thisLayer('Effects')('Pseudo/0.25')('Speed')", 0.0).unwrap().as_f64(), 3.0); } #[test] diff --git a/docs/aep-import.md b/docs/aep-import.md index 9d44006b2..08efa6123 100644 --- a/docs/aep-import.md +++ b/docs/aep-import.md @@ -320,7 +320,8 @@ parameter gets the file's value: is imported and exported, and so is an expression on a stored effect parameter (its text and its switch). - **Effects without a counterpart** (third-party plug-ins, pseudo effects of expression controls, `Pseudo/…`) arrive as **placeholders** (#3087): an effect group whose effect id is the After - Effects match name. Like a missing plug-in it renders nothing and is listed by Find Missing + Effects match name (expressions also find it by that name, `effect("Pseudo/0.25")`; its match id + is that name as one path segment, `Pseudo_0_25`). Like a missing plug-in it renders nothing and is listed by Find Missing Effects. It holds the effect's parameters flat (without the twirl-down group markers), named as 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, From a2ada5a5feeef838271728851975479582efeb53 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 13:14:16 +0530 Subject: [PATCH 7/9] #3087: count every parameter a placeholder walks, and popup names by length A definition of parameters that are not kept, shared by many instances, was walked again for each one without spending the budget; long popup item names were kept per instance with each item counted once. --- crates/aep/src/import/placeholder.rs | 28 +++++++++++++++------- crates/aep/tests/it/import/placeholders.rs | 23 ++++++++++++++++++ docs/aep-import.md | 3 ++- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/crates/aep/src/import/placeholder.rs b/crates/aep/src/import/placeholder.rs index a12ffa6c5..4186c9e28 100644 --- a/crates/aep/src/import/placeholder.rs +++ b/crates/aep/src/import/placeholder.rs @@ -11,8 +11,9 @@ //! 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. Once +//! The parameters placeholders walk, kept or not, and their popup items (by name length) are +//! bounded per import ([`MAX_COST`]): definitions can be 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; @@ -24,12 +25,16 @@ use crate::import::props::{Kind, import_expression, import_keys}; use crate::import::report::{Collector, LossCategory}; use crate::{Effect as AepEffect, EffectParam, ParamKind, ParamValue}; -/// Most placeholder properties and popup items one import creates. +/// Most placeholder parameters (kept or not) and popup items one import walks. pub(crate) const MAX_COST: usize = 200_000; /// Most items read from one placeholder popup. const MAX_POPUP_ITEMS: usize = 1024; +/// A popup item costs one more for each of these bytes in its name, so the names kept stay +/// bounded too. +const NAME_BYTES: usize = 16; + /// The placeholder of `fx`, with `params` (its parameter definitions) as properties. pub(crate) fn placeholder( fx: &AepEffect<'_, '_>, @@ -45,26 +50,33 @@ pub(crate) fn placeholder( let mut group = ids.group(&segment(fx.match_name()), shown); group.kind = GroupKind::Effect { effect: fx.match_name().to_string() }; let here = format!("{}: '{}' effect '{shown}' ({})", cx.comp, cx.layer, fx.match_name()); - for p in params.iter().filter(|p| !carries_nothing(p)) { + for p in params { + // Every parameter walked counts, kept or not: a definition shared by many instances is + // otherwise walked again for each one. + if carries_nothing(p) { + col.placeholder_cost = col.placeholder_cost.saturating_add(1).min(MAX_COST); + continue; + } if col.placeholder_cost >= MAX_COST { col.lose(LossCategory::Effects, format!("{here}: parameters past the import's {MAX_COST} placeholder parameters not kept")); break; } let label = param_label(p); let Some((kind, ui, default)) = shape(p, cx.size) else { + col.placeholder_cost = col.placeholder_cost.saturating_add(1); if p.stored().is_some() { col.lose(LossCategory::Effects, format!("{here}: parameter {label} not kept (type {:?})", p.kind)); } continue; }; - let items = match &ui { - ParamUi::Popup { options } => options.len(), - _ => 0, + let (items, names) = match &ui { + ParamUi::Popup { options } => (options.len(), options.iter().map(|o| 1 + o.len() / NAME_BYTES).fold(0, usize::saturating_add)), + _ => (0, 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); + let cost = col.placeholder_cost.saturating_add(names).saturating_add(1); if cost > MAX_COST { // 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). diff --git a/crates/aep/tests/it/import/placeholders.rs b/crates/aep/tests/it/import/placeholders.rs index 679d508af..01f1cbc43 100644 --- a/crates/aep/tests/it/import/placeholders.rs +++ b/crates/aep/tests/it/import/placeholders.rs @@ -190,6 +190,29 @@ fn an_effect_past_the_spent_budget_gets_no_placeholder() { assert!(found.iter().any(|d| d.contains("kept without rendering") && !d.contains("more like it")), "the kept instance alone: {found:?}"); } +/// Parameters a placeholder does not keep (markers, arbitrary data) still spend the budget: a +/// definition of very many of them, shared by many instances, is not walked again for each one. +#[test] +fn parameters_not_kept_still_spend_the_budget() { + let defs = (0..200_000).map(|i| other(if i % 2 == 0 { K::NoData } else { K::ArbitraryData }, "X", Default::None)).collect(); + let fx = Fx { match_name: "Odd Fx", name: "Odd", defs, values: vec![], data: vec![], keys: vec![] }; + let (_, r) = import_bytes(&twice(&bytes_with(&fx), "Odd Fx"), "Odd Fx"); + let found = losses(&r, LossCategory::Effects, "Odd Fx"); + assert!(found.iter().any(|d| d.contains("not kept (past the import's placeholder parameters)")), "{found:?}"); +} + +/// Long popup item names spend the budget by their length, so the names kept stay bounded. +#[test] +fn long_popup_item_names_spend_the_budget() { + let long = "x".repeat(1600); + let items: &'static [&'static str] = + Box::leak((0..1024).map(|i| &*Box::leak(format!("{long}{i}").into_boxed_str())).collect::>().into_boxed_slice()); + let fx = Fx { match_name: "Long Fx", name: "Long", defs: vec![popup("A", 1, items), popup("B", 1, items)], values: vec![], data: vec![], keys: vec![] }; + let (g, r) = placeholder_of(&fx); + assert_eq!(g.props().count(), 1); + assert!(losses(&r, LossCategory::Effects, "Long Fx").iter().any(|d| d.contains("placeholder parameters"))); +} + /// `bytes` with the layer's instance of `match_name` (its `tdmn` and `sspc`) repeated after it. fn twice(bytes: &[u8], match_name: &str) -> Vec { let mut file = parse(bytes).unwrap(); diff --git a/docs/aep-import.md b/docs/aep-import.md index 08efa6123..ffaa3f733 100644 --- a/docs/aep-import.md +++ b/docs/aep-import.md @@ -333,7 +333,8 @@ parameter gets the file's value: 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 + most 200 000 placeholder parameters and popup items (a popup at most 1024 items; parameters it + does not keep count, and an item one more per 16 bytes of its name); the rest is 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. From 04bbc67528f7c2579ec66332d07e522e32e217ea Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 13:42:53 +0530 Subject: [PATCH 8/9] #3087: test a placeholder found by its match name under ADBE Effect Parade --- crates/expr/src/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/expr/src/tests.rs b/crates/expr/src/tests.rs index 93e66e68f..a80f47b82 100644 --- a/crates/expr/src/tests.rs +++ b/crates/expr/src/tests.rs @@ -1395,6 +1395,7 @@ fn an_unknown_effects_parameters_are_read_by_name() { assert_close!(f.ev(f.b, ROT, "effect('Settings')(1)", 0.0).unwrap().as_f64(), 3.0); assert_close!(f.ev(f.b, ROT, "effect('Pseudo/0.25')('Speed')", 0.0).unwrap().as_f64(), 3.0); assert_close!(f.ev(f.b, ROT, "thisLayer('Effects')('Pseudo/0.25')('Speed')", 0.0).unwrap().as_f64(), 3.0); + assert_close!(f.ev(f.b, ROT, "thisLayer('ADBE Effect Parade')('Pseudo/0.25')('Speed')", 0.0).unwrap().as_f64(), 3.0); } #[test] From e7a396856def9b69ab89d813d440f3e8eb82240e Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Thu, 12 Nov 2026 13:54:03 +0530 Subject: [PATCH 9/9] #3087: a placeholder's float slider keeps its range It arrived as a plain number, so Effect Controls neither dragged it over its slider range nor clamped an edit to its valid range, as After Effects does. --- crates/aep/src/import/placeholder.rs | 13 ++++++++++++- crates/aep/tests/it/import/placeholders.rs | 18 ++++++++++++++++++ docs/aep-import.md | 2 +- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/aep/src/import/placeholder.rs b/crates/aep/src/import/placeholder.rs index 4186c9e28..2fc40b71b 100644 --- a/crates/aep/src/import/placeholder.rs +++ b/crates/aep/src/import/placeholder.rs @@ -111,7 +111,7 @@ pub(crate) fn placeholder( /// parameter), its control and its value until one is read. `None` for a type that is not kept. fn shape(p: &EffectParam<'_, '_>, size: (u32, u32)) -> Option<(Option, ParamUi, Value)> { let (kind, ui, default) = match p.kind { - ParamKind::IntSlider | ParamKind::FixedSlider | ParamKind::FloatSlider => (Kind::EffectNumber(1.0), ParamUi::Number, Value::Scalar(0.0)), + ParamKind::IntSlider | ParamKind::FixedSlider | ParamKind::FloatSlider => (Kind::EffectNumber(1.0), slider_ui(p), Value::Scalar(0.0)), ParamKind::Angle => (Kind::EffectNumber(1.0), ParamUi::Angle, Value::Scalar(0.0)), ParamKind::Checkbox => (Kind::EffectBool, ParamUi::Checkbox, Value::Bool(false)), ParamKind::Color => (Kind::EffectColor, ParamUi::Color, Value::Color([0.0, 0.0, 0.0, 1.0])), @@ -131,6 +131,17 @@ fn shape(p: &EffectParam<'_, '_>, size: (u32, u32)) -> Option<(Option, Par Some((Some(kind), ui, default)) } +/// A slider's control: a float slider's valid range and drag range when they are ranges (only a +/// float slider's are decoded), else a plain number. +fn slider_ui(p: &EffectParam<'_, '_>) -> ParamUi { + match (p.valid_min(), p.valid_max(), p.slider_range()) { + (Some(min), Some(max), Some((smin, smax))) if [min, max, smin, smax].iter().all(|v| v.is_finite()) && min <= max && smin <= smax => { + ParamUi::Slider { min, max, slider_min: smin.clamp(min, max), slider_max: smax.clamp(min, max), decimals: 2 } + } + _ => ParamUi::Number, + } +} + /// 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)) diff --git a/crates/aep/tests/it/import/placeholders.rs b/crates/aep/tests/it/import/placeholders.rs index 01f1cbc43..1a8ec697e 100644 --- a/crates/aep/tests/it/import/placeholders.rs +++ b/crates/aep/tests/it/import/placeholders.rs @@ -109,6 +109,24 @@ fn a_pseudo_effect_is_kept_with_its_parameter_names() { assert_eq!(path.as_deref().and_then(|p| layer.prop(p)).map(|p| p.name.as_str()), Some("Speed"), "{path:?}"); } +/// A float slider keeps its valid range and slider range, so Effect Controls drags and clamps it +/// as After Effects does; a range that is not a range stays a plain number. +#[test] +fn a_float_slider_keeps_its_range() { + let fx = Fx { + match_name: "Pseudo/r", + name: "Rig", + defs: vec![slider(K::FloatSlider, "Speed", 1.0, (-10.0, 100.0)), slider(K::FloatSlider, "Odd", 1.0, (5.0, 1.0))], + values: vec![], + data: vec![], + keys: vec![], + }; + let (g, _) = placeholder_of(&fx); + let ui = |id: &str| g.prop(id).map(|p| p.ui.clone()); + assert_eq!(ui("param1"), Some(ParamUi::Slider { min: -10.0, max: 100.0, slider_min: -10.0, slider_max: 100.0, decimals: 2 })); + assert_eq!(ui("param2"), Some(ParamUi::Number)); +} + /// A popup whose definition is borrowed (the project copy) and has no value record: its default /// slot holds another copy's selection, so the first item is used and that is reported. #[test] diff --git a/docs/aep-import.md b/docs/aep-import.md index ffaa3f733..8fc01d5ca 100644 --- a/docs/aep-import.md +++ b/docs/aep-import.md @@ -324,7 +324,7 @@ parameter gets the file's value: is that name as one path segment, `Pseudo_0_25`). Like a missing plug-in it renders nothing and is listed by Find Missing Effects. It holds the effect's parameters flat (without the twirl-down group markers), named as 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, + expressions read (sliders and angles as stored, a float slider with its range, 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, and their keys and expression are reported),