Math correctness foundation

CLAUDE.md

@@ -150,6 +150,7 @@ assets/               glTF samples + HDR skyboxes
 - **One declaration of every shared GPU data-layout limit** — the sizes and indices that a C++ block and a shader block must agree on (caster/light/joint/morph/emitter/kernel counts, the map-validity bits). Purely GLSL-side algorithm constants are NOT in scope and this mechanism does not own them: a compute workgroup size, a scan radix, a tap count with no C++ counterpart stays where it is used. `shaders/gpu_limits.glsl` is written in the subset that is valid GLSL *and* valid C++; shaders `#include` it and `graphics/gpu_limits.hpp` includes it inside a `shader_limits` namespace, re-exporting each value under its `k`-name. Add a shader-visible limit **there**, never as a literal on either side, and keep the file inside the common subset (no `constexpr`, `inline`, `namespace`, `static_cast`, unsigned suffixes — each breaks the *other* language, in files that never mention this one). The `gpu_limits_guard` CTest case sweeps `shaders/` for a re-declaration, requires each consumer to use the name rather than a literal, and requires each `k`-constant to be defined *as* the shared declaration, so C++ cannot drift back to hard-coded values behind green shader checks.
 - **A shadow family's recording and its uploaded validity are one value** — `ShadowMapValidity` (`graphics/shadow_map_validity.hpp`) is applied twice per frame in `Renderer::prepareShadowPlan`, in a fixed order, both from the COMPLETED view set: as ELIGIBILITY, deciding which families may be PREPARED at all (preparation resolves casters and stages hysteresis, so a family that will be neither recorded nor sampled must not be resolved); then as CONFIRMATION (`shadowMapValidityFromPlan`), derived from the finished plan and judged against the counts eligibility expected, which is what `uploadFrameLighting` writes to `LightUBO::shadowMapValidMask` for every sampling path in `shader.frag`. Never skip a family's recording without routing the decision through it — a skipped family's depth image holds an earlier frame's content, and sampling it produces no error, no crash, and shadows from a frame that is gone.
 - **The shadow pass decides in preparation and records from the plan** — `prepareShadowFrame` (`graphics/shadow_pass_prepare.hpp`) filters, resolves each caster's LOD per view, claims the diagnostic row and builds a `ShadowFramePlan`; `Shadows::recordPass` consumes that plan and nothing else (no draw spans, no view set, no resolver). Anything the pass rasterises with must live in the prepared view or draw: a value read at record time that the comparison never saw is a cached shadow map kept when it should have been re-rendered.
