Skip to content
This repository was archived by the owner on Jun 18, 2021. It is now read-only.

Implement PUSH_CONSTANTS feature #435

Merged
merged 1 commit into from
Jul 13, 2020
Merged

Conversation

cwfitzgerald
Copy link
Member

The rust half of the push constant extensions, continuing from gfx-rs/wgpu#777.

@cwfitzgerald cwfitzgerald marked this pull request as draft July 11, 2020 04:38
@kvark kvark self-requested a review July 13, 2020 05:15
@cwfitzgerald cwfitzgerald force-pushed the push-constants branch 2 times, most recently from d3f8f60 to be75abe Compare July 13, 2020 16:37
bors bot added a commit to gfx-rs/wgpu that referenced this pull request Jul 13, 2020
777: Implement PUSH_CONSTANTS feature r=kvark a=cwfitzgerald

**Connections**

Closes #734. Makes minor progress on #689.

**Description**

This one is a doozy.

Implements Push Constant support in wgpu.

Implementation Notes:
- Push constants are unconditionally cleared on change to a pipeline with a different pipeline layout. This could be elided in a future revision, possibly making push constants slightly faster on non-vulkan platforms.
- This exposes basically a direct port of vulkan push constants to wgpu. There might be design decisions that would want to be changed for an upstream webgpu implementation.

Code Notes:
- The render bundle code needs to be heavily scrutinized because I wasn't able to test it and it requires pipeline invalidation.
- Validation should be correct as I have tested it pretty throughly, but there are a lot of factors involved, so I could have accidentally missed something (or not allowed something I should have).

Other Work:
- `PipelineLayoutDescriptor` was 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


Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
@cwfitzgerald
Copy link
Member Author

Should be ready for review.

@cwfitzgerald cwfitzgerald marked this pull request as ready for review July 13, 2020 18:06
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful! Just needs a rebase

@cwfitzgerald
Copy link
Member Author

Rebased and fixed a warning on wasm that has been bugging me for the last 2 months.
bors r+

@cwfitzgerald
Copy link
Member Author

bors r-

@bors
Copy link
Contributor

bors bot commented Jul 13, 2020

Canceled.

@cwfitzgerald
Copy link
Member Author

bors r=kvark

@bors
Copy link
Contributor

bors bot commented Jul 13, 2020

@bors bors bot merged commit 32de700 into gfx-rs:master Jul 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants