#3231: evaluate prop.get and expression errors with the footage source

prop.get and Session::expression_errors (Reveal Expression Errors, the
viewer's error banner) built their expression context without the
footage source, so an expression that reads footage pixels
(sampleImage) gave a different value or a spurious error there while
it worked in the render and in expr.errors (#1000).

expr_tools::eval_ctx builds the render's context (expression host and
footage source) for a comp at a time; expr.errors, prop.get,
Session::expression_errors and the script value query all use it.

Fixes #3231
This commit is contained in:
vangel76
2026-11-07 15:42:40 +01:00
parent bb2d2005ce
commit fc9eca99a0
5 changed files with 71 additions and 11 deletions
+11 -3
View File
@@ -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<ExprError> {
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
+2 -2
View File
@@ -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}));
}
+3 -2
View File
@@ -558,8 +558,9 @@ fn get(s: &mut Session, p: &Value) -> Result<Value> {
} 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) => {
+54
View File
@@ -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<effectcraft_keyframe::Value, String> {
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::<serde_json::Value>::new());
}
+1 -4
View File
@@ -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))
}