Extract the AABB family, b3Plane, and the wide position validators - #2
Merged
Merged
Conversation
Moves the remaining fixed-point-typed aggregates out of box3d. Additive only:
367 insertions, no existing line touched.
WHAT MOVED
narrow (fixed_vec.h): b3Plane + b3IsValidPlane; b3AABB + b3MakeAABB, Contains,
Area, Center, Extents, Union, Inflate, Overlaps, Transform, b3ClosestPointToAABB,
b3IsValidAABB.
wide (fixed_wide.h): b3AABBWide and the same operations; b3WorldTransformWide;
b3IsValidWideCoord, b3IsValidPosWide, b3IsValidWorldTransformWide,
b3IsValidAABBWide; b3OffsetAABBWide.
WHY BOTH WIDTHS ARE UNCONDITIONAL, AND WHY THAT IS THE POINT
In box3d these two implementations sit in opposite branches of BOX3D_LUDICROUS_MODE.
That flag is OFF by default, so the wide half does not compile in an ordinary build --
it is dark code. Here both are unconditional and both are exercised on every CI run.
Putting that path under test is not a side effect of the extraction; on the evidence
below it is the main value of it.
ONE DELIBERATE BEHAVIOUR CHANGE, CALLED OUT BECAUSE EVERYTHING ELSE IS A PORT
box3d's wide b3AABB_Extents narrows each bound to b3Fixed and then subtracts. Past
Q48.16 range both bounds saturate to INT64_MAX, their difference is zero, and an
ordinary box reports zero extents -- at exactly the distances ludicrous mode exists to
serve. b3AABB_Transform consumes Extents, so transformed distant boxes collapse with it.
This version differences in 128-bit and then narrows. box3d's own wide b3AABB_Area
already does it that way round, so this restores consistency inside that file rather
than inventing a convention. For any box whose bounds both fit local range the two
forms agree bit-for-bit, which the correspondence cases check directly -- the fix is
invisible to every build that was already correct.
I found this by writing the test, not by reading the code. The first version of this
commit ported Extents faithfully and the test failed, which is the extraction paying
for itself inside an hour.
WHAT DID NOT MOVE, AND WHY
b3IsBoundedAABB / b3IsSaneAABB -- both read B3_HUGE, which resolves through
b3GetLengthUnitsPerMeter(), a mutable box3d global. They encode world-scale POLICY.
A pure math library should not carry an opinion about how big a world is, and should
not acquire a runtime global to do it.
b3SegmentDistance, b3LineDistance, b3PointToSegmentDistance, b3Steiner -- physics and
geometry that happen to be written in fixed point. They belong with the solver.
b3ClosestPointToAABB DID move, correcting my own earlier classification of it as a
geometric query: it is b3Clamp against the box, an operation of the type itself.
TESTING
test/aabb_test.c calls every symbol this commit adds. A header-only helper nothing
references is invisible to a passing build, which is exactly how b3WideMin/b3WideMax
shipped unreachable in the previous PR.
Proven able to fail, both directions:
- restoring box3d's narrow-first Extents fails the two distant-box checks (266, 274)
- breaking narrow b3AABB_Union fails its direct assertions AND the narrow/wide
correspondence checks at line 223
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Moves the remaining fixed-point-typed aggregates out of box3d. Additive only — 367 insertions, no existing line touched.
What moved
narrow (
fixed_vec.h) —b3Plane+b3IsValidPlane;b3AABBwithb3MakeAABB,Contains,Area,Center,Extents,Union,Inflate,Overlaps,Transform,b3ClosestPointToAABB,b3IsValidAABB.wide (
fixed_wide.h) —b3AABBWideand the same operations;b3WorldTransformWide;b3IsValidWideCoord,b3IsValidPosWide,b3IsValidWorldTransformWide,b3IsValidAABBWide;b3OffsetAABBWide.Both widths are unconditional, and that is the point
In box3d these two implementations sit in opposite branches of
BOX3D_LUDICROUS_MODE. That flag is off by default, so the wide half does not compile in an ordinary build — it is dark code. Here both are unconditional and both run on every CI job.One deliberate behaviour change
Everything else is a faithful port. This is not, so it gets its own heading.
box3d's wide
b3AABB_Extentsnarrows each bound tob3Fixedand then subtracts. Past Q48.16 range both bounds saturate toINT64_MAX, their difference is zero, and an ordinary box reports zero extents — at exactly the distances ludicrous mode exists to serve.b3AABB_TransformconsumesExtents, so transformed distant boxes collapse with it.This version differences in 128-bit and then narrows. box3d's own wide
b3AABB_Areaalready does it that way round, so this restores consistency inside that file rather than inventing a convention. For any box whose bounds both fit local range the two forms agree bit-for-bit — checked directly by the correspondence cases — so the fix is invisible to every build that was already correct.I found this by writing the test, not by reading the code. The first draft of this commit ported
Extentsfaithfully and the test failed.What did not move
b3IsBoundedAABB/b3IsSaneAABB— both readB3_HUGE, which resolves throughb3GetLengthUnitsPerMeter(), a mutable box3d global. They encode world-scale policy; a pure math library should not hold an opinion about how big a world is, nor acquire a runtime global to do it.b3SegmentDistance,b3LineDistance,b3PointToSegmentDistance,b3Steiner— physics and geometry that happen to be written in fixed point.b3ClosestPointToAABBdid move, correcting my earlier classification of it as a geometric query: it isb3Clampagainst the box, an operation of the type itself.Testing
test/aabb_test.ccalls every symbol this PR adds. A header-only helper that nothing references is invisible to a passing build — which is exactly howb3WideMin/b3WideMaxshipped unreachable in #1.Proven able to fail in both directions:
Extentsb3AABB_Union