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

Ensure that pipeline creation errors register layouts as errors also #4624

Merged

Conversation

bradwerth
Copy link
Contributor

@bradwerth bradwerth commented Nov 2, 2023

Since the pipeline id is provided by the caller, the caller may presume that an implicit pipeline layout id is also created, even in error conditions such as with an invalid device. Since our registry system will panic if asked to retrieve a pipeline layout id that has never been registered, it's dangerous to leave that id unregistered. This ensures that the layout ids also get error values when the pipeline creation returns an error.

Checklist

  • Run cargo clippy.
  • Run cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections
Link to the issues addressed by this PR, or dependent PRs in other repositories

Description
Allows user agents to drop the implicit pipeline layouts associated with a pipeline, without worrying about whether or not the registry has created an entry for that layout.

Testing
None yet.

Since the pipeline id is provided by the caller, the caller may presume
that an implicit pipeline layout id is also created, even in error
conditions such as with an invalid device. Since our registry system
will panic if asked to retrieve a pipeline layout id that has never been
registered, it's dangerous to leave that id unregistered. This ensures
that the layout ids also get error values when the pipeline creation
returns an error.
@bradwerth bradwerth requested a review from a team as a code owner November 2, 2023 22:13
@bradwerth bradwerth marked this pull request as draft November 3, 2023 21:16
@bradwerth
Copy link
Contributor Author

Through some manual testing, found that repeated calls to insert_error will panic. Need to use some method of setting values that can tolerate an existing value already being present.

@bradwerth bradwerth marked this pull request as ready for review November 3, 2023 21:27
daxpedda and others added 11 commits November 4, 2023 19:53
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Introduce a new struct `Scalar`, holding a scalar kind and width, and
use it as appropriate in the WGSL front end. This consolidates
many (kind, width) pairs, and lets us name the two components.

Ideally, `Scalar` would be used throughout Naga, but this would be a large
change, touching hundreds of use sites. This patch begins by
introducing `Scalar` to the WGSL front end only.
wgpu-core/src/device/global.rs Outdated Show resolved Hide resolved
cwfitzgerald and others added 9 commits November 6, 2023 07:58
* It verks!

* More tests

* Fixes

* Working multi-stage push constants

* Comments

* Add push constant partial update teste

* Docs

* Update Cargo.toml

* Comments
Since the pipeline id is provided by the caller, the caller may presume
that an implicit pipeline layout id is also created, even in error
conditions such as with an invalid device. Since our registry system
will panic if asked to retrieve a pipeline layout id that has never been
registered, it's dangerous to leave that id unregistered. This ensures
that the layout ids also get error values when the pipeline creation
returns an error.
@bradwerth bradwerth requested a review from a team as a code owner November 6, 2023 18:22
Since the pipeline id is provided by the caller, the caller may presume
that an implicit pipeline layout id is also created, even in error
conditions such as with an invalid device. Since our registry system
will panic if asked to retrieve a pipeline layout id that has never been
registered, it's dangerous to leave that id unregistered. This ensures
that the layout ids also get error values when the pipeline creation
returns an error.
@ErichDonGubler ErichDonGubler merged commit 7aaf672 into gfx-rs:trunk Nov 6, 2023
27 checks passed
@bradwerth bradwerth deleted the registerImplicitPipelineLayouts branch November 6, 2023 20:54
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.

8 participants