Implement PUSH_CONSTANTS feature #777
Conversation
![monocodus[bot]](https://avatars.githubusercontent.com/in/28526?s=60&v=4){kind=small}
There was a problem hiding this comment.
The .monocodus config not found in your repo. Default config is used.
Check config documentation here
There was a problem hiding this comment.
The .monocodus config not found in your repo. Default config is used.
Check config documentation here
cwfitzgerald
force-pushed
the
push-constants
branch
from
1e54766 to
f2d8222
Compare
cwfitzgerald
force-pushed
the
push-constants
branch
from
f2d8222 to
3d05aa1
Compare
|
Alright, I think this is good to go, will update the wgpu-rs PR shortly. |
wgpu-core/src/command/render.rs
Outdated
| /// None means there is no data and the data should be an array of zeros. | ||
| /// | ||
| /// Facilitates clears in renderbundles which explicitly do their clears. | ||
| values_offset: Option<usize>, |
There was a problem hiding this comment.
does this need to be usize? I.e. can we say that you aren't going to have more than 64k of push constants ever, and have this offset being u16 instead? We'd want to keep the enum RenderCommand size to be small. Even though right now it may not be the enum variant that makes it big, we don't want to face this later down the road when we try to shrink it.
There was a problem hiding this comment.
I'm going to make this a u32, as this index can be as large as the sum of all push constants used in a render pass, and if someone sets a lot of push constants (or used metal's 4k to their full advantage), it would overrun a u16 pretty quickly.
wgpu-core/src/command/render.rs
Outdated
| ); | ||
| for range in non_overlapping { | ||
| let clear_bytes = | ||
| vec![0_u32; (range.range.end - range.range.start) as usize / 4]; |
There was a problem hiding this comment.
I think the way we'd set this up in the near future (or now?) is: having a fixed-size zero vector somewhere, say in Device, say of 256 bytes. Then this code would have a loop that re-uses the same vector and calls push_graphics_constants as many times as needed to fill in zeroes.
cwfitzgerald
force-pushed
the
push-constants
branch
from
3d05aa1 to
e721984
Compare
|
Alrighty, should be good, rebasing... |
cwfitzgerald
force-pushed
the
push-constants
branch
from
e721984 to
c7c4c3e
Compare
|
That was a pretty nasty rebase, let me get wgpu-rs up to date to make sure I didn't break anything. |
cwfitzgerald
force-pushed
the
push-constants
branch
from
c7c4c3e to
c8bcc50
Compare
|
Fixed! |
435: Implement PUSH_CONSTANTS feature r=kvark a=cwfitzgerald The rust half of the push constant extensions, continuing from gfx-rs/wgpu#777. Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
435: Implement PUSH_CONSTANTS feature r=kvark a=cwfitzgerald The rust half of the push constant extensions, continuing from gfx-rs#777. Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
778: Return None from get_swap_chain_preferred_format if the adapter does not support the surface r=kvark a=OlegOAndreev Fixes gfx-rs#777 Almost every other method in wgpu panics on failure, so I decided to make the return value an `Option<TextureFormat>`, not a `Result<Option<TextureFormat>>` Co-authored-by: Oleg Andreev <ooandreev@yandex-team.ru>
Connections
Closes #734. Makes minor progress on #689.
Description
This one is a doozy.
Implements Push Constant support in wgpu.
Implementation Notes:
Code Notes:
Other Work:
PipelineLayoutDescriptorwas moved into wgt from wgpu-rs with a generic.Testing
I have modified the wgpu-rs example to use push constants for its uniform fallback, which is a perfect use case for push constants. I have not tested compute constants, but I expect no issues with them.
gfx-rs/wgpu-rs#435