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

Add configuration for Image to be shown without proper TF/fixed frame #639

Closed
RemiRigal opened this issue Jan 25, 2021 · 7 comments · Fixed by #788
Closed

Add configuration for Image to be shown without proper TF/fixed frame #639

RemiRigal opened this issue Jan 25, 2021 · 7 comments · Fixed by #788
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@RemiRigal
Copy link

I think that it would be nice to be able to see an image with the Image tool even when the frame_id provided by the image header is not found in the TF tree or does not match the fixed frame in Rviz.

It could maybe just be a checkbox to enable in the Image tool ?

@clalancette clalancette added the help wanted Extra attention is needed label Feb 11, 2021
@jacobperron jacobperron added the enhancement New feature or request label Mar 12, 2021
@bergercookie
Copy link

bergercookie commented Aug 10, 2021

I think this is going to be an issue especially with Visual SLAM algorithms, e.g., orbslam. When a vSLAM algorithm loses tracking, e.g., due to poor visual information, rapid motion etc. they will stop publishing updates for their corresponding frame until they relocalise / regain tracking. During that duration where tracking is lost, users of rviz wouldn't see any images whatsoever because there wouldn't be a valid link of the frame to the rest of the TF tree.

  • Out of cusiosity, what's the rationale behind this? Why not show images that don't have an associated frame? rviz1 was happy to render images regardless of their frame.
  • @clalancette , @jacobperron , roughly what changes would need to be made to show images without an associated frame?

@jacobperron
Copy link
Member

Out of cusiosity, what's the rationale behind this? Why not show images that don't have an associated frame? rviz1 was happy to render images regardless of their frame.

TBH, I'm not sure this was a conscious decision. It may be more of a bug. It makes sense to me to ignore the fixed from for the image display.

Other displays actually check if the transform exists before deciding to render anything, e.g.

if (!context_->getFrameManager()->transform(msg->header, msg->pose.pose, position, orientation)) {

However, the ImageDisplay does not do such a check, so I'm not sure why images are not being rendered...:

void ImageDisplay::processMessage(sensor_msgs::msg::Image::ConstSharedPtr msg)
{
bool got_float_image = msg->encoding == sensor_msgs::image_encodings::TYPE_32FC1 ||
msg->encoding == sensor_msgs::image_encodings::TYPE_16UC1 ||
msg->encoding == sensor_msgs::image_encodings::TYPE_16SC1 ||
msg->encoding == sensor_msgs::image_encodings::MONO16;
if (got_float_image != got_float_image_) {
got_float_image_ = got_float_image;
updateNormalizeOptions();
}
texture_->addMessage(msg);
}

Maybe it's something to do with the texture?

@helenol
Copy link

helenol commented Oct 5, 2021

+1 to this topic, blocking development for me and a huge regression from rviz1. Why would I need my whole TF tree to be resolved correctly to see 2D images?

@helenol
Copy link

helenol commented Oct 5, 2021

It appears to be because of the TF filter on incoming image messages? https://github.com/ros2/rviz/blob/ros2/rviz_default_plugins/include/rviz_default_plugins/displays/image/image_transport_display.hpp#L117-L140 So this is explicitly added? Why?

@jacobperron
Copy link
Member

@helenol Good find! In the ROS 1 implementation, the display's base class provides a method for opting into the TF filter:

https://github.com/ros-visualization/rviz/blob/a4919f52fb37da074fcea1648e257691037a3feb/src/rviz/image/image_display_base.h#L89-L94

https://github.com/ros-visualization/rviz/blob/a4919f52fb37da074fcea1648e257691037a3feb/src/rviz/image/image_display_base.cpp#L182-L193

The ImageDisplay does not enable the TF filter, however the CameraDisplay does:

https://github.com/ros-visualization/rviz/blob/a4919f52fb37da074fcea1648e257691037a3feb/src/rviz/default_plugin/camera_display.cpp#L249

I think we should do something similar in the ROS 2 version: add an option to toggle the filter on/off.

@helenol Would you like to contribute a pull request? I'm happy to review one.

jacobperron added a commit that referenced this issue Oct 28, 2021
Fixes #639

Only the CameraDisplay cares about TF data, so this change moves the TF filter logic into the CameraDisplay.

This allows users to view images with the ImageDisplay no matter what the fixed frame is set to.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Member

After a discussion with @wjwwood, I think removing the TF filter from ImageTransportDisplay is a good solution. Since it's only the CameraDisplay that cares about TF information, we can move the TF filter there. See #788

@andreasBihlmaier
Copy link

+1 on changing this behavior back to ROS1 behavior: Show images no matter what is available in TF.

At the very least, do not show "(checkmark-symbol) Status: OK", when it should be "(error-symbol) Status: Missing TF transform".

jacobperron added a commit that referenced this issue Jan 13, 2022
* Remove TF filter from ImageTransportDisplay

Fixes #639

Only the CameraDisplay cares about TF data, so this change moves the TF filter logic into the CameraDisplay.

This allows users to view images with the ImageDisplay no matter what the fixed frame is set to.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants