diff --git a/crates/engine/src/cmd/expr_tools.rs b/crates/engine/src/cmd/expr_tools.rs index cb39077af..bd62a8e46 100644 --- a/crates/engine/src/cmd/expr_tools.rs +++ b/crates/engine/src/cmd/expr_tools.rs @@ -2,7 +2,7 @@ //! Language menu (the ⓕ button next to an expression), written from the public Expression //! Language Reference. -use effectcraft_project::{Expression, ItemId, Layer, Property}; +use effectcraft_project::{Comp, Expression, ItemId, Layer, Property}; use effectcraft_render::EvalCtx; use effectcraft_time::Tick; use serde::Serialize; @@ -42,8 +42,16 @@ impl ExprError { /// expressions and the syntax errors that disabled others. pub fn errors(s: &Session, cid: ItemId, t: Tick) -> Vec { let Some(comp) = s.project.comp(cid) else { return vec![] }; - let ctx = EvalCtx { footage: Some(s.footage.as_ref()), expr: s.expr.as_deref(), ..EvalCtx::new(&s.project, cid, comp, t) }; - errors_in(&ctx, s.expr_check, || true).unwrap_or_default() + errors_in(&eval_ctx(s, cid, comp, t), s.expr_check, || true).unwrap_or_default() +} + +/// The context to evaluate expressions of comp `cid` at comp time `t` as the render does for the +/// comp at top level: with the session's expression host and its footage source, so +/// `sampleImage()` reads footage pixels instead of failing or sampling transparent (#1000, +/// #3231). `expr.errors`, `prop.get`, [`Session::expression_errors`] and the script value query +/// all evaluate through this one context. +pub fn eval_ctx<'a>(s: &'a Session, cid: ItemId, comp: &'a Comp, t: Tick) -> EvalCtx<'a> { + EvalCtx { footage: Some(s.footage.as_ref()), expr: s.expr.as_deref(), ..EvalCtx::new(&s.project, cid, comp, t) } } /// Evaluate diagnostics against an immutable snapshot. Background callers can cancel between diff --git a/crates/engine/src/cmd/layer_menu.rs b/crates/engine/src/cmd/layer_menu.rs index 11a5a3665..8047baa39 100644 --- a/crates/engine/src/cmd/layer_menu.rs +++ b/crates/engine/src/cmd/layer_menu.rs @@ -867,8 +867,8 @@ impl Session { }); for pr in has { let text = pr.expr.as_ref().map(|e| e.text.clone()).unwrap_or_default(); - let mut ctx = effectcraft_render::EvalCtx::new(&self.project, cid, comp, t); - ctx.expr = Some(host.as_ref()); + // With the footage source, so `sampleImage()` doesn't fail here only (#3231). + let ctx = super::expr_tools::eval_ctx(self, cid, comp, t); if let Err(e) = host.eval(&ctx, l, pr, &pr.value_at(l.layer_time(t))) { errors.push(json!({"layer": l.id.0, "prop": pr.uid, "name": pr.name, "expression": text, "error": e})); } diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index b4432dd2f..4b91649cf 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -558,8 +558,9 @@ fn get(s: &mut Session, p: &Value) -> Result { } else if pr.has_expression() && let (Some(h), Some(comp)) = (s.expr.as_deref(), s.project.comp(cid)) { - let mut ctx = crate::render::EvalCtx::new(&s.project, cid, comp, t); - ctx.expr = Some(h); + // The render's context, footage source included, so `sampleImage()` evaluates here as it + // renders and as `expr.errors` reports it (#3231). + let ctx = super::expr_tools::eval_ctx(s, cid, comp, t); match h.eval(&ctx, l, pr, &raw) { Ok(v) => out["evaluated"] = v.to_json(), Err(e) => { diff --git a/crates/engine/src/tests/track.rs b/crates/engine/src/tests/track.rs index 8c7037b69..bd1adaeca 100644 --- a/crates/engine/src/tests/track.rs +++ b/crates/engine/src/tests/track.rs @@ -433,3 +433,57 @@ fn analyze_1080p_single_point_speed() { assert!(dist(fc.value_at(frame_time(30)).as_vec2(), [990.0, 555.0]) < 0.25); assert!(fps > 30.0, "{fps} frames/s"); } + +/// A stand-in for `sampleImage()` on a footage layer (the engine has no expression language): the +/// pixel at (0, 0) of the layer's footage, read through the context's footage source. The real +/// one samples transparent without the source; this one fails, so either symptom shows. +struct SampleFootage; + +impl crate::render::ExprHost for SampleFootage { + fn eval( + &self, + ctx: &EvalCtx, + layer: &effectcraft_project::Layer, + _: &effectcraft_project::Property, + _: &effectcraft_keyframe::Value, + ) -> Result { + let item = layer.source.item().ok_or("not a footage layer")?; + let Some(ItemKind::Footage(footage)) = ctx.project.item(item).map(|i| &i.kind) else { return Err("not a footage layer".into()) }; + let src = ctx.footage.ok_or("sampleImage: no footage source")?; + let img = src.frame(item, footage, ctx.time).ok_or("no frame")?; + Ok(effectcraft_keyframe::Value::Scalar(f64::from(img.get(0, 0)[0]) * 100.0)) + } +} + +/// The clip of [`setup`] (uniform 25 % grey footage) with an opacity expression that samples its +/// footage: 25 with the footage source. Here for `setup`'s footage layer (#3231). +fn sampling_clip() -> (Session, ItemId, LayerId) { + let (mut s, clip, _) = setup(|_| Image::filled(W, H, [0.25, 0.25, 0.25, 1.0])); + let cid = s.active_comp_id().unwrap(); + s.expr = Some(Arc::new(SampleFootage)); + s.execute("prop.setExpression", json!({"layer": clip.0, "path": "transform/opacity", "expression": "sampleImage([0, 0])[0] * 100"})).unwrap(); + (s, cid, clip) +} + +/// #3231: `prop.get` evaluates an expression that samples footage pixels with the footage source, +/// as the render and `expr.errors` do: the same value and no spurious `expressionError`. +#[test] +fn prop_get_evaluates_footage_sampling_expressions_with_the_footage_source() { + let (mut s, cid, clip) = sampling_clip(); + let got = s.execute("prop.get", json!({"layer": clip.0, "path": "transform/opacity", "time": 0.0})).unwrap(); + assert!(got.get("expressionError").is_none(), "no spurious error: {got}"); + assert_eq!(got["evaluated"].as_f64(), Some(25.0), "the footage pixel, not the keyframed 100: {got}"); + // The render applies the same opacity: the clip alone (outside the solid) covers 25 %. + let img = s.render(cid, Tick::ZERO, crate::render::RenderOpts::default()); + let a = img.get(5, 5)[3]; + assert!((a - 0.25).abs() < 0.01, "rendered opacity {a}"); + assert!(crate::cmd::expr_tools::errors(&s, cid, Tick::ZERO).is_empty()); +} + +/// #3231: Reveal Expression Errors and the viewer's error banner evaluate with the footage source, +/// so an expression that samples footage pixels isn't reported as failing. +#[test] +fn expression_errors_evaluate_footage_sampling_expressions_with_the_footage_source() { + let (s, cid, _) = sampling_clip(); + assert_eq!(s.expression_errors(cid), Vec::::new()); +} diff --git a/crates/script/src/model.rs b/crates/script/src/model.rs index e662c1cdc..694d82d39 100644 --- a/crates/script/src/model.rs +++ b/crates/script/src/model.rs @@ -519,12 +519,9 @@ fn value(s: &Session, a: &J) -> R { let v = if pre || !p.has_expression() { p.value_at(l.layer_time(t)) } else { - let mut ctx = EvalCtx::new(&s.project, cid, c, t); - ctx.expr = s.expr.as_deref(); // Sample expressions (`sampleImage`) read footage through the session's source; without // it footage layers sample as transparent (#1000). - ctx.footage = Some(s.footage.as_ref()); - ctx.value(l, p) + effectcraft_engine::commands::expr_tools::eval_ctx(s, cid, c, t).value(l, p) }; Ok(js_value(c, &v)) }