PlanarSceneGraphVisualizer: Store convex hulls of meshes #13857
Conversation
There was a problem hiding this comment.
+@SeanCurtis-TRI for feature review.
Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform), needs at least two assigned reviewers (waiting on @SeanCurtis-TRI)
There was a problem hiding this comment.
I've never run this visualizer. Can you give me a quick recipe for running something that exercises this? (I'm too lazy to go spelunking myself.)
Reviewed 1 of 1 files at r1.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform), needs at least two assigned reviewers (waiting on @avalenzu)
bindings/pydrake/systems/planar_scenegraph_visualizer.py, line 278 at r1 (raw file):
# vertices after projection, and will therefore be removed # in _update_body_fill_verts(). hull = spatial.ConvexHull(patch_G.T)
BTW If you removed the .T in the definition of patch_G, then you wouldn't need one here when you call ConvexHull.
There was a problem hiding this comment.
Sure! Try this on for size
bazel run -- //examples/manipulation_station:end_effector_teleop_sliders --setup planar
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform), needs at least two assigned reviewers (waiting on @avalenzu)
Prior to this commit, we stored all of the vertices of each mesh and then took their convex hull after projecting them to the visualization plane every dt. Now we only store the vertices of the 3D convex hull of each mesh. The 2D convex hull must still be computed at each dt, but, for meshes with lots of interior vertices, it will now be over a much smaller set of points.
avalenzu
force-pushed
the
psgv-chull-first-ask-questions-later
branch
from
4d075a1
to
25e280e
Compare
There was a problem hiding this comment.
Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform), needs at least two assigned reviewers (waiting on @SeanCurtis-TRI)
bindings/pydrake/systems/planar_scenegraph_visualizer.py, line 278 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
BTW If you removed the
.Tin the definition ofpatch_G, then you wouldn't need one here when you callConvexHull.
Done.
There was a problem hiding this comment.
+@rpoyner-tri for platform review, please.
Reviewed 1 of 1 files at r2.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform) (waiting on @rpoyner-tri)
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform) (waiting on @rpoyner-tri)
Prior to this commit, we stored all of the vertices of each mesh and then took
their convex hull after projecting them to the visualization plane every dt. Now
we only store the vertices of the 3D convex hull of each mesh. The 2D convex
hull must still be computed at each dt, but, for meshes with lots of interior
vertices, it will now be over a much smaller set of points.
This change is