diff --git a/crates/engine/src/analysis/frames_key.rs b/crates/engine/src/analysis/frames_key.rs index 9e80a9dc7..b6c542666 100644 --- a/crates/engine/src/analysis/frames_key.rs +++ b/crates/engine/src/analysis/frames_key.rs @@ -9,7 +9,7 @@ //! the layer's frames, left out (with a reason) if it doesn't. Renaming a field or variant doesn't //! change keys. Of the analysed layer itself, only the fields named in `layer_frames` and //! [`analysis_key`] are read. 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 +//! data that never changes pixels (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 @@ -494,11 +494,11 @@ impl Text { fn nested_layer(&mut self, p: &Project, l: &Layer, depth: usize, seen: &mut HashSet) { let Layer { id, - // Names, labels and comments only show the layer. (An expression can find a layer - // by its name, as it can an effect, which the key leaves out too.) - name: _, + name, + // Whether the name follows the Source Text changes nothing until the name changes. name_follows_text: _, source, + // Labels and comments only show the layer. label: _, comment: _, start_time, @@ -519,6 +519,8 @@ impl Text { props, } = l; self.field("layer", id.0); + // Expressions find layers by name (`thisComp.layer("Clip")`), as they find effects. + self.str("name", name); self.layer_source(source); if let Some(item) = source.item() { match p.item(item).map(|i| &i.kind) { @@ -571,13 +573,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 { @@ -638,24 +642,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_inputs.rs b/crates/engine/src/tests/analysis_inputs.rs index ee75bead2..5eb75c115 100644 --- a/crates/engine/src/tests/analysis_inputs.rs +++ b/crates/engine/src/tests/analysis_inputs.rs @@ -145,9 +145,16 @@ fn precomp_contents(saved: Saved) { check(&mut s, "prop.set", json!({"comp": inner.0, "layer": clip, "path": "transform/opacity", "value": 50}), [false, false]); check(&mut s, "effect.apply", json!({"comp": inner.0, "layers": [clip], "effect": "ec.blur.gaussian"}), [false, false]); check(&mut s, "layer.timing", json!({"comp": inner.0, "layers": [clip], "out": 0.25}), [false, false]); - // A name, label or comment inside it doesn't. - run(&mut s, "layer.rename", json!({"comp": inner.0, "layer": clip, "name": "Inner"})); + // A label or comment inside it doesn't. A name does: expressions find layers by name. + s.edit("label", None, |p, _| { + let l = p.comp_mut(inner).unwrap().layer_mut(LayerId(clip)).unwrap(); + l.label = effectcraft_color::Label::Pink; + l.comment = "a note".into(); + Ok(()) + }) + .unwrap(); assert_eq!(analysed(&s), [true, true]); + check(&mut s, "layer.rename", json!({"comp": inner.0, "layer": clip, "name": "Inner"}), [false, false]); // A precomp inside the precomp: its contents too. let r = run(&mut s, "layer.precompose", json!({"comp": inner.0, "layers": [clip], "mode": "leave"})); diff --git a/crates/engine/src/tests/analysis_signature.rs b/crates/engine/src/tests/analysis_signature.rs index 96323e6be..045e279de 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,73 @@ 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)); +} + +/// Non-default values of the types the key writes are written as their fixed words (#3536). +#[test] +fn nested_values_are_written_as_fixed_words() { + use effectcraft_project::{ + AlphaMode, CineonUnits, FieldOrder, FootageKind, GroupKind, ItemKind, LayerSource, MaskMode, MaskMotionBlur, Solid, SourceLayer, + }; + let mut s = open(ANALYSED_0_24); + let p = std::sync::Arc::make_mut(&mut s.project); + if let Some(ItemKind::Footage(f)) = p.item_mut(ItemId(1)).map(|i| &mut i.kind) { + f.kind = FootageKind::Sequence; + f.sequence = vec!["a_0001.png".into(), "a_0002.png".into()]; + f.alpha = AlphaMode::Premultiplied; + f.premul_color = [0.5, 0.25, 1.0]; + f.fields = FieldOrder::UpperFirst; + f.cineon.units = CineonUnits::Float; + f.cineon.log_conversion = true; + f.color_profile = Some(effectcraft_color::ColorSpace::DisplayP3); + f.layer = Some(SourceLayer { index: 3, name: "Sky".into(), layer_size: true, embedded: Some("id-7".into()), placed: true }); + f.native_rate = Some(effectcraft_time::FrameRate { num: 30000, den: 1001 }); + f.start_timecode = Some(42); + f.data = Some("{\"k\":1}".into()); + } + let layer = p.comp(COMP).unwrap().layer(CLIP).unwrap().clone(); + let text = crate::analysis::frames_key::signature_text(p, COMP, &layer); + let footage = text.split(";in=").next().unwrap(); + assert_eq!( + footage, + r#"frames/1;footage=1;footage;path="/tmp/effectcraft-3456-shake.mp4";kind=sequence;w=160;h=120;par=1.0;rate=12/1;nativeRate{rate=30000/1001;};duration=254016000000;video=true;audio=false;alpha=premultiplied;matte=[0.5, 0.25, 1.0];loop=1;codec="H.264 (High)";frames=2;file="a_0001.png";file="a_0002.png";profile{p3;};layer{index=3;layerSize=true;embedded{id="id-7";};placed=true;};fields=upper;invertAlpha=false;linearLight=false;preserveRgb=false;cineon;log=true;points=[0.0, 1023.0, 95.0, 685.0, 1.7, 20.0];units=float;data{text="{\"k\":1}";};page=0;alphabetical=false;startTimecode{frame=42;};fileTimecode=none;placedPage=false"# + ); + + let solid = p.add_item("Solid", Default::default(), None, ItemKind::Solid(Solid { color: [1.0, 0.5, 0.0], width: 64, height: 48, pixel_aspect: 2.0 })); + let mut on_solid = layer.clone(); + on_solid.source = LayerSource::Solid { item: solid }; + let text = crate::analysis::frames_key::signature_text(p, COMP, &on_solid); + assert_eq!(text.split(";in=").next().unwrap(), format!("frames/1;solid={};solid;color=[1.0, 0.5, 0.0];w=64;h=48;par=2.0", solid.0)); + + // Every mask mode and motion blur setting gives its own Input Key. + let mask = layer.masks().unwrap().groups().next().unwrap().uid; + let mut keys = std::collections::BTreeSet::new(); + for mode in MaskMode::ALL { + for blur in [MaskMotionBlur::SameAsLayer, MaskMotionBlur::On, MaskMotionBlur::Off] { + let mut l = layer.clone(); + if let GroupKind::Mask { mode: m, motion_blur, .. } = &mut l.props.find_group_mut(mask).unwrap().kind { + (*m, *motion_blur) = (mode, blur); + } + keys.insert(crate::analysis::frames_key::input_key(p, COMP, &l, ROTO)); + } + } + assert_eq!(keys.len(), MaskMode::ALL.len() * 3); +}