Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add intersects(Sphere, Point) and intersects(Point, Sphere) #584

Merged
merged 1 commit into from
Dec 14, 2021

Conversation

aprokop
Copy link
Contributor

@aprokop aprokop commented Dec 12, 2021

I've been using this for a while when testing different things. It makes sense to add it to the master.

@aprokop aprokop added the enhancement New feature or request label Dec 12, 2021
Copy link
Contributor

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

git grep -n "bool intersects(.*,.*)"
src/details/ArborX_DetailsAlgorithms.hpp:189:constexpr bool intersects(Box const &box, Box const &other)
src/details/ArborX_DetailsAlgorithms.hpp:199:constexpr bool intersects(Point const &point, Box const &other)
src/details/ArborX_DetailsAlgorithms.hpp:209:bool intersects(Sphere const &sphere, Box const &box)
src/details/ArborX_KDOP.hpp:283:KOKKOS_INLINE_FUNCTION bool intersects(Box const &a, KDOP<k> const &b)
src/details/ArborX_KDOP.hpp:289:KOKKOS_INLINE_FUNCTION bool intersects(KDOP<k> const &a, Box const &b)
src/details/ArborX_KDOP.hpp:295:KOKKOS_INLINE_FUNCTION bool intersects(Point const &p, KDOP<k> const &x)
src/details/ArborX_KDOP.hpp:301:KOKKOS_INLINE_FUNCTION bool intersects(KDOP<k> const &a, KDOP<k> const &b)
src/details/ArborX_Ray.hpp:193:bool intersects(Ray const &ray, Box const &box)
test/tstCompileOnlyTypeRequirements.cpp:34:KOKKOS_FUNCTION bool intersects(FakePredicateGeometry, FakeBoundingVolume) { return true; }

A few observations:

  • we only have one occurrence where we provided both intersects(Foo, Bar) and intersects(Bar, Foo) (KDOP and Box) but there we actually need it for the BVH tests. I'd rather not do it for Point and Sphere, especially with no obvious motivation to do so. If we decide we want to support this, we could generate them with a single generic function.
  • we usually have intersects(Geometry, BoundingVolume) so I would suggest to only implement intersects(Point, Sphere)

@aprokop
Copy link
Contributor Author

aprokop commented Dec 12, 2021

So I often play around with construction a hierarchy on points (hacking in storing Points as bounding volumes, or other search structures (kd-tree, grid)), and search using spheres. So I actually need intersects(Sphere, Point) even more. See #586 for example.

I thought it's a no brainer to merge this small patch in. But if you are opposed, I suppose I cherry-pick this patch whenever I need it.

@aprokop aprokop merged commit 686dbf2 into arborx:master Dec 14, 2021
@aprokop aprokop deleted the point_sphere_intersection branch December 14, 2021 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants