compas-dev/compas

What happened to intersection_sphere_sphere

yck011522 opened this issue · 3 comments

yck011522 commented

@tomvanmele

I noticed in #1340 , you have changed two lines in the function of intersection_sphere_sphere(sphere1, sphere2) to go back to an earlier implementation.
Specifically here:
https://github.com/compas-dev/compas/pull/1340/files#diff-521558cf7170fc7e330b54e5f23c6c50b25dbf147efd2705b7c21e960888aca2L496-L497

    center1, radius1 = sphere1.base, sphere1.radius
    center2, radius2 = sphere2.base, sphere2.radius

back to this:

    center1, radius1 = sphere1
    center2, radius2 = sphere2

This was actually a problem fixed in an earlier issue #1281 where the function would not work with a proper Sphere object. The Sphere object cannot be iterated to give center and radius. This function is breaking something in compas_fab. Is there a reason to go back to that earlier unpack?

tomvanmele commented

yes, the lower level functions are meant to work with pure Python inputs and not rely on the structure of the higher level classes.

the above fix was not consistent with this and therefore created unexpected behaviour.

yck011522 commented

Then does that mean that the Sphere class should be changed to allow center1, radius1 = sphere1 to work?

tomvanmele commented

no, there is no meaningful way in which this can be done. if the sphere/sphere intersection doesn't exist yet on the sphere class it should be added. ideally your codebase then uses one of the two APIs exclusively...

  • 👍1