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

Melodic release #1414

Closed
rhaschke opened this issue Sep 1, 2019 · 14 comments
Closed

Melodic release #1414

rhaschke opened this issue Sep 1, 2019 · 14 comments

Comments

@rhaschke
Copy link
Contributor

rhaschke commented Sep 1, 2019

I would like to prepare a new release into Melodic asap. However, because there are many new commits, I would like to get some feedback from the community first to resolve potential regressions. Particularly, @wjwwood, @simonschmeisser, @VictorLamoine, @v4hn could you please use the melodic-devel branch on a daily basis and report (new) issues?

@v4hn
Copy link
Contributor

v4hn commented Sep 2, 2019

Thanks for asking.
Running it, I noticed a regression introduced by #1275 . See #1415 .

Also, I've been fighting a lot with the bugs mentioned in #1409 recently.
I'll test the latest branch with our setup later today.

@simonschmeisser
Copy link
Contributor

There seems to be quite some visual regression. Before:
pastedImage

after:
new

Those are all stls with a color set from urdf.

Also I see a new crash which I'll debug now. We should hold back the release a bit

@rhaschke
Copy link
Contributor Author

rhaschke commented Sep 2, 2019

@simonschmeisser, can you please verify whether the visual regression is due to your #1365?
The only other commit, I can think of, that affects rendering is #1387. If those are not the culprit, could you please do a git bisect. Alternatively, please provide your URDF (and meshes), so that I can do it.
Thanks a lot.

@simonschmeisser
Copy link
Contributor

it worked after #1387 , will test further but first fix the crash

@simonschmeisser
Copy link
Contributor

simonschmeisser commented Sep 2, 2019

So the crash is here:

if ( text_->getMaterial().get() )
with text_ being 0

1   rviz::TextViewFacingMarker::getMaterials                                                                                                                                                                                                                                                                                                                                                                                                text_view_facing_marker.cpp    86   0x7f111d5be0a8 
2   rviz::InteractiveMarkerControl::makeMarkers                                                                                                                                                                                                                                                                                                                                                                                             interactive_marker_control.cpp 122  0x7f111d57ac61 
3   rviz::InteractiveMarkerControl::processMessage                                                                                                                                                                                                                                                                                                                                                                                          interactive_marker_control.cpp 189  0x7f111d57b650 
4   rviz::InteractiveMarker::processMessage                                                                                                                                                                                                                                                                                                                                                                                                 interactive_marker.cpp         198  0x7f111d581a49 
5   rviz::InteractiveMarkerDisplay::updateMarkers                                                                                                                                                                                                                                                                                                                                                                                           interactive_marker_display.cpp 269  0x7f111d5716ff 
6   rviz::InteractiveMarkerDisplay::initCb                                                                                                                                                                                                                                                                                                                                                                                                  interactive_marker_display.cpp 337  0x7f111d571e8b 
7   boost::_mfi::mf1<void, rviz::InteractiveMarkerDisplay, boost::shared_ptr<visualization_msgs::InteractiveMarkerInit_<std::allocator<void>> const>>::operator()                                                                                                                                                                                                                                                                           mem_fn_template.hpp            165  0x7f111d572eca 
8   boost::_bi::list2<boost::_bi::value<rviz::InteractiveMarkerDisplay *>, boost::arg<1>>::operator()<boost::_mfi::mf1<void, rviz::InteractiveMarkerDisplay, boost::shared_ptr<visualization_msgs::InteractiveMarkerInit_<std::allocator<void>> const>>, boost::_bi::rrlist1<boost::shared_ptr<visualization_msgs::InteractiveMarkerInit_<std::allocator<void>> const> const&>>                                                             bind.hpp                       319  0x7f111d572eca 
9   boost::_bi::bind_t<void, boost::_mfi::mf1<void, rviz::InteractiveMarkerDisplay, boost::shared_ptr<visualization_msgs::InteractiveMarkerInit_<std::allocator<void>> const>>, boost::_bi::list2<boost::_bi::value<rviz::InteractiveMarkerDisplay *>, boost::arg<1>>>::operator()<boost::shared_ptr<visualization_msgs::InteractiveMarkerInit_<std::allocator<void>> const> const&>                                                        bind.hpp                       1306 0x7f111d572eca 
10  boost::detail::function::void_function_obj_invoker1<boost::_bi::bind_t<void, boost::_mfi::mf1<void, rviz::InteractiveMarkerDisplay, boost::shared_ptr<visualization_msgs::InteractiveMarkerInit_<std::allocator<void>> const>>, boost::_bi::list2<boost::_bi::value<rviz::InteractiveMarkerDisplay *>, boost::arg<1>>>, void, boost::shared_ptr<visualization_msgs::InteractiveMarkerInit_<std::allocator<void>> const> const&>::invoke function_template.hpp          159  0x7f111d572eca 
11  interactive_markers::SingleClient::checkInitFinished()                                                                                                                                                                                                                                                                                                                                                                                                                      0x7f112d53f2b0 
12  interactive_markers::SingleClient::update()                                                                                                                                                                                                                                                                                                                                                                                                                                 0x7f112d5402cd 
13  interactive_markers::InteractiveMarkerClient::update()                                                                                                                                                                                                                                                                                                                                                                                                                      0x7f112d5351e9 
14  rviz::InteractiveMarkerDisplay::update                                                                                                                                                                                                                                                                                                                                                                                                  interactive_marker_display.cpp 200  0x7f111d56e456 
15  rviz::DisplayGroup::update                                                                                                                                                                                                                                                                                                                                                                                                              display_group.cpp              240  0x7f112dac2758 
16  rviz::VisualizationManager::onUpdate                                                                                                                                                                                                                                                                                                                                                                                                    visualization_manager.cpp      353  0x7f112db8c345 
17  QMetaObject::activate(QObject *, int, int, void * *)                                                                                                                                                                                                                                                                                                                                                                                                                        0x7f1130f7ea45 
18  QTimer::timeout(QTimer::QPrivateSignal)                                                                                                                                                                                                                                                                                                                                                                                                                                     0x7f1130f8b927 
19  QTimer::timerEvent(QTimerEvent *)                                                                                                                                                                                                                                                                                                                                                                                                                                           0x7f1130f8bc88 
20  QObject::event(QEvent *)                                                                                                                                                                                                                                                                                                                                                                                                                                                    0x7f1130f7f56b 
... <Mehr>                                                                                                                                                                                                                                                                                                                                                                                                                                                                                     

