Review fixes: a failed environment source lights nothing (#2436)

Two review points on the merge:

- env_pixels returned None both for an unconfigured light (no Source, no
  Environment Layer) and for a failed source (a deleted layer, missing
  footage, a failed decode), so a selected source that failed silently
  swapped in the default studio environment — relighting and reshadowing
  the scene behind the user's back. It now tells the three apart
  (EnvPixels::Absent/Image/Failed): only the unconfigured light uses the
  default; a failed source contributes nothing, exactly like a layer whose
  source fails renders nothing. The repair the user can act on stays where
  it already lives — missing footage is flagged on the item, with the
  relink commands to fix it.

- The docs claimed only a uniform image casts no shadow. A non-uniform one
  can be directionless too: opposing bright regions cancel, and anything
  under 1% directional is rejected. agents.md, architecture.md and the
  sun/dominant_direction doc comments now say an image without a
  sufficiently strong dominant direction.

Tests: an unreadable Environment Layer (auto-picked), an unreadable
selected Source and a deleted one all light nothing — pixel-equal to the
unlit render, no studio default, no default-environment shadow — while an
unconfigured light still lights; two equal bright spots at opposite
azimuths cancel to no sun and no key, and tilting the balance re-emerges
pointing at the brighter spot.

crates/render/src/three_d/adv/scene.rs

@@ -174,8 +174,8 @@ pub struct EnvInfo {
     pub rotation: f32,
     /// Unit direction from the scene towards the environment's dominant (radiance-weighted)
     /// bright region — the direction its shadow is cast from, like a parallel light's
-    /// `to_light`. `[0; 3]` when the image is directionless (uniform) or the light casts no
-    /// shadow (nothing reads it then): no shadow.
+    /// `to_light`. `[0; 3]` when the image is directionless (uniform, or opposing bright
+    /// regions that cancel) or the light casts no shadow (nothing reads it then): no shadow.
     pub sun: [f32; 3],
     /// The image's directional (key) fraction — `|Σ dir·radiance| / Σ radiance` over the
     /// sphere, 0 for a uniform image and near 1 for a single bright spot. An Environment
@@ -821,7 +821,8 @@ pub fn irradiance_map(sw: u32, sh: u32, src: &[[f32; 4]], w: u32, h: u32) -> Vec
 
 /// The environment image's dominant direction and key strength: the radiance-weighted mean
 /// texel direction (solid-angle weighted, rotation applied), or `([0; 3], 0.0)` when the image
-/// is directionless (a uniform environment cancels to zero). The mean of a single bright spot
+/// is directionless (a uniform environment cancels to zero, and opposing bright regions that
+/// cancel leave no mean either). The mean of a single bright spot
 /// on black points straight at it; a gradient points somewhere between its ends. The returned
 /// key fraction is the mean vector's length over the radiance weight — how directional the
 /// image is, 0 (uniform) to ~1 (a lone spot) — and is the most of its light an Environment
@@ -915,13 +916,30 @@ pub(crate) fn add_environment(
     });
 }
 
-/// The environment layer's pixels as linear straight RGBA.
-fn env_pixels(r: &Renderer, ctx: &EvalCtx, src: Option<u64>, lin: bool) -> Option<(u32, u32, Vec<[f32; 4]>)> {
+/// The Environment light's image: `Absent` when the light has no Source and the comp has no
+/// Environment Layer (the default studio environment applies — After Effects "uses a default
+/// environment map" for an Environment light too), `Image` when the source renders, `Failed`
+/// when a selected or found source produced no pixels (a deleted layer, missing footage, a
+/// failed decode). A failed source must not silently become the default environment — that
+/// would relight and reshadow the scene behind the user's back — so, like any layer whose
+/// source fails renders nothing, it contributes nothing (#2436).
+enum EnvPixels {
+    Absent,
+    Image((u32, u32, Vec<[f32; 4]>)),
+    Failed,
+}
+
+fn env_pixels(r: &Renderer, ctx: &EvalCtx, src: Option<u64>, lin: bool) -> EnvPixels {
     let layer = match src {
-        Some(id) => ctx.comp.layer(effectcraft_project::LayerId(id))?,
-        None => ctx.comp.layers.iter().find(|l| l.environment && l.is_active_at(ctx.time))?,
+        Some(id) => ctx.comp.layer(effectcraft_project::LayerId(id)),
+        None => ctx.comp.layers.iter().find(|l| l.environment && l.is_active_at(ctx.time)),
     };
-    let buf = r.blend_layer_buf(ctx, layer)?;
+    let Some(layer) = layer else {
+        // A set Source that resolves to no layer (deleted) failed; an unset one with no
+        // Environment Layer is simply unconfigured.
+        return if src.is_some() { EnvPixels::Failed } else { EnvPixels::Absent };
+    };
+    let Some(buf) = r.blend_layer_buf(ctx, layer) else { return EnvPixels::Failed };
     let img = &buf.img;
     let data = img
         .data
@@ -935,7 +953,7 @@ fn env_pixels(r: &Renderer, ctx: &EvalCtx, src: Option<u64>, lin: bool) -> Optio
             if lin { [s[0], s[1], s[2], 1.0] } else { [srgb_to_linear(s[0]), srgb_to_linear(s[1]), srgb_to_linear(s[2]), 1.0] }
         })
         .collect();
-    Some((img.width, img.height, data))
+    EnvPixels::Image((img.width, img.height, data))
 }
 
 /// The default environment when the light has no Source and no Environment Layer: a dim studio
