feat(render): Camera/View types + the missing camera math (perspective, inverse, unproject, pick_ray) - #465
Merged
Merged
Conversation
…e, inverse, unproject, pick_ray) M9 e11a, the foundation child of the decomposed e11 viewports task (design m9-editor, decision D5). Adds the render-side Camera/View abstraction and, more importantly, the INVERSE direction that did not exist anywhere in the tree: nothing could answer "which point in the world does this pixel show?", which is what click-to-pick in a scene viewport is. - context/render/view.h: the D5 View value type (camera transform, projection params, mode 2D|3D, type Scene|Game, viewport id) plus view_matrix / projection_matrix / view_proj / project / unproject / pick_ray. The framed aspect ratio is deliberately NOT stored on a View -- it belongs to the target it is rendered into, and a View whose aspect disagreed with the region it was picked in would put the pick ray somewhere the user did not click. - context/render/math.h: perspective(), Mat4 inverse(), determinant() and rotation_from_quaternion(), alongside the existing 3D math PROMOTED here from render/lit/lit_math.h. The promotion is a layering fix, not a rewrite: view.h is a context_render header, so leaving Mat4 in the downstream context_render_lit would have made context_render's own public surface depend on a library that links against it. lit_math.h stays and re-exports every promoted name, so no lit call site changed and lit-test_lit_math still covers that math. - ortho() gains the zero-extent denominator guard its sprite:: sibling has always had, so a degenerate camera is finite rather than NaN. Tests ship with the behaviour (R-QA-013): render-test_math and render-test_view. They ride the existing render-* family, so no ci.yml --target bookkeeping (pre-push audit check 8 confirms). Every assertion was verified by planting a deliberate defect: 20 plants, 20 RED, each attributed to its own failing assertion from its own per-plant log. Two defects the planting round found are worth recording -- a round-trip assertion is invariant to ANY invertible error in the view-projection, so it cannot stand alone (P12 transposes the view matrix and the round trips stay green), and the inverse()'s overflowed-determinant branch was unprovable until its fixture was retuned so the adjugate stays finite. Closes #464
Review + cleanup pass over the e11a diff. Two shipped defects and five proven-vacuous claims, all found by report-only review helpers and reproduced against the real sources before being acted on. Fixes: - projection_matrix: a 3D View inheriting a 2D view's near_z = 0 built a SINGULAR view-projection, so inverse() took its identity fallback and unproject()/pick_ray() returned the NDC point as if it were world space -- finite, so no finiteness check could see it. Reachable with no authoring error: Projection deliberately keeps both framings so a viewport can toggle modes. Guarded at the View layer, which is where input policy belongs; math.h now says so. - test_web_parity: promoting Mat4 into context::render made an unqualified Mat4 ambiguous in the one TU carrying both context::render and context::render::sprite using-directives. Latent today, a hard break as soon as math.h enters that include closure. Test coverage for claims that were unpinned (each proven by a plant): - the 2D orthographic framing entirely: box height, aspect-times-height width, and the aspect's orientation -- all invisible to the round trips and the ortho/perspective contrasts, which read one projection on both sides; - the depth range through the View seam in BOTH modes (near->0, far->1, harmonic vs arithmetic midpoint); - the "camera scale is ignored" contract, which no fixture could observe (all scales were 1); - rotation handedness, pinned inside test_view rather than only in the sibling binary; - pick_ray's origin lying on the near plane; - the degenerate FALLBACK VALUES in perspective/ortho, and determinant's cofactor expansion against a non-diagonal fixture. Corrections: - kRoundTripEps 3.0e-4 -> 1.0e-3: the documented 8x margin is this host's FMA-contracted figure; without contraction the round trip reaches 9.9e-5, a ~3x margin on the CI-only toolchains. - aspect_ratio uses rhi.h's is_empty(); math.h's extract.cpp cross-reference corrected (it tests a squared length, it does not call length()); determinant documented as necessary-but-not-sufficient for invertibility; a warning that sprite::ortho is NOT this ortho renamed (opposite depth sign); lit/README.md's deleted lit_math.* bullet fixed. Gates: Suite 1 457/457 (build rc 0, warnings-as-errors), Suite 2 ASan+UBSan 457/457, pre-push audit 10/10 PASS, plant round 34/34 matched expect | 34 RED | 0 GREEN | restore-failures 0, every RED attributed from its own per-plant log. Part of #464
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Camera/Viewvalue type (src/render/include/context/render/view.h) — camera transform, projection parameters, mode (2D | 3D), type (Scene | Game), viewport id — the D5 abstraction the viewport work builds on, and a plain value so a later child can carry it inRenderSnapshotwith no conversion.perspective(),Mat4 inverse()(+determinant()),view_proj(),project()/unproject(), andpick_ray(view, region_pixel, region_size)returning a world-space ray. Nothing could previously answer "which point in the world does this pixel show?", which is exactly what click-to-pick is.render/lit/lit_math.hintocontext_render(context/render/math.h+src/math.cpp) rather than duplicating it;lit_math.hstays and re-exports every name, so no lit call site changed.Scope is the e11a foundation only: no render target, no render pass, no
extract_render_worldchange, no Shell wiring, no spatial index, no gizmos, no daemon/contract work.Why the math moved instead of growing in
lit/The task left the seam to the implementer.
view.his mandated atsrc/render/include/context/render/view.h, i.e. insidecontext_render, and aViewhas to name aMat4.context_render_litlinksPUBLIC context_render— so leavingMat4in lit would have madecontext_render's own public surface depend on a library that links against it, and that inversion gets worse in e11b/e11c when theViewentersRenderSnapshot(render_world.h, alsocontext_render).The promotion is cheap and provably non-disruptive: only four files include
lit_math.h, all insidesrc/render/lit/, and a tree-wide grep for qualifiedlit::Vec3/lit::Mat4/lit::mul/ … returns 0 hits, because every consumer is already insidenamespace context::render::lit.lit_math.hre-exports each promoted name, solit-test_lit_math— unchanged — still covers that math and doubles as the regression guard on the re-export.Alternative considered and rejected: keeping the math in
lit_math.hand havingview.hinclude it. There is precedent for that shape in this directory (viewport_scene.handgolden.hinclude sub-package headers and document that consumers must link the sub-package), but those are header-only proof scenes wired into the wgpu offscreen exe — not a foundation value type that later lands in the snapshot. The layering fix was the better seam for what e11b/e11g will do with it.One behaviour change came with the move:
ortho()gained the zero-extent denominator guard that itssprite::orthosibling has always had. A degenerate 2D camera is now finite instead of NaN, which the DoD requires.Design notes worth reviewing
Viewfield. It belongs to the target the view is rendered into, soprojection_matrix/view_proj/project/unproject/pick_rayall take that target'sExtent2Dand derive it. Storing it as well is the classic pair that drifts (the reasoningdpi.hspells out for DPI vs its scale factor), and aViewwhose aspect disagreed with the region it was picked in would put the pick ray somewhere the user did not click.view_matrixdoes not useinverse(). A camera placement is rigid, so its inverse isR^T * T(-eye)exactly, with no cofactor expansion. A camera's scale is deliberately ignored — the projection owns framing.pick_rayis the only entry point that speaks pixels: physical, region-relative, top-left origin, y-down (whatPointerDispatch::region_positionalready carries), sampled at the pixel centre, flipped into y-up NDC. Both the near- and far-plane points come from one inverse, which is why the perspective and orthographic cases need no special-casing — perspective varies the direction, ortho varies the origin.normalize()'s existing house rule: a singular or overflowinginverse()returns the identity (askdeterminant()if you must reject), a zero quaternion is the identity, a zero-extent target frames a square.Verification
cmake --build --preset dev && ctest --preset devCONTEXT_WARNINGS_AS_ERRORS=ONsanitize(ASan+UBSan)tools/**,bench/**or dependency change (audit check 7 agrees)Host row (per the profile's sanctioned table): macOS arm64,
Unix Makefiles+/usr/bin/c++(Apple Clang/libc++). A local green therefore covers thebuild (macos-latest)tier — libc++ overload resolution, all three Clang-Wunused-*, and both sanitizer presets. MSVC/W4 /WXand Linux remain CI-only and are not claimed here.No
ci.ymlchange is needed:render-test_math/render-test_viewjoin the existing plainrender-*family, which--preset devbuilds and thebuildjob's general ctest step auto-runs on all three legs. Pre-push audit check 8 (the "Not Run = RED"--targettripwire) confirms it. They do ride bothsanitizelegs, as the task noted — measured above at 0.38 s and 0.35 s instrumented. NoCONTEXT_TSAN_BUILD/CONTEXT_ASAN_BUILDwiden is included, deliberately: neither test asserts a wall-clock budget, measures elapsed time, or includes<chrono>, so there is no ceiling to widen and a define with no assertion behind it would be dead configuration. The CE #335 rule is scoped inconventions.mdto "a new test that asserts a REAL wall-clock budget".Anti-vacuity: 34 plants, 34 RED, each attributed
The 20 plants below are 02's original round, re-run against the refined tree (P17 re-anchored
after
aspect_ratiomoved tois_empty()); the refine pass added 14 more, one per claim itauthored. Final round: 34/34 matched their expect | 34 RED | 0 GREEN | restore-failures 0,
every RED attributed from its own per-plant log.
Every new assertion was verified by planting a deliberate defect through
plant_and_revert.pyand reading the per-plant log for which assertion failed — never a suite-level count.The finding that shaped the tests: an
unproject(project(p)) == pround trip is invariant to any invertible error in the view-projection. Transpose the view matrix, halve the field of view, swap the aspect — the round trip still closes, becauseunprojectinverts whateverprojectdid. So the round trip proves exactly one thing, thatinverse()inverts. Plant P12 demonstrates it: transposing the view matrix reddens 13 direct assertions and leaves every round-trip assertion green. The projection itself is therefore pinned by independent positive assertions naming expected values (test_view_matrix_is_the_inverse_of_the_camera_placement,test_projection_pins_frustum_edges,test_two_d_mode_is_orthographic).perspectivedivides x by the aspectnear_f(narrow.x, 1.0f / kAspect)perspectiveis projective (w' = -view_z)near_f(on_far.z, 1.0f)far, notnearnear_f(on_near.z, 0.0f)inversescales the adjugate by1/detnear_f(lhs.at(col, row), expected, kInverseEps)inverserejects a non-finite determinantnear_f(overflowed.m[i], identity.m[i])inversesweeps the result for inf/NaNall_finite(inv)orthoguards a zero-extent boxall_finite(ortho(5,5,5,5,0,0))orthocarries the principal-point translationnear_f(min_corner.x, -1.0f) && near_f(min_corner.y, -1.0f)near_f(x_axis.x,0) && near_f(x_axis.y,1) && near_f(x_axis.z,0)near_f(scaled.m[i], unit.m[i])all_finite(zero)view_matrixtransposes the rotationnear_f(ahead.z, -d)— round trips stay greenprojection_matrixreadsViewModenear_f(a.x, b.x)pick_rayflips the pixel y axisupper_component > 0.2fpick_rayfar point at NDC depth 1near_f(length(ray.direction), 1.0f)near_f(aspect_ratio(target()), 16.0f / 9.0f)near_f(aspect_ratio(Extent2D{0u,0u}), 1.0f)pick_rayguards a zero-extent regionfinite3(r.origin) && finite3(r.direction)unprojectinverts the view-projectionnear_f(back.x, p.x, kRoundTripEps)projectgoes through the viewnear_f(centre.x, 0.0f) && near_f(centre.y, 0.0f)Restores were byte-exact for all 20 (md5 verified against the pre-round digests), and the gate was re-run GREEN afterwards on the byte-exact tree.
Two defects the planting round found, that reading did not
inverse()— the one case a result-finiteness sweep structurally cannot catch, because1/infis exactly0and scales the adjugate to a finite matrix of zeros — was fixtured withdiag(1e30). At that magnitude the adjugate overflows too, so the products are NaN, the result sweep catches them, and the branch is never observed: the plant came back GREEN. Retuned todiag(1e12)(adjugate1e36, finite; determinant1e48, overflows) the same plant reddens, and the test now asserts the identity rather than mere finiteness. The redundantdet == 0clause it exposed was deleted rather than shipped unprovable — a branch no input can reach on its own is a branch no plant can justify.-Wunused-variableunder-Werrorreddened the compile — a RED that proves nothing. Reshaped to keep the symbol referenced.Why these fixtures can discriminate
Field of view 50°, never 90° (where
tan(fov/2) == 1hides a dropped factor entirely); aspect 16:9, never 1:1 (where a dropped and an inverted aspect divide are both invisible); near/far 0.25 / 120, never 0/1; the orthographic box asymmetric[-3,11] x [-2,9], which is what makes the non-centred principal point observable at all — a symmetric box has zero translation terms and cannot distinguish a correct projection from one with no principal-point handling. The camera is off-axis (37° about a non-axis-aligned axis) at a position off-centre in all three world axes, and every probe point is off-centre in all three camera axes — asserted in the test rather than assumed. Depth is checked at the harmonic mean of near/far, where a perspective mapping reads 0.5 and an affine one reads ~1.0.Epsilons are measured, not guessed: worst observed error over exactly these fixtures is 1.19e-7 (direct projection), 2.27e-5 (
mul(M, inverse(M))vs identity), 9.5e-7 (view/projection through aView), 3.6e-5 (3D round trip, on coordinates of magnitude ~21), 1.1e-5 (pick-ray incidence). Each constant sits 8x-21x above its measurement on this host (see the refine pass below:kRoundTripEpswas re-derived from a no-FMA measurement and raised to 1.0e-3) — loose enough to survive another toolchain's rounding, and orders of magnitude below what any wrong matrix produces here.Test plan
test.md): Suite 1 457/457, Suite 2 (ASan+UBSan) 457/457, pre-push audit exit 0Refine pass (
refine: address code review findings,b9b9860)A review + cleanup pass over the diff above surfaced two shipped defects and five
proven-vacuous claims. Every finding was reproduced against the real sources before being acted
on; a helper's recommended fix was declined twice where its finding was right but its prescription
was not.
Defects fixed
Viewinheriting a 2D view'snear_z = 0built a SINGULAR view-projection.Projectiondeliberately keeps both framings so a viewport can toggle 2D <-> 3D, and a 2D view legitimately
sits at
near_z = 0— so toggling that view to 3D made rows 2 and 3 of the frustum coincide.inverse()then took its identity fallback andunproject()/pick_ray()returned the NDCpoint as if it were world space: finite, so the degenerate-finiteness test was structurally
blind to it, and completely wrong — exactly what picking would have inherited. Reproduced against
the real sources (
determinant = 0,pick_raydirection(0,0,1)), then guarded at theViewlayer, which is where input policy belongs;
math.hnow states that boundary explicitly.Mat4promotion made an unqualifiedMat4ambiguous in the one TU carrying bothusing namespace context::render;andusing namespace context::render::sprite;(
test_web_parity.cpp). Latent today only because that TU's includes do not yet reachmath.h;a hard compile break as soon as the viewport render pass lands. Reproduced with
-include context/render/math.h, then qualified tosprite::Mat4.Claims that were unpinned, now each proven by its own plant
orientation. All three survive the round trips (invertible) and the ortho/perspective contrasts
(one projection read on both sides of the comparison), so the half-height could have been ignored
outright and the aspect dropped or inverted with the file still green;
Viewseam in BOTH modes — near -> 0, far -> 1, and the harmonic(3D) vs arithmetic (2D) midpoint.
test_mathpinnedperspective()given the right two floats;nothing pinned that
projection_matrixpassed them, unswapped and unscaled;scale at
1,1,1;test_viewrather than only in the sibling binary: thisfile's probe helpers share
rotation_from_quaternionwithview_matrix, which transposes it, sothe two cancel and a handedness inversion left the whole file green;
pick_ray's origin lying on the near plane (every other assertion is invariant to which depthis unprojected, since a projective map sends the constant-(x,y) NDC line to one world line);
perspective/ortho, anddeterminant's cofactorexpansion against a non-diagonal fixture — all four fixtures above were diagonal, so a
determinant implemented as the product of the diagonal agreed with every one of them.
Corrections
kRoundTripEps3.0e-4 -> 1.0e-3. The "8x-21x above its measurement" claim below is thishost's FMA-contracted figure; re-measured under strictly-rounded float32 with no contraction the
3D round trip reaches 9.9e-5, i.e. a ~3x margin on precisely the toolchains that are CI-only.
aspect_rationow usesrhi.h'sis_empty();math.h'sextract.cppcross-reference corrected(the extract tests a squared length — it never calls
length());determinant()documented asnecessary but not sufficient for invertibility (a subnormal determinant is non-zero and finite
yet still yields the identity fallback); a warning that
sprite::orthois not thisorthorenamed (opposite depth sign — collapsing it into a re-export would silently invert 2D depth);
lit/README.md's bullet for the now-deletedlit_math.*fixed.Gates on the refined tree: Suite 1 457/457 (build rc 0, warnings-as-errors), Suite 2
ASan+UBSan 457/457, pre-push audit 10/10 PASS, and the plant round below re-run against the
final tree.
Closes #464