+- **One comparison authority, and norms with a scaled fallback** — `math/scalar.hpp`'s `almostEqual` is the only thing that decides whether two floats are close, and every `approxEqual` on `Vec*`/`Mat3`/`Mat4`/`Quaternion` delegates to it. `a == b` first (equal infinities pass), any remaining non-finite operand unequal (a NaN is never equal to anything, itself included), then absolute and relative tolerances in `double`. THREE OVERLOADS so an explicit tolerance is never loosened by an implicit one — none means both defaults, one is ABSOLUTE ONLY, two are both stated — and an invalid tolerance (negative, NaN, infinite) returns false rather than being reinterpreted, checked before the `a == b` shortcut so equal operands cannot hide a bad constant. `magnitude()`/`normalise()` compute `sqrt(dot(v, v))` FIRST and fall back to a scaled form only when that sum is not finite and NORMAL: the fast path keeps ordinary results bit-identical (so the physics goldens do not move for a change about extreme values), and the fallback covers overflow, total underflow AND the subnormal region, where a finite positive sum has already lost its precision. `normalise` divides ONCE per component wherever the length is representable — dividing twice costs an ulp each, which is not nothing: it tripled a box stack's settle time and only one platform's test noticed. Its three answers are load-bearing: non-finite → NaNs (VISIBLY invalid, never laundered into a plausible value), below `float_normalise_cutoff` (its own constant, not `float_epsilon`) → zero vector or identity rotation, otherwise normalised. `Mat3::tryInverse()` is the only inversion: scale-invariant, `double`, and its `std::optional` means "a REPRESENTABLE inverse exists" — an engaged value is never a matrix of infinities. Physics sites with a genuine invariant use the physics-local fail-fast helper, never an unchecked dereference.
 - **A reused shadow map is a claim about the GPU, so it is only ever made after the submit** — `ShadowResidencyStore` (`graphics/shadow_pass_plan.hpp`) records what each physical view's depth image HOLDS, and `prepareShadowFrame` compares this frame's prepared content against it to mark each view `Reused` or `Recorded`. It is owned by `Shadows`, beside the images it describes: that is the whole invalidation story, and why there is no `invalidate()` to forget to call — recreating the images means reconstructing the object that owns both. Two rules the type enforces rather than its callers: only a `Recorded` view commits (a `Reused` one never touched its image, so committing its prepared work would replace the record of what the image holds with a description of a frame that wrote nothing), and an `Invalid` slot is left alone (nothing recorded means nothing overwrote the image, so its record is still true). The commit sits beside `shadowLodResolver_.commitFrame()` BETWEEN `submitFrame` and `presentFrame`, for the same reason: content adopted by a frame that was abandoned would claim an image holds pixels the GPU never drew, and committing after PRESENTATION would be worse still — raii `presentKHR` throws on an out-of-date swapchain, so a resize would skip the commit for a frame whose depth was already being rasterised — and it is `noexcept`, adopting by MOVE out of the plan (`ShadowFramePlan::takeRecorded`, with `static_assert`s pinning the no-throw moves), because on the far side of a submit there is no useful answer to a failed allocation. `RenderTunables::shadowResidencyReuseEnabled` (overlay: "Reuse unchanged shadow views") forces every engaged view to record; it is SCHEDULING, so it is an argument to the law and never part of the content descriptor — a frame recorded with reuse off commits as usual and is reusable the moment it is switched back on. Each SH-01 row carries the disposition it ended up with, because zero raster passes alone cannot separate "reused" from "never engaged".
 - **GPU data-layout discipline** — every CPU struct shared with a shader (UBO/SSBO) lives in `render/ubo.hpp` with `alignas` + `static_assert`s pinning its std140/std430 offsets and size. Preserve this: when you change a shader-visible struct, update both sides and keep the static_asserts — they are the only thing catching a silent host↔GPU layout mismatch. Mapped host-visible writes go through `graphics/mapped_buffer.hpp` `writeMapped` (a bounds-checked `std::span<std::byte>`), never a raw `void*`. **And a block bound by more than one shader is declared ONCE, in a shared `shaders/*.glsl` include** (`light_ubo.glsl` for `LightUBO`, `material.glsl` for the bindless `Materials` SSBO + `MaterialData`, `shadow_push.glsl` for the `ShadowPushConstants` push block), never hand-copied per shader: field offsets depend on every field before them, so a copy missing an inserted field misreads everything after it, with no validation error and no crash. That is how the sky came to be multiplied by a shadow matrix — `selfShadowViewProj` was added to the struct and `shader.frag`, not to `skybox.frag`, and the wrong value read 1.0 until a scene had two skinned self-shadow casters. The `shader_block_guards` CTest case (`cmake/check_shader_blocks.cmake`) fails on a re-declared block *and* on a shared include that stops declaring it.
 

CMakeLists.txt

