From 863a6aff6cc6828088367a7748f6943e9d086978 Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Tue, 13 Oct 2026 02:19:47 +0200 Subject: [PATCH] #1167: frames: keep the viewer frames when only plain markers change Adding a marker replaced the comp, so every cached frame stopped matching and the preview had to re-render before its audio could carry on. Plain markers change no pixels; protected time markers with a duration still do (Responsive Design). Markers count in full again when a project uses expressions, which can read them. Co-Authored-By: Claude Haiku 5.5 --- crates/project/src/lib.rs | 17 +++++++++++++++++ crates/ui-egui/src/frames.rs | 37 +++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index fee3b8ce..c9b49147 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -865,6 +865,23 @@ impl Comp { && layers.iter().zip(&o.layers).all(|(a, b)| a.same_pixels(b)) } + /// Draws the same pixels as `o` once plain markers are ignored: they change no pixels. Protected + /// time markers with a duration set the timing (Responsive Design), so those still count. + pub fn same_pixels_ignoring_markers(&self, o: &Comp) -> bool { + let protected = + |m: &[Marker]| -> Vec<(Tick, Tick)> { m.iter().filter(|m| m.protected && m.duration > Tick::ZERO).map(|m| (m.time, m.duration)).collect() }; + if protected(&self.markers) != protected(&o.markers) { + return false; + } + let mut c = self.clone(); + c.markers = o.markers.clone(); + for (a, b) in c.layers.iter_mut().zip(&o.layers) { + a.markers = b.markers.clone(); + a.markers_locked = b.markers_locked; + } + c.same_pixels(o) + } + /// The comp with those switches and the Resolution at their defaults (borrowed when they /// already are), for keys that hash the whole comp. pub fn pixel_form(&self) -> std::borrow::Cow<'_, Comp> { diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index ae0c5497..059bc3a0 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -291,6 +291,23 @@ pub fn comp_content(project: &Project, comp: ItemId) -> u64 { comp_content_with(project, comp, |_, c| Arc::as_ptr(c)) } +/// Whether two versions of a comp draw the same pixels. Markers only count when an expression +/// could read them (`markers_count`). +fn same_pixels(a: &Comp, b: &Comp, markers_count: bool) -> bool { + if markers_count { a.same_pixels(b) } else { a.same_pixels_ignoring_markers(b) } +} + +/// Whether a layer anywhere in the project has an expression, which can read markers. +fn project_has_expressions(project: &Project) -> bool { + fn has_expression(g: &PropGroup) -> bool { + g.children.iter().any(|c| match c { + Node::Prop(p) => p.expr.is_some(), + Node::Group(sub) => has_expression(sub), + }) + } + project.items.values().any(|i| matches!(&i.kind, ItemKind::Comp(c) if c.layers.iter().any(|l| has_expression(&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) -> *const Comp) -> u64 { use std::hash::{Hash, Hasher}; @@ -703,10 +720,12 @@ impl Frames { fn identity(&self, project: &Arc, comp: ItemId) -> Identity { let plain = comp_content(project, comp); let Ok(mut bases) = self.bases.lock() else { return (plain, None) }; + // Markers can only change pixels through expressions, which read them. + let markers_count = project_has_expressions(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) || same_pixels(b, c, markers_count) => Arc::as_ptr(b), _ => Arc::as_ptr(c), }) == *base_id { @@ -1985,4 +2004,20 @@ mod tests { s.execute("layer.addMarker", json!({"comp": outer.0, "layers": [solid], "time": 0.5, "comment": "beat"})).unwrap(); assert_eq!(at(&s), o0, "a marker changes no pixels, so it keeps the frames"); } + + #[test] + fn markers_count_when_an_expression_can_read_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 outer = ItemId(id(s.execute("comp.new", json!({"name": "Outer", "width": 8, "height": 8, "duration": 1})).unwrap(), "comp")); + let solid = id(s.execute("layer.newSolid", json!({"comp": outer.0, "color": "#ff0000"})).unwrap(), "layer"); + s.execute("prop.setExpression", json!({"comp": outer.0, "layer": solid, "path": "transform/opacity", "expression": "1"})).unwrap(); + let f = Frames::default(); + let at = |s: &Session| f.content_of(&s.project, s.revision, outer); + let o0 = at(&s); + s.execute("layer.addMarker", json!({"comp": outer.0, "layers": [solid], "time": 0.5, "comment": "beat"})).unwrap(); + assert_ne!(at(&s), o0, "an expression can read the marker, so the frames change"); + } }