Move standard geometric functions out of Ray - #1388
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors ray-related geometric operations by moving “standard” functions (e.g., intersects, distance, equals, centroid) out of ArborX::Experimental::Ray’s header-local free functions and into the centralized geometry algorithm dispatch headers, while relocating the associated unit tests into the corresponding geometry test suites.
Changes:
- Moved ray
intersectsimplementations (ray-box, ray-triangle) intosrc/geometry/algorithms/ArborX_Intersects.hppand added corresponding tests totest/tstGeometryIntersects.cpp. - Moved ray
distance(ray-box) intosrc/geometry/algorithms/ArborX_Distance.hppand added corresponding tests totest/tstGeometryDistance.cpp. - Added dispatch support for ray
equalsandcentroid, and removed ray-specific test cases fromtest/tstRay.cpp.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/tstRay.cpp | Removes ray intersects/distance test cases; keeps ray intersection/overlapDistance tests (but adds an unused include). |
| test/tstGeometryIntersects.cpp | Adds templated ray-box and ray-triangle intersects test coverage. |
| test/tstGeometryDistance.cpp | Adds templated ray-box distance test coverage. |
| src/geometry/ArborX_Ray.hpp | Removes ray-box/ray-triangle intersects and ray-box distance from this header; relocates traits placement. |
| src/geometry/algorithms/ArborX_Intersects.hpp | Adds dispatch specializations for ray-box and ray-triangle intersects. |
| src/geometry/algorithms/ArborX_Equals.hpp | Adds dispatch specialization for ray-ray equals (currently incorrect). |
| src/geometry/algorithms/ArborX_Distance.hpp | Adds dispatch specialization for ray-box distance and associated documentation comment. |
| src/geometry/algorithms/ArborX_Centroid.hpp | Adds dispatch specialization for ray centroid. |
Suppressed comments (1)
src/geometry/algorithms/ArborX_Equals.hpp:82
- Ray equality calls l.centroid()/r.centroid(), but ArborX::Experimental::Ray does not define a centroid() member (it has origin()). This will not compile and also changes semantics from the previous ray-ray equals implementation.
return Details::equals(l.centroid(), r.centroid()) &&
l.direction() == r.direction();
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| }; | ||
|
|
||
| // equals sphere-sphere |
| { | ||
| // Returns the first positive value for t such that ray.origin + t * direction | ||
| // intersects the given box. If no such value exists, returns inf. | ||
| // Note that this definiton is different from the standard |
| #include <ArborX_Triangle.hpp> | ||
| #include <algorithms/ArborX_Intersects.hpp> |
dalg24
left a comment
dalg24
left a comment
There was a problem hiding this comment.
I think the original intent was that the user "opt in" and include the Ray header if he wants to use it, but that it is not considered a basic core geometry.
I don't necessarily have a problem with these changes but I would like you to discuss the rationale for that change.
No description provided.