@@ -410,6 +410,7 @@ add_executable(test_fire_engine
     tests/physics/test_physics_handle.cpp
     tests/physics/test_physics_determinism.cpp
     tests/physics/test_demos.cpp
+    tests/math/test_scalar.cpp
     tests/math/test_mat3.cpp
     tests/math/test_singular_value.cpp
     tests/math/test_mat4.cpp

docs/codereview.md

@@ -261,6 +261,59 @@ property/invariant tests for:
 The existing `Mat3` tests particularly need expansion: inversion is a critical operation, but the
 current suite does not cover scale invariance or the absolute-determinant failure above.
 
+### Phase 1 resolution — ✅ landed (`math-correctness-foundation`, five commits)
+
+Findings 1, 2, 4 and 5 are cleared. Finding 3 (the rotation invariant) and findings 6–8 belong to
+phases 2–3 and remain open.
+
+1. **One comparison authority** (finding 2). `math/scalar.hpp`'s `almostEqual` is the only thing that
+   decides whether two floats are close: `a == b` first (equal infinities pass), any remaining
+   non-finite operand unequal (a NaN is never equal to anything, itself included), then absolute and
+   relative tolerances in `double` so the difference itself cannot overflow. Every type's
+   `approxEqual` delegates. THREE OVERLOADS, because an explicit tolerance must not be loosened by
+   an implicit one — none means both defaults, one is absolute only, two are both stated. Invalid
+   tolerances (negative, NaN, infinite) return false rather than being reinterpreted, and the check
+   precedes the `a == b` shortcut so equal operands cannot hide a bad constant.
+2. **`Mat3::tryInverse()`** (finding 1). Scale-invariant, `double` intermediates, `std::optional`
+   instead of a zero-matrix sentinel that was both a value and an error report. It refuses an
+   invalid tolerance before examining the matrix (`magnitude > tolerance` is TRUE for a zero
+   determinant against a negative threshold — a singular matrix accepted, then divided by its own
+   zero), and refuses an inverse `float` cannot represent (a `1e-39` uniform scale is perfectly
+   conditioned and its inverse is `1e39`, so an engaged optional would have held infinities). The
+   three physics invariant sites use a physics-local fail-fast helper — assert, then a logged reason
+   and `abort` in release, never an unchecked dereference. VDPM's two disagreeing invertibility
+   decisions became one, with its `|det| > 1e-6·σ_max³` policy reproduced exactly rather than
+   inherited; `determinant()` returns `double` so a tiny reflection's sign survives instead of
+   underflowing to `-0.0f` and inverting the cone facing.
+3. **Equality is exact, not bitwise** (finding 5). `bitwiseEqual` was `return self() == rhs` — it
+   duplicated `operator==` and misdescribed it in both directions (`-0.0f` equals `+0.0f` with
+   different bits; a NaN equals nothing with identical bits). Deleted, comments corrected on all
+   four types, and the tests now assert those two IEEE cases plus the `q` vs `-q` seam that phase 2
+   must close.
+4. **Robust norms** (finding 4), and the shape of this one is the finding worth keeping.
+   `sqrt(dot(v, v))` is computed FIRST and trusted only when the sum is finite and **normal**; the
+   scaled form runs otherwise. That ordering keeps ordinary vectors bit-identical to the previous
+   arithmetic — **neither physics golden moved** — while covering both failure regions. Requiring a
+   normal sum rather than merely a positive one matters: `(3e-23, 3e-23, 0)` sums to `2.8e-45`,
+   which is finite, positive and carries about two significant bits, and answered 24.8% high. A
+   finite subnormal sum does reach the scaled fallback in `normalise` as well, but its magnitude is
+   necessarily below `float_normalise_cutoff`, so normalisation still returns the configured
+   degenerate value. `normalise` divides ONCE per component wherever the length is representable:
+   the first attempt divided twice, and that single extra ulp per component delayed a settling box
+   stack from step 169 to 425 on macOS and 167 to 1309 on Linux.
+5. **A settle-time tripwire** (no finding — it exists because of how (4) was caught). Every endpoint
+   assertion in the suite passed on macOS while the broken version tripled the settle time; only
+   Linux failed, and only because its trajectory was slower still. `Demos.Sleep` now records the
+   first post-impact step at which the whole island sleeps and bounds it at 260 — measured from
+   healthy runs of 167 (macOS/arm64) and 172 (Linux/x86_64), decisively below the 425 a real
+   regression produced — then steps a further 120 to separate "crossed the threshold once" from
+   stable rest.
+
+The lesson worth carrying into phases 2 and 3: a green suite plus a dutifully re-baselined golden
+looked exactly like success while the change was a regression. What separated them was measuring a
+QUANTITY (settle steps) rather than asserting an endpoint, and treating a golden move as a question
+rather than a chore.
+
 ### Recommended implementation sequence
 
 #### Phase 1: correctness foundation

docs/onboarding.md

@@ -84,13 +84,17 @@ Start here because these classes are small, heavily tested, and used everywhere.
 - `Vec2`, `Vec3`, `Vec4`: numeric vector types with constexpr arithmetic and component
   accessors. `Vec3` also provides operations used by lighting, transforms, normals, and
   physics response. `magnitude()` / `normalise()` call `std::sqrt` and are intentionally
-  *not* `constexpr` (sqrt only became constexpr in C++26). `operator==` is strict bit
-  equality — use `approxEqual(rhs, eps)` for tolerance-based comparison (or `bitwiseEqual`
-  if you want to name the bit-identity intent explicitly). Vec3 ↔ Vec4 conversion is
-  `explicit` in both directions to prevent silent w-component loss/gain.
+  *not* `constexpr` (sqrt only became constexpr in C++26). `operator==` is **exact
+  component-wise IEEE equality — not bitwise**: `-0.0f` equals `+0.0f` though their bits
+  differ, and a NaN equals nothing though its bits are identical to itself. Use
+  `approxEqual(rhs, eps)` for tolerance-based comparison. (There was a `bitwiseEqual` that
+  simply called `operator==`; it claimed semantics it did not have and is gone. If you ever
+  need real bit identity, say so with `std::bit_cast` at the point that needs it.)
+  Vec3 ↔ Vec4 conversion is `explicit` in both directions to prevent silent w-component
+  loss/gain.
 - `Mat4`: column-major transform/projection matrix type. Look at translation, rotation,
   scale, perspective, and look-at helpers. Renderer, scene traversal, skinning, and physics
-  transforms all depend on this behaving predictably. Same `approxEqual` / `bitwiseEqual`
+  transforms all depend on this behaving predictably. Same `operator==` / `approxEqual`
   convention as the vector types.
 - `Quaternion`: runtime rotation representation for scene transforms. glTF rotations round
   trip better through quaternions than Euler angles. Animation uses SLERP for rotation
@@ -957,6 +961,38 @@ the same change — most have a test or guard that will catch you, but not all.
   one constructed value so they cannot drift), and the per-frame-ring buffer handles are carried for
   recording but EXCLUDED from the comparison, since identical content alternates handles every
   frame.
