Refactor Ray-Triangle intersection #1229
Conversation
Benchmark Results (Julia vlts)Time benchmarks
Memory benchmarks
|
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1229 +/- ##
==========================================
- Coverage 87.94% 87.93% -0.01%
==========================================
Files 196 196
Lines 6222 6218 -4
==========================================
- Hits 5472 5468 -4
Misses 750 750 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@halleysfifthinc given that you touched this code recently to improve type stability, I appreciate if you could take a look to make sure we are not missing something. Please let me know if you are busy and I can try to ping other contributors for review 🙂 |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the ray–triangle intersection implementation to match the non-culling branch of the Möller–Trumbore (1997) algorithm, removing intermediate computations and aligning notation with the paper.
- Rewrote
intersectionto use the paper’s step-by-step notation - Removed the
Tintermediate until after determinant inversion - Updated test comment for clarity
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/intersections.jl | Fixed grammar in a test comment |
| src/intersections/rays.jl | Refactored triangle intersection to follow Möller–Trumbore non-culling algorithm |
|
Is this intersection benchmarked by CI yet? Probably a good idea to add now given that the intent of this PR is at least partially to improve performance, afaict? |
That is a good idea. I will add a benchmark to make sure we don't introduce regressions. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Benchmarks show great speedups when the ray and the triangle do not intersect. Given that this is the most likely configuration, this can make a real difference in practice. |
I refactored the code to follow the exact same steps of the non-culling branch described in the Möller, T. & Trumbore, B. 1997 paper. I understand that this branch will compute the correct intersection even when triangles are back-faces.
The new implementation avoids unnecessary intermediate computations like the variable
Tthat is only computed after the determinant check. It also follows the notation of the paper more closely to facilitate future maintenance.