Instancing support (shapegroup and instance plugins)

[Speierers]

This is PR ports the shapegroup and instance plugins to Mitsuba 2.

Thanks @nathan96g for his great work!

TODOs

• Minor fixes in current implementation
• Check Embree support
• OptiX support
• Documentation
• Add more tests

[Speierers]

Awesome work @4str0m ! Just a few minor feedbacks.

[wjakob]

This generally looks great -- I left a few low-level comments. Here are a few higher level thoughts on the interfaces:

The amount of backend-specific methods in the Shape class has gotten a little out of hand IMO, and I would like this to be reduced if possible. For example: I think it should be possible to merge init_embree_scene into embree_geometry, and put cleanup logic in destructor without losing anything.

The various OptiX-specific methods in shape.h are very hard to understand (just short 1-liner comments) that don’t really tell us much about their role in the process (this was fine when there was just a single one like optix_geometry that does "the obvious", but now that is no longer the case). Like this one where the comment is really just a placeholder that provides no extra information over the function signature:

/// Fill the OptixBuildInput struct
virtual void optix_build_input(OptixBuildInput&) const;

So: this will need significantly more extensive documentation (A rough guide is probably > 10 lines — a thorough explanation followed by separate documentation for each function parameter explaining its role in relation to the intro). Can you add this? I’d then like to do another pass over these functions to see if it can be condensed a bit more.

Another thing to consider: sometimes such methods are extremely specific to a single plugin like “instance” or “shapegroup” — check out Mitsuba 1. There, the instance actually had its own header file (int the “shape” directory) to add a few function declarations that were not in shape.h. I propose that we follow a similar approach.

Testing needs to be improved: right now we’re only testing scalar RGB mode, and we are only testing an analytic sphere with a single instance. It would be good to test the “wide” backends on CPU and GPU with something more interesting where we will truly find out when something breaks.

[4str0m]

Another thing to consider: sometimes such methods are extremely specific to a single plugin like “instance” or “shapegroup” — check out Mitsuba 1. There, the instance actually had its own header file (int the “shape” directory) to add a few function declarations that were not in shape.h. I propose that we follow a similar approach.

This method was usefull for cases like the optix_accel_handle method, which was only defined inside the ShapeGroup plugin, and used in the Instance plugin. Unfortunately, even though it could be done for the optix_prepare_instance method (that is currently only defined inside the Instance plugin), this method is used in librender (more specifically in the include/mitsuba/render/optix/shapes.h file). This prevents it to be defined in an instance.h header, as the actual implementation would have to be visible to the linker when compiling librender.so.

@wjakob : do you have an idea on how we could keep the current architecture (Instance and Shapegroup classes as separate plugins) while still allowing the optix_prepare_instance method to be specific to the instance's plugin only.

[wjakob]

@4strom: thinking about this a bit more, I think it would only be possible if you declare these as inline method in a hypothetical instance.h/shapegroup.h that is then included called from librender. That is the only way to avoid having to link librender to the plugin. Effectively it would mean that librender knows about the instance layout of these two classes and then performs any method calls directly there. What do you think?

[wjakob]

(This, by the way, is IIRC also how it was handled in Mitsuba 1.)