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

Derive storage bindings via naga::StorageAccess instead of naga::GlobalUse #3985

Merged
merged 4 commits into from
Jul 31, 2023

Conversation

teoxoy
Copy link
Member

@teoxoy teoxoy commented Jul 27, 2023

Resolves #3983.

Changes

  • Derive storage bindings via naga::StorageAccess instead of naga::GlobalUse.
  • Removes the feature check for read-only and read-write storage textures as it's later checked by create_bind_group_layout.
    match access {
    wgt::StorageTextureAccess::ReadOnly
    | wgt::StorageTextureAccess::ReadWrite
    if !self.features.contains(
    wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES,
    ) =>
    {
    return Err(binding_model::CreateBindGroupLayoutError::Entry {
    binding: entry.binding,
    error: binding_model::BindGroupLayoutEntryError::StorageTextureReadWrite,
    });
    }
    _ => (),
    }
  • Also removes the GlobalUse checks from check_binding_use as naga is already checking those.
    https://github.com/gfx-rs/naga/blob/535701f6b2f38f7e0088701ee4cf6d8024f74a21/src/valid/interface.rs#L639-L669

…lobalUse`

Removes the feature check for read-only and read-write storage textures as it's later checked by `create_bind_group_layout`.

https://github.com/gfx-rs/wgpu/blob/335f40f85a492a5d9a69eda6835a30173f2e6b03/wgpu-core/src/device/resource.rs#L1505-L1518

Also removes the `GlobalUse` checks from `check_binding_use` as naga is already checking those.

https://github.com/gfx-rs/naga/blob/535701f6b2f38f7e0088701ee4cf6d8024f74a21/src/valid/interface.rs#L639-L669
@ErichDonGubler
Copy link
Member

I'll take this as a reviewer! I might not be able to do it until my tomorrow morning, but I'll strive to do it before next office hours on the Mozilla side.

Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

LGTM (noting that my review is impotent still, according to GH)! It took me some reading, but after getting to #3983 (comment), the changes here clicked for me. It's great that this makes for simpler code, too.

CHANGELOG.md Show resolved Hide resolved
@teoxoy teoxoy merged commit 48078a8 into gfx-rs:trunk Jul 31, 2023
20 checks passed
@teoxoy teoxoy deleted the derive-bindings-via-access branch July 31, 2023 13:30
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.

Shader translation from SPIR-V fails targeting DX12
2 participants