Implement push constants for pipeline cache #4908
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
james7132
added
C-Feature
james7132
requested changes
Jun 4, 2022
| #[derive(Default)] | ||
| struct LayoutCache { | ||
| layouts: HashMap<Vec<BindGroupLayoutId>, wgpu::PipelineLayout>, | ||
| layouts: HashMap<(Vec<BindGroupLayoutId>, Vec<PushConstantRange>), wgpu::PipelineLayout>, |
There was a problem hiding this comment.
This key is getting pretty darn big. We should probably consider making this a dedicated type and use bevy_utils::PreHashMap
There was a problem hiding this comment.
@james7132 I don't understand how PreHashMap helps in this case. Hashed<K> also stores K. If anything, using PreHashMap makes the key bigger because it memoizes the u64 hash.
I'm necromancing this thread b/c @Neo-Zhixing is trying to do something similar in #7681 .
MDeiml
force-pushed
the
push_constants
branch
5 times, most recently
from
a83116e to
cd8482c
Compare
MDeiml
force-pushed
the
push_constants
branch
from
11b5e81 to
658a6b0
Compare
|
This feature would be super useful. Any updates on this? It's been a while. @cart @superdump |
|
Adopted in #7681. |
bors bot
pushed a commit
that referenced
this pull request
Feb 17, 2023
# Objective Allow for creating pipelines that use push constants. To be able to use push constants. Fixes #4825 As of right now, trying to call `RenderPass::set_push_constants` will trigger the following error: ``` thread 'main' panicked at 'wgpu error: Validation Error Caused by: In a RenderPass note: encoder = `<CommandBuffer-(0, 59, Vulkan)>` In a set_push_constant command provided push constant is for stage(s) VERTEX | FRAGMENT | VERTEX_FRAGMENT, however the pipeline layout has no push constant range for the stage(s) VERTEX | FRAGMENT | VERTEX_FRAGMENT ``` ## Solution Add a field push_constant_ranges to` RenderPipelineDescriptor` and `ComputePipelineDescriptor`. This PR supersedes #4908 which now contains merge conflicts due to significant changes to `bevy_render`. Meanwhile, this PR also made the `layout` field of `RenderPipelineDescriptor` and `ComputePipelineDescriptor` non-optional. If the user do not need to specify the bind group layouts, they can simply supply an empty vector here. No need for it to be optional. --- ## Changelog - Add a field push_constant_ranges to RenderPipelineDescriptor and ComputePipelineDescriptor - Made the `layout` field of RenderPipelineDescriptor and ComputePipelineDescriptor non-optional. ## Migration Guide - Add push_constant_ranges: Vec::new() to every `RenderPipelineDescriptor` and `ComputePipelineDescriptor` - Unwrap the optional values on the `layout` field of `RenderPipelineDescriptor` and `ComputePipelineDescriptor`. If the descriptor has no layout, supply an empty vector. Co-authored-by: Zhixing Zhang <[email protected]>
myreprise1
pushed a commit
to myreprise1/bevy
that referenced
this pull request
Feb 18, 2023
# Objective Allow for creating pipelines that use push constants. To be able to use push constants. Fixes bevyengine#4825 As of right now, trying to call `RenderPass::set_push_constants` will trigger the following error: ``` thread 'main' panicked at 'wgpu error: Validation Error Caused by: In a RenderPass note: encoder = `<CommandBuffer-(0, 59, Vulkan)>` In a set_push_constant command provided push constant is for stage(s) VERTEX | FRAGMENT | VERTEX_FRAGMENT, however the pipeline layout has no push constant range for the stage(s) VERTEX | FRAGMENT | VERTEX_FRAGMENT ``` ## Solution Add a field push_constant_ranges to` RenderPipelineDescriptor` and `ComputePipelineDescriptor`. This PR supersedes bevyengine#4908 which now contains merge conflicts due to significant changes to `bevy_render`. Meanwhile, this PR also made the `layout` field of `RenderPipelineDescriptor` and `ComputePipelineDescriptor` non-optional. If the user do not need to specify the bind group layouts, they can simply supply an empty vector here. No need for it to be optional. --- ## Changelog - Add a field push_constant_ranges to RenderPipelineDescriptor and ComputePipelineDescriptor - Made the `layout` field of RenderPipelineDescriptor and ComputePipelineDescriptor non-optional. ## Migration Guide - Add push_constant_ranges: Vec::new() to every `RenderPipelineDescriptor` and `ComputePipelineDescriptor` - Unwrap the optional values on the `layout` field of `RenderPipelineDescriptor` and `ComputePipelineDescriptor`. If the descriptor has no layout, supply an empty vector. Co-authored-by: Zhixing Zhang <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Allow for creating pipelines that use push constants. To be able to use push constants this depends on #4824. Fixes #4825.
Solution
Add a field
push_constant_rangestoRenderPipelineDescriptorandComputePipelineDescriptorChangelog
push_constant_rangestoRenderPipelineDescriptorandComputePipelineDescriptorbevy_sprite,bevy_uiandbevy_pbrto declare the new fieldMigration Guide
push_constant_ranges: Noneto everyRenderPipelineDescriptorandComputePipelineDescriptor