Repository navigation
An empty tree node's bounds are inverted infinities, not the float limits - #99
Merged
Merged
Conversation
… float limits An empty node passes for a leaf (EMPTY_NODE carries the leaf flag), so only its bounds keep a traversal out of it. They were math.aabb_empty(), the float limits (+-MAX_FLOAT) inverted, where the reference's b3MakeEmptyNode uses infinities. A query box reaching the limits -- the natural 'everything' box -- overlapped them: in a tree of one proxy, whose root is a leaf beside an empty node, tree_query took the empty node for a leaf and read proxy 0x3FFFFFFF, an access violation; tree_box_cast had the same path for a cast box grown to the limits. The empty node's bounds are +-infinity now (math.infinity(), new), which no box overlaps. test_dynamic_tree: the leaf root queried with a box out to the float limits finds only its proxy (on main the test crashes). Found by an audit of the dynamic tree and hull creation against Box3D e77352c, otherwise faithful.
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
1 participant
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.
I audited the dynamic tree and hull creation against Box3D
e77352c. Both match upstream line for line, apart from this one defect.An empty node counts as a leaf (
EMPTY_NODEcarries the leaf flag), so only its bounds keep a traversal out of it. Those bounds weremath.aabb_empty(): the float limits (±MAX_FLOAT), inverted. Upstream'sb3MakeEmptyNodeuses infinities instead.The failure: a query box reaching the float limits, which is the natural 'query everything' box, overlapped those bounds. In a tree with a single proxy, the root is a leaf with an empty node beside it.
tree_querytook the empty node for a leaf and read proxy0x3FFFFFFF, which is an access violation.tree_box_casthad the same path for a cast box grown to the limits.The fix: empty nodes now get ±infinity bounds from a new
math.infinity(), and no box overlaps those.test_dynamic_treequeries a one-proxy tree with a box out to the float limits and expects only that proxy. On main the test crashes.All 28 test programs pass locally on aether 0.772.0, with no warnings.