Skip to content

Math: add AABB and bounding sphere algorithms - #557

Closed
pezcode wants to merge 5 commits into
mosra:masterfrom
pezcode:bounding-volume
Closed

Math: add AABB and bounding sphere algorithms#557
pezcode wants to merge 5 commits into
mosra:masterfrom
pezcode:bounding-volume

Conversation

@pezcode

@pezcode pezcode commented Apr 6, 2022

Copy link
Copy Markdown
Contributor

👋 This PR adds a function for generating (approximate) bounding spheres to MeshTools. It lives in the new BoundingVolume.h header (+ matching .cpp).

The algorithm used is "Bouncing Bubble: A fast algorithm for Minimal Enclosing Ball problem", specifically the "Ritter substitution algorithm" proposed in the paper. Time complexity between Ritter and this algorithm is identical (3 passes over all position values) but it produces spheres with smaller radius. One side effect of this algorithm is that the radius is always at least some minimal hard-coded value, currently set to TypeTraits<Float>::epsilon(). Since the use case for this function is primarily expected to be some sort of culling, this seems acceptable to me.

Because it was pretty straight-forward, I also added a function for generating axis-aligned bounding boxes. It's just a wrapper around minmax() but it doesn't hurt to have.

@mosra mosra added this to the 2022.0a milestone Apr 6, 2022

@mosra mosra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is great, thank you!

Mostly just minor / style things, nothing serious.

Comment thread src/Magnum/MeshTools/BoundingVolume.cpp Outdated
Comment thread src/Magnum/MeshTools/BoundingVolume.cpp Outdated
Comment thread src/Magnum/MeshTools/BoundingVolume.cpp
Comment thread src/Magnum/MeshTools/BoundingVolume.cpp Outdated
Comment thread src/Magnum/MeshTools/BoundingVolume.h
Comment thread src/Magnum/MeshTools/Test/BoundingVolumeTest.cpp Outdated
Comment thread src/Magnum/MeshTools/Test/BoundingVolumeTest.cpp
Comment thread src/Magnum/MeshTools/Test/BoundingVolumeTest.cpp Outdated
Comment thread src/Magnum/MeshTools/BoundingVolume.h Outdated
Comment thread src/Magnum/MeshTools/Test/BoundingVolumeTest.cpp Outdated
Comment thread src/Magnum/MeshTools/Test/BoundingVolumeTest.cpp Outdated
@pezcode

pezcode commented Apr 6, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review, much appreciated 😊 There's one open question regarding numerical stability, I'll get back to you on that after some reading.

Note: I force-pushed all the minor fixes into the initial commit (since the commit message was wrong, it had the Math: prefix) and the new test cases/benchmarks into extra commits.

@codecov

codecov Bot commented Apr 6, 2022

Copy link
Copy Markdown

Codecov Report

Merging #557 (4694b59) into master (7f4500d) will decrease coverage by 17.16%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master     #557       +/-   ##
===========================================
- Coverage   83.84%   66.67%   -17.17%     
===========================================
  Files         523      483       -40     
  Lines       33893    30070     -3823     
===========================================
- Hits        28417    20050     -8367     
- Misses       5476    10020     +4544     
Impacted Files Coverage Δ
src/Magnum/MeshTools/BoundingVolume.cpp 100.00% <100.00%> (ø)
src/Magnum/GL/Renderer.h 0.00% <0.00%> (-100.00%) ⬇️
src/Magnum/Text/Renderer.h 0.00% <0.00%> (-100.00%) ⬇️
src/Magnum/Audio/Extensions.h 0.00% <0.00%> (-100.00%) ⬇️
src/Magnum/MeshTools/Compile.h 0.00% <0.00%> (-100.00%) ⬇️
src/Magnum/DebugTools/BufferData.cpp 0.00% <0.00%> (-100.00%) ⬇️
src/Magnum/DebugTools/ForceRenderer.h 0.00% <0.00%> (-100.00%) ⬇️
src/Magnum/DebugTools/ObjectRenderer.h 0.00% <0.00%> (-100.00%) ⬇️
src/Magnum/DebugTools/ForceRenderer.cpp 0.00% <0.00%> (-100.00%) ⬇️
src/Magnum/DebugTools/ResourceManager.h 0.00% <0.00%> (-100.00%) ⬇️
... and 349 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f4500d...4694b59. Read the comment docs.

@mosra

mosra commented Apr 8, 2022

Copy link
Copy Markdown
Owner

Thank you! Merged as 7f4500d...90b5379. While cross-linking those with the algorithms in Math::Intersection, I realized the AABB calculating function could be named boundingRange() instead to be consistent with the intersection algos, so I renamed it. Besides that, I left the public header at just forward declarations to not drag along unneeded dependencies when people would want to use just one of the APIs.

@mosra mosra closed this Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

2 participants