From 5922dabd3a087388450d1e115e1a097ea811b012 Mon Sep 17 00:00:00 2001 From: arnav-hiwarkar Date: Wed, 11 Nov 2026 16:04:11 +0530 Subject: [PATCH 1/4] Time entry: a negative displayed timecode goes to the frame it shows (#3373) In a comp whose Start Timecode is negative, the current-time display shows the first frames as `-0:00:00:12`, but typing that back (the Go to Time dialog opens with it since #3419, and the Timeline's time field takes it) read the leading `-` as "move back", so OK on an unedited dialog jumped to another frame. There, a `-` before a full timecode is now the time's sign, as it already is before Feet + Frames; a bare `-n` still steps back, and comps that start at 0 or later are unchanged. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/time.rs | 18 +++++++++++++++++- crates/engine/src/tests/timeline.rs | 25 +++++++++++++++++++++++++ crates/ui-egui/src/panels/forms.rs | 16 ++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/time.rs b/crates/engine/src/cmd/time.rs index 556c02bcb..49a3caede 100644 --- a/crates/engine/src/cmd/time.rs +++ b/crates/engine/src/cmd/time.rs @@ -43,7 +43,9 @@ pub fn display_duration(s: &Session, comp: &Comp, len: Tick) -> String { } /// Parse a time typed in the project's Time Display Style (relative `+n`/`-n` from frame -/// `current`) into a comp frame. +/// `current`) into a comp frame. In Timecode style, in a comp whose Start Timecode is negative, a +/// `-` before a timecode with separators (`-0:00:00:12`, `-1.00`) is the sign of a displayed time, +/// not a step back; a bare `-n` still steps back. pub fn parse_display_time(s: &Session, comp: &Comp, text: &str, current: i64) -> std::result::Result { let fr = comp.frame_rate; let st = &s.project.settings; @@ -53,6 +55,20 @@ pub fn parse_display_time(s: &Session, comp: &Comp, text: &str, current: i64) -> effectcraft_time::parse_feet_frames(text, pf, current + start).map(|f| f - start).map_err(|e| e.0) } None => { + // A comp whose Start Timecode is negative shows the times before 0 with a `-` + // (`-0:00:00:12`), so there a `-` before a timecode with separators is the sign of a + // displayed time and typing what the display shows lands on the frame it shows. A bare + // `-n` still moves n frames back. + if st.time_display == TimeDisplayStyle::Timecode + && comp.display_start < Tick::ZERO + && let Some(body) = text.trim().strip_prefix('-').map(str::trim_start) + && body.contains([':', ';', '.', ',']) + && !body.starts_with(['+', '-']) + { + let drop_frame = fr.supports_drop_frame() && body.contains(';'); + let shown = effectcraft_time::parse_timecode(body, fr, drop_frame, current).map_err(|e| e.0)?; + return Ok(shown.saturating_neg().saturating_sub(fr.frame_at(comp.display_start))); + } // The Timeline shows non-drop timecode (`:`), so typed `:`/`.` text is read as non-drop // too; only explicit `;` fields mean drop-frame (#890). let drop_frame = fr.supports_drop_frame() && text.contains(';'); diff --git a/crates/engine/src/tests/timeline.rs b/crates/engine/src/tests/timeline.rs index 55475eef3..b5a44239b 100644 --- a/crates/engine/src/tests/timeline.rs +++ b/crates/engine/src/tests/timeline.rs @@ -649,6 +649,31 @@ fn time_set_keeps_the_frame_shown_in_the_current_time_field() { assert_eq!(r["frame"], 13); } +/// A comp that starts before 0 shows its first frames as negative timecode (`-0:00:00:12`). +/// Typing back what the current-time display shows (Go to Time opens with it) stays on that frame; +/// the leading `-` used to move back from the current frame instead. A bare `-n` still steps back, +/// and in a comp that starts at 0 a `-` stays relative. +#[test] +fn time_set_round_trips_negative_displayed_timecode() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Pre-roll", "width": 64, "height": 64, "frameRate": 24, "duration": 4, "startTime": -1})).unwrap(); + for frame in [0_i64, 1, 12, 23, 24, 25, 60] { + let r = s.execute("time.set", json!({"frame": frame})).unwrap(); + let display = r["display"].as_str().unwrap().to_string(); + let r = s.execute("time.set", json!({"timecode": display})).unwrap(); + assert_eq!(r["frame"], frame, "displayed {display}"); + } + s.execute("time.set", json!({"frame": 12})).unwrap(); + assert_eq!(s.execute("time.set", json!({"timecode": "-0:00:00:12"})).unwrap()["display"], "-0:00:00:12"); + assert_eq!(s.execute("time.set", json!({"timecode": "-5"})).unwrap()["frame"], 7, "a bare -n steps back"); + assert_eq!(s.execute("time.set", json!({"timecode": "-0:00:00:20"})).unwrap()["frame"], 4); + assert!(s.execute("time.set", json!({"timecode": "- -1:00"})).is_err(), "a second sign is still an error"); + + s.execute("comp.settings", json!({"startTime": 0})).unwrap(); + s.execute("time.set", json!({"frame": 12})).unwrap(); + assert_eq!(s.execute("time.set", json!({"timecode": "-0:00:00:02"})).unwrap()["frame"], 10, "relative when the comp starts at 0"); +} + /// Frames display style: typed digits are a frame count, so 00100 is frame 100, not 1 s (#530). #[test] fn time_set_reads_frames_style_digits_as_a_frame_count() { diff --git a/crates/ui-egui/src/panels/forms.rs b/crates/ui-egui/src/panels/forms.rs index 9b94360a5..58f89e39d 100644 --- a/crates/ui-egui/src/panels/forms.rs +++ b/crates/ui-egui/src/panels/forms.rs @@ -1130,6 +1130,22 @@ mod tests { assert_eq!(at, 19, "displayed 00020 with frames starting at 1"); } + /// In a comp that starts 1 s before 0, Go to Time at frame 12 opens on the negative time the + /// viewer shows, and OK without editing it stays on frame 12 (it used to jump to frame 0). + #[test] + fn go_to_time_ok_keeps_a_negative_displayed_time() { + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + app.session.execute("comp.new", json!({"name": "Main", "width": 64, "height": 36, "frameRate": 24, "duration": 4, "startTime": -1})).unwrap(); + app.session.execute("time.set", json!({"frame": 12})).unwrap(); + assert!(open_form(&mut app, "time.set", &json!({}))); + let f = &app.dialog_state.form; + assert!(matches!(&f.fields[0].kind, FieldKind::Text(t) if t == "-0:00:00:12"), "{:?}", f.fields[0]); + let (command, params) = (f.command.clone(), f.params()); + app.session.execute(&command, params).unwrap(); + let at = app.session.active_comp().unwrap().frame_rate.frame_at(app.session.time()); + assert_eq!(at, 12, "OK on the unedited time stays put"); + } + /// File ▸ Export ▸ Adobe Premiere Pro Project… opens a form that says it writes Final Cut Pro /// XML; its fields register automation ids and OK runs `file.exportTimeline`. #[test] From 47fc00f85d9169751ad2d0674f228afaf233681a Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Wed, 11 Nov 2026 11:58:07 +0100 Subject: [PATCH 2/4] test(ui-egui): split the crypto-pick viewer matrix into one test per window size picks_what_the_viewer_shows_across_sizes_scales_zoom_pan_resolution_and_proxies looped over four independent window-size and display-scale cases, each with its own session. As one test it took ~90 s in the gate and ~590 s under coverage, where it alone set the slowest SonarCloud shard. Each case is now its own test, running the same checks, so nextest runs them in parallel and partitions them across shards. The "more than 100 hovers checked" floor becomes more than 25 per case, which still adds up to more than 100. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/tests/it/ui_crypto_pick.rs | 153 ++++++++++++---------- 1 file changed, 86 insertions(+), 67 deletions(-) diff --git a/crates/ui-egui/tests/it/ui_crypto_pick.rs b/crates/ui-egui/tests/it/ui_crypto_pick.rs index 0d1767c7e..03e49ca18 100644 --- a/crates/ui-egui/tests/it/ui_crypto_pick.rs +++ b/crates/ui-egui/tests/it/ui_crypto_pick.rs @@ -277,84 +277,103 @@ fn eyedropper_arms_hovers_picks_with_modifiers_and_stays_armed_until_esc() { let _ = std::fs::remove_dir_all(&ids.dir); } -/// Window and panel sizes × display scales × viewer zoom / pan × Resolution (and a proxy): hover -/// and click name the ID of the pixel the viewer shows, next to cell edges too. Each hover is -/// checked against the rendered matte of the named ID at the point the screen maps back to. +// Window and panel sizes × display scales, one test each so they run in parallel. Together they +// took ~90 s as one test, and ~10 min under coverage, where that one test set the slowest shard. #[test] -fn picks_what_the_viewer_shows_across_sizes_scales_zoom_pan_resolution_and_proxies() { +fn picks_what_the_viewer_shows_at_900x640_and_1x() { + picks_what_the_viewer_shows(0, vec2(900.0, 640.0), 1.0, 300.0); +} + +#[test] +fn picks_what_the_viewer_shows_at_1280x720_and_1_25x() { + picks_what_the_viewer_shows(1, vec2(1280.0, 720.0), 1.25, 420.0); +} + +#[test] +fn picks_what_the_viewer_shows_at_1600x1000_and_1_5x() { + picks_what_the_viewer_shows(2, vec2(1600.0, 1000.0), 1.5, 360.0); +} + +#[test] +fn picks_what_the_viewer_shows_at_2400x1400_and_2x() { + picks_what_the_viewer_shows(3, vec2(2400.0, 1400.0), 2.0, 600.0); +} + +/// Case `i`: a window of `size` at display scale `ppp`, with an Effect Controls panel `ec` wide, +/// × viewer zoom / pan × Resolution (and a proxy). Hover and click name the ID of the pixel the +/// viewer shows, next to cell edges too. Each hover is checked against the rendered matte of the +/// named ID at the point the screen maps back to. Case 0 also checks frames from the idle cache. +fn picks_what_the_viewer_shows(i: usize, size: egui::Vec2, ppp: f32, ec: f32) { let mut checked = 0; let mut failures = vec![]; - let cases = [(vec2(900.0, 640.0), 1.0, 300.0), (vec2(1280.0, 720.0), 1.25, 420.0), (vec2(1600.0, 1000.0), 1.5, 360.0), (vec2(2400.0, 1400.0), 2.0, 600.0)]; - for (i, (size, ppp, ec)) in cases.into_iter().enumerate() { - let (s, ids) = crypto_session(&format!("matrix{i}"), 1919, 1079, false); - let mut h = harness(s, size, ppp, ec); - h.state_mut().session.execute_checked("prop.set", json!({"path": "effects/#1/matteOnly", "value": true})).unwrap(); - arm(&mut h, &ids); - for proxy in [false, true] { - if proxy { - let p = ids.dir.join("proxy").join("plate_proxy.exr"); - h.state_mut().session.execute("file.setProxy", json!({"item": ids.footage, "path": p.to_string_lossy()})).unwrap(); - } - for res in [Resolution::Full, Resolution::Half, Resolution::Third, Resolution::Quarter, Resolution::Custom(6)] { - for (zoom, pan) in [(None, [0.0, 0.0]), (Some(0.5), [0.0, 0.0]), (Some(2.0), [-300.0, 120.0])] { - let cid = h.state().session.active_comp_id().unwrap(); - h.state_mut().session.set_resolution(cid, res).unwrap(); - h.state_mut().ui.viewer.zoom = zoom.map(|z: f32| z / ppp); - h.state_mut().ui.viewer.pan = pan; - settle(&mut h); - let area = h.state().auto.find("viewer.fxPick").unwrap().rect; - let scale = shown_scale(&h); - // Points next to cell edges, in the middle and at the far corner. - // (Cell edges far from the origin: 925, 1044 shows the 1/3 vs 0.333 rounding.) - for p in [[36.6, 28.7], [37.4, 29.3], [959.7, 539.2], [1918.4, 1078.6], [74.1, 57.9], [925.4, 1044.4], [1110.6, 1043.6]] { - let Some(sp) = comp_to_screen(&h.ctx, [p[0] as f32, p[1] as f32]) else { continue }; - if sp.x < area[0] + 4.0 || sp.y < area[1] + 40.0 || sp.x > area[0] + area[2] - 4.0 || sp.y > area[1] + area[3] - 4.0 { - continue; - } - let got = hover(&mut h, sp); - let got = got.trim_end_matches(" (selected)").to_string(); - // The comp point under the cursor, as the viewer maps it back. - let q = to_comp(&h, sp); - checked += 1; - // The named ID has the largest coverage of the shown pixel (a blended edge - // pixel of a proxy at Quarter has several). No ID named: the frame shows - // (almost) no matte there either (an odd size's last half-resolution pixel - // samples just outside the layer). - let ok = if got.starts_with('c') { largest(&mut h, &got, q, scale) } else { got == HINT && lit(&mut h, "*", q, scale) < 0.5 }; - if !ok { - failures.push(format!( - "{size:?} ppp {ppp} proxy {proxy} {res:?} zoom {zoom:?} (scale {scale}): at {q:?} hover {got:?} is not lit there" - )); - } + let (s, ids) = crypto_session(&format!("matrix{i}"), 1919, 1079, false); + let mut h = harness(s, size, ppp, ec); + h.state_mut().session.execute_checked("prop.set", json!({"path": "effects/#1/matteOnly", "value": true})).unwrap(); + arm(&mut h, &ids); + for proxy in [false, true] { + if proxy { + let p = ids.dir.join("proxy").join("plate_proxy.exr"); + h.state_mut().session.execute("file.setProxy", json!({"item": ids.footage, "path": p.to_string_lossy()})).unwrap(); + } + for res in [Resolution::Full, Resolution::Half, Resolution::Third, Resolution::Quarter, Resolution::Custom(6)] { + for (zoom, pan) in [(None, [0.0, 0.0]), (Some(0.5), [0.0, 0.0]), (Some(2.0), [-300.0, 120.0])] { + let cid = h.state().session.active_comp_id().unwrap(); + h.state_mut().session.set_resolution(cid, res).unwrap(); + h.state_mut().ui.viewer.zoom = zoom.map(|z: f32| z / ppp); + h.state_mut().ui.viewer.pan = pan; + settle(&mut h); + let area = h.state().auto.find("viewer.fxPick").unwrap().rect; + let scale = shown_scale(&h); + // Points next to cell edges, in the middle and at the far corner. + // (Cell edges far from the origin: 925, 1044 shows the 1/3 vs 0.333 rounding.) + for p in [[36.6, 28.7], [37.4, 29.3], [959.7, 539.2], [1918.4, 1078.6], [74.1, 57.9], [925.4, 1044.4], [1110.6, 1043.6]] { + let Some(sp) = comp_to_screen(&h.ctx, [p[0] as f32, p[1] as f32]) else { continue }; + if sp.x < area[0] + 4.0 || sp.y < area[1] + 40.0 || sp.x > area[0] + area[2] - 4.0 || sp.y > area[1] + area[3] - 4.0 { + continue; + } + let got = hover(&mut h, sp); + let got = got.trim_end_matches(" (selected)").to_string(); + // The comp point under the cursor, as the viewer maps it back. + let q = to_comp(&h, sp); + checked += 1; + // The named ID has the largest coverage of the shown pixel (a blended edge + // pixel of a proxy at Quarter has several). No ID named: the frame shows + // (almost) no matte there either (an odd size's last half-resolution pixel + // samples just outside the layer). + let ok = if got.starts_with('c') { largest(&mut h, &got, q, scale) } else { got == HINT && lit(&mut h, "*", q, scale) < 0.5 }; + if !ok { + failures + .push(format!("{size:?} ppp {ppp} proxy {proxy} {res:?} zoom {zoom:?} (scale {scale}): at {q:?} hover {got:?} is not lit there")); } } } - // Frames from Cache Frames When Idle (the first case): rendered on the grid the - // viewer renders and the pick reads (1/3, not the key's 0.333), so frame 20 from the - // idle cache is frame 0 as the viewer rendered it, and hovers there are right too. - if i == 0 && !proxy { - (h.state_mut().ui.viewer.zoom, h.state_mut().ui.viewer.pan) = (None, [0.0, 0.0]); - for res in [Resolution::Third, Resolution::Custom(6)] { - let fails = idle_cached_frames_are_shown_frames(&mut h, res); - checked += 1; - failures.extend(fails); - } - } - // A click picks what the hover named. + } + // Frames from Cache Frames When Idle (the first case): rendered on the grid the + // viewer renders and the pick reads (1/3, not the key's 0.333), so frame 20 from the + // idle cache is frame 0 as the viewer rendered it, and hovers there are right too. + if i == 0 && !proxy { (h.state_mut().ui.viewer.zoom, h.state_mut().ui.viewer.pan) = (None, [0.0, 0.0]); - settle(&mut h); - let sp = comp_to_screen(&h.ctx, [959.7, 539.2]).unwrap(); - let named = hover(&mut h, sp); - click(&mut h, sp, Modifiers::NONE); - assert_eq!(selection(&h, &ids), named.trim_end_matches(" (selected)")); - if proxy { - h.state_mut().session.execute("file.setProxy", json!({"item": ids.footage, "path": null})).ok(); + for res in [Resolution::Third, Resolution::Custom(6)] { + let fails = idle_cached_frames_are_shown_frames(&mut h, res); + checked += 1; + failures.extend(fails); } } - let _ = std::fs::remove_dir_all(&ids.dir); + // A click picks what the hover named. + (h.state_mut().ui.viewer.zoom, h.state_mut().ui.viewer.pan) = (None, [0.0, 0.0]); + settle(&mut h); + let sp = comp_to_screen(&h.ctx, [959.7, 539.2]).unwrap(); + let named = hover(&mut h, sp); + click(&mut h, sp, Modifiers::NONE); + assert_eq!(selection(&h, &ids), named.trim_end_matches(" (selected)")); + if proxy { + h.state_mut().session.execute("file.setProxy", json!({"item": ids.footage, "path": null})).ok(); + } } + let _ = std::fs::remove_dir_all(&ids.dir); assert!(failures.is_empty(), "{} of {checked} hovers wrong:\n{}", failures.len(), failures.join("\n")); - assert!(checked > 100, "{checked}"); + // more than 100 over the four cases, as when they were one test + assert!(checked > 25, "{checked}"); } fn set_frame(h: &mut Harness<'_, EffectcraftApp>, f: i64) { From 12260b288ca6b6e43e1904e468fc81a2c6cc618f Mon Sep 17 00:00:00 2001 From: sunkidda Date: Wed, 11 Nov 2026 12:46:25 +0100 Subject: [PATCH 3/4] .aep import: recognize French names for Levels' Channel and Clip popups (#3432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * .aep import: recognize French names for Levels' Channel and Clip popups After Effects stores a popup's items in the UI's own language. A project saved by a French After Effects writes "RVB", "Activé" and "Désactivé pour la couleur 32 bpc" for Levels' Channel and Clip to Output Black / White. EffectCraft's own popup options are English only, so these never matched by name and the parameter fell back to its default, reported as "has no counterpart". Add popup_alias, a small table of confirmed After Effects labels in other languages, used as a fallback in popup_mapping after the exact name match fails. A synthetic test reproduces the three losses from a real file and checks they are gone, with the same values as the English original. * Cover Levels' French Red, Green and Blue popup items too popup_alias only mapped "rvb" (RGB), so a French Levels with Red, Green or Blue selected still lost that selection the same way. Add "rouge", "vert" and "bleu" (from the same real pdnm list, confirmed against it); "Alpha" is already the same word in both languages. A direct test on popup_alias covers every item of the Channel popup it claims to know. --------- Co-authored-by: sunkidda --- crates/aep/src/container/effect_map.rs | 22 ++++++++++ crates/aep/src/import/effects.rs | 13 ++++-- crates/aep/tests/it/import/synthetic.rs | 56 +++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/crates/aep/src/container/effect_map.rs b/crates/aep/src/container/effect_map.rs index aea320395..06b80cc3a 100644 --- a/crates/aep/src/container/effect_map.rs +++ b/crates/aep/src/container/effect_map.rs @@ -1147,6 +1147,28 @@ pub fn popup_key(item: &str) -> String { s.strip_suffix(" channel").map(str::to_string).unwrap_or(s) } +/// The English [`popup_key`] a non-English After Effects popup item name stands for, when this +/// table knows it; `None` otherwise. EffectCraft's own popup options are English only, so a +/// project saved by an After Effects set to another UI language never matches them by name and +/// the parameter falls back to its default (observed on a real "Niveaux" project, French After +/// Effects 2026 26.0.0x67: Levels' Channel and Clip to Output Black / White). Add an entry only +/// from a confirmed After Effects label, never a guess at a whole menu's translation. +pub fn popup_alias(key: &str) -> Option<&'static str> { + Some(match key { + // French: Levels' Channel popup (`ADBE Easy Levels2-0001`). "Alpha" is the same word in + // both languages, so it needs no entry. + "rvb" => "rgb", + "rouge" => "red", + "vert" => "green", + "bleu" => "blue", + // French: Levels' Clip to Output Black / White popups (`-0008`, `-0009`). + "activé" => "on", + "désactivé" => "off", + "désactivé pour la couleur 32 bpc" => "off for 32 bpc color", + _ => return None, + }) +} + /// The table row of an After Effects match name. pub fn by_match_name(match_name: &str) -> Option<&'static EffectMap> { EFFECTS.iter().find(|m| m.match_name == match_name) diff --git a/crates/aep/src/import/effects.rs b/crates/aep/src/import/effects.rs index 1ab4b208a..a09ef37b6 100644 --- a/crates/aep/src/import/effects.rs +++ b/crates/aep/src/import/effects.rs @@ -34,7 +34,7 @@ //! import. Hue/Saturation's master values and Levels' values are arbitrary data, decoded by the //! reader (444-446, 512-516). -use crate::effect_map::{CRYPTOMATTE_OUTPUT, Conv, EFFECTS, EffectMap, Extra, HUE_SATURATION_MASTER, LEVELS_RGB, ParamMap, popup_key}; +use crate::effect_map::{CRYPTOMATTE_OUTPUT, Conv, EFFECTS, EffectMap, Extra, HUE_SATURATION_MASTER, LEVELS_RGB, ParamMap, popup_alias, popup_key}; use crate::{ CryptomatteData, CurvesData, DefinitionSource, Effect as AepEffect, EffectDefinitions, EffectParam, ExtractorChannels, HueSaturationData, Layer as AepLayer, LayerRef, LevelsData, MeshData, ParamKind, ParamValue, @@ -413,8 +413,8 @@ pub(crate) fn resolve_layer_refs( } } -/// The EffectCraft item of each file item (by name, ignoring case and separators); `None` when -/// the parameter here is not a popup. +/// The EffectCraft item of each file item (by name, ignoring case and separators, and a known +/// non-English name for it, [`popup_alias`]); `None` when the parameter here is not a popup. fn popup_mapping(file_items: &[String], ui: &ParamUi) -> Option>> { let ParamUi::Popup { options } = ui else { return None }; let norm = popup_key; @@ -425,7 +425,12 @@ fn popup_mapping(file_items: &[String], ui: &ParamUi) -> Option> if item.trim() == "(-" { return None; } - options.iter().position(|o| norm(o) == norm(item)).and_then(|i| u32::try_from(i).ok()) + let key = norm(item); + options + .iter() + .position(|o| norm(o) == key) + .or_else(|| popup_alias(&key).and_then(|a| options.iter().position(|o| norm(o) == a))) + .and_then(|i| u32::try_from(i).ok()) }) .collect(), ) diff --git a/crates/aep/tests/it/import/synthetic.rs b/crates/aep/tests/it/import/synthetic.rs index 8e4c28f16..5e01c9f9b 100644 --- a/crates/aep/tests/it/import/synthetic.rs +++ b/crates/aep/tests/it/import/synthetic.rs @@ -842,3 +842,59 @@ fn a_default_state_colour_record_holds_the_value() { assert_eq!(colour(&default_state), colour(&written)); assert_ne!(colour(&default_state), colour(&unchanged), "the recorded colour, not the default"); } + +/// `SYNTHETIC` with Levels' Channel and Clip to Output Black / White popups' item names (`pdnm`) +/// rewritten to the French labels a French-language After Effects stores there, keeping the same +/// item order so the index each popup is set to is unaffected. +fn with_french_levels_popups() -> Vec { + let (mut channel, mut clip) = (0, 0); + let bytes = mutate(&mut |c| { + if !c.is(b"Utf8") { + return; + } + if c.bytes() == Some(b"RGB|Red|Green|Blue|Alpha".as_slice()) { + *c = Chunk::data(*b"Utf8", b"RVB|Rouge|Vert|Bleu|Alpha".to_vec()); + channel += 1; + } else if c.bytes() == Some(b"On|Off|Off for 32 bpc Color".as_slice()) { + *c = Chunk::data(*b"Utf8", "Activé|Désactivé|Désactivé pour la couleur 32 bpc".as_bytes().to_vec()); + clip += 1; + } + }); + assert_eq!(channel, 2, "Levels' Channel pdnm"); + assert_eq!(clip, 4, "Levels' two Clip pdnm (Black and White), each stored twice"); + bytes +} + +/// A popup's item names in the file are matched to EffectCraft's own (English) ones by name +/// (`popup_key`): a project saved by a French After Effects stores "RVB", "Activé", "Désactivé +/// pour la couleur 32 bpc" for Levels' Channel and Clip to Output Black / White (observed in a +/// real "Niveaux" effect, After Effects 2026 26.0.0x67). Before `popup_alias`, none of these +/// matched EffectCraft's English-only options, so each came back "has no counterpart, default +/// used" and the real selection was silently lost. +#[test] +fn french_levels_popup_items_are_recognized() { + let (unchanged, r1) = run(SYNTHETIC, true); + let (french, r2) = run(&with_french_levels_popups(), true); + let (r1, r2) = (r1.unwrap(), r2.unwrap()); + let found = losses(&r2, LossCategory::Effects); + assert!(!found.iter().any(|d| d.contains("has no counterpart")), "{found:?}"); + for id in ["channel", "clipToOutputBlack", "clipToOutputWhite"] { + let want: Vec<_> = effect_values(&unchanged).into_iter().filter(|(p, _)| p == id).collect(); + let got: Vec<_> = effect_values(&french).into_iter().filter(|(p, _)| p == id).collect(); + assert_eq!(got, want, "{id}"); + } + assert_eq!(r1.report.imported.effects, r2.report.imported.effects); +} + +/// `popup_alias` must cover every item of a popup it claims to know, not only the one a first +/// report happened to show: Channel's French "Rouge", "Vert", "Bleu" ("Alpha" is the same word in +/// both languages) alongside the already-covered "RVB" (`effect_map.rs`, reviewed on #3432). +#[test] +fn french_levels_channel_aliases_cover_every_item() { + use effectcraft_aep::effect_map::popup_alias; + for (french, english) in [("rvb", "rgb"), ("rouge", "red"), ("vert", "green"), ("bleu", "blue")] { + assert_eq!(popup_alias(french), Some(english), "{french}"); + } + // "Alpha" needs no alias: the same word in both languages. + assert_eq!(popup_alias("alpha"), None); +} From 284cec6e461a5ea4e23ceec6cb5499b01b094b01 Mon Sep 17 00:00:00 2001 From: ash Date: Wed, 11 Nov 2026 17:16:46 +0530 Subject: [PATCH 4/4] .aep: popup items match ignoring case outside ASCII too (#3432 follow-up) (#3447) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit popup_key lowered only ASCII letters, so an item starting with a capital outside ASCII (French "Écran") kept it and never matched the same word in lower case. After Effects stores popup items in its UI language, and #3432 starts matching such names through popup_alias, keyed by popup_key. Use Unicode lower case; ASCII keys are unchanged. --- crates/aep/src/container/effect_map.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/aep/src/container/effect_map.rs b/crates/aep/src/container/effect_map.rs index 06b80cc3a..49a53d2e7 100644 --- a/crates/aep/src/container/effect_map.rs +++ b/crates/aep/src/container/effect_map.rs @@ -1142,8 +1142,12 @@ pub const GLOW_MIDPOINT_MAX: f64 = 0.990_005_493_164_062_5; /// The key a popup item is matched by between the two programs: trimmed, lower case, and /// Set Matte's "Red Channel" ... "Alpha Channel" are EffectCraft's "Red" ... "Alpha" (904 E24). +/// +/// Lower case is Unicode's, not only ASCII's: After Effects stores popup items in its UI +/// language, so an item can start with a capital outside ASCII (French "Écran"), and its key must +/// be the same as for the same word in lower case. pub fn popup_key(item: &str) -> String { - let s = item.trim().to_ascii_lowercase(); + let s = item.trim().to_lowercase(); s.strip_suffix(" channel").map(str::to_string).unwrap_or(s) } @@ -1186,3 +1190,17 @@ pub const HUE_SATURATION_MASTER: [&str; 3] = ["hue", "saturation", "lightness"]; /// The EffectCraft parameters Levels' RGB channel values (`Extra::Levels`) go to, in the order of /// the data (input black, input white, gamma, output black, output white: 512-516). pub const LEVELS_RGB: [&str; 5] = ["inBlack", "inWhite", "gamma", "outBlack", "outWhite"]; + +#[cfg(test)] +mod tests { + use super::*; + + /// A capital outside ASCII has the same key as its lower case; ASCII keys are unchanged. + #[test] + fn popup_key_lowers_non_ascii_capitals() { + assert_eq!(popup_key("ÉCRAN"), popup_key("écran")); + assert_eq!(popup_key(" Désactivé "), "désactivé"); + assert_eq!(popup_key("Off for 32 bpc Color"), "off for 32 bpc color"); + assert_eq!(popup_key("Red Channel"), "red"); + } +}