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: Reduce overhead from excessive cache sharing for postprocessing nodes. #29198

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

aardgoose
Copy link
Contributor

Related issue: #27447

An alternative approach to reducing the overhead from the cache sharing that results from multiple renderTargets that mapping to a single internal RenderContext. This adds a unique id and a boolean property 'isolate' - default false, to RenderTarget().

These are used to control renderContext sharing as required in the post processing nodes: GaussianBlurNode, BloomNode, AfterImageNode and PMREMGenerator.

I think this is a cleaner mechanism than creating and managing additional QuadMesh objects.

For the PMREMGenerator this PR combined with #29197 prevents all per frame WebGPU object creation in the webgpu_cubemap_dynamic example.

Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
685.4 kB (169.7 kB) 685.5 kB (169.7 kB) +71 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
462 kB (111.4 kB) 462 kB (111.5 kB) +71 B

@RenaudRohlinger RenaudRohlinger added WebGPU TSL Three.js Shading Language labels Aug 22, 2024
@RenaudRohlinger
Copy link
Collaborator

A lot of exciting performance improvements with #29197 and #29183. Nice!

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 5, 2024

The current workaround of creating an instance of QuadMesh per render target isn't ideal as well as having a separate render target property. Both approaches are not intuitive and I fear developers won't get this right. I do think we need a solution that requires no additional work on app level.

Couldn't we enhance the renderer instead so a render object for each QuadMesh + render target combination is created instead? If that isn't doable, we should have something similar to simply avoid the creation of BindGroups and BindGroupLayouts. Maybe we can use an additional caching mechanism to save BindGroups and BindGroupLayouts and then reuse them.

@RenaudRohlinger
Copy link
Collaborator

RenaudRohlinger commented Nov 5, 2024

I cannot agree more on this. Even yesterday I stumbled upon this issue when swapping between 2 RTT without carefully re-binding in a custom Postprocessing Node, infinite amount of bingGroup were stacking. It's very common if you're not careful with RenderTarget and Postprocessing setups. I also agree that the real solution would be to fix it on the caching strategy level. /cc @sunag

@sunag
Copy link
Collaborator

sunag commented Nov 5, 2024

I also think the cache management should take care of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TSL Three.js Shading Language WebGPU
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants