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

Make shader write&read storage buffers match non readonly layouts #3893

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Make shader write&read storage buffers match non readonly layouts #3893

merged 1 commit into from
Jun 29, 2023

Conversation

fornwall
Copy link
Contributor

@fornwall fornwall commented Jun 28, 2023

Checklist

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

Connections
See #2897, write-only storage buffer cannot match a Pipeline Layout.

Description
From the linked issue: When you provide a storage buffer pipeline layout with read-only set to false, if the shader marks the buffer is write, it will complain about a mismatched PLL and shader

Testing
Mark a storage storage buffer as write in a shader, and use it with a buffer pipeline layout with read-only set to false. There is also a unit test of the address_space_matches function.

@fornwall
Copy link
Contributor Author

After doing this, I realised that perhaps write-only storage buffers are not allowed in WGSL? From https://www.w3.org/TR/WGSL/#storage-buffer:

A variable in the storage address space is a storage buffer variable. Its store type must be a host-shareable type and must satisfy the address space layout constraints. The variable may be declared with a read or read_write access mode; the default is read.

If so, should #2897 be closed instead?

@cwfitzgerald
Copy link
Member

Welp, lets go ahead and merge this as it fixes the current behavior, thanks! But apparently they aren't anymore. There was never really a point, but whatever. Removing that is a separate issue.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Some small nits, but code looks good

wgpu-core/src/validation.rs Outdated Show resolved Hide resolved
wgpu-core/src/validation.rs Outdated Show resolved Hide resolved
@fornwall
Copy link
Contributor Author

@cwfitzgerald Thanks for the feedback! I addressed your comments, you could re-review?

@fornwall fornwall requested a review from cwfitzgerald June 29, 2023 20:54
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) June 29, 2023 22:21
@cwfitzgerald cwfitzgerald merged commit 17143c1 into gfx-rs:trunk Jun 29, 2023
@fornwall fornwall deleted the write-only-storage-buffer-match branch June 30, 2023 07:33
teoxoy added a commit to teoxoy/wgpu that referenced this pull request Jul 27, 2023
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