diff --git a/crates/engine/src/analysis/frames_key.rs b/crates/engine/src/analysis/frames_key.rs index 44eb93d57..248b3b4f5 100644 --- a/crates/engine/src/analysis/frames_key.rs +++ b/crates/engine/src/analysis/frames_key.rs @@ -9,7 +9,7 @@ //! doesn't change keys. Of the layer and of a precomp, only the fields named in `layer_frames` are //! read (a precomp's contents are #3535). The //! text leaves out observations (whether footage is available) and data that never changes pixels -//! (names, labels, UI hints, markers), so they never drop saved analyses (#3534, #3536). +//! (colour labels, UI hints, markers), so they never drop saved analyses (#3534, #3536). //! //! Keys saved by 0.22–0.24 hashed derived `Debug` text instead. [`legacy`] still recognises //! them, and a matching saved key is kept rather than rewritten: frozen Roto Brush mattes and @@ -348,13 +348,15 @@ impl Text { } fn group(&mut self, g: &PropGroup, depth: usize) { - // The uid, name and colour label identify and label the group; they never change pixels. - let PropGroup { uid: _, match_id, name: _, kind, enabled, label: _, children } = g; + // Expressions find effects and groups by name (`effect("Blur")`, `propertyGroup(1).name`). + // The uid and colour label never change pixels. + let PropGroup { uid: _, match_id, name, kind, enabled, label: _, children } = g; if depth >= MAX_DEPTH { self.word("tooDeep"); return; } self.str("group", match_id); + self.str("name", name); self.group_kind(kind); self.field("enabled", enabled); for c in children { @@ -415,24 +417,16 @@ impl Text { } fn property(&mut self, pr: &Property) { - // The uid, name and UI hints identify and show the property; the 2D/3D-only and - // hold-only flags say where and how it can be edited, not what it evaluates to. - let Property { - uid: _, - match_id, - name: _, - value, - keys, - expr, - ui: _, - spatial, - hold_only: _, - three_d_only: _, - two_d_only: _, - shown_dims: _, - static_only: _, - } = pr; + // Expressions read the name (`thisProperty.name`) and seed `random`/`wiggle` with the + // uid: the uid is written for a property with an expression. UI hints and the 2D/3D-only + // and hold-only flags say where and how it can be edited, not what it evaluates to. + let Property { uid, match_id, name, value, keys, expr, ui: _, spatial, hold_only: _, three_d_only: _, two_d_only: _, shown_dims: _, static_only: _ } = + pr; self.str("prop", match_id); + self.str("name", name); + if expr.is_some() { + self.field("uid", uid); + } self.value(value); self.field("spatial", spatial); for k in keys { diff --git a/crates/engine/src/tests/analysis_signature.rs b/crates/engine/src/tests/analysis_signature.rs index 96323e6be..b73c79edd 100644 --- a/crates/engine/src/tests/analysis_signature.rs +++ b/crates/engine/src/tests/analysis_signature.rs @@ -100,7 +100,7 @@ fn analyses_saved_by_0_24_survive_edits_that_keep_their_frames() { fn signature_text_is_pinned() { let s = open(ANALYSED_0_24); let layer = s.project.comp(COMP).unwrap().layer(CLIP).unwrap(); - let key = r#"frames/1;footage=1;footage;path="/tmp/effectcraft-3456-shake.mp4";kind=video;w=160;h=120;par=1.0;rate=12/1;nativeRate=none;duration=254016000000;video=true;audio=false;alpha=ignore;matte=[0.0, 0.0, 0.0];loop=1;codec="H.264 (High)";frames=0;profile{rec709;};layer=none;fields=off;invertAlpha=false;linearLight=false;preserveRgb=false;cineon;log=false;points=[0.0, 1023.0, 95.0, 685.0, 1.7, 20.0];units=10bit;data=none;page=0;alphabetical=false;startTimecode=none;fileTimecode=none;placedPage=false;in=0;out=254016000000;start=0;stretch=100.0;timeRemap{prop="timeRemap";scalar=0.0;spatial=false;key=0;scalar=0.2;interp=linear/linear;continuous=false;autoBezier=false;spatialIn=[0.0, 0.0, 0.0];spatialOut=[0.0, 0.0, 0.0];spatialAuto=true;spatialContinuous=false;roving=false;muted=false;key=254016000000;scalar=1.0;interp=linear/linear;continuous=false;autoBezier=false;spatialIn=[0.0, 0.0, 0.0];spatialOut=[0.0, 0.0, 0.0];spatialAuto=true;spatialContinuous=false;roving=false;muted=false;expr=none;};compRate{rate=12/1;};"#; + let key = r#"frames/1;footage=1;footage;path="/tmp/effectcraft-3456-shake.mp4";kind=video;w=160;h=120;par=1.0;rate=12/1;nativeRate=none;duration=254016000000;video=true;audio=false;alpha=ignore;matte=[0.0, 0.0, 0.0];loop=1;codec="H.264 (High)";frames=0;profile{rec709;};layer=none;fields=off;invertAlpha=false;linearLight=false;preserveRgb=false;cineon;log=false;points=[0.0, 1023.0, 95.0, 685.0, 1.7, 20.0];units=10bit;data=none;page=0;alphabetical=false;startTimecode=none;fileTimecode=none;placedPage=false;in=0;out=254016000000;start=0;stretch=100.0;timeRemap{prop="timeRemap";name="Time Remap";scalar=0.0;spatial=false;key=0;scalar=0.2;interp=linear/linear;continuous=false;autoBezier=false;spatialIn=[0.0, 0.0, 0.0];spatialOut=[0.0, 0.0, 0.0];spatialAuto=true;spatialContinuous=false;roving=false;muted=false;key=254016000000;scalar=1.0;interp=linear/linear;continuous=false;autoBezier=false;spatialIn=[0.0, 0.0, 0.0];spatialOut=[0.0, 0.0, 0.0];spatialAuto=true;spatialContinuous=false;roving=false;muted=false;expr=none;};compRate{rate=12/1;};"#; assert_eq!(crate::analysis::frames_key::signature_text(&s.project, COMP, layer), key); } @@ -151,11 +151,10 @@ fn todays_keys_survive_edits_that_keep_their_frames() { add_to_render_queue(&mut s); assert_eq!(saved(&s), before); } - // Names and labels only show things. - s.execute_checked("layer.rename", json!({"layer": CLIP.0, "name": "Renamed"})).unwrap(); - s.edit("rename", None, |p, _| { + // Colour labels and footage markers only show things. + s.edit("label", None, |p, _| { let l = p.comp_mut(COMP).unwrap().layer_mut(CLIP).unwrap(); - l.props.find_group_mut(82).unwrap().name = "Soft".into(); + l.props.find_group_mut(82).unwrap().label = Some(effectcraft_color::Label::Yellow); if let Some(effectcraft_project::ItemKind::Footage(f)) = p.item_mut(ItemId(1)).map(|i| &mut i.kind) { f.markers.push(effectcraft_project::Marker::default()); } @@ -163,6 +162,16 @@ fn todays_keys_survive_edits_that_keep_their_frames() { }) .unwrap(); assert_eq!(saved(&s), before); + // Expressions find effects by name: renaming the effect above the Roto Brush is a change for it. + s.edit("rename", None, |p, _| { + p.comp_mut(COMP).unwrap().layer_mut(CLIP).unwrap().props.find_group_mut(82).unwrap().name = "Soft".into(); + Ok(()) + }) + .unwrap(); + assert_eq!(saved(&s)[..2], before[..2]); + assert_eq!(saved(&s)[2].0, ""); + s.execute_checked("edit.undo", json!({})).unwrap(); + assert_eq!(saved(&s), before); // A real change still clears them. s.execute_checked("file.interpretFootage", json!({"items": [1], "invertAlpha": true})).unwrap(); @@ -234,3 +243,22 @@ fn keys_saved_by_0_24_while_offline_still_match() { assert_eq!(roto(&s), ("saved frozen".into(), ROTO_INPUT_0_24_OFFLINE.into())); } } + +/// Expressions seed `random` and `wiggle` with the property's uid: a Time Remap with an expression +/// keys its uid (a duplicated layer gets new uids), one without doesn't. +#[test] +fn an_expression_keys_its_property_uid() { + let s = open(ANALYSED_0_24); + let layer = s.project.comp(COMP).unwrap().layer(CLIP).unwrap().clone(); + let text = |l: &effectcraft_project::Layer| crate::analysis::frames_key::signature_text(&s.project, COMP, l); + let renumbered = |l: &effectcraft_project::Layer| { + let mut l = l.clone(); + l.props.get_mut("timeRemap").unwrap().uid += 1000; + l + }; + assert_eq!(text(&renumbered(&layer)), text(&layer)); + let mut with_expr = layer.clone(); + with_expr.props.get_mut("timeRemap").unwrap().expr = + Some(effectcraft_project::Expression { text: "seedRandom(0, true); random(0, 1)".into(), enabled: true }); + assert_ne!(text(&renumbered(&with_expr)), text(&with_expr)); +}