Accessor methods for geom::Primitives?

[simongeilfus]

I'm using the geom API a lot and was wondering if it would make sense to add accessor methods to the different primitive classes. I understand the idea for classes like ci::geom::Capsule or ci::geom::Cube is more about carrying the geometry and attributes around than representing the primitive itself. But as there's very little reasons for creating more math classes along the existing ci::Sphere, ci::AxisAlignedBox, etc..., I was wondering if we could extend the geom classes to have accessor methods (just to get access to the protected mRadius, mSize, mLength,...).

The downside of this is that the interface would be a tiny bit more cluttered, but on the other hand it would offer more flexibility. It would for example allow us to use the same data for both gl objects and a physics engine. To be honest I don't see a lot of other use cases, except for scene serialisation and physics engine integration. But it's a small change that might open some nice doors.

(Edit: Probably any library or engine that has the concept of geometric primitives would benefit from this small change.)

So for instance having this:

class Sphere : public Source {
  public:
    Sphere();
    Sphere( const ci::Sphere &sphere );

    ....

    vec3        getCenter() const { return mCenter; }
    float       getRadius() const { return mRadius; }
    int     getSubdivisions() const { return mSubdivisions; }

  protected:
        ....
};

Would allow that kind of code :

auto source = geom::Sphere().radius( 10.0f ).center( vec3( 0.0f, 100.0f, 0.0f ) );
auto sphereBatch = gl::Batch::create( source, glslProg );
auto sphereRigidBody = bt::createRigidBody( source, mass );

I can work on a PR if this is something you would accept.

[andrewfb]

I'm open to this - does anybody else have an opinion?

[paulhoux]

To be honest, I am opposed to this. The classes in the geom namespace currently aren't meant to be small, concise, mathematical descriptions of primitives. We have separate AxisAlignedBox and Sphere classes for that. Instead, they are "just" there to create a bunch of vertices and normals.

I would personally like to see a few clean, small classes just for intersection and collision purposes, in the cinder namespace, or a different namespace altogether. Apart from AxisAlignedBox and Sphere, I'd add an OrientedBox, a Cone and a Capsule and that's about it. Those classes would contain functions for efficient culling, intersection and collision, no more and no less.

With that being said, I see no reason why we would not offer simple conversion operators, e.g. from geom::Sphere to ci::Sphere, so that your use case of passing a geom::Sphere to Bullet3D would still work.

[simongeilfus]

Thanks for your feedback guys, I think you're right Paul, ultimately it would make more sense to have them as separated classes. My suggestion is clearly more of a quick patch than a clean solution. To be honest this is definitely not the most urgent thing I would see added to cinder, just some thoughts I had when trying to come up with a nice design for a Bullet block.

That said I'd be happy working on what we think would be the right approach (isn't this the right moment to bring back ci::Triangle on the table as well?).

If we go that road, I guess it would be good to have a base class or at least a shared common interface. Probably something along those lines (based on ci::AxisAlignedBox and ci::Sphere):

bool contains( const vec3 &point ) const;
bool intersects( const Primitive &other ) const;
bool intersects( const Ray &ray ) const;
int intersect( const Ray &ray, float *min, float *max ) const;
void transform( const mat4 &transform );
Primitive transformed( const mat4 &transform ) const;

And obviously add the new types to our ci::Frustum class.

So again this merely my two cents on a not-so-urgent topic.

ps: @paulhoux: I can definitely see some uses for the ci::Cone class. But just out of curiosity, what do you have in mind for the ci::Capsule one?

[paulhoux]

A capsule usually is the best bounding shape for human characters. It's also nice to have when culling linear lights (tubes).

And a ci::Cylinder might come in handy, too :)

[paulhoux]

Regarding ci::Triangle: that might be handy for calculating barycentric coordinates, like I recently had to do when helping someone out with Nanogui. As long as we're not going to implement a gl::draw( ci::Triangle ), because I swear to the big G up there that I will leave the Cinder team if we do.

[paulhoux]

One more thing: I would not add a bunch of methods to ci::Frustum for culling. I'd prefer having free-standing function for that, like:

bool intersects( const Frustum&, const Cone& );

[richardeakin]

I don't see any problems with adding getters for geom properties that are settable, it seems logical to me. However, we have previously discussed that geometric tests (like what AxisAlignedBox, OrientedBox, etc are for) is out of the scope of the ci::geom namespace.

So if there is a ci::Cone and a ci::geom::Cone, the former is for testing while the latter is for generating geometry, however both can expose all of their properties with getters..

[simongeilfus]

Well, I get Paul's point. And if we end up having both classes, there wont really be a need to access the properties of ci::geom classes. On the other hand the only negative outcomes I can see by adding getters is longer classes in GeomIo.h and more semantical confusion between the two types of classes. But well, I guess having a ci::Sphere and ci::geom::Sphere will probably be confusing for a few already.

That said, lets try to think about a few more use cases where those getters might come handy. Most of the time the geom::Objects are used as temporary objects carrying geometry information around. You pretty much use them like you would use other class::Format or class::Options classes to initialise other objects. It might be less frequent but in some cases you might want to keep those geom::Objects around and that's where the lack of getter/setters might become annoying. Let's say that you are working on a small 3d editor with basic editing options. You can create primitives, move them around and change their properties. Not having traditional setters here is more or less ok as it's fairly inexpensive to rebuild the whole object on each modification. But not having access to the properties themselves probably means that you would have to duplicate them outside of the class.

It's not a big deal, but it kind of limit those classes to "initialisation classes".

If this is the intent, and I would understand it, I wouldn't add anything else to those classes. If it's not, I don't see why it would be an issue (apart from the semantical / conceptual one I mentioned above).

[richardeakin]

The only reason why I think getters for those properties haven't been added is that they got overlooked as the geom architecture was the main focus and in constant flux throughout the last development cycle. Same reason many of the methods don't have one line doxygen docs - in my opinion both of these should be added for completeness.

Aside from that, in general the design philosophy is to make cinder types transparent to the user, so if there is a property that is settable, it is also gettable, unless there is some valid reason to not have one or the other. Say you just want to print the dimensions of a geom that was constructed through some abstract factory (like script), it would be annoying not to be able to see the size or dimensions from code.

Concerning the confusion between ci::Sphere and ci::geom::Sphere, this was a conscious choice (I believe) to protect the complexity of the geoms so they don't start getting filled up with testing routines, which could make things like handling the caching of vertices a pain. I think my argument at the time was to place things like ci::Sphere in a separate namespace as well (example ci::math::Sphere), but then one could argue that most of the types in the ci:: namespace are mathy and that is a good place to place things like geometrical testing among other things.

[andrewfb]

I agree; the setters should almost always have matching getters, but geom::Sphere should not have additional "mathematical" functionality on it. And would definitely like to see additional intersection (and similar) functionality on ci::Sphere et al, as well as additional new primitive types.

[morphogencc]

@simongeilfus Did you ever put together any of the changes for this? I'm running into a very similar issue integrating Cinder with NVIDIA PhysX.

It seems to me the quickest "fix" is just to add getters for anything we can set, such as center, radius, or rect in the case of a rectangle.

The next best thing would be some way to convert the GPU object generated from geom:: into a CPU object (ci::Rectf, ci::Circle, etc.). If this were the case it may be beneficial to have the CPU-based classes inherit from a common parent class with the interface @simongeilfus mentioned above (ci::Shape?), but that may be beyond the scope of what makes sense.

I'd be happy to do any combination of the above if it would be accepted in principle.

(tagging @paulhoux and @richardeakin in case their opinions have changed at all in the past 7 years ;) )

[simongeilfus]

What about using the geom::Target interface to send your data to PhysX? If you don't need to keep the primitive representation and would rather have access to the actual geom data I would use geom::Target. Otherwise just make small structs to represent your primitives. You might need a bit of both.

[morphogencc]

How does the geom::Target solve this problem? Are there any examples to clarify the intended use case for it?

It seems like I would only have access to the mesh vertices, which wouldn't provide me with information about the primitives (like radius or center of a circle).

As you demonstrated in your first post, there's a lot of convenience in the geom::Objects all sharing the geom::Source interface, which allows for functions to accept a geom and then determine what type it is and act accordingly. It's unfortunate that the 'math' classes don't share a similar interface; the symmetry would be rather convenient!