Meshcat::SetObject() reports files a Mesh uses

[SeanCurtis-TRI]

The mesh file specified in a Mesh shape may actually depend on an arbitrary number of additional files on disk. When adding a Shape to Meshcat, Meshcat will report all of the files that it is making available to a meshcat client.

Meldis uses this API to learn about mesh files.

Closes #21194

---

This change is 

[SeanCurtis-TRI]

+(release notes: none)
+@jwnimmer-tri for feature review, please.

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

a discussion (no related file):
The meshcat changes are relatively simple and straightforward.

The meldis changes are much more complex because the change logic is now more fractured. Although, it's nice that GeometryFileHasher knows nothing about mesh file formats. :)

---

[SeanCurtis-TRI]

-(release notes: none)
+(type: feature request)

The feature is the change to Meshcat::SetObject() has a return value now. Changes in meldis need not be reported because this doesn't appreciably change meldis's behavior (only how it does what it does).

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

[SeanCurtis-TRI]

-(type: feature request)
+(release notes: feature)

[jwnimmer-tri]

I'm taking this one top-level. Starting with Meldis... (see below).

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

a discussion (no related file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

The meshcat changes are relatively simple and straightforward.

The meldis changes are much more complex because the change logic is now more fractured. Although, it's nice that GeometryFileHasher knows nothing about mesh file formats. :)

I think SeanCurtis-TRI#11 helps a lot. WDYT?

---

[SeanCurtis-TRI]

I've merged your PR, commented, and pushed a couple of changes. Let me know what you think. I can imagine that the cost of encoding is not something worth worrying about.

Reviewable status: 3 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I think SeanCurtis-TRI#11 helps a lot. WDYT?

I like it. Very succinct.

---

---

bindings/pydrake/visualization/_meldis.py line 130 at r4 (raw file):

        # check, but is a very fast way detect common kinds of novelty.)
        if (message.num_links == self._prior_message.num_links
                and message.encode() == self._prior_message.encode()

nit: With the assumption that this feature assumes we will frequently declare the new load message to not be different from the old (the primary reason to favor meldis), it might be better to store the encoding of the last loaded message than re-encode it over and over.

---

bindings/pydrake/visualization/test/meldis_test.py line 354 at r4 (raw file):

            f.write("newmtl test_mat\n")
            f.write("Kd 1 0 0")
        old_hashes = applet._load_deduplicator._path_hashes.copy()

nit: oops, unused.

[jwnimmer-tri]

Checkpoint LGTM on pydrake half. I'll circle back to the C++ half later on; lots of meetings today.

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

---

bindings/pydrake/visualization/_meldis.py line 130 at r4 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit: With the assumption that this feature assumes we will frequently declare the new load message to not be different from the old (the primary reason to favor meldis), it might be better to store the encoding of the last loaded message than re-encode it over and over.

I believe the encoding cost is in the noise (and it better be, because now we're eating that cost even on novel messages), but anyway the new code that leans into the encoded bytes isn't that much more complicated, so it's fine.

---

bindings/pydrake/visualization/_meldis.py line 132 at r5 (raw file):

        # remain unchanged. (The num_links check is redundant with the encode
        # check, but is a very fast way detect common kinds of novelty.)
        encoding = None

nit In all control flow paths below, we need to re-encode now. We might as well just do it in one place for simplicity.

Suggestion:

encoding = message.encode()

---

bindings/pydrake/visualization/_meldis.py line 160 at r5 (raw file):

    def add_dependencies(self, *, paths: typing.Iterable[Path]):
        """Adds ...

nit Oops, I forgot to finish writing this comment.

[SeanCurtis-TRI]

Reviewable status: LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

---

bindings/pydrake/visualization/_meldis.py line 132 at r5 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit In all control flow paths below, we need to re-encode now. We might as well just do it in one place for simplicity.

Done Not just below, but we are inevitably going to encode right from the beginning.

---

bindings/pydrake/visualization/_meldis.py line 160 at r5 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit Oops, I forgot to finish writing this comment.

I had the same mental note and still didn't do it.

[jwnimmer-tri]

Reviewed 1 of 1 files at r6.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

a discussion (no related file):
We have #21297 already in place with a brittle (but passable) solution. I see the goal of this PR as rewriting the solution to be much less brittle -- to be simpler to maintain, and to be correct in 100% of cases instead of just 99%.

With that in mind, while reviewing the C++ code I was coming up with some small problems with certainty (soundness), relating to whether we correctly detect changes. I could post those and we could iterate here, but instead I'd like to entertain a change in direction to see if it will work better for the long-term goals. If we end up not liking it, we can circle back here. (I imagine the new solution will be a new PR. Either you or I could write it. I'll explain and we can negotiate from there.)

Here's the idea -- remove all re-validation logic from the Meldis side. Fully encapsulate the "what file contents did I load" and "has what I loaded from disk changed out from under me" logic inside meshcat.cc. The only job for Meldis would be either:

(1a) To ask Meshcat "Did meshes a,b,c,d I just loaded change on disk?", and in case any were "yes" then reset everything just like today; or

(1b) To ask Meshcat "Did meshes a,b,c,d I just loaded change on disk?", and for those which were "yes" only reload those particular paths; or

(2) Command Meshcat to "Reload any and all SetObject paths whose backing files on disk have changed, compared to the version you sent to the JS side". (This would be an incremental reload, similar to 1b above.)

In writing that out, I don't really see anything Meldis-specific about that logic? The feature to "reload from disk" seems like it could be useful outside of Meldis.

For example, for ModelVisualizer we've long wanted the option to "auto-reload" when the files on disk change. There is probably a flavor of (2) where we can say "bool Meshcat::Reload(dry_run=true)` or similar to interrogate whether a reload would change anything vs being a no-op.

On the other hand, the lower-level query on a per-Mesh basis from (1) would allow users access to more details, which might unlock some more applications we haven't thought out. My only challenge with (1) is exactly how to phrase the API. When we call SetObject, do we return a "Handle" object with methods like IsDirty() and/or Reload()? Or do we just have bool ReloadObject(path=..., dry_run=...) and use the string path as the handle? Maybe if we push on that a little bit, we can find a nice answer.

Anyway, back to you to mull over this. We can have a call later today or tomorrow if you'd like to discuss.

---

[SeanCurtis-TRI]

I'm happy to discuss this later...

... small problems with certainty (soundness), relating to whether we correctly detect changes.

I'd be curious to hear more about this. Without details on this front, it seems like what you're suggesting is largely less about correctness and more about feature.

In addition, this philosophical push seems toward pushing things in to Meshcat. meldis currently serves as an external memory for Meshcat and has specific events which trigger questioning the validity of what's been loaded. Do you anticipate that Meshcat will begin monitoring the filesystem? Otherwise, how will the owner of a Meshact instance (say, ModelVisualizer) know that it should be updating objects? Or will Meshcat undertake that on its own?

No need to answer these here -- I just wanted to make note of the questions while this is fresh in my mind, but we can defer discussion to a meeting.

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

a discussion (no related file):

I'd be curious to hear more about this.

Two main points, the first one more important than the second:

(1) TOCTOU. If the files are being rewritten during this loading process, the version Meshcat sends to the browser and the version that Meldis checksummed might not be the same version. The only way to be sure is to only ever read from each file once, and checksum that. I knew that going in, and was initially OK to just hand-wave it away, but the more I looked at the details the less comfortable I became.

(2) The new calls to lexically_normal. I struggled to prove that these can never ever have any effect on the hash, especially since we do need to affirmatively hash the presence/absence of a file.

Do you anticipate that Meshcat will begin monitoring the filesystem?

We can discuss various options, but the answer I proposed upthread was "no". Things like model_visualizer would poll Meshcat to either ask if anything had changed (meshcat.ReloadAll(dry_run=True) -> bool or meshcat.ReloadObject(path=..., dry_run=True) -> bool) in case they need to inquire to the user before reloading, or they could just poll meshcat.ReloadAll() and in case nothing had changed, there would be no side-effects.

---

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

a discussion (no related file):

... more about feature.

Yes, in a sense. Big picture, the angle plays out like: Meldis is somewhat vestigial, and might eventually die. It's supposed to be a low-investment tool to help transition and solve a narrow class of problems. If we can invest in a more centralized solution to this particular defect, that will retain more value long-term.

---

[SeanCurtis-TRI]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

... more about feature.

Yes, in a sense. Big picture, the angle plays out like: Meldis is somewhat vestigial, and might eventually die. It's supposed to be a low-investment tool to help transition and solve a narrow class of problems. If we can invest in a more centralized solution to this particular defect, that will retain more value long-term.

Alrighty -- tag me when you have some free time and we can discuss the alternative.

---

[SeanCurtis-TRI]

+(status: do not merge) +(status: do not review)

We'll put this on hold while we (read as: Jeremy) investigate an alternative.

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @SeanCurtis-TRI)