From f263a32ab7eb4168c5bb7035c2a5ad83c06f0db2 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Tue, 13 Oct 2026 21:59:03 -0400 Subject: [PATCH] Render: an effect parameter's history is read at its exact layer time (#1373) FxHost::param_at mapped the requested layer time to comp time, and EvalCtx::value mapped it back. Under a fractional stretch (33.3 %, 133.33 %) that round trip truncates a tick early, so a Hold key at 0.5 s read its previous value at the step starting there: a Birth Rate cut to 0 at 0.5 s bore one more step of particles (six). EvalCtx::value_at_layer_time reads the keyframes at the given layer time exactly, and still runs an expression at the comp time. EvalCtx::value now uses it, and param_at reads through it. EffectHost::param_is_static says when a parameter has no keyframes and no expression. CC Particle World then skips sampling its Birth Rate at every step, since it's the same throughout. --- crates/effects/src/lib.rs | 5 ++++ crates/effects/src/sim.rs | 3 ++- crates/render/src/eval.rs | 8 ++++++- crates/render/src/lib.rs | 48 ++++++++++++++++++++++++--------------- 4 files changed, 44 insertions(+), 20 deletions(-) diff --git a/crates/effects/src/lib.rs b/crates/effects/src/lib.rs index 388b287c5..e7d43f1ec 100644 --- a/crates/effects/src/lib.rs +++ b/crates/effects/src/lib.rs @@ -390,6 +390,11 @@ pub trait EffectHost: Sync { fn param_at(&self, _effect_index: usize, _id: &str, _layer_time: f64) -> Option { None } + /// Parameter `id` of effect `effect_index` has one value at every time (no keyframes, no + /// expression), so sampling it over time can be skipped. `false` when unknown. + fn param_is_static(&self, _effect_index: usize, _id: &str) -> bool { + false + } /// A particle simulation backend (GPU particles; see [`psim`]). `None` = simulate on the /// CPU. fn particles(&self) -> Option<&dyn psim::ParticleSim> { diff --git a/crates/effects/src/sim.rs b/crates/effects/src/sim.rs index 15544eb81..ea664ba41 100644 --- a/crates/effects/src/sim.rs +++ b/crates/effects/src/sim.rs @@ -1006,7 +1006,8 @@ fn accelerated(ctx: &EffectCtx, key: u64, phys: &Phys, rates: Option<&[f64]>) -> /// other times, the current value for every step. fn rate_history(ctx: &EffectCtx, id: &str, k: f64) -> Vec { let now = ctx.params.f(id); - let host = ctx.env.host; + // A rate without keyframes or an expression is the same at every step. + let host = ctx.env.host.filter(|h| !h.param_is_static(ctx.env.effect_index, id)); (0..steps_at(ctx.time)) .map(|i| { let v = host.and_then(|h| h.param_at(ctx.env.effect_index, id, i as f64 / SPS)).map_or(now, |v| v.as_f64()); diff --git a/crates/render/src/eval.rs b/crates/render/src/eval.rs index 8a193b87a..14249900f 100644 --- a/crates/render/src/eval.rs +++ b/crates/render/src/eval.rs @@ -60,7 +60,13 @@ impl<'a> EvalCtx<'a> { let y = tr.get("positionY").map(|p| self.value(layer, p).as_f64()).unwrap_or(0.0); return Value::Vec3([self.value(layer, px).as_f64(), y, z]); } - let lt = layer.layer_time(self.time); + self.value_at_layer_time(layer, prop, layer.layer_time(self.time)) + } + /// [`Self::value`] with the keyframes read at layer time `lt` exactly, rather than at the + /// context time mapped through the layer's start and stretch (which rounds under a + /// fractional stretch: a Hold key at `lt` could read its previous value). An expression + /// still runs at the context time. + pub fn value_at_layer_time(&self, layer: &Layer, prop: &Property, lt: Tick) -> Value { let v = prop.value_at(lt); if prop.has_expression() && let Some(h) = self.expr diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index a5597de93..787fe1245 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -344,24 +344,16 @@ impl EffectHost for FxHost<'_, '_, '_> { } fn param_at(&self, effect_index: usize, id: &str, layer_time: f64) -> Option { - // Only the one property: a simulation samples it at every step. - let mut g = self.layer.effects()?.groups().nth(effect_index)?; - let mut path = id.split('/').peekable(); - while let Some(m) = path.next() { - if path.peek().is_none() { - let p = g.children.iter().find_map(|c| match c { - effectcraft_project::Node::Prop(p) if p.match_id == m => Some(p), - _ => None, - })?; - let ctx = self.ctx.at(self.layer.comp_time(Tick::from_seconds_f64(layer_time))); - return Some(ctx.value(self.layer, p)); - } - g = g.children.iter().find_map(|c| match c { - effectcraft_project::Node::Group(sg) if sg.match_id == m => Some(sg), - _ => None, - })?; - } - None + // Only the one property: a simulation samples it at every step. Its keyframes are read + // at the requested layer time exactly (mapping it to comp time and back rounds under a + // fractional stretch, so a Hold key there could read its previous value). + let p = effect_param(self.layer, effect_index, id)?; + let lt = Tick::from_seconds_f64(layer_time); + Some(self.ctx.at(self.layer.comp_time(lt)).value_at_layer_time(self.layer, p, lt)) + } + + fn param_is_static(&self, effect_index: usize, id: &str) -> bool { + effect_param(self.layer, effect_index, id).is_some_and(|p| p.keys.is_empty() && !p.has_expression()) } fn comp_scene(&self) -> Option { @@ -386,6 +378,26 @@ impl EffectHost for FxHost<'_, '_, '_> { } } +/// Parameter `id` (a flattened id: match ids joined by `/`) of effect `effect_index` in +/// `layer`'s stack. +fn effect_param<'a>(layer: &'a Layer, effect_index: usize, id: &str) -> Option<&'a effectcraft_project::Property> { + let mut g = layer.effects()?.groups().nth(effect_index)?; + let mut path = id.split('/').peekable(); + while let Some(m) = path.next() { + if path.peek().is_none() { + return g.children.iter().find_map(|c| match c { + effectcraft_project::Node::Prop(p) if p.match_id == m => Some(p), + _ => None, + }); + } + g = g.children.iter().find_map(|c| match c { + effectcraft_project::Node::Group(sg) if sg.match_id == m => Some(sg), + _ => None, + })?; + } + None +} + /// The Essential Properties overrides of a precomp layer, evaluated at the context time. pub fn essential_overrides(ctx: &EvalCtx, layer: &Layer) -> Vec { use effectcraft_project::essential;