Math: Fix Math::Intersection::sphereFrustum#482
Conversation
Signed-off-by: Squareys <squareys@googlemail.com>
Squareys
force-pushed
the
fix-sphere-frustum
branch
from
1dbe8ab to
47e7f6b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #482 +/- ##
==========================================
- Coverage 77.12% 77.12% -0.01%
==========================================
Files 415 415
Lines 25498 25497 -1
==========================================
- Hits 19665 19664 -1
Misses 5833 5833 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| {0.0f, 1.0f, 0.0f, 0.0f}, | ||
| {0.0f, -1.0f, 0.0f, 10.0f}, | ||
| {0.0f, 0.0f, 1.0f, 0.0f}, | ||
| {0.0f, 0.0f, -1.0f, 10.0f}}; |
There was a problem hiding this comment.
Hmm, so if there's multiple ways how to represent the same plane and the planes can be in any order, maybe the test could be made instanced with testing a few equivalent variants and expecting the same points get always the same result? That could harden it quite nicely.
And that could be done for all *Frustum tests -- I see you already updated planeFrustum(), so the same instancing change could be done to all.
EDIT: and while you're at it, I think including arbitrary (instanced) transformation in both the frustum planes and the points could make sense as well.
There was a problem hiding this comment.
Instanced test for permutating the plane order sounds like it would get out of hand in the test results pretty quickly, I think a loop with iteration description is probably enough?
Re the transformation: Do you mean offsetting/transforming the entire space, frustum and points, sphere's etc by a certain transform?
2 participants
Hi @mosra !
As per #453 (Fix sphere frustum intersection), here's the fix plus extensive tests.
I made a conscious descision to mark "touching" geometries as non-intersection. Even though mathematically, the volume is not empty, it's infinitely small and hence useless for the main use case, which will be frustum culling.
Best,
Jonathan