RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.24k stars 1.25k forks source link

MeshcatVisualizer should display the convex hull for the proximity role if ProximityEngine is using the convex hull for collision queries #20618

Closed RussTedrake closed 5 months ago

RussTedrake commented 9 months ago

I've been seeing many students confused by the fact that proximity engine often uses the convex hull of the geometry. Here are two recent examples:

image image

Everyone loves being able to visualize the collision geometry directly in meshcat (via AddDefaultVisualization), and they do know to check it. But the collision geometry still renders the nonconvex mesh even when ProximityEngine is using the convex hull for collision queries.

Proposed resolution: MeshcatVisualizer should display the convex hull for the proximity role if ProximityEngine is using the convex hull for collision queries.

Tagging @jwnimmer-tri for triage.

joemasterjohn commented 9 months ago

Since we send the binary contents of mesh files over the wire to meshcat, I think the most feasible way to do this would be by computing computing the convex hull of the transmitted mesh on the javascript side.

Modify the meshcat JSON type _meshfile_object to have an additional field use_convex_hull. When Meshcat loads a meshfile with this flag set to true, construct a ConvexGeometry in place of the original geometry from the file.

We might want to throw on meshfiles that provide anything other than a simple single color material, as texture coordinates cannot be preserved in any meaningful way through taking the convex hull.

jwnimmer-tri commented 9 months ago

We need to think about this somewhat carefully. It matters what Meldis and Rviz will show for the proximity geometry as well. We can't throw during visualization -- anything that is a valid SceneGraph needs to be visualized one way or another.

SeanCurtis-TRI commented 9 months ago

Drake's current implementation of hull for meshes is, at best, a stop gap. What it should be doing is actually computing a convex hull (that'll speed up contact computations and unlock some other calculations). One of the implications of that is our ability to actually send the mesh (in the same way we send the tessellated hydroelastic surface).

When it was first implemented, Drake hadn't picked up its dependency on qhull. Now that qhull is present, there's no reason to not push ProximityEngine over the edge to make use of it.

jwnimmer-tri commented 9 months ago

One related note from f2f...

I asked Sean if the "use the hull of this mesh for collisions" capability is a desirable long-term feature, or merely a temporary implementation deficiency. The idea is that it is a durable feature we want to keep (possibly with different defaults or syntax?). For me, that tips the solution here from "figure out the most expedient hack that papers over the bug" to "the 'promote to convex hull' capability is a first-class citizen, so needs appropriate accessors as part of QueryObject" so that all visualizers can display the same source of truth.

