Add visual geometry mesh URL by felixvd · Pull Request #2854 · moveit/moveit

felixvd · 2021-08-28T05:16:58Z

Description

I finally got around to implementing this. CollisionObjects can now have a visual geometry, and the pick-place demo now picks up a beer bottle from a table:

2021-08-28.visual.geom.32.mp4

The collision geometry is as simple as before. See the new GUI fields on the left:

There are some limitations about this PR:

All meshes need to have the same scale, because they are scaled by the parameter in the Rviz display. This is because I initially thought that adding visual_geometry_mesh_scaling_factor to each AttachedBody would make the constructor too long, since it is already a little ridiculous. But in retrospect, I should have done it. Or added a VisualGeometry object that both world.cpp and AttachedBody objects use. But I'm low on time, so it is how it is for now.
There are no tests, even though it's easy to lose the geometry during copying (thanks for finding the last bug @simonschmeisser ). I also won't have the time to write any ;(

I would merge this because the functionality is useful, but I would prefer the scale to be defined per object. I don't know when I can make this change though, so if anyone feels so inclined, please be my guest. It should be straightforward to follow the changes in this PR and add a per-object scaling factor.

Some other notes:

I split PlanningSceneRender into a visual and collision node, like the rviz Robot. I originally tried to avoid adding another scene node, but changed my mind after trying it. This is definitely the better way.
I am increasingly convinced that the separation into robot and scene is (and creates) cruft and should go. The conversion from world objects to AttachedBody objects is probably where it is the most obvious. Unifying them would go a long way to de-cluttering the PlanningScene code base. I would refer to Tesseract's messages (and maybe environment) to improve on this. No need to reinvent the wheel. This has come up before (e.g. Representation of Attached vs Non-Attached objects in planning_scene #1980 ) and relates to the scene graph discussions.
The tutorial demo sadly doesn't use this prettier table. Maybe @simonschmeisser will add color for the visual mesh and tackle this some day :)
@simonschmeisser I boldly added your name to a TODO in the code that you seemed most knowledgeable about.

Fixes #2284. Depends on a PR in moveit_msgs and is required for the PR to moveit_tutorials.

Checklist

Required by CI: Code is auto formatted using clang-format
Extend the tutorials / documentation reference
Document API changes relevant to the user in the MIGRATION.md notes
Create tests, which fail without this PR reference
Include a screenshot if changing a GUI

simonschmeisser

some small first remarks while looking at the code as reference for another PR

simonschmeisser · 2021-08-28T20:15:47Z

moveit_core/collision_detection/include/moveit/collision_detection/world.h

+    std::string visual_geometry_mesh_url_;
+
+    /** \brief The pose of the visual geometry (mesh). */
+    Eigen::Isometry3d visual_geometry_pose_;


Suggested change

Eigen::Isometry3d visual_geometry_pose_;

Eigen::Isometry3d visual_geometry_pose_ = Eigen::Isometry3d::Identity();

then you can skip that later

If this creates issues with Eigen::Isometry3d being forward declared (but I don't think it is ...) you could also set the value in the constructor and move the constructor to the cpp file (where it actually belongs ...)

simonschmeisser · 2021-08-28T20:17:00Z

moveit_core/collision_detection/src/world.cpp

+    obj->visual_geometry_mesh_url_ = "";
+    obj->visual_geometry_pose_ = Eigen::Isometry3d::Identity();


Suggested change

obj->visual_geometry_mesh_url_ = "";

obj->visual_geometry_pose_ = Eigen::Isometry3d::Identity();

we did that in the header already, see above

simonschmeisser · 2021-08-28T20:17:54Z

moveit_core/collision_detection/src/world.cpp

+    obj->visual_geometry_mesh_url_ = "";
+    obj->visual_geometry_pose_ = Eigen::Isometry3d::Identity();


Suggested change

obj->visual_geometry_mesh_url_ = "";

obj->visual_geometry_pose_ = Eigen::Isometry3d::Identity();

same here, strings are always empty when created

simonschmeisser · 2021-10-02T08:29:15Z

Requires felixvd/moveit_msgs#1 (or moveit/moveit_msgs#129 ) as well

Open Todos:

Less noise
There is some jumping around when attaching/detaching objects, something is strange with transformations
Decide when to enable/disable mesh_use_embedded_material
Check if the pretty table works ✨

Seriously though, textured objects now work. The tinting sometimes looks really horrible sometimes just a bit horrible, not sure how to fix that

…ch to pass CI

branch to pass CI

codecov · 2021-11-20T22:44:56Z

Codecov Report

Merging #2854 (a97026d) into master (ab42a1d) will increase coverage by 0.04%.
The diff coverage is 71.06%.

@@            Coverage Diff             @@
##           master    #2854      +/-   ##
==========================================
+ Coverage   61.56%   61.60%   +0.04%     
==========================================
  Files         370      370              
  Lines       33147    33220      +73     
==========================================
+ Hits        20405    20461      +56     
- Misses      12742    12759      +17

Impacted Files	Coverage Δ
...tection/include/moveit/collision_detection/world.h	`96.67% <ø> (-3.33%)`	⬇️
...g_scene_interface/src/planning_scene_interface.cpp	`52.20% <ø> (+1.10%)`	⬆️
moveit_core/planning_scene/src/planning_scene.cpp	`63.73% <56.53%> (-0.35%)`	⬇️
moveit_core/collision_detection/src/world.cpp	`89.85% <84.62%> (-0.36%)`	⬇️
...t_state/include/moveit/robot_state/attached_body.h	`100.00% <100.00%> (ø)`
...bot_state/include/moveit/robot_state/robot_state.h	`84.36% <100.00%> (ø)`
moveit_core/robot_state/src/attached_body.cpp	`75.76% <100.00%> (ø)`
moveit_core/robot_state/src/conversions.cpp	`65.79% <100.00%> (+0.79%)`	⬆️
moveit_core/robot_state/src/robot_state.cpp	`52.59% <100.00%> (+0.04%)`	⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab42a1d...a97026d. Read the comment docs.

simonschmeisser · 2021-11-27T22:07:23Z

So colored or textured objects are still not that impressive but it works. Tinting is to strong but I don't understand how to influence that yet.

Let the reviews begin! I will squash some of the commits before merging.

felixvd · 2021-11-29T04:28:36Z

So colored or textured objects are still not that impressive

Speak for yourself!!! I think they look great. Thanks a lot for pursuing this PR.

ahmadabouelainein · 2025-01-28T12:20:07Z

Has this feature been discarded or is still pending?

felixvd force-pushed the add-visual-geometry-url branch from 0d4b637 to 73230a6 Compare August 28, 2021 05:17

This was referenced Aug 28, 2021

Add visual geometry url to CollisionObject moveit/moveit_msgs#129

Open

Add visual geometry to pick-place tutorial moveit/moveit_tutorials#663

Open

simonschmeisser requested changes Aug 28, 2021

View reviewed changes

simonschmeisser mentioned this pull request Aug 28, 2021

use default copy constructor to clone attached object #2855

Merged

felixvd force-pushed the add-visual-geometry-url branch from 73230a6 to 35e7d3b Compare September 17, 2021 12:35

tylerjw mentioned this pull request Oct 14, 2021

Review Felix's PRs #2904

Open

6 tasks

felixvd and others added 13 commits November 20, 2021 21:50

Add visual geometry mesh url to CollisionObject

e56ae8d

Address feedback

87dceff

remove global mesh scaling factor

3142341

individual mesh scaling factor per object

c1f6e70

load and save visual geometry urls

9d0ef74

copy rviz::MeshResourceMarker

389444b

turn marker into entity

14bf189

keep visual geometries and update as needed

8be5e54

Use MeshResourceEntity for AttachedBodies as well

9ad0f05

[TEMPORARY] Adapted rosinstall file to use different moveit_msgs bran…

1dd7c16

…ch to pass CI

Fixup: reorder initialization in AttachedBody

76fac74

[TEMPORARY] Adapted rosinstall file to use different moveit_msgs

50259b5

branch to pass CI

fix ROS_ERROR include missing

d973d8b

simonschmeisser force-pushed the add-visual-geometry-url branch from 38dd937 to d973d8b Compare November 20, 2021 20:53

simonschmeisser added 5 commits November 21, 2021 13:12

Fixup: Handle end of file correctly for old scene files

fe1792d

Fixup: clang-tidy fixes

6beca76

Temporary Robostack: check out matching moveit_msgs

2f7f67c

Fixup: remove debug messages

f8fc69a

enable "use_embed_materials" to support textures

a97026d

simonschmeisser self-requested a review November 27, 2021 22:08

simonschmeisser requested review from rhaschke and v4hn November 27, 2021 22:09

simonschmeisser marked this pull request as ready for review November 27, 2021 22:09

simonschmeisser requested review from mlautman and tylerjw as code owners November 27, 2021 22:09

wxmerkt mentioned this pull request Dec 9, 2021

Hotfix: Add backwards compatibility for old scene serialization format #2986

Merged

v4hn mentioned this pull request May 8, 2023

Refactor the colors portion of the PlanningScene class moveit/moveit2#2132

Closed

rhaschke force-pushed the master branch 5 times, most recently from e1b0ec1 to fdc3b1c Compare July 28, 2023 10:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add visual geometry mesh URL#2854

Add visual geometry mesh URL#2854
felixvd wants to merge 18 commits intomoveit:masterfrom
felixvd:add-visual-geometry-url

felixvd commented Aug 28, 2021

Uh oh!

simonschmeisser left a comment

Uh oh!

simonschmeisser Aug 28, 2021

Uh oh!

simonschmeisser Aug 28, 2021

Uh oh!

simonschmeisser Aug 28, 2021

Uh oh!

simonschmeisser Aug 28, 2021

Uh oh!

simonschmeisser commented Oct 2, 2021 •

edited

Loading

Uh oh!

codecov bot commented Nov 20, 2021 •

edited

Loading

Uh oh!

simonschmeisser commented Nov 27, 2021

Uh oh!

felixvd commented Nov 29, 2021

Uh oh!

ahmadabouelainein commented Jan 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	Eigen::Isometry3d visual_geometry_pose_;
	Eigen::Isometry3d visual_geometry_pose_ = Eigen::Isometry3d::Identity();

		obj->visual_geometry_mesh_url_ = "";
		obj->visual_geometry_pose_ = Eigen::Isometry3d::Identity();

Conversation

felixvd commented Aug 28, 2021

Description

Checklist

Uh oh!

simonschmeisser left a comment

Choose a reason for hiding this comment

Uh oh!

simonschmeisser Aug 28, 2021

Choose a reason for hiding this comment

Uh oh!

simonschmeisser Aug 28, 2021

Choose a reason for hiding this comment

Uh oh!

simonschmeisser Aug 28, 2021

Choose a reason for hiding this comment

Uh oh!

simonschmeisser Aug 28, 2021

Choose a reason for hiding this comment

Uh oh!

simonschmeisser commented Oct 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

simonschmeisser commented Nov 27, 2021

Uh oh!

felixvd commented Nov 29, 2021

Uh oh!

ahmadabouelainein commented Jan 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simonschmeisser commented Oct 2, 2021 •

edited

Loading

codecov bot commented Nov 20, 2021 •

edited

Loading