-
Notifications
You must be signed in to change notification settings - Fork 205
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
Consider region of interest in CameraDisplay #864
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay in getting to review this. Thank you for the addition!
The changes LGTM! There's just two small comments to address. After that, I'll trigger CI :)
rviz_default_plugins/src/rviz_default_plugins/displays/camera/camera_display.cpp
Outdated
Show resolved
Hide resolved
rviz_default_plugins/include/rviz_default_plugins/displays/camera/camera_display.hpp
Outdated
Show resolved
Hide resolved
942aeb7
to
7e4383c
Compare
@jacobperron Thank you very much for your feedback. I added your suggestions. |
7e4383c
to
9b30006
Compare
I applied uncrustify and found that I overlooked one more formatting issue. Now it should be fine. |
rviz_default_plugins/src/rviz_default_plugins/displays/camera/camera_display.cpp
Outdated
Show resolved
Hide resolved
This feature was already implemented in Rviz1
9b30006
to
e638fa2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
this commit considers the region of interest (roi) in CameraInfo.msg. It accounts for the fact that an image was cropped by the camera driver to reduce the required bandwidth. The values in CameraInfo.msg are the same as for the full image. Just the ROI values are set to non-zero values. This was already implemented in Rviz1. So I took these lines.