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

allow classes inheriting from image display access to more state #1161

Closed
wants to merge 1 commit into from
Closed

allow classes inheriting from image display access to more state #1161

wants to merge 1 commit into from

Conversation

daizr
Copy link

@daizr daizr commented Oct 10, 2017

needed when adding roi selection overlay to image display here https://github.com/dispatch-ai/rviz-image-roi

@daizr daizr changed the title allow inheriting classes access to more state allow classes inheriting from image display access to more state Oct 10, 2017
Copy link
Contributor

@dhood dhood left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @daizr, we're happy to consider your use case. However, we should only expose the minimum to subclasses, so we can't accept this proposal as is.

Would just moving texture_ and render_panel_ be sufficient? (these are what have been moved to protected for CameraDisplay in ddd7f35). If so, that would be an acceptable change.

@dhood
Copy link
Contributor

dhood commented Apr 12, 2018

replaced by https://github.com/ros-visualization/rviz/pull/1221/files exposing less state ( @daizr appears to have deleted their fork)

@dhood dhood closed this Apr 26, 2018
130s pushed a commit to 130s/rviz that referenced this pull request Aug 21, 2024
…ization#1162)

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
(cherry picked from commit 92023c966414d4d9a044ad8f609a1c6f3ca402d3)

Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants