Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add InstancedMesh example #187

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

siddancha
Copy link

@siddancha siddancha commented Feb 3, 2025

Resolves #141

The MeshCat API already implicitly supports threejs' InstancedMesh. Since InstancedMesh is a subclass of Object3D, the set_object API can be used to create an InstancedMesh of a given geometry and material.

This PR adds an example (in the test folder) that creates an InstancedMesh of BoxGeometries arranged in a 3x3x3 grid. Furthermore, the example updates the color buffer of the InstancedMesh using the set_property API. This showcases how one might update the colors (and similarly, poses) of instances in an InstancedMesh without deleting and creating a new InstancedMesh object.

After set_object:

After set_property:

This change is Reviewable

@joemasterjohn joemasterjohn self-assigned this Feb 19, 2025
Copy link
Contributor

@joemasterjohn joemasterjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 files reviewed, all discussions resolved


a discussion (no related file):
+@joemasterjohn for review.

Copy link
Contributor

@joemasterjohn joemasterjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions


a discussion (no related file):
Hi @siddancha, thanks for the contribution. Please consider some of the comments I left below.


test/instanced_mesh.html line 21 at r1 (raw file):

            type: "set_object",
            path: "/meshcat/instanced_cubes",
            object: {

I don't find much value in writing out JSON by hand for examples, especially JavaScript examples.

  1. It has to be manually updated to keep up with newer THREE.js object format versioning.
  2. It doesn't provide the user a method for generating their own JSON for their own purposes. It is still mysterious which properties must exist or how they should be formatted. Especially things like uuids etc.

I think an example written in JS that gives the user a way to spit out the JSON representation of whatever THREE.js object they're dealing with would be good. That would give them the reference to implement the object format correctly in whatever client code they are writing. Consider my update to your example here that does exactly that: https://github.com/joemasterjohn/meshcat/tree/instanced-mesh-example

If you agree, add me as a collaborator on your fork. I can push that commit (with co-author credits), and we can merge this very nice example.


test/instanced_mesh.html line 100 at r1 (raw file):

        // Set InstancedMesh properties.
        // Here, we are updating the colors of the cubes.
        viewer.handle_command({

This happens to work because the command executes before the first render call. Indeed if you put this call to set_property into a setTimeout(..., 5000) you won't see the color change. This is because any change to a buffer needs to be indicated by setting the needsUpdate attribute to true (see https://threejs.org/docs/#manual/en/introduction/How-to-update-things).

Copy link
Contributor

@joemasterjohn joemasterjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @siddancha)

Copy link
Author

@siddancha siddancha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for building on this example -- your interactive demo is awesome! I'm not convinced that the example should be written in native Javascript/THREE.js code vs. handwritten JSONs; more details in my reply to your comment.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @joemasterjohn)


test/instanced_mesh.html line 21 at r1 (raw file):

Previously, joemasterjohn (Joe Masterjohn) wrote…

I don't find much value in writing out JSON by hand for examples, especially JavaScript examples.

  1. It has to be manually updated to keep up with newer THREE.js object format versioning.
  2. It doesn't provide the user a method for generating their own JSON for their own purposes. It is still mysterious which properties must exist or how they should be formatted. Especially things like uuids etc.

I think an example written in JS that gives the user a way to spit out the JSON representation of whatever THREE.js object they're dealing with would be good. That would give them the reference to implement the object format correctly in whatever client code they are writing. Consider my update to your example here that does exactly that: https://github.com/joemasterjohn/meshcat/tree/instanced-mesh-example

If you agree, add me as a collaborator on your fork. I can push that commit (with co-author credits), and we can merge this very nice example.

Hi @joemasterjohn! Thanks for building on this example -- your interactive demo is awesome! Correct me if I'm wrong, but I still think that writing out the JSON is what we need to demo this in the Meshcat API. Here's my reasoning:

  1. I'm just following the other examples in the test folder, which also write out JSONs by hand. Instead, you created an InstancedMesh object in native Javascript/THREE.js, and are passing its JSON to the set_object command via the .toJSON() call. None of the other test examples use the .toJSON() call.
  2. I think this makes sense because the way a non-JavaScript user would interact with the Meshcat API (e.g. via meshcat-python) is precisely by hand-writing JSONs and passing them to the set_object command. For example, I'm working on a PR to create InstancedMeshes in Meshcat from Drake: SEE CODE HERE. To implement this, I need to know what InstancedMesh's JSON looks like. Your native Javascript/THREE.js code unfortunately doesn't tell me much about what its JSON looks like and what fields I should set (THREE.js is to blame for that).

With this example (along the same lines as other examples) I'm hoping to expose what the nominal format of Meshcat's API would look like for InstancedMeshes. Does that make sense?


test/instanced_mesh.html line 100 at r1 (raw file):

Previously, joemasterjohn (Joe Masterjohn) wrote…

This happens to work because the command executes before the first render call. Indeed if you put this call to set_property into a setTimeout(..., 5000) you won't see the color change. This is because any change to a buffer needs to be indicated by setting the needsUpdate attribute to true (see https://threejs.org/docs/#manual/en/introduction/How-to-update-things).

Agreed! I agree that we need to set instanceMatrix.needsUpdate after executing set_property commands, like you did in your branch. I will copy that once the previous comment is resolved.

Copy link
Author

@siddancha siddancha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could include both my hand-written JSON example and your THREE.js based example?

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @joemasterjohn)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InstancedMesh support
2 participants