Enable ray query for depth, segmentation and thermal cameras

[srmainwaring]

🦟 Bug fix

Fixes #414

Summary

This PR enables depth, segmentation and thermal cameras to set a ray query for the ogre and ogre2 render engines.

The approach Introduces mix-in classes to provide access to underlying Ogre objects in the OgreCamera / Ogre2Camera and related camera classes.

Update the mouse handling in the segmentation_camera and thermal_camera examples.

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

[codecov]

Codecov Report

Merging #483 (2b2e72b) into main (50410bf) will decrease coverage by 0.03%.
The diff coverage is 30.55%.

❗ Current head 2b2e72b differs from pull request most recent head a5c1aaf. Consider uploading reports for the commit a5c1aaf to get more accurate results

@@            Coverage Diff             @@
##             main     #483      +/-   ##
==========================================
- Coverage   53.52%   53.48%   -0.04%     
==========================================
  Files         194      198       +4     
  Lines       19655    19679      +24     
==========================================
+ Hits        10520    10526       +6     
- Misses       9135     9153      +18     

Impacted Files	Coverage Δ
...ude/ignition/rendering/ogre/OgreObjectInterface.hh	0.00% <0.00%> (ø)
ogre/src/OgreCamera.cc	0.00% <0.00%> (ø)
ogre/src/OgreDepthCamera.cc	0.00% <0.00%> (ø)
ogre/src/OgreObjectInterface.cc	0.00% <0.00%> (ø)
ogre/src/OgreRayQuery.cc	0.00% <0.00%> (ø)
ogre/src/OgreThermalCamera.cc	0.00% <0.00%> (ø)
ogre2/src/Ogre2DepthCamera.cc	86.96% <0.00%> (-0.34%)	⬇️
ogre2/src/Ogre2ObjectInterface.cc	0.00% <0.00%> (ø)
ogre2/src/Ogre2SegmentationCamera.cc	74.86% <0.00%> (-0.81%)	⬇️
ogre2/src/Ogre2ThermalCamera.cc	89.00% <0.00%> (-0.48%)	⬇️
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50410bf...a5c1aaf. Read the comment docs.

[srmainwaring]

Converting to draft: while this works I'm concerned that using RTTI will result in maintenance problems later.

Using a virtual function to access the Ogre::Camera would be better, but it's not clear to me the on best way to alter the inheritance structure or introduce a mix-in class.

(There is also a problem with variable shadowing in the if - else clause which can be fixed with additional scoping if we did want to proceed with this approach).

[iche033]

looks like the problem is that the depth, thermal, segmentation camera etc does not derive from OgreCamera / Ogre2Camera so it can't access the camera member variable.

Not sure what the consequence of changing them to derive from OgreCamera / Ogre2Camera but that's one possible option.

Another workaround I can think off is to add a function function called OgreObject() to OgreSensor / Ogre2Sensor that returns an Ogre::MovableObject object, and in the derived Ogre/Ogre2 classes just return the Ogre::Camera object. In the ray query class, you can then call the OgreObject() function and cast it to a Ogre::Camera.

open to other ideas.

[srmainwaring]

Another workaround I can think off is to add a function function called OgreObject() to OgreSensor / Ogre2Sensor that returns an Ogre::MovableObject object, and in the derived Ogre/Ogre2 classes just return the Ogre::Camera object. In the ray query class, you can then call the OgreObject() function and cast it to a Ogre::Camera

This might be a good way forward. It retains some type safety and doesn't require a change to the inheritance structure. All the ignition cameras contain the ogre camera object required so can implement the function. We could add a parameter to request the type of object needed (similar to the getCustomObject() method on the Ogre::TextureGpu class that we added for Metal support). That allows for other objects to be requested if needed later without a further change to the interface. I'll update the PR with the changes for review.

[iche033]

sounds good, thanks

[srmainwaring]

I took a slightly different tack here: OgreSensor / Ogre2Sensor is concerned with scene nodes rather than movable objects. Rather than alter their purpose I've introduced small abstract mix-in classes to expose the underlying Ogre objects and implemented their methods in the cameras. In some cases these coincide with existing non-virtual methods on the camera class.

The mix-in class has two accessors. A specific one to access an Ogre::Camera and a general one to access an Ogre::MovableObject. I have not implemented the general case (so could remove it), but it may be useful to have if other underlying objects are required and you don't want to break ABI.

Coming up with a mix-in class name that makes sense and does not clash with existing objects is always a challenge, I went with OgreObjectInterface which is a bit clumsy and I'm not tied to it by any means. The camera access method has been chosen to coincide with any existing accessor already used (which happen to be different for the Ogre and Ogre2 render engines).

[iche033]

ok yeah your approach is fine. Given that the OgreMovableObject accessor function is currently not used, one suggestion is to make it non-pure-virtual and just return nullptr inline so that the derived classes do not have to implement it. I think it may break ABI if the derived classes decide to override the function later but I'm leaning towards reducing the overhead of adding new camera classes for now.

[iche033]

I think you'll need to add IGNITION_RENDERING_OGRE2_VISIBLE here for ignition to export the symbols, i.e.

class IGNITION_RENDERING_OGRE2_VISIBLE Ogre2ObjectInterface

[srmainwaring]

Added export declarations and provided a default implementation for the OgreMovableObject method. Removed empty methods from the camera classes.

I'm not sure that a const char* _typename parameter is the best choice, something like the Ogre::IdString would be better but I'm not sure of the Ignition equivalent. Given that the function is a placeholder to signal intent, and that any changes to implement the method in camera classes would require a major release in any case, it's probably ok to leave it as is for now.

[iche033]

works well for me. Just one minor comment, otherwise good to go.