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

Validate that BufferDescriptor::usage is not zero #3928

Merged
merged 2 commits into from Jul 14, 2023

Conversation

nical
Copy link
Contributor

@nical nical commented Jul 13, 2023

Checklist

  • Run cargo clippy.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections

Found this while investigating CTS failures in Firefox.

Description

WebGPU's spec says that descriptor.usage must not be 0. The vulkan spec as well, in fact it's the validation layer that helped me notice.

Testing

It's covered by the CTS run by Firefox, so I got lazy and did not produce a test. Let me know if you want a test here as well.

@nical nical changed the title Validate that BufferDesctor::usage is not zero. Validate that BufferDesctor::usage is not zero Jul 13, 2023
@ErichDonGubler
Copy link
Member

Nit on OP title: s/BufferDesctor/BufferDescriptor

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.

Pretty easy to validate this against the spec.:

5.1.3. Buffer Creation

createBuffer(descriptor)

Device timeline initialization steps:

  1. If any of the following requirements are unmet, generate a validation error, make bi invalid, and stop.

  • descriptor.usage must not be 0.

LGTM!; Note, though, that I don't have approving or merging rights, so…😅🤷🏻‍♂️

@nical nical changed the title Validate that BufferDesctor::usage is not zero Validate that BufferDescriptor::usage is not zero Jul 13, 2023
Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

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

Let's get that Cargo.lock change out of there. Otherwise, looks good.

@ErichDonGubler Thanks for checking the spec.

@nical
Copy link
Contributor Author

nical commented Jul 13, 2023

Let's get that Cargo.lock change out of there. Otherwise, looks good.

Annoyingly rust-analyzer always instantly puts it back by building its thing

@ErichDonGubler
Copy link
Member

ErichDonGubler commented Jul 13, 2023 via email

@nical nical merged commit 7198d60 into gfx-rs:trunk Jul 14, 2023
20 checks passed
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.

None yet

3 participants