Skip to content

Improve the performances of BVH-BVH collision detection#858

Open
lmontaut wants to merge 8 commits into
coal-library:develfrom
lmontaut:topic/improve-bvh-bvh-perf
Open

Improve the performances of BVH-BVH collision detection#858
lmontaut wants to merge 8 commits into
coal-library:develfrom
lmontaut:topic/improve-bvh-bvh-perf

Conversation

@lmontaut

Copy link
Copy Markdown
Contributor

Multiple users have recently complained about the performance of BVH-BVH collision detection #857 #837.
This PR impacts BVH-BVH collisions:

I also added a test to make sure the GJK and triangle-triangle specializations are identical.

No API change

This should make BVH-BVH about 2x faster.

// relative transform RT = (R1^T*R2, R1^T*(t2-t1)).
// P1/P2/P3 stay in mesh1's local frame — zero transform cost.
Vec3s Q1r, Q2r, Q3r;
if (RTIsIdentity) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry about the drive by comment, i just wanted to ask.

I believe that one of the tweaks from the original PR set was to make RTIsIdentity a true compile time constant and let the compiler strip the if/else accordingly. It might still be worth doing this type of change (not strictly required in this PR). Or did you find that it was not worth it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @sgayda2 what PR are you refering to?

I'd love to use consexpr but coal is <= c++17. I just tried on my local benchmarks with constexpr and I see almost no gain. I think we can wait for the next coal release to maybe bump to C++17 and use constexpr everywhere.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats my bad, i guess i imagined it. I was thinking about #838 and its subsequent split PRs. Its also possible the compiler is smart enough to recognize it as constant and do the optimization anyway so there wouldn't be any difference in the generated code.

As for coal being <= c++17 i did not pay enough attention to that and did not realize this was the case. Definitely moving this to 20+ would be great, but it depends on the users/deps.

@lmontaut

Copy link
Copy Markdown
Contributor Author

Note: #846 proposed to use a different algo than the one used in this PR for triangle-triangle intersection (for now we use SAT).
I tried it, and it's a little bit better but it does not deal with coplanar triangles + it's harder to maintain. If someone wants to use the Möller algo from #846 but deal with its edge cases, you are more than welcome.

@facontidavide

Copy link
Copy Markdown

Even if you closed all my PRs, I hope they provided some source of inspiration 😁

Hopefully I can get a small acknowledgement in the release note/ changeling?

lmontaut added 8 commits May 17, 2026 12:15
Nice for everything which is not AABB (most used is probably OBBRSS)
Now `enable_distance_lower_bound` means wether or not the computations
try to make the bound as tight as possible.
Specialize the triangle-triangle collision/distance query instead of
using the generic GJK+EPA solver. Also adds a SAT fast path when no
contact info is requested.
@lmontaut lmontaut force-pushed the topic/improve-bvh-bvh-perf branch from 00b1382 to 2d8b52d Compare May 17, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants