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] Compute minimum binding size correctly for arrays. #5222

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

jimblandy
Copy link
Member

@jimblandy jimblandy commented Feb 8, 2024

In early versions of WGSL, storage or uniform global variables had
to be either structs or runtime-sized arrays. This rule was relaxed,
and now globals can have any type; Naga automatically wraps such
variables in structs when required by the backend shading language.

Under the old rules, whenever wgpu-core saw a storage or uniform
global variable with an array type, it could assume it was a
runtime-sized array, and take the stride as the minimum binding size.
Under the new rules, wgpu-core must consider fixed-sized and
runtime-sized arrays separately.

Checklist

  • Run cargo fmt.
  • Run cargo clippy.
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@jimblandy jimblandy added area: validation Issues related to validation, diagnostics, and error handling area: correctness We're behaving incorrectly labels Feb 8, 2024
@jimblandy jimblandy requested a review from a team as a code owner February 8, 2024 00:11
@nical nical enabled auto-merge (squash) February 8, 2024 09:53
@ErichDonGubler
Copy link
Member

@jimblandy
Copy link
Member Author

Hoho, maybe we can reliably reproduce #3193 now??

In early versions of WGSL, `storage` or `uniform` global variables had
to be either structs or runtime-sized arrays. This rule was relaxed,
and now globals can have any type; Naga automatically wraps such
variables in structs when required by the backend shading language.

Under the old rules, whenever wgpu-core saw a `storage` or `uniform`
global variable with an array type, it could assume it was a
runtime-sized array, and take the stride as the minimum binding size.
Under the new rules, wgpu-core must consider fixed-sized and
runtime-sized arrays separately.

Fixes gfx-rs#5219.
@nical nical merged commit 4af531c into gfx-rs:trunk Feb 9, 2024
27 checks passed
@jimblandy
Copy link
Member Author

Okay, the change between 3194089 and 7ffe936 made CI stop complaining, which is further info for #3189. I'll publish the former commit somewhere accessible.

suspistew pushed a commit to suspistew/wgpu that referenced this pull request Mar 23, 2024
…s#5222)

* [wgpu-core] Add tests for minimum binding size validation.

* [wgpu-core] Compute minimum binding size correctly for arrays.

In early versions of WGSL, `storage` or `uniform` global variables had
to be either structs or runtime-sized arrays. This rule was relaxed,
and now globals can have any type; Naga automatically wraps such
variables in structs when required by the backend shading language.

Under the old rules, whenever wgpu-core saw a `storage` or `uniform`
global variable with an array type, it could assume it was a
runtime-sized array, and take the stride as the minimum binding size.
Under the new rules, wgpu-core must consider fixed-sized and
runtime-sized arrays separately.
@jimblandy jimblandy deleted the array-min-binding-size branch March 31, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: correctness We're behaving incorrectly area: validation Issues related to validation, diagnostics, and error handling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants