Skip to content

Add optimized is_in_half_space_identity, contains_aabb_identity, and intersects_obb_identity#21249

Merged
mockersf merged 12 commits intobevyengine:mainfrom
Saratii:main
Dec 8, 2025
Merged

Add optimized is_in_half_space_identity, contains_aabb_identity, and intersects_obb_identity#21249
mockersf merged 12 commits intobevyengine:mainfrom
Saratii:main

Conversation

@Saratii
Copy link
Contributor

@Saratii Saratii commented Sep 28, 2025

Objective

This adds an optimized version of is_in_half_space, Frustum::contains_aabb_identity, Aabb::is_in_half_space_identity, and Frustum::intersects_obb_identity that takes advantage of how calling with IDENTITY (common) reduces the amount of matrix multiplications.

Solution

Add a specialized function without touching the original that assumes Identity was passed to simplify math.

Testing

I use this function extensively in my own project. I asserted the old function called with identity always returns the same as my new function.
I bench marked and profiled the usage in my real application and noticed a 16% speed up on Linux and 20% on windows for contains_aabb.
I bench marked intersects_obb_identity and noticed a 39% increase on linux and 38% on windows.
Both functions have unit tests that assert calling with identity yields the same result in both versions.

@Saratii
Copy link
Contributor Author

Saratii commented Sep 28, 2025

Please squash commits, I see no button to do so.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible C-Performance A change motivated by improving speed, memory usage or compile times A-Math Fundamental domain-agnostic mathematical operations D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 28, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I like this! Before I approve, can you check if we can get the same performance improvements by using const if branching on the existing methods?

If that fails, please:

  1. Add doc links to these methods from the unspecialized versions.
  2. Add true doc links back to the unspecialized versions.
  3. Add some basic tests asserting equivalence.

@alice-i-cecile
Copy link
Member

Please squash commits, I see no button to do so.

We always squash on merge: don't worry about it for this project :)

Saratii and others added 5 commits September 30, 2025 16:51
The word "entire" is badly needed here. its not obvious that this returns false when only part of the aabb is in the frustum
Add optimized intersects_obb_identity method for frustum culling.
@Saratii
Copy link
Contributor Author

Saratii commented Oct 1, 2025

@alice-i-cecile 38% increase on windows and 39% increase on linux with a similar identity fn for intersects_obb. intersects_obb also seems to be more heavily used and heavier to compute. I would appreciate another code review :)

@Saratii Saratii changed the title Add optimized is_in_half_space_identity and contains_aabb_identity Add optimized is_in_half_space_identity, contains_aabb_identity, and intersects_obb_identity Oct 1, 2025
@alice-i-cecile alice-i-cecile added this to the 0.18 milestone Oct 1, 2025
@hymm hymm added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 18, 2025
Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
@mockersf mockersf enabled auto-merge December 8, 2025 21:36
@mockersf mockersf added this pull request to the merge queue Dec 8, 2025
Merged via the queue into bevyengine:main with commit 80e17cc Dec 8, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Math Fundamental domain-agnostic mathematical operations C-Feature A new feature, making something new possible C-Performance A change motivated by improving speed, memory usage or compile times D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants