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.

Sign up for GitHub

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

Remove deprecations: tock #1091

Merged
merged 4 commits into from
Dec 16, 2024
Merged

Remove deprecations: tock #1091

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
852e009
Remove deprecations: tock
caguero Dec 5, 2024
2fe623c
Update Migration.md
caguero Dec 5, 2024
6bd5db5
Tweak
caguero Dec 15, 2024
c383af1
Migration
caguero Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,6 @@ endif()
set(GZ_RENDERING_ENGINE_RELATIVE_INSTALL_DIR
${GZ_LIB_INSTALL_DIR}/gz-${GZ_DESIGNATION}-${PROJECT_VERSION_MAJOR}/engine-plugins
)
set(GZ_RENDERING_ENGINE_INSTALL_DIR
${CMAKE_INSTALL_PREFIX}/${GZ_RENDERING_ENGINE_RELATIVE_INSTALL_DIR})
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we may still need to keep this cmake variable here as I see that these are being used in ogre/src/CMakeLists.txt and ogre2/src/CMakeLists.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we replace these calls with getResourcePath()?

Copy link
Contributor

Choose a reason for hiding this comment

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

getResourcePath() is a C++ function so we won't be able to replace those calls in CMakeLists.txt. I think we can just keep this cmake var here. Other changes in this PR are fine.

Copy link
Contributor Author

@caguero caguero Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry I meant GZ_RENDERING_RESOURCE_PATH, that it's used in some .cc files (although also in optix/src/CMakeLists.txt).

Copy link
Contributor

Choose a reason for hiding this comment

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

oh do you see the macro used in cc files? For example, I see usage like this that it just gets the value directly from the GZ_RENDERING_RESOURCE_PATH environment variable (instead of the macro from config.hh).

Copy link
Contributor

Choose a reason for hiding this comment

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

to clarify, I think we are still keeping the environment variable and only deprecating the macro? Or are we deprecating / removing both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the confusion. I think what we want is to remove just the GZ_RENDERING_RESOURCE_PATH macro and nothing else. We'll keep supporting the environment variables. 6bd5db5


#--------------------------------------
# Find DL if doing relocatable installation
Expand Down Expand Up @@ -196,7 +194,6 @@ else()
endif()

set(GZ_RENDERING_RELATIVE_RESOURCE_PATH ${GZ_DATA_INSTALL_DIR})
set(GZ_RENDERING_RESOURCE_PATH ${CMAKE_INSTALL_PREFIX}/${GZ_RENDERING_RELATIVE_RESOURCE_PATH})

#============================================================================
# Configure the build
Expand Down
14 changes: 14 additions & 0 deletions Migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ Deprecated code produces compile-time warnings. These warning serve as
notification to users that their code should be upgraded. The next major
release will remove the deprecated code.

## Gazebo Rendering 9.x to 10.x

### Removals

1. The macro `GZ_RENDERING_RESOURCE_PATH` is removed. Use
`gz::rendering::getResourcePath()` instead.

1. The macro `GZ_RENDERING_ENGINE_INSTALL_DIR` is removed. Use
`gz::rendering::getEngineInstallDir()` instead.

1. **Ogre2SelectionBuffer**
+ Removed: `bool ExecuteQuery(const int _x, const int _y, Ogre::Item *&_item, math::Vector3d &_point)`
+ Replacement: `bool ExecuteQuery(int _x, int _y, Ogre::MovableObject *&_obj, math::Vector3d &_point)`

## Gazebo Rendering 8.x to 9.x

### Deprecations
Expand Down
4 changes: 0 additions & 4 deletions include/gz/rendering/config.hh.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@

#define GZ_RENDERING_VERSION_HEADER "Gazebo Rendering, version ${PROJECT_VERSION_FULL}\nCopyright (C) 2014 Open Source Robotics Foundation.\nReleased under the Apache 2.0 License.\n\n"

#define GZ_RENDERING_RESOURCE_PATH _Pragma ("GCC warning \"'GZ_RENDERING_RESOURCE_PATH' macro is deprecated, use gz::rendering::getResourcePath() function instead. \"") "${GZ_RENDERING_RESOURCE_PATH}"

#define GZ_RENDERING_ENGINE_INSTALL_DIR _Pragma ("GCC warning \"'GZ_RENDERING_ENGINE_INSTALL_DIR' macro is deprecated, use gz::rendering::getEngineInstallDir() function instead. \"") "${GZ_RENDERING_ENGINE_INSTALL_DIR}"

#cmakedefine GZ_RENDERING_HAVE_OGRE 1
#cmakedefine GZ_RENDERING_HAVE_OGRE2 1
#cmakedefine GZ_RENDERING_HAVE_OPTIX 1
Expand Down
10 changes: 0 additions & 10 deletions ogre2/include/gz/rendering/ogre2/Ogre2SelectionBuffer.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,6 @@ namespace gz
/// \return Returns the Ogre movable object at the coordinate.
public: Ogre::MovableObject *OnSelectionClick(int _x, int _y);

/// \brief Perform selection operation and get ogre item and
/// point of intersection.
/// \param[in] _x X coordinate in pixels.
/// \param[in] _y Y coordinate in pixels.
/// \param[out] _item Ogre item at the coordinate.
/// \param[out] _point 3D point of intersection with the ogre item's mesh.
/// \return True if an ogre item is found, false otherwise
public: bool GZ_DEPRECATED(9) ExecuteQuery(const int _x, const int _y,
Ogre::Item *&_item, math::Vector3d &_point);

/// \brief Perform selection operation and get ogre item and
/// point of intersection.
/// \param[in] _x X coordinate in pixels.
Expand Down
14 changes: 0 additions & 14 deletions ogre2/src/Ogre2SelectionBuffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,20 +421,6 @@ Ogre::MovableObject *Ogre2SelectionBuffer::OnSelectionClick(int _x, int _y)
return obj;
}

/////////////////////////////////////////////////
bool Ogre2SelectionBuffer::ExecuteQuery(const int _x, const int _y,
Ogre::Item *&_item, math::Vector3d &_point)
{
Ogre::MovableObject *obj = nullptr;
bool out = this->ExecuteQuery(_x, _y, obj, _point);

Ogre::Item *item = dynamic_cast<Ogre::Item *>(obj);
if (item)
_item = item;

return out;
}

/////////////////////////////////////////////////
bool Ogre2SelectionBuffer::ExecuteQuery(int _x, int _y,
Ogre::MovableObject *&_obj, math::Vector3d &_point)
Expand Down
Loading