[geometry] Move CalcBarycentric to be non-inline

[jwnimmer-tri]

Towards #23477. (Eigen 5's updated linear solvers cause Drake a bit of trouble; having that isolated into a few files instead of spammed into everything downstream of these headers improves developer iteration speed.)

---

This change is 

[jwnimmer-tri]

+assignee:@SeanCurtis-TRI for both reviews, please.

[SeanCurtis-TRI]

With two editorial bits.

@SeanCurtis-TRI reviewed 6 of 8 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: 2 unresolved discussions

---

geometry/proximity/mesh_field_linear.h line 333 at r2 (raw file):

   @throws std::exception if the field does not have gradients defined _and_ the
           MeshType doesn't support Barycentric coordinates.
   @tparam C must be either `double` or `AutoDiffXd`. */

BTW This seems to be the only function that picked up template documentation that didn't pick up the corresponding requires clause. Any reason to omit it?

---

geometry/proximity/polygon_surface_mesh.cc line 61 at r2 (raw file):

  requires scalar_predicate<C>::is_bool
{
  unused(p_MQ, p);

BTW Why not eschew the parameter names and unused as long as we're in the .cc file? Simply in the name of minimizing the churn? That churn seems worthwhile.

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion

---

geometry/proximity/mesh_field_linear.h line 333 at r2 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

BTW This seems to be the only function that picked up template documentation that didn't pick up the corresponding requires clause. Any reason to omit it?

This one is less important since its not out-of-line, but in any case I'm OK to add it.

---

geometry/proximity/polygon_surface_mesh.cc line 61 at r2 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

BTW Why not eschew the parameter names and unused as long as we're in the .cc file? Simply in the name of minimizing the churn? That churn seems worthwhile.

The style guide rule is to keep the names unless they are obvious: https://drake.mit.edu/styleguide/cppguide.html#Function_Declarations_and_Definitions

For now, I'm happy to err on the side of "might not be obvious".

[SeanCurtis-TRI]

@SeanCurtis-TRI reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignee SeanCurtis-TRI(platform)

---

geometry/proximity/polygon_surface_mesh.cc line 61 at r2 (raw file):
Hmmm... I interpret that differently. The examples of "you may omit if obvious" seem all to show declarations as the example.

But for the nonobvious, it seems to specifically advocate for simply commenting out the name (in place).

Unused parameters that might not be obvious should comment out the variable name in the function definition (emphasis mine).

That still seems superior to the use of unused.

[jwnimmer-tri]

Reviewable status: complete! all discussions resolved, LGTM from assignee SeanCurtis-TRI(platform)

---

geometry/proximity/polygon_surface_mesh.cc line 61 at r2 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

Hmmm... I interpret that differently. The examples of "you may omit if obvious" seem all to show declarations as the example.

But for the nonobvious, it seems to specifically advocate for simply commenting out the name (in place).

Unused parameters that might not be obvious should comment out the variable name in the function definition (emphasis mine).

That still seems superior to the use of unused.

Switched to comments.

[SeanCurtis-TRI]

@SeanCurtis-TRI reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: 1 unresolved discussion

---

geometry/proximity/polygon_surface_mesh.h line 270 at r1 (raw file):

  Vector3<FieldValue> CalcGradientVectorOfLinearField(
      const std::array<FieldValue, 3>& field_value, int p) const {
    unused(field_value, p);

BTW I'm not sure this is appropriate. In the current documentation, the parameter names get called out. With this change, the new documentation drops the names:

The method is described as "stub" so it might be appropriate to omit the parameter names, but it certainly leaves the doxygen with non-obvious place holders which seems to go against the spirit of the styleguide.

[jwnimmer-tri]

Reviewable status: complete! all discussions resolved, LGTM from assignee SeanCurtis-TRI(platform)

---

geometry/proximity/polygon_surface_mesh.h line 270 at r1 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

BTW I'm not sure this is appropriate. In the current documentation, the parameter names get called out. With this change, the new documentation drops the names:

The method is described as "stub" so it might be appropriate to omit the parameter names, but it certainly leaves the doxygen with non-obvious place holders which seems to go against the spirit of the styleguide.

In that case, going back to r2 is the winner.

---

geometry/proximity/polygon_surface_mesh.cc line 61 at r2 (raw file):

Why not eschew the parameter names and unused as long as we're in the .cc file? Simply in the name of minimizing the churn?

Yes. And also for consistency with the header which still uses unused.

[SeanCurtis-TRI]

@SeanCurtis-TRI reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: 1 unresolved discussion

---

geometry/proximity/polygon_surface_mesh.h line 270 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

In that case, going back to r2 is the winner.

I don't agree, but I'm not going to fight it.

In the name of completeness, I'm stating the basis of my disagreement in a completely unbinding way. I think the .cc file avoiding use of unused where the header uses it is perfectly reasonable. It certainly makes the .cc file hew to the styleguide more accurately. (Which, of course, makes no mention of .h vs .cc in this regard.)

---

geometry/proximity/mesh_field_linear.h line 337 at r3 (raw file):

  promoted_numerical_t<C, T> EvaluateCartesian(int e,
                                               const Vector3<C>& p_MQ) const
    requires scalar_predicate<C>::is_bool

btw Did you mean to revert this as well?

[SeanCurtis-TRI]

@SeanCurtis-TRI reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignee SeanCurtis-TRI(platform)