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

WebGPURenderPipelines: Add pipeline cache. #21741

Merged
merged 2 commits into from
Apr 29, 2021
Merged

WebGPURenderPipelines: Add pipeline cache. #21741

merged 2 commits into from
Apr 29, 2021

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Apr 28, 2021

Related issue: -

Description

In WebGL features like depth testing or blending were controlled via WebGLState. These configurations could be changed independently of shader programs. This is not possible in WebGPU where pipelines are used to control the rendering stages.

Up to now, WebGPURenderer did not support changes of many material properties after the object's creation. The PR changes this by introducing a new method that checks if a new rendering pipeline is required or not.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 28, 2021

@Kangz It seems there is no way to explicitly destroy an instance of GPURenderPipeline. Or did I miss something?

Depending on how the user configures a material, the required pipelines could change over time. Do you think it makes sense to save existing pipelines in a cache based on a key in order to avoid calls of createRenderPipeline()? Or is this over-engineering?

If WebGPU implementations already do some sort of pipeline caching it's questionable if engines should do this as well.

@Mugen87 Mugen87 marked this pull request as draft April 28, 2021 15:14
@Kangz
Copy link

Kangz commented Apr 28, 2021

@Kangz It seems there is no way to explicitly destroy an instance of GPURenderPipeline. Or did I miss something?

Nope there's no way to explicitly destroy them at the moment.

Depending on how the user configures a material, the required pipelines could change over time. Do you think it makes sense to save existing pipelines in a cache based on a key in order to avoid calls of createRenderPipeline()?

It depends on how frequently the pipeline would be recreated. If it's once in a while, Chromium has an internal cache so it should be fine (and I expect other browser will have a cache too). If it's every frame or even multiple times per frame then it's probably better to have some kind of cache. But that has a CPU cost on the JS side as well.

Babylon.js has more dynamic state and found the need to cache pipelines on their side, but my understanding is that Three.js's API allows less dynamic changes so you ideally you can let the browser cache.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 28, 2021

Thanks for the feedback! I guess this topic needs to be investigated in more detail in order to find out what setup makes sense for three.js.

E.g. when we started to cache uniforms in WebGL, we have found out that caching uniform arrays introduced a performance issue (see #16355) and thus was not beneficial. I'd like to avoid the same conceptual mistake in WebGPU right from the beginning 😅 . Meaning if we introduce a caching mechanism, the implementation should be straightforward and manageable so we are not accidentally degrading the overall engine performance. In any event, it's good to now that Chromium has an internal pipeline cache!

Before this PR can be merged, I'd like to find a solution for sharing render pipelines across 3D objects. Also testing material and render properties for changes does not make sense for each 3D object. This can be done once per frame. I guess it make sense to have a similar logic like the program cache in WebGLRenderer (WebGLPrograms). Only for pipelines.

@Mugen87 Mugen87 changed the title WebGPURenderPipelines: Determine pipeline based on parameters. WebGPURenderPipelines: Add pipeline cache. Apr 29, 2021
@Mugen87 Mugen87 marked this pull request as ready for review April 29, 2021 13:08
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 29, 2021

Okay, I have added a cache for render pipelines so they can be shared across render items. There is no 1:1 relationship between 3D object and render pipeline anymore (which was a temporary solution anyway). We can check on dev how this system goes.

@Mugen87 Mugen87 merged commit 4ad8243 into mrdoob:dev Apr 29, 2021
@Mugen87 Mugen87 added this to the r129 milestone Apr 29, 2021
@mrdoob
Copy link
Owner

mrdoob commented May 3, 2021

Thanks!

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