+- **Every approximate comparison goes through `almostEqual`, and every norm has a scaled fallback.**
+  `math/scalar.hpp` is the one place that decides whether two floats are close: `a == b` first (so
+  equal infinities pass), non-finite operands unequal (so a NaN is never equal to anything, itself
+  included), then absolute and relative tolerances in `double`. The types' `approxEqual` delegate;
+  none of them re-implements the test. Three overloads, because an explicit tolerance must not be
+  loosened by an implicit one — no argument means both defaults, ONE argument is absolute only, two
+  are both stated — and an invalid tolerance (negative, NaN, infinite) makes the comparison false
+  rather than being reinterpreted as a policy.
+
+  `magnitude()` and `normalise()` compute `sqrt(dot(v, v))` FIRST and fall back to a scaled form
+  whenever that sum is not finite and NORMAL — zero, SUBNORMAL, infinite or NaN — which is exactly
+  when the naive computation had no accurate answer (components above ~1.8e19 square to infinity;
+  below ~1e-22 they flush to zero; and in between, a subnormal sum is finite and positive while
+  carrying only a couple of significant bits, which is how `(3e-23, 3e-23, 0)` answered 24.8% high).
+  Two
+  consequences to preserve if you touch this. Ordinary vectors take the arithmetic the engine always
+  used, BIT FOR BIT, so the physics goldens do not move for a change about extreme values. And the
+  fallback normalises through ONE division wherever the length is representable: dividing twice
+  (by the largest component, then by a scaled norm) costs an ulp per component, which delayed a
+  settling box stack from step 169 to 425 here and 1309 on Linux against a 600-step budget.
+
+  Three answers from `normalise`, and the middle one is the one people get wrong: a non-finite input
+  yields NaNs (**visibly invalid**), a magnitude below `float_normalise_cutoff` yields the zero
+  vector or the identity rotation (degenerate, as documented), and anything else is normalised —
+  including a vector whose LENGTH is unrepresentable but whose direction is ordinary. Laundering an
+  invalid input into the degenerate answer is what makes a corrupt orientation surface three seconds
+  later somewhere unrelated.
+
+  If you do change the arithmetic here, check `ReplayIsBitIdentical` and `FreeFallMatchesClosedForm`
+  before re-baselining any golden: those separate "last-bit arithmetic changed" from "the physics
+  changed", and a settle-time probe separates both from "the solver now takes three times as long to
+  come to rest".
 - **What an image HOLDS is committed only after the submit, and only for a view that recorded.**
   `ShadowResidencyStore` (`graphics/shadow_pass_plan.hpp`) is the other operand of the disposition
   law: preparation compares this frame's prepared content against it, and a view whose content