Merge pull request #3530 from arnav-hiwarkar/fix/3456-tracker-signature

Warp Stabilizer, 3D Camera Tracker: keep analyses saved before 0.23 (#3456)

ATTRIBUTION.md

@@ -134,6 +134,7 @@ configuration from these Apache-2.0 projects:
 | Asset | Author | Source | Licence |
 |---|---|---|---|
 | `crates/engine/src/tests/fixtures/object-matte-0.22.ecproj` | lisn0 | Original work, synthetic project saved by EffectCraft 0.22.0 | MIT OR Apache-2.0 |
+| `crates/engine/src/tests/fixtures/tracked-0.22.ecproj` | arnav-hiwarkar | Original work, synthetic project saved by EffectCraft 0.22.0 | MIT OR Apache-2.0 |
 
 ## First-party brand marks
 

crates/engine/src/analysis/camera_track.rs

@@ -124,8 +124,17 @@ fn str_of(g: &PropGroup, m: &str) -> String {
 
 /// What a tracker's tracks are made from: the layer's frames and Detailed Analysis.
 fn tracks_key(project: &Project, cid: ItemId, l: &Layer, g: &PropGroup) -> String {
+    tracks_key_of(signature(project, cid, l), g)
+}
+
+fn tracks_key_of(signature: String, g: &PropGroup) -> String {
     let detailed = effectcraft_effects::warp_stab::param(&static_params(g), "advanced/detailedAnalysis").is_some_and(Value::as_bool);
-    format!("{}{}", signature(project, cid, l), if detailed { "+detailed" } else { "" })
+    format!("{signature}{}", if detailed { "+detailed" } else { "" })
+}
+
+/// Whether the tracker's saved tracks are made from the layer's current frames and settings.
+fn tracks_match(project: &Project, cid: ItemId, l: &Layer, g: &PropGroup) -> bool {
+    crate::warp::key_matches(project, cid, l, &str_of(g, TRACKS_KEY), |s| tracks_key_of(s, g))
 }
 
 /// Parameters of an instance from static values (the tracker's settings are not animated).
@@ -165,7 +174,7 @@ pub(crate) fn invalidate(before: &Project, after: &mut Project) -> Vec<(ItemId,
             let old_layer = before.comp(*cid).and_then(|bc| bc.layer(l.id));
             for g in fx.groups().filter(|g| is_tracker(g)) {
                 let tracked = !str_of(g, TRACKS).is_empty();
-                if tracked && str_of(g, TRACKS_KEY) != tracks_key(after, *cid, l, g) {
+                if tracked && !tracks_match(after, *cid, l, g) {
                     clears.push((*cid, l.id, g.uid, true));
                     todo.push((*cid, l.id, g.uid));
                 } else if tracked && str_of(g, SOLVE_KEY) != ct::solve_key(&ct::settings(&static_params(g))) {
@@ -325,7 +334,7 @@ impl Session {
         let key = tracks_key(&self.project, comp, l, g);
         let settings = ct::settings(&params);
         let solve_key = ct::solve_key(&settings);
-        let tracks = ct::tracks(&params).filter(|_| str_of(g, TRACKS_KEY) == key);
+        let tracks = ct::tracks(&params).filter(|_| tracks_match(&self.project, comp, l, g));
         let n = times.len();
         let work = CamWork {
             project: self.project.clone(),

crates/engine/src/analysis/roto.rs

@@ -222,12 +222,6 @@ pub fn input_key(p: &Project, comp: ItemId, layer: &Layer, uid: Uid) -> String {
     input_key_with_signature(crate::warp::signature(p, comp, layer), layer, uid)
 }
 
-/// Recognize the input key serialized before 0.23 without re-keying its frozen matte.
-fn input_key_before_placed_page(p: &Project, comp: ItemId, layer: &Layer, uid: Uid) -> Option<String> {
-    let signature = crate::warp::signature_before_placed_page(p, comp, layer)?;
-    Some(input_key_with_signature(signature, layer, uid))
-}
-
 fn input_key_with_signature(mut s: String, layer: &Layer, uid: Uid) -> String {
     if let Some(m) = layer.masks() {
         s.push_str(&format!("{m:?}"));
@@ -258,7 +252,7 @@ pub(crate) fn sync(before: &Project, after: &mut Project) -> Vec<(ItemId, LayerI
                 let old = before.comp(*cid).and_then(|bc| bc.layer(l.id)).and_then(|ol| ol.props.find_group(g.uid));
                 let saved_key = str_of(g, INPUT_KEY);
                 // Keep a matching old key: the frozen cache itself also hashes that key.
-                if saved_key != key && input_key_before_placed_page(after, *cid, l, g.uid).as_deref() != Some(saved_key.as_str()) {
+                if !crate::warp::key_matches(after, *cid, l, &saved_key, |s| input_key_with_signature(s, l, g.uid)) {
                     set.push((*cid, l.id, g.uid, key));
                     todo.push((*cid, l.id, g.uid));
                 } else if old.is_none_or(|o| guard_of(o) != guard_of(g)) {

crates/engine/src/analysis/warp.rs

@@ -17,7 +17,7 @@ use std::sync::{Arc, Mutex};
 
 use effectcraft_effects::warp_stab::{self, ANALYSIS, ANALYSIS_KEY};
 use effectcraft_keyframe::Value;
-use effectcraft_project::{GroupKind, ItemId, ItemKind, Layer, LayerId, LayerSource, Project, PropGroup, Uid};
+use effectcraft_project::{Footage, GroupKind, ItemId, ItemKind, Layer, LayerId, LayerSource, Project, PropGroup, Uid};
 use effectcraft_raster::Image;
 use effectcraft_render::{EvalCtx, ExprHost, FootageSource, LayerCache, Renderer};
 use effectcraft_time::Tick;
@@ -112,7 +112,7 @@ fn fnv(s: &str) -> u64 {
 pub fn signature(p: &Project, comp: ItemId, layer: &Layer) -> String {
     let src = match &layer.source {
         LayerSource::Footage { item } | LayerSource::Solid { item } | LayerSource::Comp { item } => match p.item(*item).map(|i| &i.kind) {
-            Some(ItemKind::Footage(f)) => format!("{f:?}"),
+            Some(ItemKind::Footage(f)) => footage_text(f, true),
             Some(ItemKind::Solid(s)) => format!("{s:?}"),
             Some(ItemKind::Comp(c)) => format!("{}x{}@{:?}/{:?}", c.width, c.height, c.frame_rate, c.duration),
             _ => String::new(),
@@ -122,14 +122,108 @@ pub fn signature(p: &Project, comp: ItemId, layer: &Layer) -> String {
     signature_with_source(p, comp, layer, &src)
 }
 
+/// Whether `saved`, a key an analysis of `layer` was saved with, is for the layer's current frames.
+/// `key` makes such a key from a signature. A key made from the signature an earlier release wrote
+/// for the same frames matches too: keep it rather than rewrite it (analysing writes today's).
+pub(crate) fn key_matches(p: &Project, comp: ItemId, layer: &Layer, saved: &str, key: impl Fn(String) -> String) -> bool {
+    key(signature(p, comp, layer)) == saved || signature_before_placed_page(p, comp, layer).is_some_and(|s| key(s) == saved)
+}
+
 /// Before `Footage::placed_page` was added in 0.23, the persisted signature hashed the
-/// same Debug representation without that last field. Only ordinary footage can match it.
-pub(crate) fn signature_before_placed_page(p: &Project, comp: ItemId, layer: &Layer) -> Option<String> {
+/// same footage text without that last field. Only ordinary footage can match it.
+fn signature_before_placed_page(p: &Project, comp: ItemId, layer: &Layer) -> Option<String> {
     let LayerSource::Footage { item } = &layer.source else { return None };
     let ItemKind::Footage(footage) = &p.item(*item)?.kind else { return None };
-    let src = format!("{footage:?}");
-    let old = src.strip_suffix(", placed_page: false }")?;
-    Some(signature_with_source(p, comp, layer, &format!("{old} }}")))
+    if footage.placed_page {
+        return None;
+    }
+    Some(signature_with_source(p, comp, layer, &footage_text(footage, false)))
+}
+
+/// A footage item in a signature: its fields as `Debug` wrote them in 0.23, the text saved
+/// analysis keys were hashed from (without `placed_page` before 0.23). It is written out here so
+/// that a new `Footage` field doesn't change every signature and drop every saved analysis.
+pub(crate) fn footage_text(f: &Footage, placed_page: bool) -> String {
+    format!("{:?}", FootageText { f, placed_page })
+}
+
+struct FootageText<'a> {
+    f: &'a Footage,
+    placed_page: bool,
+}
+
+impl std::fmt::Debug for FootageText<'_> {
+    fn fmt(&self, out: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        // Every field is named, so a new one doesn't compile until it is decided here. A new field
+        // that changes the frames goes last, written only when it isn't its default, so keys saved
+        // before it still match. A new field that doesn't change the frames stays out.
+        let Footage {
+            path,
+            kind,
+            width,
+            height,
+            pixel_aspect,
+            frame_rate,
+            native_rate,
+            duration,
+            has_video,
+            has_audio,
+            alpha,
+            premul_color,
+            loop_count,
+            codec,
+            missing,
+            sequence,
+            color_profile,
+            layer,
+            fields,
+            invert_alpha,
+            linear_light,
+            preserve_rgb,
+            cineon,
+            data,
+            page,
+            alphabetical,
+            start_timecode,
+            file_timecode,
+            markers,
+            placed_page,
+        } = self.f;
+        let mut d = out.debug_struct("Footage");
+        d.field("path", path)
+            .field("kind", kind)
+            .field("width", width)
+            .field("height", height)
+            .field("pixel_aspect", pixel_aspect)
+            .field("frame_rate", frame_rate)
+            .field("native_rate", native_rate)
+            .field("duration", duration)
+            .field("has_video", has_video)
+            .field("has_audio", has_audio)
+            .field("alpha", alpha)
+            .field("premul_color", premul_color)
+            .field("loop_count", loop_count)
+            .field("codec", codec)
+            .field("missing", missing)
+            .field("sequence", sequence)
+            .field("color_profile", color_profile)
+            .field("layer", layer)
+            .field("fields", fields)
+            .field("invert_alpha", invert_alpha)
+            .field("linear_light", linear_light)
+            .field("preserve_rgb", preserve_rgb)
+            .field("cineon", cineon)
+            .field("data", data)
+            .field("page", page)
+            .field("alphabetical", alphabetical)
+            .field("start_timecode", start_timecode)
+            .field("file_timecode", file_timecode)
+            .field("markers", markers);
+        if self.placed_page {
+            d.field("placed_page", placed_page);
+        }
+        d.finish()
+    }
 }
 
 fn signature_with_source(p: &Project, comp: ItemId, layer: &Layer, src: &str) -> String {
@@ -169,11 +263,10 @@ pub(crate) fn invalidate(before: &Project, after: &mut Project) -> Vec<(ItemId,
             if !fx.groups().any(is_warp) {
                 continue;
             }
-            let sig = signature(after, *cid, l);
             let old_layer = before.comp(*cid).and_then(|bc| bc.layer(l.id));
             for g in fx.groups().filter(|g| is_warp(g)) {
                 let analysed = !str_of(g, ANALYSIS).is_empty();
-                if analysed && str_of(g, ANALYSIS_KEY) != sig {
+                if analysed && !key_matches(after, *cid, l, &str_of(g, ANALYSIS_KEY), |s| s) {
                     clear.push((*cid, l.id, g.uid));
                     todo.push((*cid, l.id, g.uid));
                 } else if !analysed && old_layer.and_then(|ol| ol.props.find_group(g.uid)).is_none() {

crates/engine/src/tests.rs

@@ -27,6 +27,7 @@ mod key_metadata_order;
 mod key_time;
 mod keyframe_text;
 mod keylight;
+mod legacy_analysis_keys;
 mod limits;
 mod lottie;
 mod m137;

crates/engine/src/tests/fixtures/tracked-0.22.ecproj.attribution

@@ -0,0 +1,8 @@
+asset: tracked-0.22.ecproj
+title: One clip with an analysed Warp Stabilizer and 3D Camera Tracker from EffectCraft 0.22.0
+author: arnav-hiwarkar
+source: original work
+license: MIT OR Apache-2.0
+license-file: ../../../../../LICENSE-MIT
+added: 2026-11-12 by arnav-hiwarkar
+notes: Saved by the released 0.22.0 CLI after importing a generated 160x120, 12-frame shaking crop of an ffmpeg testsrc2 pattern, applying both effects and analysing them. The source path is a generic test path; the video is not needed by the tests, which only check that the saved analyses are kept or cleared.

crates/engine/src/tests/legacy_analysis_keys.rs

@@ -0,0 +1,127 @@
+//! Warp Stabilizer and 3D Camera Tracker analyses saved by an earlier release keep matching
+//! their footage after an edit that doesn't change it (#3456), and a real change still clears them.
+
+use effectcraft_effects::camera_tracker::{SOLVE, TRACKS, TRACKS_KEY};
+use effectcraft_effects::warp_stab::{ANALYSIS, ANALYSIS_KEY};
+use effectcraft_keyframe::Value;
+use effectcraft_project::{ItemId, LayerId, Project};
+use serde_json::json;
+
+use crate::Session;
+
+const FOOTAGE: ItemId = ItemId(1);
+const COMP: ItemId = ItemId(2);
+const CLIP: LayerId = LayerId(3);
+const WARP: u64 = 26;
+const TRACKER: u64 = 54;
+
+/// The analysis (as its length and a hash: it's long) and key of the Warp Stabilizer and the
+/// 3D Camera Tracker.
+fn saved(s: &Session) -> [(usize, u64, String); 2] {
+    use std::hash::{Hash, Hasher};
+    let layer = s.project.comp(COMP).unwrap().layer(CLIP).unwrap();
+    let get = |uid: u64, data: &str, key: &str| {
+        let params = crate::camera_track::static_params(layer.props.find_group(uid).unwrap());
+        let text = |id: &str| match params.get(id) {
+            Some(Value::Str(s)) => s.clone(),
+            other => panic!("{id}: {other:?}"),
+        };
+        let analysis = text(data);
+        let mut h = std::collections::hash_map::DefaultHasher::new();
+        analysis.hash(&mut h);
+        (analysis.len(), h.finish(), text(key))
+    };
+    [get(WARP, ANALYSIS, ANALYSIS_KEY), get(TRACKER, TRACKS, TRACKS_KEY)]
+}
+
+fn open(json: &str) -> Session {
+    let mut s = Session::default();
+    s.replace_project(Project::from_json(json).unwrap(), None);
+    s.open_comp(COMP);
+    s
+}
+
+#[test]
+fn analyses_saved_by_0_22_survive_edits_that_keep_their_frames() {
+    // Saved by the released 0.22.0 CLI after analysing both effects; its footage is offline here.
+    let mut s = open(include_str!("fixtures/tracked-0.22.ecproj"));
+    let before = saved(&s);
+    let layer = s.project.comp(COMP).unwrap().layer(CLIP).unwrap();
+    for (len, _, key) in &before {
+        assert!(*len > 0);
+        // The key predates Footage::placed_page, so it isn't today's signature.
+        assert_ne!(key, &crate::warp::signature(&s.project, COMP, layer));
+    }
+
+    // Adding to the render queue edits the project, as `effectcraft-cli render` does.
+    s.execute_checked("renderQueue.add", json!({"comp": COMP.0, "format": "png", "output": "frame.png"})).unwrap();
+    assert_eq!(saved(&s), before);
+    s.execute_checked("edit.undo", json!({})).unwrap();
+    assert_eq!(saved(&s), before);
+
+    // Saved and reopened (by this version), the old keys are kept and still match.
+    let mut s = open(&s.project.to_json());
+    s.execute_checked("renderQueue.add", json!({"comp": COMP.0, "format": "png", "output": "frame.png"})).unwrap();
+    assert_eq!(saved(&s), before);
+
+    // Other frames, from other timing or a reinterpreted footage item: both analyses are
+    // cleared, and undo brings them back.
+    let cleared = |s: &Session| saved(s).iter().all(|(len, _, key)| *len == 0 && key.is_empty());
+    s.execute_checked("layer.timing", json!({"layer": CLIP.0, "out": 0.5})).unwrap();
+    assert!(cleared(&s));
+    s.execute_checked("edit.undo", json!({})).unwrap();
+    assert_eq!(saved(&s), before);
+    s.execute_checked("file.interpretFootage", json!({"items": [FOOTAGE.0], "invertAlpha": true})).unwrap();
+    assert!(cleared(&s));
+    s.execute_checked("edit.undo", json!({})).unwrap();
+    assert_eq!(saved(&s), before);
+    // Footage framed as a placed page has other frames too, though 0.22 had no such field.
+    s.edit("place page", None, |p, _| {
+        if let Some(effectcraft_project::ItemKind::Footage(f)) = p.item_mut(FOOTAGE).map(|i| &mut i.kind) {
+            f.placed_page = true;
+        }
+        Ok(())
+    })
+    .unwrap();
+    assert!(cleared(&s));
+}
+
+/// Solving again reuses the 0.22 tracks rather than tracking the (here offline) footage again.
+#[test]
+fn camera_tracker_solves_again_from_tracks_saved_by_0_22() {
+    let mut s = open(include_str!("fixtures/tracked-0.22.ecproj"));
+    let tracks = saved(&s)[1].clone();
+    let solve = |s: &Session| {
+        let g = s.project.comp(COMP).unwrap().layer(CLIP).unwrap().props.find_group(TRACKER).unwrap();
+        crate::camera_track::static_params(g).get(SOLVE).cloned().unwrap()
+    };
+    // Another solve method, and back, clears the solve and keeps the tracks.
+    for method in [1, 0] {
+        s.execute_checked("prop.set", json!({"layer": CLIP.0, "path": "effects/#2/advanced/solveMethod", "value": method})).unwrap();
+    }
+    assert_eq!(solve(&s), Value::Str(String::new()));
+    assert_eq!(saved(&s)[1], tracks);
+    s.execute_checked("camera.analyze", json!({"layer": CLIP.0, "effect": TRACKER, "wait": true})).unwrap();
+    assert!(matches!(solve(&s), Value::Str(v) if !v.is_empty()));
+    assert_eq!(saved(&s)[1], tracks);
+}
+
+/// The footage text analysis keys are hashed from, as 0.23 saved it. A new `Footage` field must
+/// not change it (or every saved analysis is dropped again).
+#[test]
+fn footage_signature_text_is_pinned() {
+    use effectcraft_project::{Footage, FootageKind};
+    let f = Footage {
+        path: "clip.mp4".into(),
+        kind: FootageKind::Video,
+        width: 160,
+        height: 120,
+        invert_alpha: true,
+        page: 2,
+        placed_page: true,
+        ..Footage::default()
+    };
+    let fields = r#"path: "clip.mp4", kind: Video, width: 160, height: 120, pixel_aspect: 1.0, frame_rate: FrameRate { num: 30, den: 1 }, native_rate: None, duration: Tick(0), has_video: false, has_audio: false, alpha: Straight, premul_color: [0.0, 0.0, 0.0], loop_count: 1, codec: "", missing: false, sequence: [], color_profile: None, layer: None, fields: Off, invert_alpha: true, linear_light: false, preserve_rgb: false, cineon: CineonSettings { log_conversion: false, converted_black: 0.0, converted_white: 1023.0, black: 95.0, white: 685.0, gamma: 1.7, rolloff: 20.0, units: TenBit }, data: None, page: 2, alphabetical: false, start_timecode: None, file_timecode: None, markers: []"#;
+    assert_eq!(crate::warp::footage_text(&f, true), format!("Footage {{ {fields}, placed_page: true }}"));
+    assert_eq!(crate::warp::footage_text(&f, false), format!("Footage {{ {fields} }}"));
+}