In other words, we either need to pre-process the mesh file during parsing (so that proximity engine doesn't need to further reprocess), or (more likely) we need to provide a query object getter where the proximity engine can return the "refined" flavor of the current Shape that is actually in effect for collisions. Probably @SeanCurtis-TRI will have a better explanation of the rough API we'd be looking for.

jinliangwii commented 8 months ago

Rq94Z Hey folks, I think I have encountered the same issue, and thanks to Professor Tedrake leading me here. I resolved this unknown "contact force" by splitting my model in a certain way to avoid this invisible collision.

Visualize the calculated geometry from mesh files sounds like a good idea to me. And may I ask to also update the API description somewhere to declare that the collision geometry is not what you seen when you use mesh files to represent it.

at the end... I really like that Drake can show me exact states of each step and that helps me a lot while debugging this issue. I once doubt it was my algorithms problem that I can drive my car go straight, but it turns out to be there is a "default force" consistently applied to a direction. PID won't work very well in the situation.

Thank you!

jinliangwii commented 8 months ago

If we are lacking of hands on this issue, I would love to contribute my time on this issue under someone's guidance. Please let me know.

jwnimmer-tri commented 6 months ago

Here's one other implementation idea we could consider here: introduce a specified geometry properties name like use_convex_hull, and if the property is set to true on a geometry instance then it's the responsibility of whoever is consuming that mesh to use convex hull when interpreting that shape. We could teach our various visualizers to all obey that (including RViz), and then we don't need to change how shape descriptions work.

Part of my inspiration for that idea was that I was hoping certain visualization back-ends already had "use convex hull" as a flag, in which case we could just pass it along without copying the whole mesh. From a quick skim though, nothing seems to offer that in easy reach.

Therefore, I think probably https://github.com/RobotLocomotion/drake/issues/20618#issuecomment-1838688412 is still the best idea.

@SeanCurtis-TRI when you have some free brain cycles, could you please work up a rough outline of either pseudo-code and/or new API calls to make available, showing how this might look in the ideal way?

I imagine that once it all boils down we have the MeshcatVisualizer<T>::SetObjects end up with TriangleSurfaceMesh for the GeometryId in question instead of a geometry::Mesh, but how does that look in term of the SceneGraphInspector?

SeanCurtis-TRI commented 6 months ago

I'm doing some investigating and I'll outline what I think the solution is. Stay tuned.

SeanCurtis-TRI commented 6 months ago

As I started sketching out what it would take, I felt it would be a small step to just implement it. So, I'm currently knocking it out.

@jwnimmer-tri The first pass is going to produce a "smoothed" mesh. In my estimation, it will be far better to have a faceted mesh. I think that can easily be addressed on the MeshcatVisualizer side, but trickier on the meldis side. We should chat about how we want to do that.

jwnimmer-tri commented 6 months ago

As I started sketching out what it would take, I felt it would be a small step to just implement it. So, I'm currently knocking it out.

SGTM

The first pass is going to produce a "smoothed" mesh. In my estimation, it will be far better to have a faceted mesh.

Yes, that's my intuition as well.

I think that can easily be addressed on the MeshcatVisualizer side, but trickier on the meldis side. We should chat about how we want to do that.

Sure. I'll be curious where the trouble comes from. In my mind, TriangleSurfaceMesh is already a supported LCM geometry type (using lcmt_viewer_geometry_data.float_data) and surface meshes should always be drawn using with faceted normals instead of smooothed normals, so all Meldis has to do is dump the mesh into Meshcat.

SeanCurtis-TRI commented 6 months ago

The trouble is the fact that the triangle data we send is impoverished; we're not passing normals. When that happens, we are subject to whatever the graphics engine resorts to. If meshcat doesn't have normals, it explicitly adds them (smoothing everything).

So, to get faceted geometry (which, if you look at the hydroelastic proximity representations, you'll not we don't currently get), we have to communicate those normals to meshcat. Augmenting Meshcat::SetTriangleMesh() will be sufficient.

To handle it via meldis, we'll either have to add normals in the mesh data we send out on lcm (a change to the message), or meldis will have to compute normals when registering it with meshcat. In typing that sentence, it's clear what the solution is. Just have meldis compute the per-triangle normals and walk away. (It does have the downside of the fact that if anything else ever reads the meshes from the LCM, they'll be obliged to do the same.)

Thoughts?

jwnimmer-tri commented 6 months ago

I'm lost. Assume we do this part:

Augmenting Meshcat::SetTriangleMesh() will be sufficient.

Do you mean that inside of that function, we compute faceted normals and sends them to the JS side? That seems like the right idea.

If we have that as a base, then all Meldis needs to do is take the existing vertices (transmitted over LCM as a vertex/tri buffer, not a mesh filename), pop them into a TriangleSurfaceMesh, and then hand that to Meshcat.

SeanCurtis-TRI commented 6 months ago

To be specific, this is what I expect:

  1. SetObject(TriangleSurfaceMesh) would compute normals.
    • semantically, if all you provide is a literal TriangleSurfaceMesh, we visualize it as faceted.
    • As TriangleSurfaceMesh has already computed normals, we'll assemble them together in this function.
    • This method simply invokes SetTriangleMesh() below (passing the normals).
  2. SetTriangleMesh(vertices, faces) would accept some optional normals as well.
    • If you provide normals, we use them.
    • If you don't provide normals, we calcualte them.
    • The idea is that if you are willing to define the mesh elements, you can define your own normals (allowing for smooth, faceted, or arbitrary creases).

But, yes, even with those details, your summary is correct. Populating a TriangleSurfaceMesh in meldis would be sufficient. And, now that I look, I see that's what meldis is already doing. So, modifying Meshcat will be sufficient and meldis will inherit for free. Huzzah!

joemasterjohn commented 6 months ago

As for smooth/flat shading: The THREE.js MeshPhongMaterial (the material we use for triangle meshes in Meshcat) has an option for flatShading. I think just adding this option to the MaterialData type in meshcat_types_internal.h and giving SetTriangleMesh() an optional argument for flat shading would do the trick without having to pass face normals (pretty sure the THREE.js Mesh/BufferedGeometry type computes the face and vertex normals anyway upon initialization).

SeanCurtis-TRI commented 6 months ago

There's a tricky subtlety in this. If a mesh has both a hydroelastic representation and a convex hull (i.e., using hydro-with-fallback, it might collide sometimes as a non-convex mesh and sometimes as a convex hull), what do we render? My current implementation treats them exclusively with the heuristic that we draw the hydro surface if available, otherwise a convex hull if available, otherwise just the shape.

Should we pick a different heuristic? (This might be related to #20987 where we use some visual indicator to help classify the nature of the proximity geometry.

jwnimmer-tri commented 6 months ago

Let me toss out another idea to stretch our imagination: when not in collision, use your current heuristic; when in collision, show any/all representations that are colliding. I think that boils down to adding both objects to Meshcat, but the chull is visible=false by default. Then iff we encounter a fallback point contact we toggle the mesh chull's "visible" property to true in the contact visualizer, so the silly shape suddenly appears in the scene.

SeanCurtis-TRI commented 6 months ago

Let me ponder that.

SeanCurtis-TRI commented 6 months ago

In retrospect, #21061 shouldn't have closed this issue until we resolve the question of hydro vs hull.

SeanCurtis-TRI commented 5 months ago

Having pondered the suggestion, while the "toggle the visual representation to match the current contact" approach has appeal, it doesn't seem reasonably feasible.

The reason is that we have different paths in the meshcat tree -- one for contact information and one for proximity geometry. These two paths are typically managed by different entities. I don't think it's a good idea to have them reach into each other's playgrounds. That seems dangerous.

Alternative proposal:

  1. This only applies to Mesh type where there exists a rigid hydroelastic representation.
  2. In this case, we render the rigid mesh as a faceted mesh. We render the convex hull as wireframe over the top of it.
    • As an optimization, we could attempt to detect if they are the same and omit the wireframe.
jwnimmer-tri commented 5 months ago

Sure, that's plausibly good. I think we just need to try an experiment and see what we like.

I don't think it's a good idea to have them reach into each other's playgrounds. That seems dangerous.

We could do it with a responsible API between the two objects (e.g., formally communicate which geometries are in what kinds of collisions), not just setting meshcat property strings and hoping for the best.

If we don't like the UX of it then that's fine, but I don't think software design concerns have any bearing on the question.

jwnimmer-tri commented 5 months ago

In retrospect, https://github.com/RobotLocomotion/drake/pull/21061 shouldn't have closed this issue until we resolve the question of hydro vs hull.

Do we think that the corner case of point contact fallback should be addressed with priority?

I'm leaning towards "no", in which case we should file that feature request as a separate issue, with low priority and not on the board, and then close this issue as completed.

SeanCurtis-TRI commented 5 months ago

I'm likewise inclined for "no". I'll open a separate issue.

SeanCurtis-TRI commented 4 months ago

FTR The issue spawned was: #21291