Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OgreVector3.h is deprecated in favor of OgreVector.h #1741

Merged
merged 1 commit into from
May 21, 2022

Conversation

lucasw
Copy link
Contributor

@lucasw lucasw commented May 7, 2022

Description

Replace all includes of OgreVector3.h with OgreVector.h for Ogre 1.12 (I noticed this because locus Fuse has Werror and errored on the OgreVector3.h with ogre 1.12).

Checklist

  • If you are addressing rendering issues, please provide:
    • Images of both, broken and fixed renderings.
    • Source code to reproduce the issue, e.g. a YAML or rosbag file with a MarkerArray msg.
  • If you are changing GUI, please include screenshots showing how things looked before and after.
  • Choose the proper target branch: latest release branch, for non-ABI-breaking changes, future release branch otherwise.
    Due to the lack of active maintainers, we cannot provide support for older release branches anymore.
  • Did you change how RViz works? Added new functionality? Do not forget to update the tutorials and/or documentation on the ROS wiki
  • While waiting for someone to review your request, please consider reviewing another open pull request to support the maintainers of RViz. Refer to the RViz Wiki for reviewing guidelines.

@rhaschke
Copy link
Contributor

Replacing OgreVector3.h with more generic Ogre.h isn't a good way to go: this requires including many more files, which slows down compilation. In Ogre 1.12, there is OgreVector.h (without 3), but this is not available in earlier versions...

@lucasw lucasw force-pushed the ogre_vector branch 2 times, most recently from e27ddaa to db9d866 Compare May 14, 2022 17:47
@lucasw
Copy link
Contributor Author

lucasw commented May 14, 2022

It looks like it takes around 24 minutes instead of 20 in the ci system, assuming the ci is consistent.

What do you think of this intermediary include with an ogre version conditional include in it?

@rhaschke
Copy link
Contributor

Using conditional includes is fine. What kind of deprecation warnings do you get? From which source file?

@lucasw
Copy link
Contributor Author

lucasw commented May 18, 2022

In file included from /home/lucasw/base_catkin_ws/src/ros/rviz/src/rviz/robot/robot_link.h:40,
                 from /home/lucasw/base_catkin_ws/src/ros/rviz/src/rviz/robot/robot_joint.cpp:31:
/usr/include/OGRE/OgreVector3.h:2:62: note: ‘#pragma message: /usr/include/OGRE/OgreVector3.h is deprecated, migrate to Ogre.h’
    2 | #pragma message( __FILE__ " is deprecated, migrate to Ogre.h")

It's Ogre 1.12.10

@rhaschke
Copy link
Contributor

I added a CI build job for OGRE 1.12. Please rebase your branch onto latest noetic-devel to see remaining issues. Thanks.

@lucasw lucasw changed the title OgreVector3.h is deprecated in favor of Ogre.h OgreVector3.h is deprecated in favor of OgreVector.h May 19, 2022
Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I don't see a warning about OgreVector3.h anymore. Do you?

@lucasw lucasw marked this pull request as ready for review May 20, 2022 23:05
@lucasw
Copy link
Contributor Author

lucasw commented May 20, 2022

Nope, just a bunch of the deprecated-copy warnings on the jenkins focal build- but that must be failing everywhere, not related to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants