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

Compute Shader Integration #3970

Closed
kurtkuehnert opened this issue Feb 17, 2022 · 2 comments
Closed

Compute Shader Integration #3970

kurtkuehnert opened this issue Feb 17, 2022 · 2 comments
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@kurtkuehnert
Copy link
Contributor

What problem does this solve or what need does it fill?

For now Bevy does not have a dedicated compute pipeline abstraction, but only exposes the raw wgpu API for this use case.
The game of life example shows how to implement your own compute pipelines.

Currently there is no way to use Bevy's shader abstraction(shader defs, shader imports and hot shader reloading) for compute, which makes writing those shaders quite cumbersome.

What solution would you like?

I would like to have a way of turning a Handle<Shader> into a ShaderModule, which can be use for creating the compute pipeline.
For the render pipelines we use a RenderPipelineCache, which intern stores a ShaderCache, for processing the shaders.
We probably need something similar for compute pipelines (ComputePipelineCache), or we decouple the ShaderCache entirely from the RenderPipelineCache to cache all shaders in a single place (resource).

@kurtkuehnert kurtkuehnert added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Feb 17, 2022
@superdump superdump moved this to Todo in Rendering Feb 17, 2022
@superdump superdump added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Feb 17, 2022
@cart
Copy link
Member

cart commented Feb 17, 2022

Agreed. I think we should heavily consider renaming RenderPipelineCache to PipelineCache and extending it to include compute pipelines. Render and Compute pipelines could (and should) both utilize the same LayoutCache and ShaderCache. It would be a pretty nasty UX reduction to break those out / force users to contend with passing them around directly.

The only missing piece here is adding a higher level ComputePipelineDescriptor (that largely mirrors our RenderPipelineDescriptor) and wiring that up to the PipelineCache.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! label Feb 18, 2022
@kurtkuehnert
Copy link
Contributor Author

Sounds reasonable, I will try taking a stab at it .

@superdump superdump moved this from Todo to In Progress in Rendering Feb 25, 2022
@superdump superdump moved this from In Progress to In Review in Rendering Mar 7, 2022
@bors bors bot closed this as completed in 9e450f2 Mar 23, 2022
Repository owner moved this from In Review to Done in Rendering Mar 23, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this issue Jun 7, 2022
# Objective

- Fixes bevyengine#3970
- To support Bevy's shader abstraction(shader defs, shader imports and hot shader reloading) for compute shaders, I have followed carts advice and change the `PipelinenCache` to accommodate both compute and render pipelines.

## Solution

- renamed `RenderPipelineCache` to `PipelineCache`
- Cached Pipelines are now represented by an enum (render, compute)
- split the `SpecializedPipelines` into `SpecializedRenderPipelines` and `SpecializedComputePipelines`
- updated the game of life example

## Open Questions

- should `SpecializedRenderPipelines` and `SpecializedComputePipelines` be merged and how would we do that?
- should the `get_render_pipeline` and `get_compute_pipeline` methods be merged?
- is pipeline specialization for different entry points a good pattern




Co-authored-by: Kurt Kühnert <51823519+Ku95@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

- Fixes bevyengine#3970
- To support Bevy's shader abstraction(shader defs, shader imports and hot shader reloading) for compute shaders, I have followed carts advice and change the `PipelinenCache` to accommodate both compute and render pipelines.

## Solution

- renamed `RenderPipelineCache` to `PipelineCache`
- Cached Pipelines are now represented by an enum (render, compute)
- split the `SpecializedPipelines` into `SpecializedRenderPipelines` and `SpecializedComputePipelines`
- updated the game of life example

## Open Questions

- should `SpecializedRenderPipelines` and `SpecializedComputePipelines` be merged and how would we do that?
- should the `get_render_pipeline` and `get_compute_pipeline` methods be merged?
- is pipeline specialization for different entry points a good pattern




Co-authored-by: Kurt Kühnert <51823519+Ku95@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants