Fix min node queryray closest#14
Open
Ph0t0nX wants to merge 3 commits into
Open
Conversation
… (SortedNodes.Length < BVHUtils.NbLeavesPerNode)" to "if (SortedNodes.Length <= 1)" . This fixes an issue where if the tree had 2 or 3 nodes, only one node was getting included. Now the early-out only triggers for 0 or 1 nodes, and lets the existing padding code handle cases with 2 and 3 nodes.
…e it is needed by QueryRayClosest().
… have not tested QueryRayAny() yet, but QueryRayClosest() seems to work as expected. It uses pruning so that every subsequent ray-AABB test uses a shrunken length, and IntersectsRay rejects any AABB whose entry is past it. This required a small change in the AABB struct's IntersectRay() function.
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.
This PR has two main things:
I made a small change in BVHPrecomputedHierarchyJob.Execute() that fixes the error where: if there are 2 or 3 nodes inserted in the tree, only one of them gets added. I verified it with a gizmo showing all bvh nodes in the scene - before the fix with 3 entities, only one showed a box around it, and after the fix all 3 entities had boxes around them. I tested with 1, 2, and 4 as well.
I added 3 functions. 2 versions of QueryRayClosest(), and one QueryRayAny(). In order to make QueryRayClosest() more efficient, I had to make a tiny modification to AABB.IntersectRay() -- I needed the tEntry parameter. I overloaded it so that all calls to IntersectRay() simply call the new IntersectRay() and ignore the tEntry. Using the tEntry allowed me to keep shrinking the ray and prune out a lot of the ray-AABB tests, so it's faster than running a normal QueryRay() and using a collector. I tested it for a while using my selection/hover code in my game and it seemed to run fine - selecting entities in front of other entities.
Disclosure: I am not a BVH expert and I used Claude. I looked over the code as best I could and tested with it, but it's possible that there is a mistake that slipped through. I apologize for that, but I have so many things on my platet right now and I don't have a week to study the BVH implementation. The good news is that aside from the really tiny 2-3 node fix and the tiny change to AABB.IntersectRay() (which I can verify as correct), it's just 3 new functions which can't break anything existing so the risk is low. I will be using QueryRayClosest() for all my projectiles and QueryRayAny() for my turret LOS tests soon, so it'll be getting a lot of testing.