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

BufferGeometryUtils: Add mergeGroups(). #23756

Merged
merged 1 commit into from
Mar 21, 2022
Merged

BufferGeometryUtils: Add mergeGroups(). #23756

merged 1 commit into from
Mar 21, 2022

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Mar 21, 2022

Depends on #23740.

Description

Introduces a utility function to merge groups in geometries. Can be used to optimize the rendering of multi-material meshes by lowering draw calls. Also required for an upcoming routine that converts a multi-material mesh into separate meshes.

As suggested in #13748 (comment), this PR adds an index if not present and only sorts the index. It can also work on interleaved index data.

Closes #13748.

@mrdoob mrdoob added this to the r139 milestone Mar 21, 2022
@mrdoob mrdoob merged commit 24ca284 into mrdoob:dev Mar 21, 2022
@mrdoob
Copy link
Owner

mrdoob commented Mar 21, 2022

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Mar 21, 2022

However... I'm starting to really dislike groups and multi materials...

I'm not sure if it helps, but what about an API like this?

const geometry = new BufferGeometry();
geometry.addAtrribute( 'position', new BufferAtrribute( data, 3 ) );

// SubBufferGeometry is basically like geometry.group... maybe BufferGeometryView is a better name?
const subGeometry1 = new SubBufferGeometry( geometry, 0, 100 );
const subGeometry2 = new SubBufferGeometry( geometry, 100, 200 );

// Compound would be like Group, but transforms for the children are ignored
const compound = new Compound(); 
compound.add( new Mesh( subGeometry1, material1 ) );
compound.add( new Mesh( subGeometry2, material2 ) );
scene.add( compound );

I feel like something like this could be easier to handle (for the developer and for the renderer).

@Mugen87 @donmccurdy Thoughts?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 22, 2022

That looks indeed cleaner however I'm not sure multi-materials or a similar feature like Compound is required at all. It's actually easier to enable/use stuff like batching or instanced rendering if no such logic exists. Besides, the engine provides an existing way to achieve more or less the same in a different way (by using multiple instances of BufferGeometry and a combination of Group/Meshes).

WebGPURenderer does currently not support multi-materials and if there will be no compelling demand from user side, I think it's better to stop multi-material support and provide no substitute.

@mrdoob
Copy link
Owner

mrdoob commented Mar 22, 2022

I think something like SubBufferGeometry is more intuitive and more flexible. With .groups you need to make sure that the index of the array matches the index in the multi-material array. The SubBufferGeometry approach is less error prone in that regard.

It also makes it much easier to render different parts of the same geometry data in different parts of the scene. With groups is much more convoluted to do that.

Hmm... BufferGeometryView may be a better name indeed.

The Compound class is to keep the current behavior in which groups do not have transforms. But also, I think it matches better with GLTF because you can have different meshes in the same node/transform.

@donmccurdy What do you think?

@mrdoob
Copy link
Owner

mrdoob commented Mar 22, 2022

Hmm... This approach would also make multi-material obsolete, which is also code/design I would love to get rid of...

The only use case I can think of where this makes things more difficult are when dealing with SkinnedMesh with groups.

@takahirox
Copy link
Collaborator

MMD might be a good example to check if the functionalities keep working because it consists of groups, SkinnedMesh, morph, and keyframe animations.

@donmccurdy
Copy link
Collaborator

multi-material / geometry.groups / Compound

Removing multi-material and geometry.groups would be a good direction, I think. What makes things more difficult with SkinnedMesh, though?

I'm not sure I see the need for a Compound class that ignores transforms, but I'm not opposed to it. Perhaps it should not allow >1 layer of nesting? Or what would happen if you put an Object3D or Group into a Compound? Another idea for a name would be something like TransformGroup to imply it's similar to Group and that all objects share the same transform.


BufferGeometryView / SubBufferGeometry

If we introduce an API like BufferGeometryView / SubBufferGeometry I hope we could plan how they might work with (or replace) the proposals below:

More concretely, do we want an API allowing multiple Mesh instances to use different parts of a BufferGeometry, or of a WebGL buffer? The latter is more low-level, but potentially allows more optimization too.

Perhaps it's too different but I'll bring up the BatchedMesh proposal as well ...

... this isn't a direct parallel for BufferGeometryView of course, but it is another way to draw many meshes from a single geometry.

@joshuaellis joshuaellis mentioned this pull request Mar 26, 2022
10 tasks
abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
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.

4 participants