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

GLTFExporter: Support EXT_mesh_gpu_instancing to export InstancedMesh. #26854

Merged
merged 3 commits into from
Oct 5, 2023

Conversation

repalash
Copy link
Contributor

Description

Adds support to export InstancedMesh in gltf for both the instanceMatrix and instanceColor. Also, change in GLTFLoader to load the exported instanceColor.
Changed the gltf_exporter example to add a test InstancedMesh in scene1.

This contribution is funded by Threepipe

@Mugen87 Mugen87 added this to the r157 milestone Sep 27, 2023
@mrdoob mrdoob modified the milestones: r157, r158 Sep 28, 2023
TRANSLATION: writer.processAccessor( new BufferAttribute( translationAttr, 3 ) ),
ROTATION: writer.processAccessor( new BufferAttribute( rotationAttr, 4 ) ),
SCALE: writer.processAccessor( new BufferAttribute( scaleAttr, 3 ) ),
};
Copy link
Collaborator

@takahirox takahirox Oct 1, 2023

Choose a reason for hiding this comment

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

A comment for potential future optimization. (So no need to do in this PR.)

As commented at

https://github.com/takahirox/three-gltf-extensions/blob/00e4233e4fc6c086f13c1c2de80ecc8a3c7eec29/exporters/EXT_mesh_gpu_instancing/EXT_mesh_gpu_instancing_exporter.js#L55

the attributes are optional and we may be able to think of exporting an attribute only if it has non-default value. For example, it may not be too rare that all scales are default (1, 1, 1) and in such a case the exported file size can be reducted.

Another approach would be the exporter always exports all the attributes as this PR does, and the optimization can be done in external gltf post-processing pipeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I have tried it here: PalashBansal96@85bac3d

If it looks fine I can push it in this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can also merge the PR first and then apply the optimization. In this way, the changes presented in this PR are not blocked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can make another PR later.

@Mugen87 Mugen87 merged commit b72b7db into mrdoob:dev Oct 5, 2023
18 checks passed
@Mugen87 Mugen87 changed the title GLTFExporter: Support EXT_mesh_gpu_instancing to export InstancedMesh. Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants