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

WebGPURenderer: Update attribute only when needed #28701

Merged
merged 7 commits into from
Jun 19, 2024

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Jun 19, 2024

Related issue: #28697

Description

This PR addresses an issue where WebGPURenderer was updating all attributes in every render frame, regardless of their state. With this fix, attributes are now updated only if attribute.needsUpdate has been called, enhancing performance. /cc @sunag

This PR fixes the handling of interleavedBuffer management.

InstanceNode was missing a way to notify the associated buffers if the instanceMatrix and instanceColor were flagged to be updated, I also added a way to transfer the correct version to the corresponding buffers.

This contribution is funded by Utsubo

@RenaudRohlinger
Copy link
Collaborator Author

Consequently the example webgpu_instance_mesh didn't use mesh.instanceMatrix.needsUpdate = true; but was updated regardless since the WebGPURenderer was updating every attributes every render.

I will also need to fix the fact adding mesh.instanceMatrix.needsUpdate = true; doesn't seems to fix the current example.

@RenaudRohlinger
Copy link
Collaborator Author

RenaudRohlinger commented Jun 19, 2024

Added a fix in InstanceNode to notify if the buffers needs to be updated or not and fixed the webgpu_instance_mesh example.

@RenaudRohlinger RenaudRohlinger added this to the r166 milestone Jun 19, 2024
@@ -85,6 +93,22 @@ class InstanceNode extends Node {

}

update( /*frame*/ ) {

if ( this.buffer !== null && this.instanceMesh.instanceMatrix.version !== this.buffer.version ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This check necessary? this.buffer !== null update() should be called after setup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed! Updated the PR accordingly 👍

@sunag sunag merged commit a83652d into mrdoob:dev Jun 19, 2024
11 checks passed
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.

2 participants