Open Alvinosaur opened 2 years ago
Getting a cone in is a good thing. I'm all in favor of doing this. So, let's talk about what it takes:
Cone
shape.
MeshcatCone
the Cone
and add a deprecated alias called MeshcatCone
so that we can systematically replace references of MeshcatCone
to Cone
.Cone
code hasn't been evaluated yet. We've only added a shape once we've confirmed FCL's implementation and correctness. So, we need to do that.vtkConeSource
for RenderEngineVtk
and custom tessellation (and uvs) for RenderEngineGl
.As I think of other tasks, I'll add them to the list above.
Assigning @SeanCurtis-TRI for now; Sean, feel free to reassign.
@RussTedrake MeshcatCone
supports an elliptical base. I had originally thought I'd just convert MeshcatCone
to Cone
but the FCL infrastructure only supports circular cross sections.
So, my tentative plan is to make Cone
with a circular base and leave the MeshcatCone
in place for the purpose of supporting elliptical cross sections in Meshcat
visualization. I believe as we address the FCL limitation in the future, the Cone
can organically grow from a default circular base to an optional elliptical base.
Thoughts/concerns?
Sounds good to me. That was the original plan that you proposed back when we chose the name "MeshcatCone" in the first place: https://drakedevelopers.slack.com/archives/C43KX47A9/p1633699257016600?thread_ts=1633692434.016300&cid=C43KX47A9 Obviously, we'll want to add documentation to MeshcatCone to make it clear why it survives in addition to Cone (elliptical cross sections).
Finally -- remember that you were surprised that MeshcatCone "points down" (as discussed in the original PR). This is the natural choice for me, since I like the mathematical object that is the second-order (e.g. Lorentz) cone. For the eventual deprecation to be seamless, we should make Cone that way, too.
No worries about orientation and alignment. I wasn't going to change that at all. One could make an argument about positioning the cone so that it's center of mass is coincident with the cone's origin (so it matches the other mathematical primitives). But I don't think the arguments are compelling. So, point at the origin, expanding in the +Gz direction is what we're going to stay with. We're definitely not going to support two cones with different semantics. That would be madness.
A small pertinent development: the sdformat parser library has added cone support. See https://github.com/RobotLocomotion/drake/pull/21798
Is your feature request related to a problem? Please describe. MeshcatCone is the only existing Cone class and only supports visualization.
Describe the solution you'd like I would like to replace MeshcatCone with a new Cone class that supports proximity queries. This would require implementing ProximityEngine::ImplementGeometry() as well as other functions.
Describe alternatives you've considered I would try implementing this myself and building from source, but I have to use Drake pre-built binaries. One could convert cones to meshes, but this is unnecessary since Cones are convex and should be straightforward to handle (fcl already supports them).