Markers: adding one during playback keeps the cached frames and the sound (#1083, #1085)

Markers counted as pixels in a comp's frame identity, so every marker edit
dropped the cached frames; the playing preview then hit an uncached frame
and stopped its sound. They now count only when an expression mentions markers.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/project/src/lib.rs

@@ -759,6 +759,12 @@ impl Switches {
 impl Layer {
     /// Draws the same pixels as `o`: equal but for the Audio, Lock and Shy switches.
     pub fn same_pixels(&self, o: &Layer) -> bool {
+        self.same_pixels_with(o, true)
+    }
+
+    /// [`Self::same_pixels`], with the markers counting only when `with_markers` (they are read by
+    /// expressions alone, so a project without any can edit them freely).
+    pub fn same_pixels_with(&self, o: &Layer, with_markers: bool) -> bool {
         // Every field (no `..`: a new field must be considered here); the property tree last.
         let Layer {
             id,
@@ -796,7 +802,7 @@ impl Layer {
             && *preserve_transparency == o.preserve_transparency
             && *track_matte == o.track_matte
             && *parent == o.parent
-            && *markers == o.markers
+            && (!with_markers || *markers == o.markers)
             && *markers_locked == o.markers_locked
             && *auto_orient == o.auto_orient
             && *environment == o.environment
@@ -809,6 +815,13 @@ impl Comp {
     /// Draws the same pixels as `o`: equal but for its layers' Audio, Lock and Shy switches, Hide
     /// Shy Layers and the viewer's Resolution.
     pub fn same_pixels(&self, o: &Comp) -> bool {
+        self.same_pixels_with(o, true)
+    }
+
+    /// [`Self::same_pixels`], with the comp's and its layers' markers counting only when
+    /// `with_markers`: only expressions read them, so while none does, adding or moving a marker
+    /// (even during playback) leaves every rendered frame valid.
+    pub fn same_pixels_with(&self, o: &Comp, with_markers: bool) -> bool {
         // Every field (no `..`: a new field must be considered here); the layers last.
         let Comp {
             width,
@@ -846,7 +859,7 @@ impl Comp {
             && *display_start == o.display_start
             && *background == o.background
             && *work_area == o.work_area
-            && *markers == o.markers
+            && (!with_markers || *markers == o.markers)
             && *shutter_angle == o.shutter_angle
             && *shutter_phase == o.shutter_phase
             && *motion_blur_samples == o.motion_blur_samples
@@ -862,7 +875,7 @@ impl Comp {
             && *guides == o.guides
             && *essential == o.essential
             && layers.len() == o.layers.len()
-            && layers.iter().zip(&o.layers).all(|(a, b)| a.same_pixels(b))
+            && layers.iter().zip(&o.layers).all(|(a, b)| a.same_pixels_with(b, with_markers))
     }
 
     /// The comp with those switches and the Resolution at their defaults (borrowed when they

crates/ui-egui/src/frames.rs

@@ -291,6 +291,18 @@ pub fn comp_content(project: &Project, comp: ItemId) -> u64 {
     comp_content_with(project, comp, |_, c| Arc::as_ptr(c))
 }
 
+/// Some expression in the project mentions markers (`marker.key(1)`, `numKeys`, `nearestKey`…).
+/// Only then do the markers of its comps and layers affect what is rendered.
+fn reads_markers(project: &Project) -> bool {
+    fn group(g: &PropGroup) -> bool {
+        g.children.iter().any(|c| match c {
+            Node::Prop(p) => p.expr.as_ref().is_some_and(|e| e.text.to_ascii_lowercase().contains("marker")),
+            Node::Group(sub) => group(sub),
+        })
+    }
+    project.comps().any(|(_, c)| c.layers.iter().any(|l| group(&l.props)))
+}
+
 /// [`comp_content`] with each comp's address given by `addr`.
 fn comp_content_with(project: &Project, comp: ItemId, addr: impl Fn(ItemId, &Arc<Comp>) -> *const Comp) -> u64 {
     use std::hash::{Hash, Hasher};
@@ -703,10 +715,12 @@ impl Frames {
     fn identity(&self, project: &Arc<Project>, comp: ItemId) -> Identity {
         let plain = comp_content(project, comp);
         let Ok(mut bases) = self.bases.lock() else { return (plain, None) };
+        // Markers are pixels only to the expressions that read them (#1083).
+        let markers = reads_markers(project);
         if let Some((base_id, base)) = bases.get(&comp.0)
             && *base_id != plain
             && comp_content_with(project, comp, |id, c| match base.item(id).map(|i| &i.kind) {
-                Some(ItemKind::Comp(b)) if Arc::ptr_eq(b, c) || b.same_pixels(c) => Arc::as_ptr(b),
+                Some(ItemKind::Comp(b)) if Arc::ptr_eq(b, c) || b.same_pixels_with(c, markers) => Arc::as_ptr(b),
                 _ => Arc::as_ptr(c),
             }) == *base_id
         {
@@ -1970,4 +1984,36 @@ mod tests {
         switch(&mut s, inner, solid, "video", true);
         assert_ne!(at(&s), o1);
     }
+
+    /// Adding, moving or deleting a marker (Num* while a preview plays) keeps the cached frames:
+    /// the sound would otherwise stop at the next frame that has to be rendered again (#1083).
+    /// Only expressions that read markers make them part of the pixels.
+    #[test]
+    fn markers_keep_the_frames_unless_an_expression_reads_them() {
+        use effectcraft_engine::Session;
+        use serde_json::json;
+        let mut s = Session::default();
+        let id = |v: serde_json::Value, k: &str| v[k].as_u64().unwrap();
+        let inner = ItemId(id(s.execute("comp.new", json!({"name": "Inner", "width": 8, "height": 8, "duration": 2})).unwrap(), "comp"));
+        let solid = id(s.execute("layer.newSolid", json!({"comp": inner.0, "color": "#ff0000"})).unwrap(), "layer");
+        let outer = ItemId(id(s.execute("comp.new", json!({"name": "Outer", "width": 8, "height": 8, "duration": 2})).unwrap(), "comp"));
+        id(s.execute("layer.addItem", json!({"comp": outer.0, "item": inner.0})).unwrap(), "layer");
+        let f = Frames::default();
+        let at = |s: &Session, c: ItemId| f.content_of(&s.project, s.revision, c);
+        let (o0, i0) = (at(&s, outer), at(&s, inner));
+        s.execute("comp.addMarker", json!({"comp": inner.0, "time": 0.5, "comment": "beat"})).unwrap();
+        s.execute("comp.addMarker", json!({"comp": outer.0, "time": 1.0})).unwrap();
+        s.execute("comp.addMarker", json!({"comp": outer.0, "time": 1.5})).unwrap();
+        assert_eq!((at(&s, outer), at(&s, inner)), (o0, i0), "comp markers");
+        s.execute("markers.set", json!({"comp": inner.0, "layer": solid, "new": true, "time": 0.25})).unwrap();
+        assert_eq!((at(&s, outer), at(&s, inner)), (o0, i0), "layer marker");
+        // An expression reading markers makes them count again.
+        s.execute("prop.setExpression", json!({"comp": inner.0, "layer": solid, "path": "transform/opacity", "expression": "thisComp.marker.numKeys"}))
+            .unwrap();
+        let (o1, i1) = (at(&s, outer), at(&s, inner));
+        assert_ne!(o1, o0);
+        s.execute("comp.addMarker", json!({"comp": inner.0, "time": 1.0})).unwrap();
+        assert_ne!(at(&s, inner), i1, "marker read by an expression");
+        assert_ne!(at(&s, outer), o1, "marker read by an expression in a nested comp");
+    }
 }

crates/ui-egui/tests/ui_audio_scrub.rs

@@ -192,3 +192,31 @@ fn preview_with_audio_sounds_once_rendering_keeps_up() {
     }
     assert!(h.state().audio.is_some() && fake.0.lock().unwrap().0.is_some(), "the sound started within 2 s, at frame {last} of 900");
 }
+
+/// Adding markers while a preview sounds (Num*, one per beat) keeps the sound going (#1083): a
+/// marker changes no pixels, so no cached frame is dropped and no frame has to be rendered again
+/// (which would stop the sound until the frames ahead are cached).
+#[test]
+fn adding_markers_does_not_stop_the_sound() {
+    let fake = Fake::default();
+    let (app, cid) = setup(true, &fake);
+    // Everything is cached before the preview starts, so the sound starts at once.
+    let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_step_dt(1.0 / 31.0).build_eframe(|_| app);
+    h.run_steps(2);
+    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(60);
+    let now = h.ctx.input(|i| i.time);
+    h.state_mut().play(now);
+    while h.state().audio.is_none() && std::time::Instant::now() < deadline {
+        h.step();
+        std::thread::sleep(std::time::Duration::from_millis(1));
+    }
+    assert!(h.state().audio.is_some(), "the sound started");
+    for beat in 0..6 {
+        let t = 0.1 + 0.05 * beat as f64;
+        h.state_mut().session.execute("comp.addMarker", json!({"comp": cid.0, "time": t, "comment": "beat"})).unwrap();
+        h.step();
+        let app = h.state();
+        assert!(app.playback.playing && app.audio.is_some() && !app.playback.audio_held, "marker {beat}: still playing with sound");
+    }
+    assert_eq!(h.state().session.project.comp(cid).unwrap().markers.len(), 6);
+}