Drake: Update call sites to use RigidTransform

[amcastro-tri]

EDIT (eric): Overarching issue: #9865

These are the MBP's APIs that were upgraded to use RigidTransform in #10959, showing the number of times they get called within Drake code:

• [x] #11074 SetFreeBodyPose: 42 - @EricCousineau-TRI
• [x] #11180 AddJoint: 98 - @EricCousineau-TRI
• [x] WeldFrames: 46 - @mitiguy
• [x] SetFreeBodyPoseInWorldFrame: 3 - @mitiguy
• [x] SetFreeBodyPoseInAnchoredFrame: 7 - @mitiguy
• [X] #11091 RegisterVisualGeometry: 37 - @SeanCurtis-TRI
• [X] #11091 RegisterCollisionGeometry: 27 - @SeanCurtis-TRI
• [x] #11181 Element constructors (WeldJoint, FixedOffsetFrame) - @EricCousineau-TRI

A total of 260 places where things need to be upgraded with lots of manual/per-case supervision.

Example PR.

@EricCousineau-TRI led the way showing us how to accomplish this. See #11074.

Proposed approach

Several Drake developer volunteer to take one (or more!) of those bullet points and address them in parallel in separate PRs. Please, if you do volunteer to address one of the bullets, ~place a check mark right away so others know you are working on it.~ edit this and write your name next to the checkbox. Check the box once your PR lands.

What needs to be done

1. Call sites need to be written as much as possible in terms of RigidTransform, at least in what using the MBP APIs concerns. We have operator*(RT, Iso3) and conversion operator RT::Iso3() that for now allow us to temporarily mix RTs with Iso3s. Avoid using them as much as possible. A later pass will deprecate those as well.
2. Ideally, we'd like to deprecate the Isometry3 versions in MBP (right now they are not deprecated yet to enable the resolution of this PR as a joint effort).

[EricCousineau-TRI]

What's the suggested process for easily catching these conversions?
Do you have a patch to disable the implicit bits, so then things fail when Isometry3 is used locally in Drake (or Anzu)?

[amcastro-tri]

Good question.

1. MBP methods soon to be deprecated are within this "non documented Doxygen" section.
2. Take SetFreeBodyPoseInWorld() as an example, and delete it.
3. Compile Drake (or global search where this method is called. Prob faster).
4. Upgrade the call sites to use the newly available API taking RigidTransform.
5. Put back the Isometry3 variant of SetFreeBodyPoseInWorld() and deprecate properly.
6. Push a PR!

[amcastro-tri]

Sorry, I just realized what you asked @EricCousineau-TRI. The easy (cave man like me) approach is to simple comment out the implicit "conversions" here (also within a "non documented Doxygen" section) and make sure your upgraded code compiles and runs correctly.

[EricCousineau-TRI]

I will take SetFreeBodyPose and see what's necessary for Python bindings; if it's not huge overhead, I may close #10982 in lieu of tracking that here, as they're most likely coupled.

[EricCousineau-TRI]

Also, rather than check a box, should they instead put their name next to it?
I think that box should only be checked once the relevant PRs are merged.

[EricCousineau-TRI]

Er... It's not good to conflate implicit conversions with this issue. Trying to delineate errors between implicit conversions and old signatures is hard when doing bazel test //.... That should be handled in the upstream issue.

[EricCousineau-TRI]

FTR: Here is my procedure-as-commits for SetFreeBodyPose:
https://github.com/EricCousineau-TRI/drake/commits/issue/11064_SetFreeBodyPose-wip

[EricCousineau-TRI]

I'll take AddJoint.

Can someone aside from Alejandro, Sean, or myself take some of the remaining ones?
\cc @siyuanfeng-tri @sherm1 @mitiguy

[mitiguy]

I'll take the rest.

[EricCousineau-TRI]

Sweet, thanks!

[EricCousineau-TRI]

FYI, it looks like we may have "untracked" bits for derived Frame and Joint class constructors (at least in Python).
Will add that on the checklist, and assign myself.

[EricCousineau-TRI]

@amcastro-tri Just as a heads up, the AddJoint(Args...) stuff is making me sad... Coupling the constructors causes ambiguities between overloads with optional<RigidTransformd> and optional<Isometry3d>. Dunno why, don't really wanna find out, but I'm guessing it's due to perfect forward (with parameter packs) screwing with affinity.
I may raise this complain a bit more in the future if I touch the code again, as I'm not sure if the cost/benefit is great...

[amcastro-tri]

Yes, I noticed, sorry about that. But that also has to do with the fact that we have the old (soon to be deprecated) Isometry3 APIs. I believe not a huge problem once only the RigidTransform API is supported?

[EricCousineau-TRI]

Yup, that'd be correct. Really, I'm pointing this out as a way to discourage this type of super-coupled sugar in the future, if possible, due to the maintenance overhead it incurs.

[amcastro-tri]

We checked all the boxes! thanks everyone who helped with this!

[jwnimmer-tri]

I think we're not actually done, because make_unique is hiding deprecation warnings. I'll check tomorrow, and report back.

[amcastro-tri]

I created a more specific #11193. Would that address the ones you were thinking of @jwnimmer-tri?

[jwnimmer-tri]

I checked quickly by changing

#ifndef DRAKE_DOXYGEN_CXX
  // These APIs using Isometry3 will be deprecated soon with the resolution of
  // #9865. Right now we offer them for backwards compatibility.

to be #if 0 instead. The portion of the tree that I compiled still worked, so I think the RT / Isometry3 implicit converion is saving us for now, so I'm okay tracking that in another issue instead.

[amcastro-tri]

ah, thanks. That's a great test.