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

Buffer usages mismatch check and documentation for mapped_at_creation size requirement. #3023

Merged
merged 6 commits into from
Sep 19, 2022

Conversation

Imberflur
Copy link
Contributor

@Imberflur Imberflur commented Sep 14, 2022

Checklist

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

Connections
I based this on the documented validation behavior:

Description
Buffer usage mismatch validation was lost to a refactor. Specifically, this is the check needed when one of the MAP_* usages is enabled. This re-adds the check.

I also added some documentation on the the validation of the buffer size when mapped_at_creation is true.

Testing
A test was added at wgpu/tests/buffer_usage.rs. I confirmed that improper combinations were being allowed via this test and that with the changes they now fail validation.

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.

I love the tests!

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) September 19, 2022 04:15
@cwfitzgerald cwfitzgerald merged commit 41006d7 into gfx-rs:master Sep 19, 2022
@Imberflur Imberflur deleted the buffer_usages_mismatch_check branch September 19, 2022 06:28
@Imberflur
Copy link
Contributor Author

I was looking over this again and realized there is a small issue with the way the tests are set up. Specifically, I tried to reduce the amount of times we need to perform the setup so that it doesn't need to be done before running each case. However, this means with the failing cases we only test that the first item fails! 😭

Maybe I could instead use error scopes to catch the errors so each failure case doesn't need to redo the whole setup?

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