fixing adding tables to the collision world #578
Conversation
| @@ -125,7 +125,7 @@ bool SemanticWorld::addTablesToCollisionWorld() | |||
| std::vector<unsigned int> triangles((vertices.size() - 2)* 3); | |||
| for (unsigned int j = 0 ; j < convex_hull.size() ; ++j) | |||
| vertices[j] = Eigen::Vector3d(convex_hull[j].x, convex_hull[j].y, convex_hull[j].z); | |||
| for (unsigned int j = 1; j < triangles.size() - 1; ++j) | |||
| for (unsigned int j = 0; j < vertices.size() - 2; ++j) | |||
| { | |||
| unsigned int i3 = j * 3; | |||
| triangles[i3++] = 0; | |||
There was a problem hiding this comment.
There is a new bug here. Consider what happens when j == 0. triangles[0] = 0, triangles[1] = 0, and triangles[2] = 1. So the first triangle is always a line. Then because of the loop ending just before vertices.size() - 2, the last (needed) triangle is missing.
I think the correct code should look like you have it, with lines 132 and 133 changed to read:
triangles[i3++] = j+1;
triangles[i3] = j+2;
|
This PR seems to have a lot of changes that are not relevant to the fix being made. @nlyubova could you please update this PR and remove all your debug comments. |
nlyubova
force-pushed
the
indigo-devel
branch
15 times, most recently
from
08745e8 to
faeccb1
Compare
|
I've just cleaned the last commit, sorry for the debug comments. |
nlyubova
force-pushed
the
indigo-devel
branch
from
faeccb1 to
885e9f5
Compare
|
Could you address hersh's comment too. |
|
I believe that hersh's comment is the correct way. If I understand this correctly, tables are added as planar meshes. This is a problem for the point cloud updaters, where a 3D convex hull is produced that degenerates. This leads to arbitrarily large portions of the octree being removed. So, I'd recommend to add a fixed table height. |
|
When I try the Hersh's way, the table is not displayed, so I do not know a better way to fix it |
|
@nlyubova do you have any interest in moving this change to the new repo? https://github.com/ros-planning/moveit otherwise I will close it for now |
|
It seems everyone involved in this request agrees that there is a problem with adding tables to the collision world. |
|
i closed it because no good solution seemed to have ever been found, but i'll close PRs with a little more caution |
@vrabaud : fyi