@@ -1065,15 +1083,22 @@ pub(crate) fn build_at(r: &Renderer, ctx: &EvalCtx, sub: Option<&EvalCtx>, run:
         b.s.ambient = amb;
         if let Some(env) = env {
             match env_pixels(r, ctx, env.src, lin) {
-                Some((w, h, d)) => add_environment(&mut b, w, h, d, env.color, env.rot, env.casts_shadows, env.shadow_darkness, env.shadow_diffusion),
+                EnvPixels::Image((w, h, d)) => {
+                    add_environment(&mut b, w, h, d, env.color, env.rot, env.casts_shadows, env.shadow_darkness, env.shadow_diffusion)
+                }
                 // No Source and no Environment Layer: the default environment, a soft studio
                 // gradient of the light's colour × intensity — After Effects "uses a default
                 // environment map" for an Environment light — so the light always lights the
                 // scene per its parameters, and its dominant region casts shadows (#2436).
-                None => {
+                EnvPixels::Absent => {
                     let (w, h, d) = default_environment();
                     add_environment(&mut b, w, h, d, env.color, env.rot, env.casts_shadows, env.shadow_darkness, env.shadow_diffusion)
                 }
+                // A selected or found source that produced no pixels (deleted layer, missing
+                // footage, failed decode): no environment at all — the light contributes
+                // nothing, exactly like a layer whose source failed renders nothing, never a
+                // silent swap to the default studio's lighting and shadows (#2436).
+                EnvPixels::Failed => {}
             }
         }
         if r.opts.shadows() {

crates/render/src/three_d/adv/tests.rs

@@ -95,6 +95,33 @@ fn environment_key_strength_tracks_directionality() {
     assert!((0.35..0.55).contains(&key), "the default studio is a broad key with fill: {key}");
 }
 
+/// #2436 review: a non-uniform image can be directionless too — two equal bright spots at
+/// opposite azimuths pull the radiance-weighted mean in opposite directions and cancel,
+/// leaving no dominant region to shadow from (under 1 % directional): no sun, no key, so no
+/// shadow, just like a uniform image. Tilt the balance and a direction re-emerges, pointing
+/// at the brighter spot.
+#[test]
+fn opposing_bright_regions_cancel_to_no_shadow() {
+    let (w, h) = (64u32, 32u32);
+    let mut opposed = vec![[0.0f32; 4]; (w * h) as usize];
+    // Rows 15 and 16 mirror about the equator (v = 0.484375 / 0.515625), and columns 16 and 48
+    // sit half a width apart (u = 0.2578125 / 0.7578125): the two texel-centre directions are
+    // exactly opposite — equal solid-angle weight, opposite pull.
+    let (a, b) = ((15 * w + 16) as usize, (16 * w + 48) as usize);
+    opposed[a] = [1.0, 1.0, 1.0, 1.0];
+    opposed[b] = [1.0, 1.0, 1.0, 1.0];
+    assert_eq!(dominant_direction(w, h, &opposed, 0.0), ([0.0; 3], 0.0));
+    // One side twice as bright: the mean points at the brighter spot with its share of the
+    // weight (key = 1/3 — two parts against one).
+    let mut tilted = opposed.clone();
+    tilted[a] = [2.0, 2.0, 2.0, 1.0];
+    let (sun, key) = dominant_direction(w, h, &tilted, 0.0);
+    assert!(sun != [0.0; 3] && key > 0.3, "the brighter side wins: {sun:?} key {key}");
+    let want = shade::equirect_dir(16.5 / 64.0, 15.5 / 32.0, 0.0);
+    let dot = sun[0] * want[0] + sun[1] * want[1] + sun[2] * want[2];
+    assert!(dot > 0.999, "the sun points at the brighter spot: {sun:?} vs {want:?}");
+}
+
 #[test]
 fn equirect_round_trip_and_env_brdf() {
     for (u, v) in [(0.1f32, 0.2f32), (0.5, 0.5), (0.9, 0.7)] {
@@ -404,6 +431,70 @@ fn environment_light_without_a_source_lights_the_scene() {
     assert!(c5[0] > 0.15 && c5[1] < 0.05 && c5[2] < 0.05, "red environment: {c5:?}");
 }
 
+/// #2436 review: a selected or auto-picked environment source that fails to render (missing
+/// footage, a deleted layer) must not silently swap in the default studio environment — the
+/// light contributes nothing, exactly like a layer whose source failed renders nothing. Only a
+/// light with no Source at all (and no Environment Layer in the comp) uses the default.
+#[test]
+fn a_failed_environment_source_lights_nothing() {
+    let (mut p, cid) = project(120, 90);
+    let sid = p.add_item(
+        "S",
+        Default::default(),
+        None,
+        ItemKind::Solid(effectcraft_project::Solid { color: [1.0, 1.0, 1.0], width: 60, height: 40, pixel_aspect: 1.0 }),
+    );
+    add(&mut p, cid, LayerSource::Solid { item: sid }, |l| l.switches.three_d = true);
+    let unlit = render(&p, cid);
+    // The Environment Layer's footage is unreadable (missing; NoFootage serves no frames).
+    let fid = p.add_item(
+        "F",
+        Default::default(),
+        None,
+        ItemKind::Footage(effectcraft_project::Footage {
+            path: "missing.exr".into(),
+            kind: effectcraft_project::FootageKind::Still,
+            width: 64,
+            height: 32,
+            pixel_aspect: 1.0,
+            duration: Tick::from_seconds_f64(1.0),
+            has_video: true,
+            missing: true,
+            ..Default::default()
+        }),
+    );
+    add(&mut p, cid, LayerSource::Footage { item: fid }, |l| {
+        l.switches.three_d = true;
+        l.environment = true;
+    });
+    let env_layer = p.comp(cid).unwrap().layers.iter().find(|l| l.environment).unwrap().id;
+    add(&mut p, cid, LayerSource::Light { kind: LightKind::Environment }, |l| {
+        set(l, "lightOptions/castsShadows", Value::Bool(true));
+    });
+    fn light(p: &mut Project, cid: ItemId) -> &mut effectcraft_project::Layer {
+        p.comp_mut(cid).unwrap().layers.iter_mut().find(|l| l.is_light()).unwrap()
+    }
+    // Auto-picked Environment Layer, unreadable: the light is off — no studio default, no
+    // default-environment shadow either.
+    let failed = render(&p, cid);
+    assert_eq!(failed.data, unlit.data, "an unreadable Environment Layer lights nothing");
+    // The light's Source set to that unreadable layer: same.
+    set(light(&mut p, cid), "lightOptions/source", Value::Layer(Some(env_layer.0)));
+    let failed = render(&p, cid);
+    assert_eq!(failed.data, unlit.data, "an unreadable selected Source lights nothing");
+    // The Source pointing at a deleted layer: same.
+    set(light(&mut p, cid), "lightOptions/source", Value::Layer(Some(999_999)));
+    let failed = render(&p, cid);
+    assert_eq!(failed.data, unlit.data, "a deleted selected Source lights nothing");
+    // Contrast: with no Source and no Environment Layer, the default studio still lights it.
+    let comp = p.comp_mut(cid).unwrap();
+    comp.layers.retain(|l| l.is_light() || !l.environment);
+    set(comp.layers.iter_mut().find(|l| l.is_light()).unwrap(), "lightOptions/source", Value::Layer(None));
+    let lit = render(&p, cid);
+    let (c, c2) = (unlit.data[20 * 120 + 30], lit.data[20 * 120 + 30]);
+    assert!(c2[3] > 0.99 && (c2[0] - c[0]).abs() > 0.05, "the unconfigured light still lights: {c2:?} vs {c:?}");
+}
+
 /// #2436: an Environment light's Cast Shadows works like After Effects' — its image's dominant
 /// (brightest) region shadows the scene. A bright disc above the image's centre, turned to
 /// shine from up and behind the camera (rotation 180°), shadows a sphere onto the floor below

docs/agents.md

@@ -215,12 +215,15 @@ Things that tripped up a long agent-driven session (a multi-scene 3D piece built
   layer by layer id; with no source it uses the comp's Environment Layer (`layer.environment`),
   or a default studio environment — bright above and toward the initial camera, dark below and
   behind, at a dim 0.3 mean level — of the light's colour × intensity when there is none; either
-  way the light brightens the scene. It has `castsShadows`/`shadowDarkness`/`shadowDiffusion`
+  way the light brightens the scene. A selected Source or Environment Layer that fails to
+  render — a deleted layer, missing footage — lights nothing instead: the light never silently
+  falls back to the studio default. It has `castsShadows`/`shadowDarkness`/`shadowDiffusion`
   like the other lights: Cast Shadows shades the scene from the image's dominant bright region
-  (the default environment's key, or a source image's brightest area; only a uniform image has
-  none, so it casts no shadow) — occluding only that key's share of the light, so the shadow
-  stays partial like After Effects' area-light shadows, and soft by default (a penumbra of at
-  least 8 pixels or four map texels, plus Shadow Diffusion). It only lights
+  (the default environment's key, or a source image's brightest area; an image without a
+  sufficiently strong dominant direction has none — a uniform one, or opposing bright regions
+  that cancel — so it casts no shadow) — occluding only that key's share of the light, so the
+  shadow stays partial like After Effects' area-light shadows, and soft by default (a penumbra
+  of at least 8 pixels or four map texels, plus Shadow Diffusion). It only lights
   Advanced 3D comps, so creating one (or changing a light's type to Environment) switches a
   Classic 3D comp to Advanced 3D
   ([#261](https://github.com/storytold/effectcraft/issues/261),

docs/architecture.md

@@ -163,10 +163,13 @@ camera, dark below and behind, at a dim 0.3 mean level — of the light's colour
 when there is neither) is an equirectangular image (cosine irradiance map; roughness-indexed
 mip chain with the split-sum environment BRDF fit), and percentage-closer-filtered shadow maps
 (spot: perspective, parallel: orthographic over the scene, point: six cube faces, environment:
-one orthographic from the image's dominant bright region — none only when the image is
-directionless, as a uniform one is — occluding only the image's directional key fraction of
+one orthographic from the image's dominant bright region — none only when the image lacks a
+sufficiently strong dominant direction (a uniform one, or opposing bright regions that
+cancel) — occluding only the image's directional key fraction of
 the light, so the shadow stays partial like After Effects' area-light shadows, with a soft
-default penumbra of at least eight pixels or four map texels). An Environment light only lights Advanced 3D comps,
+default penumbra of at least eight pixels or four map texels). A Source or Environment Layer
+that fails to render — a deleted layer, missing footage — lights nothing: no silent fallback
+to the default studio environment. An Environment light only lights Advanced 3D comps,
 so adding one (or changing a light's type to it) switches a Classic 3D comp to Advanced 3D,
 as model layers do. Cards keep
 Material Options (Accepts Lights/Shadows, Casts Shadows On/Only, Ambient, Diffuse, Specular);