Skip to content

Conversation

@sunag
Copy link
Collaborator

@sunag sunag commented Mar 12, 2024

Related issue: Closes #27895

Description

In the vast majority of cases, cacheKeys are being generated without any real need, which would be to change the TSL code, causing overloading. This PR introduces node.needsUpdate=true so that the system can identify when it is really necessary to recreate the Node's cacheKey.

Comparation

Comparison below of webgpu_backdrop_water in the range of 1 to 4 seconds.

Before Now
image image

When should I use it?

Every time you need to update a specific Node chain, for exemple:

// mix( a, b, c ) = mix( aNode, bNode, cNode )
material.colorNode = mix( color( 0x0487e2 ), color( 0x0066ff ), normalWorld.y );

// ... in the software cycle

material.colorNode.aNode = color( 0xffffff );
material.colorNode.needsUpdate = true;

@sunag sunag marked this pull request as ready for review March 12, 2024 04:49
@sunag sunag added this to the r163 milestone Mar 12, 2024
@RenaudRohlinger
Copy link
Collaborator

This solution is so good!! 🙏

@mrdoob
Copy link
Owner

mrdoob commented Mar 12, 2024

this.version and this._version?

@RenaudRohlinger
Copy link
Collaborator

In the WebGLRenderer the strategy was often this.version !== thisProperties.__version.

Would rename this._version to this.__version be better? Where this.__version is basically this.cachedVersion.
/cc @mrdoob

@mrdoob
Copy link
Owner

mrdoob commented Mar 12, 2024

Ah right yes.
Forgot about the internal usage...

Looking good then! Let's follow how we did it in WebGLRenderer. I've never seen anyone having issues with it.

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.

WebGPURenderer: Performance Drop Due to Cache Key Overhead After PR #26942

4 participants