[geometry] Unbundle glTFs before sending to Meshcat

[jwnimmer-tri]

This substantially improves loading times for large files.

Towards #19598.

• Requires [workspace] Upgrade meshcat to latest commit #20876.

---

This change is 

[jwnimmer-tri]

Reviewed 12 of 12 files at r1, 3 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @jwnimmer-tri)

---

geometry/meshcat_internal.cc line 162 at r1 (raw file):

  }
  if (edited) {
    *gltf_contents = gltf.dump();

Working

Is nlohmann output deterministic?

[jwnimmer-tri]

Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @jwnimmer-tri)

---

geometry/meshcat_internal.cc line 162 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

Is nlohmann output deterministic?

Done.

Yes, per https://json.nlohmann.me/features/object_order/ by default it's a std::map so the output should be reasonably consistent. The formatting for floating-point types might change when we upgrade nlohmann (nlohmann/json#2526) but for any given version pin it should be deterministic.

[jwnimmer-tri]

+@rpoyner-tri for feature review, please.

+@SeanCurtis-TRI please give this a quick "sniff test" to make sure the design seems unobjectionble. (It should be sufficient to just look at meshcat_internal.h and meshcat.cc to get the idea.)

Then we can tag Grant for platform tomorrow.

[jwnimmer-tri]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

[SeanCurtis-TRI]

Seems reasonable to me.

Reviewed 1 of 3 files at r2, 1 of 1 files at r3.
Reviewable status: 4 unresolved discussions, LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

---

geometry/meshcat.cc line 266 at r4 (raw file):

  DRAKE_NO_COPY_NO_MOVE_NO_ASSIGN(MeshcatShapeReifier);

  explicit MeshcatShapeReifier(

nit No longer necessary.

Code quote:

explicit 

---

geometry/meshcat_internal.h line 57 at r4 (raw file):

them. Meshcat is MUCH slower at loading bundled assets compared to unbundled.

When glTF file has relative path URIs (i.e., unbundled files on disk), this

nit: typo

Suggestion:

When a glTF

---

geometry/meshcat_internal.h line 58 at r4 (raw file):

When glTF file has relative path URIs (i.e., unbundled files on disk), this
loads the files FileStorage so that we can serve them later, even if the

nit: typo

Suggestion:

files into FileStorage

---

geometry/meshcat_internal.h line 62 at r4 (raw file):

@param[in] gltf_filename The glTF filename, used to calculate relative paths.
@param[in,out] gltf_contents The glTF data already loaded from `gltf_filename`.

nit; I think you mean "glTF file contents" as something to contrast with "glTF data" which would be the form that is stored in memory (e.g., decoded mesh data), right?

Code quote:

glTF data

[jwnimmer-tri]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

---

geometry/meshcat_internal.h line 62 at r4 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit; I think you mean "glTF file contents" as something to contrast with "glTF data" which would be the form that is stored in memory (e.g., decoded mesh data), right?

I tried to rephrase it a bit for clarity.

[SeanCurtis-TRI]

Reviewable status: 1 unresolved discussion, LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

---

geometry/meshcat.cc line 382 at r4 (raw file):

      }
    } else if (format == "gltf") {
      assets_ =

BTW It seems that we'd want to explain why we're locally overriding this parameter from the constructor. That seems a bit odd.

---

geometry/meshcat_internal.h line 62 at r4 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I tried to rephrase it a bit for clarity.

Very clear.

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

---

geometry/meshcat.cc line 382 at r4 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

BTW It seems that we'd want to explain why we're locally overriding this parameter from the constructor. That seems a bit odd.

I'm not sure what you mean. Are you saying we could envision the assets_ as [in,out] instead of just [out], so we should append to it here instead of overwriting it? (Maybe I could add Doxygen to the constructor to clarify what's going on?)

[SeanCurtis-TRI]

Reviewable status: 1 unresolved discussion, LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

---

geometry/meshcat.cc line 382 at r4 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I'm not sure what you mean. Are you saying we could envision the assets_ as [in,out] instead of just [out], so we should append to it here instead of overwriting it? (Maybe I could add Doxygen to the constructor to clarify what's going on?)

Clarification is key. You contrast [in,out] and [out], but there is, of course, no documentation to either extent. It stands in particular contrasts to its adjacent parameter, file_storage which is definitely [in,out].

Documenting the semantics is minimally sufficient.

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

---

geometry/meshcat.cc line 382 at r4 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

Clarification is key. You contrast [in,out] and [out], but there is, of course, no documentation to either extent. It stands in particular contrasts to its adjacent parameter, file_storage which is definitely [in,out].

Documenting the semantics is minimally sufficient.

Working

Right, the problem is it's really a new addition to the return value, and instead of refactoring the reifier stuff to admit as much, I hacked it into the constructor. I'll work on refactoring the reifier to "return" more than one value.

[jwnimmer-tri]

Reviewable status: LGTM missing from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

---

geometry/meshcat.cc line 382 at r4 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

Right, the problem is it's really a new addition to the return value, and instead of refactoring the reifier stuff to admit as much, I hacked it into the constructor. I'll work on refactoring the reifier to "return" more than one value.

Yup, much clearer now. Now we have consistency:

• the long-lived helper tools are constructor arguments,
• the shape details are the ImplementGeometry argument,
• the reification result is the reify output argument.

[rpoyner-tri]

Reviewed 6 of 12 files at r1, 1 of 3 files at r2, 1 of 1 files at r3, 2 of 2 files at r4, 1 of 2 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform)

[jwnimmer-tri]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform)

[jwnimmer-tri]

+@xuchenhan-tri for platform review per schedule, please.

[xuchenhan-tri]

Reviewed 6 of 12 files at r1, 1 of 3 files at r2, 1 of 1 files at r3, 2 of 2 files at r4, 1 of 2 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee SeanCurtis-TRI(platform)

---

geometry/meshcat_file_storage_internal.cc line 138 at r6 (raw file):

  // not contain a subdirectory (i.e., no "/" characters) because that confuses
  // any assets that refer to sub-assets.
  return fmt::format("cas-v1-{}", asset.sha256.to_string());

BTW, what does "v1? mean here?

Code quote:

cas-v1

---

geometry/meshcat_internal.cc line 98 at r6 (raw file):

      return nullptr;
    }
    // TODO(jwnimer-tri) Save the media type to http-serve later on.

typo

Suggestion:

// TODO(jwnimmer-tri) Save the media type to http-serve later on.

---

geometry/meshcat_internal.cc line 100 at r6 (raw file):

    // TODO(jwnimer-tri) Save the media type to http-serve later on.
    // For now, we'll just skip ahead to the actual content.
    std::string_view base64_content = uri.substr(pos + 8);

BTW, what is the magic number 8?

Code quote:

std::string_view base64_content = uri.substr(pos + 8);

[jwnimmer-tri]

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform)

---

geometry/meshcat_internal.cc line 100 at r6 (raw file):

Previously, xuchenhan-tri wrote…

BTW, what is the magic number 8?

Replaced with a more meaningful constant expression.