Found a bug in BVH8_CPU CombineLeafs when number of triangles in BLAS… #138
Conversation
This file contains 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
… <= 4, and proposed a simple workaround for now
|
Good find. It probably needs a slightly more fundamental fix, but for now, this is a good workaround. Thanks! |
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.
… <= 4, and proposed a simple workaround for now
When using the BVH8_CPU, I found that if you have less than 5 triangles in your BLAS, then, in the CombineLeafs function, there is an infinite loop because since there is an empty node, not marked as leaf, the Combine loops on it indefinitely. Since this kind of node is empty and has a null bound (min and max are set to zero), testing for the surface area before pushing it to the stack fixes the problem (at least for me, I tried in many test scenes of my render engine and nothing changes in the final output).
I duplicated the minimal example to make it buggy, and I added the fix with a define in tiny_bvh.h.
Hope this helps!