Hydro devel #25

dornhege · 2015-02-12T13:21:06Z

Fixing a couple of things with handling convex meshes. Mainly that qhull only returns proper triangles when called with "Qt".
It might be a good idea to also incorporate this into indigo, which I can't test.

skohlbr · 2015-02-27T22:33:41Z

I merged the patch into Indigo (https://github.com/team-vigir/geometric_shapes/tree/indigo-devel) and tested. Unfortunately, I still get the infinite recursion as described in moveit/moveit_ros#567. I'm not sure if things improved (i.e. if it crashes only 5% as opposed to 10% of the time or so :) ). I double checked the code from the PR is indeed used and it still crashes.

I can't easily make my test available, but it mainly involves adding a mesh collision object and repeatedly moving it around.

This causes a process to run out of file handles, where the next fopen call returns nullptr, which is passed into qhull, which segfaults. Fixes moveit/moveit_ros#481 and fixes moveit/moveit_ros#526.

mikeferguson · 2015-03-05T11:34:48Z

@dornhege I chatted with Ioan today, and will try to get this (and indigo devel) merged and released over the upcoming weekend.

dornhege · 2015-03-05T11:51:33Z

Sounds good!

davetcoleman · 2015-03-05T17:32:05Z

I'm testing this now on Indigo

davetcoleman · 2015-03-05T17:50:11Z

I ran this just now and I'm not sure how to verify it improves anything. It seems to not have changed anything - the PlanningScenePlugin is still slow to load large meshes over ROS messages. What should I look for?

My demo:

dornhege · 2015-03-05T18:22:48Z

It depends, which of the commits you're checking. Nothing speeds things up, technically it should make it slower as triangulation is enabled.
The main thing would be: Enable sending the monitored scene and watch the file handles in /proc//fd without the last commit these should keep incrementing.
The wrong triangle generation for meshes is not in the planning scene's mesh, but in the ones generated for them, i.e. bodies::ConvexMesh. There was no display, yet, so I displayed them manually as triangle marker. Without 2545... they are broken. 840ce... adds a function that also orients faces for display. This has to be called manually.

davetcoleman · 2015-03-05T18:26:55Z

I'm checking all of the commits. I'm loading each (high res) mesh into my node's planning scene then pushing it over the planning scene topic (using the planning scene monitor) to rviz where I have the PlanningScenePlugin displaying them like in the picture above. Its still not clear to me what was previously broken, I was assuming this PR would increase performance (an issue that I have).

I did notice a slight speed up when I first merged just the file handle fix (in the indigo-devel commit), but the other commits in this PR are unclear to me still.

Sorry if I'm being slow to understand!

dornhege · 2015-03-05T18:34:52Z

Sorry, no performance improvements :(
When you define padding/scaling parameters for, e.g., a point cloud octomap updater, from these meshes, a bodies::ConvexMesh object is created. This is, where scaling and padding are applied, and that is the mesh, which has broken triangles. As that is never displayed, no one ever knew this. The triangles are also not used by any code in moveit, so besides the undefined behavior, nothing is observable.
I have a hacked point_containment_filter that sends markers for these. I can push that from my work machine tomorrow if you're interested.

davetcoleman · 2015-03-05T19:54:33Z

I've observed incorrect/invisible collisions before in the collision_detection, but are you saying this convex mesh isn't used for that purpose?

In an ideal world we'd have a checkbox in the PlanningScene plugin in Rviz that allows us to visualize the convex mesh version of the planning scene...

dornhege · 2015-03-06T11:38:53Z

I've put my debug code in here:
https://github.com/dornhege/moveit_ros/tree/visualize_convex_mesh

It sends a scaled_bodies marker. Without the patch these look broken.

Unfortunately, when padding is used, there is no proper and simple way to produce a correctly padded mesh. The current implementation is not correct, and the planeProjection function that I added is also not fully correct.
The mesh isn't directly used in the library, which is why this never mattered, although the way that the padding is currently applied can get unexpected results for meshes with acute angles, but that is a different issue.

I have only looked in scaling/padding for removing things from the octomap. That is where this is used. I'm not sure if it is used for collision tests.

davetcoleman · 2015-03-24T05:31:01Z

I like that you've made a visualizer for the padded meshes - I'm really into creating effective debug visualizations for MoveIt! components. I'd like to look into this myself but, since I don't have a direct application for it at the moment, can't find time to work on the new feature. But would you be interested in cleaning this up more such that there is an optional feature for publishing this mesh, that is enabled/disabled based on some parameter or flag? Then we could merge this into the main code

dornhege · 2015-03-31T09:34:08Z

I wouldn't mind cleaning this up at all. My motivation was to visualize what's going on. However, I won't have time/knowledge to do this really properly. I'm basically missing a nice entry point for the visualization. Right now I just hook into the main maskContainment method in the point_containment_filter and bring up a publisher, which is quite ugly.

This should still be fine for debugging purposes, if it is parametrized, i.e., during normal operation it won't matter performance-wise. If you believe that a cleaned-up version of this will be merged, I'll do it.

davetcoleman · 2015-03-31T16:08:26Z

@mikeferguson you said:

I chatted with Ioan today, and will try to get this (and indigo devel) merged and released over the upcoming weekend.

any progress? I'm ok with merging this PR as I've been running it for a month with no problems.

we should also get it merged into the indigo branch

@dornhege I'm always happy to merge passive features (that can be turned on) that help improve visualization and debugging. I believe a cleaned-up version of your commit will be merged

isucan · 2015-03-31T16:34:15Z

src/bodies.cpp

@@ -660,13 +660,35 @@ bool bodies::ConvexMesh::containsPoint(const Eigen::Vector3d &p, bool verbose) c
  if (bounding_box_.containsPoint(p))
  {
    Eigen::Vector3d ip(i_pose_ * p);
-    ip = mesh_data_->mesh_center_ + (ip - mesh_data_->mesh_center_) * scale_;
+    ip = mesh_data_->mesh_center_ + (ip - mesh_data_->mesh_center_) / scale_;


Is this in sync with the scale of other objects? Not sure division is what we want

Just to be clear: This will scale the mesh UP, i.e. a scale of 1.5 will make it appear 1.5 times larger. This is what we want, right? If I understand what's happening here correctly, it transforms the coordinates to the internally unscaled mesh, thus they need to be scaled down.

I believe the main reason this was never discovered is that the bounding_box check happens before. The bounding_box is already scaled up and thus catches many cases.

I believe this issue demonstrates a bug due to scale being inverted prior to above fix: moveit/moveit_ros#537

isucan · 2015-03-31T16:37:30Z

This looks fine to me, but I would feel much better if someone tried this and confirmed it works. My main concern is with the usage of scale -- perhaps I did not understand how it works, but it should be kept in sync with how it is used for other bodies.

dornhege · 2015-04-09T12:53:33Z

@davetcoleman I have cleaned the visualize_convex_mesh branch and opened moveit/moveit_ros#574.

mikeferguson · 2015-05-01T04:38:18Z

Seems everyone who has tested this is happy with the changes -- as we are running out of time before hydro is EOL, I'll merge and release

Hydro devel

Forward port #25 into indigo-devel

mikeferguson · 2015-05-01T04:41:39Z

ported to indigo-devel here: #34

dornhege added 3 commits February 12, 2015 13:29

Fixed inverted scale for convex meshes inside check

350a67b

produce actual triangles for qhull mesh

2545759

add functions for better display of convex meshes

840ced6

skohlbr mentioned this pull request Feb 27, 2015

Adding a mesh object to the planning scene eventually causes a SIGSEGV moveit/moveit_ros#526

Closed

skohlbr mentioned this pull request Feb 27, 2015

[perception] padding scale parameters inverted for collision objects moveit/moveit_ros#537

Closed

Prevent every mesh generation opening a new file handle.

be20ca7

This causes a process to run out of file handles, where the next fopen call returns nullptr, which is passed into qhull, which segfaults. Fixes moveit/moveit_ros#481 and fixes moveit/moveit_ros#526.

dornhege mentioned this pull request Mar 3, 2015

Prevent every mesh generation opening a new file handle. #26

Merged

mikeferguson mentioned this pull request Mar 15, 2015

Release into ROS Jade #27

Closed

6 tasks

isucan reviewed Mar 31, 2015
View reviewed changes

This was referenced Apr 14, 2015

Revert "Visualize convex mesh" moveit/moveit_ros#576

Merged

Visualize convex mesh moveit/moveit_ros#574

Merged

mikeferguson added a commit that referenced this pull request May 1, 2015

Merge pull request #25 from dornhege/hydro-devel

fce1da0

Hydro devel

mikeferguson merged commit fce1da0 into moveit:hydro-devel May 1, 2015

mikeferguson added a commit that referenced this pull request May 1, 2015

Merge pull request #34 from ros-planning/indigo_25

dbf7201

Forward port #25 into indigo-devel

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hydro devel #25

Hydro devel #25

dornhege commented Feb 12, 2015

skohlbr commented Feb 27, 2015

mikeferguson commented Mar 5, 2015

dornhege commented Mar 5, 2015

davetcoleman commented Mar 5, 2015

davetcoleman commented Mar 5, 2015

dornhege commented Mar 5, 2015

davetcoleman commented Mar 5, 2015

dornhege commented Mar 5, 2015

davetcoleman commented Mar 5, 2015

dornhege commented Mar 6, 2015

davetcoleman commented Mar 24, 2015

dornhege commented Mar 31, 2015

davetcoleman commented Mar 31, 2015

isucan Mar 31, 2015

dornhege Mar 31, 2015

skohlbr Mar 31, 2015

isucan commented Mar 31, 2015

dornhege commented Apr 9, 2015

mikeferguson commented May 1, 2015

mikeferguson commented May 1, 2015

Hydro devel #25

Hydro devel #25

Conversation

dornhege commented Feb 12, 2015

skohlbr commented Feb 27, 2015

mikeferguson commented Mar 5, 2015

dornhege commented Mar 5, 2015

davetcoleman commented Mar 5, 2015

davetcoleman commented Mar 5, 2015

dornhege commented Mar 5, 2015

davetcoleman commented Mar 5, 2015

dornhege commented Mar 5, 2015

davetcoleman commented Mar 5, 2015

dornhege commented Mar 6, 2015

davetcoleman commented Mar 24, 2015

dornhege commented Mar 31, 2015

davetcoleman commented Mar 31, 2015

isucan Mar 31, 2015

Choose a reason for hiding this comment

dornhege Mar 31, 2015

Choose a reason for hiding this comment

skohlbr Mar 31, 2015

Choose a reason for hiding this comment

isucan commented Mar 31, 2015

dornhege commented Apr 9, 2015

mikeferguson commented May 1, 2015

mikeferguson commented May 1, 2015