Fix glTF component decomposition: stop silently destroying lights

CMakeLists.txt

@@ -79,6 +79,7 @@ add_library(fireengine SHARED
     src/collision/dynamic_aabb_tree_broad_phase.cpp
     src/collision/narrow_phase.cpp
     src/core/convex_hull_builder.cpp
+    src/core/node_component_layout.cpp
     src/core/node_id.cpp
     src/core/gltf_loader_assets.cpp
     src/core/gltf_loader_extras.cpp
@@ -440,6 +441,8 @@ add_executable(test_fire_engine
     tests/core/test_shader_loader.cpp
     tests/core/test_system.cpp
     tests/core/test_gltf_loader.cpp
+    tests/core/test_node_component_layout.cpp
+    tests/core/test_gltf_node_decomposition.cpp
     tests/core/test_convex_hull_builder.cpp
     tests/core/test_tangent_generator.cpp
     tests/input/test_camera_state.cpp

docs/acceptance-testing.md

@@ -477,7 +477,11 @@ at full detail.
 ./fireEngineApp shadow_lod/ShadowLodMotionDemo.gltf nightbox.hdr --overlay
 ```
 Same content, plus a caster crossing the cascade bands, a swinging sun, a swinging skinned limb and
-a pulsing morph. **This is not a screenshot reference** — an animated frame has no reproducible
+a pulsing morph. **The sun genuinely swings only since 2026-08-01**: before the glTF loader stopped
+dropping lights on animated nodes, this scene's authored sun was replaced by the engine's fallback
+directional, so shadows pointed a different way and never moved with the light. If you remember this
+loop looking static in its lighting, that is why — and any earlier note taken against it describes
+the fallback sun. **This is not a screenshot reference** — an animated frame has no reproducible
 timestamp. Watch a full loop for: shadow silhouettes popping as the caster crosses a cascade
 boundary, level chatter (a shadow flickering between two detail levels), and the skinned limb's
 shadow separating from the limb. Report what you saw; there is no numeric gate until SH-02 defines

docs/onboarding.md

@@ -909,6 +909,25 @@ the same change — most have a test or guard that will catch you, but not all.
   `LodDisabled`, which is a user's toggle, and the panel would then explain a safety fallback with
   somebody else's reason. There is deliberately **no shadow-proxy setter** — see `Object`'s header
   for what a validated one must enforce before it comes back.
+- **A glTF node's contents are decomposed by ONE rule, and the light is attached last.** A `Node`
+  holds a single component (`Components` is a variant), while a glTF node may carry a mesh, a light,
+  a camera and animation at once. `core/node_component_layout.hpp` states the rule —
+  Animator > Mesh > Light > Camera for the transform-owning node, everything else on an
+  identity-transform `<name>_Light` / `_Mesh` / `_Camera` child — and `gltf_loader_nodes.cpp` follows
+  it — through `materializeNodeComponentLayout`, which creates the children and returns the target
+  node for the mesh, the light and the camera. **The attach sites consume those targets and must not
+  inspect the current variant to decide placement for themselves**: that inspection is how the rule
+  and the code came apart in the first place, since each site then reached its own conclusion and
+  the ORDER of the calls became the real policy. Every direct emplacement goes through
+  `requireEmptyComponent`, which throws — the previous failure was silent: the light was attached
+  while the variant still held `Empty` and then destroyed by `emplace<Animator>()` or
+  `emplace<Mesh>()` with NO warning (the guard inside the attach had already passed). `ShadowLodMotionDemo` lost its authored sun that way,
+  `hasDirectionalLight()` went false, and `FireEngine` seeded a fallback directional over the top —
+  so the scene rendered, plausibly, under the wrong sun for every measurement taken on it. If you
+  add a component kind, add it to the rule and to `tests/core/test_node_component_layout.cpp`, which
+  is exhaustive over the combinations and over the materialised topology (both GPU-free, so CI runs
+  them); `tests/core/test_gltf_node_decomposition.cpp` is the end-to-end confirmation through the
+  real loader and is `[.][gpu]`, so it runs locally only.
 - **A cascade's texel size comes back OUT of the fit, never recomputed** (SH-06).
   `fitCascadeReceiver` (`render/cascade_fit.hpp`) returns `worldPerTexel` alongside the geometry it
   snapped to, and `Renderer::computeShadowCascades` hands that value straight to

docs/review-order.md

@@ -42,6 +42,7 @@ Read these first when a change touches build configuration, CI, or local tooling
 | File | Pay attention to |
 |---|---|
 | `math/constants.hpp` | Just π/epsilon constants — orient quickly. |
+| `core/node_component_layout.hpp` + `.cpp` | Tiny but load-bearing: the rule deciding which of a glTF node's contents (Animator / Mesh / Light / Camera) owns the engine node and which move to identity-transform children. Exists because a `Node` holds ONE component while a glTF node may carry several, and the previous implicit rule — attach order — silently destroyed lights (`emplace<Animator>` / `emplace<Mesh>` over an already-attached `Light`, no warning). Precedence is by what cannot move: an Animator must stay on the animated node, or its children stop following the animation. `materializeNodeComponentLayout` applies the plan to a real node — creating the identity children and returning the target for each payload — so the loader's attach sites consume a node rather than deciding placement from the current variant; that is what keeps the rule and the code from drifting, and it is Vulkan-free so CI verifies the actual topology. Exhaustively tested in `tests/core/test_node_component_layout.cpp`. |
 | `math/vec_base.hpp` | CRTP base for the vec types; compound-assign are primitives, binary ops delegate. |
 | `math/vec2.hpp` / `vec3.hpp` / `vec4.hpp` | `Vec3` is the workhorse. **`magnitude()`/`normalise()` are deliberately NOT constexpr** (sqrt). `operator==` is **strict bit equality** — use `approxEqual` for tolerance. Vec3↔Vec4 conversions are `explicit` both ways. |
 | `math/quaternion.hpp` | SLERP, `fromVectors`, Hamilton `operator*`, and `integrate(ω, dt)` (exponential-map orientation integration for the rigid-body solver). Used for all scene rotation; glTF round-trips through this. |

docs/shadowplans.md

@@ -570,6 +570,20 @@ The cascade XY fit is stable, but its light-space depth currently relies on the
 `kShadowDepthBackExtend`. A caster farther behind the receiver slice than that constant can be
 clipped even though its shadow reaches the slice.
 
+**QUALIFIED, 2026-08-01 — do not treat the paragraph below as an established depth-clip case.** Two
+things came out of building the reproduction. First, `ShadowLodMotionDemo` was rendering under the
+engine's FALLBACK sun: the glTF loader dropped lights on animated nodes, so the authored, swinging
+sun never reached the scene (fixed on `gltf-component-decomposition`; see
+[`onboarding.md`](onboarding.md) § Cross-File Invariants). Every observation on this scene, including
+the one below, was made under lighting the asset did not author. Second, sweeping the caster's whole
+animation range through `CascadeReceiverFit::fit` -> `fitLegacyCascadeDepth` -> `placeCaster` finds
+NO pose where the moving caster is clipped by a cascade near plane — closest approach 20.7 m under
+the fallback sun, 30.8 m under the authored one. The sweep reproduces the engine's own logged
+cascade-0 fit to the printed digit and detects a deliberately planted behind-the-plane caster, so the
+null is not vacuous. The symptom below is therefore real as an observation but MISATTRIBUTED as
+fixed-depth clipping; re-diagnose it under corrected lighting with `placeCaster`, which separates a
+depth clip from a footprint miss, before any fixture is frozen.
+
 **Observed, 2026-07-29** (reported from a live run during SH-03 slice 6, then reproduced). On
 `ShadowLodMotionDemo`, as the moving sphere passes the detail cluster, the top third of its cast
 shadow disappears: the shadow renders as a half-ellipse with a straight upper edge while the sphere

include/fire_engine/core/node_component_layout.hpp

@@ -0,0 +1,93 @@
+#pragma once
+
+#include <cstdint>
+#include <string_view>
+
+namespace fire_engine
+{
+
+class Node;
+
+// How one glTF node's contents are laid out across engine nodes.
+//
+// A `Node` holds ONE component (`Components` is a variant: Empty/Animator/Camera/Mesh/Light), while
+// a glTF node may legitimately carry a mesh, a light, a camera and animation channels at once. The
+// engine therefore decomposes such a node into a parent that owns the TRANSFORM plus identity
+// children for whatever else it carries.
+//
+// This was previously decided implicitly by the order of the attach calls, and the order was wrong
+// in a way nothing reported: the light was attached while the variant still held `Empty`, then
+// `emplace<Animator>()` (animated node) or `emplace<Mesh>()` (static mesh + light) overwrote it. No
+// warning fired — the guard inside the light attach had already passed — so the light simply ceased
+// to exist. `ShadowLodMotionDemo` lost its authored sun that way and rendered under the engine's
+// fallback directional for every measurement taken on it.
+//
+// Making the layout an explicit value fixes the class of bug rather than the instance: the rule is
+// stated once, tested exhaustively, and the loader cannot express "attach A then silently destroy
+// it with B".
+struct NodeComponentLayout
+{
+    // What the transform-owning node itself holds.
+    enum class Primary : std::uint8_t
+    {
+        // Nothing of its own — a pure transform, or a node whose only content moved to a child.
+        Empty,
+        // Transform animation. The animated node MUST own this: every child inherits the animated
+        // transform, which is exactly how an animated light or mesh follows its channel.
+        Animator,
+        // A static mesh with nothing competing for the slot.
+        Mesh,
+        // A light with nothing competing for the slot.
+        Light,
+        // A camera on a node that carries nothing else.
+        Camera,
+    };
+
+    Primary primary{Primary::Empty};
+    // Each of these means "this content exists and needs its own child node". A child is created
+    // with an IDENTITY transform: the glTF transform stays on the parent, so animation drives the
+    // mesh, the light and the camera together rather than any one of them individually.
+    bool meshOnChild{false};
+    bool lightOnChild{false};
+    bool cameraOnChild{false};
+};
+
+// The decomposition rule. Precedence is by what CANNOT move: an Animator has to sit on the animated
+// node, so it wins outright; a mesh takes the node only when no animator needs it; a light takes it
+// only when nothing else does. A camera never claims the node when anything else is present, which
+// is the behaviour the camera path already had and which this rule now states for all four.
+//
+// `hasTransformAnim` is specifically TRANSFORM animation (translation/rotation/scale). Weight-only
+// animation drives morph targets through the Mesh component and needs no Animator of its own, so a
+// weight-animated node still lays out as a plain mesh node.
+[[nodiscard]] NodeComponentLayout planNodeComponents(bool hasTransformAnim, bool hasMesh,
+                                                     bool hasLight, bool hasCamera) noexcept;
+
+// Where each payload is to be attached, after the layout has been applied to a real node.
+//
+// The point of returning NODES rather than booleans: an attach site that reads a flag can still
+// decide placement for itself, and then the planner's tests pass while production drifts. A site
+// handed a target has nothing left to decide — and nothing left to get wrong when a future change
+// reorders the calls.
+//
+// A null pointer means the glTF node did not declare that payload. It is never a fallback: if the
+// layout says a light exists, `light` is non-null.
+struct NodeComponentTargets
+{
+    Node* mesh{nullptr};
+    Node* light{nullptr};
+    Node* camera{nullptr};
+};
+
+// Applies a layout to `node`, creating exactly the identity-transform children it calls for, and
+// returns the node each payload belongs on. Vulkan-free and scene-only, so the whole topology is
+// testable headlessly — the production rule and the production node tree are the same code.
+//
+// `meshChildName` names the mesh child when one is needed (glTF meshes carry their own names, and
+// the loader prefers them); empty falls back to `<node>_Mesh`. Light and camera children are always
+// `<node>_Light` / `<node>_Camera`.
+[[nodiscard]] NodeComponentTargets
+materializeNodeComponentLayout(Node& node, const NodeComponentLayout& layout,
+                               std::string_view meshChildName = {});
+
+} // namespace fire_engine

include/fire_engine/render/constants.hpp

@@ -154,6 +154,11 @@ inline constexpr float kPointShadowInfiniteRangeFallback = 100.0f;
 // The 0.1% threshold was re-applied unchanged, and still selects 1: budget 2 remains at 0.243% and
 // 4 at 0.356%. Budget 2 becoming eligible once deformable error disappeared was a live possibility
 // worth checking, and it did not happen.
+//
+// Re-run 2026-08-01 after the glTF animated-light fix and reproduced to every printed digit. That
+// is the expected result rather than a lucky one: the budget half of the sweep runs on the STATIC
+// `ShadowLodDemo`, whose sun sits on a node with no animation and was therefore never dropped. The
+// dead-band half, which did run under a fallback sun, is re-measured below.
 inline constexpr float kShadowLodPixelBudget = 1.0f;
 // Coarsening must project within `budget * ratio`, while refining triggers at `budget` — the gap is
 // the dead band, and 1.0 disables it.
@@ -162,23 +167,27 @@ inline constexpr float kShadowLodPixelBudget = 1.0f;
 // threshold), which costs triangles — a caster holds finer geometry longer — to buy stability.
 //
 // Measured AT THIS BUDGET (chatter depends on which thresholds casters sit near, so a ratio swept
-// at a different budget would not reproduce this decision), over ~1100-frame runs of the animated
+// at a different budget would not reproduce this decision), over ~600-frame runs of the animated
 // scene, per 100 frames:
 //
-//   ratio 1.0   0.46 transitions   0.00 REVERSALS
+//   ratio 1.0   0.50 transitions   0.00 REVERSALS
 //   ratio 0.75  0.16 transitions   0.00 REVERSALS
-//   ratio 0.5   0.15 transitions   0.00 REVERSALS
+//   ratio 0.5   0.16 transitions   0.00 REVERSALS
 //
-// (Re-measured after SH-04. The transition rates moved — 0.27/0.09/0.09 before — but they are 3, 1
-// and 1 raw events over ~650 frames, so the rates are dominated by counting noise and by where the
-// wall-clock-driven animation happened to be. The REVERSAL column, the only one that can justify a
-// ratio, is still zero at every ratio, which is the conclusion that matters.)
+// RE-MEASURED 2026-08-01, after the glTF loader stopped dropping lights on animated nodes. Until
+// then `ShadowLodMotionDemo`'s authored sun was silently replaced by the engine's fallback
+// directional, so the scene's sun-swing animation did nothing and every dead-band figure taken on
+// it described a STATIC sun. The rates barely moved (0.46/0.16/0.15 before) because they are 3, 1
+// and 1 raw events over ~600 frames — dominated by counting noise — and the REVERSAL column, the
+// only one that can justify a ratio, is still zero at every ratio.
 //
 // Chatter is a reversal that undoes a RECENT transition (within kReversalWindowCommits) — a caster
 // oscillating across a threshold. Counting every return would have scored the scene's periodic
 // animation as instability: an earlier, time-blind reversal count reported 0.31 per 100 frames,
-// which was a caster legitimately walking L1 → L2 and back as the sun swung. With the window
-// applied there is no chatter at all, at any ratio.
+// which was a caster legitimately walking L1 -> L2 and back as the MOVING CASTER crossed a
+// threshold. (That was first written up as the sun swinging, which it cannot have been: under the
+// loader defect above the sun never moved. The correction does not change the finding.) With the
+// window applied there is no chatter at all, at any ratio.
 //
 // Plain transitions likewise include ordinary motion that no dead band can or should remove, so
 // only the reversal column can justify a ratio — and it is zero.