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

wgpu_core::command::bundle: Consolidate pipeline and vertex state. #2769

Merged

Conversation

jimblandy
Copy link
Member

@jimblandy jimblandy commented Jun 13, 2022

Refactor wgpu_core::command::bundle::State to more closely resemble the internal slots of a WebGPU GPURenderBundleEncoder, and add validation required by the WebGPU specification.

Use Option to represent state that may be left unset on the encoder: specifically, the pipeline and vertex buffers. (Previous commits have already addressed index buffers and bind groups.) Use .ok_or, etc. for unwrapping, to ensure that encoding state errors are reported.

Consolidate state derived from the pipeline in a new PipelineState struct.

Remove wgpu_core::command::bundle::PushConstantState::is_dirty; just represent push constant state as a vector of PushConstantRange values. It's sufficient to simply zero the push constants whenever the vector is non-empty.

Rename bundle::State::flush_push_constants to zero_push_constants. This is not a "flush pending state changes" function like all the others; it just ensures that each pipeline's push constant state is initialized.

@jimblandy jimblandy force-pushed the bundle-isolate-vertex-and-pipeline-state branch 2 times, most recently from 7d426b7 to 1c74a53 Compare June 14, 2022 06:35
@jimblandy jimblandy marked this pull request as ready for review June 14, 2022 06:35
@jimblandy jimblandy force-pushed the bundle-isolate-vertex-and-pipeline-state branch 2 times, most recently from 4f010c6 to d65fcd7 Compare June 14, 2022 07:06
Refactor `wgpu_core::command::bundle::State` to more closely resemble
the internal slots of a WebGPU `GPURenderBundleEncoder`, and add
validation required by the WebGPU specification.

Use `Option` to represent state that may be left unset on the encoder:
specifically, the pipeline and vertex buffers. (Previous commits have
already addressed index buffers and bind groups.) Use `.ok_or`, etc.
for unwrapping, to ensure that encoding state errors are reported.

Consolidate state derived from the pipeline in a new `PipelineState`
struct.

Remove `wgpu_core::command::bundle::PushConstantState::is_dirty`; just
represent push constant state as a vector of `PushConstantRange`
values. It's sufficient to simply zero the push constants whenever the
vector is non-empty.

Rename `bundle::State::flush_push_constants` to `zero_push_constants`a.
This is not a "flush pending state changes" function like all the
others; it just ensures that each pipeline's push constant state is
initialized.
@jimblandy jimblandy force-pushed the bundle-isolate-vertex-and-pipeline-state branch from d65fcd7 to e3573d2 Compare June 14, 2022 07:17
@jimblandy
Copy link
Member Author

jimblandy commented Jun 14, 2022

I hope this isn't too much stuff jammed into one PR. I think the easiest way to see what's going on is to look at the new definition of wgpu_core::command::bundle::State, the new PipelineState struct, and the changes to VertexState. Values are now stored based on where they originate, not where they are used. That's what makes it feasible to use Option to track what's actually there, which forces us to do validation at the right times.

@cwfitzgerald cwfitzgerald merged commit 915be10 into gfx-rs:master Jun 15, 2022
@jimblandy jimblandy deleted the bundle-isolate-vertex-and-pipeline-state branch June 28, 2022 00:44
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.

2 participants