@rhaschke
Copy link
Contributor Author

rhaschke commented Sep 2, 2019

@v4hn, this issue might be related to your #1275 as well, not defining the text marker anymore if there is some issue (e.g. an empty text?). @simonschmeisser, can you please provide a yaml or rosbag record of your marker message?

@simonschmeisser
Copy link
Contributor

@rhaschke it is indeed due to my commit #1365 , I found that the stl_loader sets

object->begin( "BaseWhiteNoLighting", Ogre::RenderOperation::OT_TRIANGLE_LIST );

will now check if something similar gets set with the assimp code path

@simonschmeisser
Copy link
Contributor

ok, changing the following

submesh->setMaterialName(material_table[input_mesh->mMaterialIndex]->getName());

to

submesh->setMaterialName("BaseWhiteNoLighting", "rviz");

fixes STL coloring, this should of course be dependent on some property of the mesh, not sure which

@simonschmeisser
Copy link
Contributor

as a test-case you could use https://github.com/ros-industrial/fanuc and then run

roslaunch fanuc_lrmate200ib_moveit_config demo.launch

it should show a bright and friendly orange instead of the gloom dark one

@simonschmeisser
Copy link
Contributor

about the crash, it appears that TextViewFacingMarker discards messages containing purely whitespace:
https://github.com/ros-visualization/rviz/blame/melodic-devel/src/rviz/default_plugin/markers/text_view_facing_marker.cpp#L60
which was introduced in #1275 however there is no check for nullptr in https://github.com/ros-visualization/rviz/blame/melodic-devel/src/rviz/default_plugin/markers/text_view_facing_marker.cpp#L88

Also the name onNewMessage implies that this might be used to update a marker? So changing the caption from non-whitespace to whitespace would get discarded here. I don't think that's correct either?

@rhaschke
Copy link
Contributor Author

rhaschke commented Sep 2, 2019

submesh->setMaterialName("BaseWhiteNoLighting", "rviz");
fixes STL coloring. This should, of course, depend on some property of the mesh. Not sure which.

Would be great, of course, if you could figure out this.

Regarding the crash, @v4hn introduced this white-space-only check in #1275, because they noticed Ogre would crash with such text. Thus, setting an empty text should delete the marker on an update.
@v4hn, could you have a look?

@simonschmeisser
Copy link
Contributor

submesh->setMaterialName("BaseWhiteNoLighting", "rviz");
fixes STL coloring. This should, of course, depend on some property of the mesh. Not sure which.

Would be great, of course, if you could figure out this.

I'll try to investigate sometime later this week what's the difference between a collada mesh and a stl mesh after it has been parsed by assimp. Alternatively we could copy the old behavior and set the material based on file extension

@simonschmeisser
Copy link
Contributor

There are two parts to the coloring issue:

  1. The coloring issues seem to be a known and fixed issue in assimp 4.1.0 (as used on ubuntu bionic): Fix white ambient in STL loader assimp/assimp#2563 a workaround cited there replaces pure white ambient light with pure black one: mosra/magnum-plugins@6444d54#diff-bd6ab8f40c2db2c93f2d75de9e4997ed

  2. We only apply the urdf color to links that have "BaseWhite" or "BaseWhiteNoLighting" as material name:

    if (material_name == "BaseWhite" || material_name == "BaseWhiteNoLighting")

So it appears we need to do some magic based on file extensions?

@rhaschke
Copy link
Contributor Author

Release prepared: ros/rosdistro#22320

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

No branches or pull requests

3 participants