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: avoid render pass/pipeline attachment mismatches #26376

Merged
merged 24 commits into from
Jul 8, 2023

Conversation

aardgoose
Copy link
Contributor

Render pass attachment characteristics and pipeline attachment expectations must match. The following sequence can result in a mismatch followed by WebGPU errors because the chain map keys used to locate matching renderObjects and related pipelines does not represent attachment status.

  1. .render( scene, camera ); // pipelines created and keyed by (scene, camera) etc.

  2. .setRenderTarget( newTarget ); // new target with mismatch of color format or sample count etc.

  3. .render( scene, camera ); // pipelines from (1) used which fail with renderPass for newTarget.

This PR adds the render target to the key for renderContexts and uses renderContext as a replacement key element for renderObjects. This prevents the reuse of incompatible pipelines.

aardgoose added 21 commits June 9, 2023 10:07
@sunag sunag self-assigned this Jul 5, 2023
@sunag sunag added this to the r155 milestone Jul 6, 2023
@sunag
Copy link
Collaborator

sunag commented Jul 6, 2023

One approach to consider is to cache information of a RenderTarget that impacts pipeline and shader creation like { colorSpace, sampler } in order to reduce the numbers of RenderObjects.

Many RenderTargets will have similar characteristics, only the stored texture will be different.

@aardgoose
Copy link
Contributor Author

I looked at using something like attachmentState = { color format, sample count, depth-stencil } as well, and agree that there is there is more renderObject creation in some cases. I didn't want to add another layer to the renderContext chain map, but that could be avoided by appending attachmentState to the passId string?

On the 'for' side to adding render target to renderContext is more opportunity for caching the renderPass descriptor and avoiding createView() calls.

@LeviPesin
Copy link
Contributor

Maybe we can implement a function like RenderContext.getCacheKey() and use that cache key instead of render context?

@sunag sunag merged commit 366d701 into mrdoob:dev Jul 8, 2023
@aardgoose aardgoose deleted the render-context branch July 28, 2023 15:08
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.

3 participants