gazebosim / gz-cmake

A set of CMake modules that are used by the C++-based Gazebo projects.
https://gazebosim.org/libs/cmake
Apache License 2.0
27 stars 31 forks source link

Drop support for OGRE2 < 2.2 #302

Closed mjcarroll closed 1 year ago

mjcarroll commented 2 years ago

🦟 Bug fix

Summary

This is a pretty substantial cleanup of the OGRE2/OGRE-Next logic.

Drops support for < 2.2 and unifies the logic between Linux and Windows.

Checklist

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

mjcarroll commented 2 years ago

CC: @darksylinc

This is going to get us most of the way on deb/conda/source compatibility

mjcarroll commented 2 years ago

As an alternative @darksylinc has proposed that we may just be able to use upstream FindOGRE, but I haven't tested this on all platforms yet.

j-rivero commented 2 years ago

Sorry for being late to review this and now that we have released gz-cmake3 seems a disruptive change to me. Actually the diff does not provide very useful information about what was changed so it is more like a new module from scratch.

Probably at this point there are no more consumers than the Garden family but it would be great to preserve the module as it is if it is not fully compatible with the one in the current code. Several options:

Whatever you prefer @mjcarroll works for me.

j-rivero commented 2 years ago

As an alternative @darksylinc has proposed that we may just be able to use upstream FindOGRE, but I haven't tested this on all platforms yet.

Aaah that would be great, indeed.

mjcarroll commented 2 years ago

Probably at this point there are no more consumers than the Garden family but it would be great to preserve the module as it is if it is not fully compatible with the one in the current code. Several options:

That was my thought as well. gz-cmake3 should only be used by the garden collection (unless there are other people out there using it). I think default behavior should be okay in this case, but I'm happy to preserve the functionality as well.

Since there is no rush, lets hold on this until I have an opportunity to test upstream cmake logic and see if that can replace this.

iche033 commented 1 year ago

I tested building gz-rendering7 with the changes in this PR and also https://github.com/gazebo-forks/ogre-next/tree/gazebo/v2-3 built from source in a colcon workspace. I saw that gz-rendering was able to find ogre-next 2.3 but had issue with include paths. I get a few of these compile errors:

/home/iche/code/ogre_ws/install/include/OGRE-Next/Hlms/Unlit/OgreHlmsUnlit.h:32:10: fatal error: OgreHlmsBufferManager.h: No such file or directory
   32 | #include "OgreHlmsBufferManager.h"