Proposal: SphereShape

[jslee02]

This might be silly (and was declined before), but I'd like to try one more time. 😓

We have used EllipsoidShape to represent sphere by setting the radii to be the same since a sphere is a special case of an ellipsoid. However, we could use the speciality of the sphere to take the advantage of cheaper collision checking algorithms for spheres from the collision detectors. We could still use ellipsoid to create sphere shape in the collision detectors when the radii are the same, but modifying the radii not to be the same would require to change the collision detector's shape to ellipsoid (and vice versa) once we have collision shape refresh feature. Still we could handle this using ellipsoid, but I imagine having SphereShape would be more straigtforward and clean in implementation.

[mxgrey]

From an abstraction standpoint it would seem silly for SphereShape to be its own class, independent from EllipsoidShape, but I agree that from an implementation standpoint it completely makes sense, because the machinery used to collision check an EllipsoidShape is so different from the optimized machinery that could get used for SphereShape. 👍

[psigen]

I agree that this is an extremely useful special case to handle independently.

If anything, the EllipsoidShape class might do some special casing to handle itself like a SphereShape (or even internally construct a SphereShape) when radii are equal, but there are many places where we would want to semantically distinguish spheres from ellipsoids.

I guess another alternative would be a lot of boilerplate involving branching on an isSphere check on EllipsoidShape, but I don't see a huge downside to simply having another Shape.

[mxgrey]

I guess another alternative would be a lot of boilerplate involving branching on an isSphere check on EllipsoidShape

I think the main issue here is that we ultimately want to be able to change parameters of shapes (such as dimensionality) on the fly. For most collision detectors, we're implementing EllipsoidShape as a mesh, because most collision detectors don't have an ellipsoid primitive. So tweaking the dimensions of the EllipsoidShape would typically mean adjusting the values in some vertices. On the other hand, if the user suddenly makes the EllipsoidShape into a sphere or away from a sphere, we would have to completely change the collision information for that Shape from a mesh into a primitive, or vice-versa. This is doable, but I think it tends to involve a significant amount of overhead. In general I've noticed that adding new shapes to the collision engines tends to take a noticeable amount of time. This would probably confuse a user who expects a change from the dimensions (1, 2, 1) to (1, 1, 1) to not take much longer than a change from (1, 2, 1) to (1, 2 ,2).

[psigen]

@mxgrey: Totally agree, I was only posting the alternative for completeness.

Trying to shoehorn the two shapes together would require a lot of boilerplate and add performance losses in rather strange places (since the underlying data structure would need to change when moving from an ellipsoid to a sphere and vice versa).

[mkoval]

I also agree with @mxgrey and @psigen. Combining the two shapes introduces a lot of boilerplate and (potentially) performance overhead. It makes sense to add a separate SphereShape class.