proximity: Don't warn when parsing meshes with mtl files

[jwnimmer-tri]

When reusing a visual mesh for collision purposes, we would get warning logspam while skipping the mtl files. This was a regression as of #14656.

See also slack discussion.

---

This change is 

[jwnimmer-tri]

+@DamrongGuoy for feature review, please.

[DamrongGuoy]

with minor questions.

Reviewed 3 of 4 files at r1, 1 of 1 files at r2.
Reviewable status: 3 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on @jwnimmer-tri)

---

geometry/proximity/obj_to_surface_mesh.h, line 3 at r2 (raw file):

#pragma once

#include <istream>

Should we add #include <functional> for std::function?

---

geometry/proximity/obj_to_surface_mesh.h, line 33 at r2 (raw file):

    const std::string& filename,
    double scale = 1.0,
    std::function<void(std::string_view)> on_warning = {});

Clang format gave this to me:

SurfaceMesh<double> ReadObjToSurfaceMesh(
    const std::string& filename, double scale = 1.0,
    std::function<void(std::string_view)> on_warning = {});

I'm fine either way. I like one parameter per line, as in the original code, too.

---

geometry/proximity/obj_to_surface_mesh.cc, line 140 at r2 (raw file):

    } else {
      drake::log()->warn(warn);
    }

Just a note that my kcov test did not cover this block of code. However, I won't take it as a defect because it's about handling warning. In fact, I saw that the change in the unit test checked that there is no warning, which is the main purpose of this PR.

I followed this link to use kcov in drake.

[DamrongGuoy]

The links to two CI jobs were broken (HTTP ERROR 404 Not Found), so I suppose it's infrastructure flake and will rerun them now.

@drake-jenkins-bot linux-bionic-clang-bazel-experimental-leak-sanitizer please
@drake-jenkins-bot linux-bionic-gcc-bazel-experimental-debug please

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on @DamrongGuoy)

---

geometry/proximity/obj_to_surface_mesh.h, line 3 at r2 (raw file):

Previously, DamrongGuoy (Damrong Guoy) wrote…

Should we add #include <functional> for std::function?

Done.

---

geometry/proximity/obj_to_surface_mesh.h, line 33 at r2 (raw file):

Previously, DamrongGuoy (Damrong Guoy) wrote…

Clang format gave this to me:

SurfaceMesh<double> ReadObjToSurfaceMesh(
    const std::string& filename, double scale = 1.0,
    std::function<void(std::string_view)> on_warning = {});

I'm fine either way. I like one parameter per line, as in the original code, too.

(... and that's why I don't use clang-format.)

---

geometry/proximity/obj_to_surface_mesh.cc, line 140 at r2 (raw file):

Previously, DamrongGuoy (Damrong Guoy) wrote…

Just a note that my kcov test did not cover this block of code. However, I won't take it as a defect because it's about handling warning. In fact, I saw that the change in the unit test checked that there is no warning, which is the main purpose of this PR.

I followed this link to use kcov in drake.

An interesting conundrum! Fixing the mtl bug causes the warning code to be unused. I've added more test cases now.

[DamrongGuoy]

Reviewed 4 of 4 files at r3.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers (waiting on @jwnimmer-tri)

[jamiesnape]

An interesting conundrum! Fixing the mtl bug causes the warning code to be unused. I've added more test cases now.

Fred Brooks once said to me (slightly tongue in cheek) that fixing bugs was a bad idea, you just replace one bug with potentially another (or worse, more than one bug).

[jwnimmer-tri]

+@SeanCurtis-TRI for platform review per schedule on Monday, please.

[SeanCurtis-TRI]

Reviewed 3 of 4 files at r3.
Reviewable status: complete! all discussions resolved, LGTM from assignees SeanCurtis-TRI(platform),DamrongGuoy (waiting on @jwnimmer-tri)