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

Better error message for 16 byte alignment error #3414

Merged
merged 9 commits into from
Jan 24, 2023

Conversation

IceSentry
Copy link
Contributor

@IceSentry IceSentry commented Jan 21, 2023

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
(Based on) Closes #3099
Closes #2832

Description
Previous error message was the generic downflags error message for BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED that didn't highlight that if it was 16 bit aligned then this is supported. But it wasn't clear which shader group / bind was not aligned.

New error message:

In Device::create_render_pipeline
      note: label = `cuboids_pipeline`
    shader group 1 binding 1 must be 16 bytes aligned as device does not support DownlevelFlags `BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED`

Also have switched the code around to check the flag once as if it's set then we can skip the check entirely.

Testing
I didn't test it myself, but the original author tested and I simply updated the message to address the comment on the PR

IceSentry and others added 2 commits January 24, 2023 13:55
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
@IceSentry
Copy link
Contributor Author

@cwfitzgerald btw, I replied to the code suggestion, but I know github notifications are a bit weird with resolved discussions so I'll ping you here.

@cwfitzgerald
Copy link
Member

@IceSentry Good shout on the ping - I didn't see this. I just realized the requirement is that the size, not the alignment, is a multiple of 16. So my suggestion is actually slightly wrong. /s/alignment/size should take care of it. The requirement should be "a multiple of 16".

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.

Party party party

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) January 24, 2023 20:34
@cwfitzgerald cwfitzgerald merged commit bb876f3 into gfx-rs:master Jan 24, 2023
@IceSentry IceSentry deleted the better-alignment-error branch April 9, 2023 22:52
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.

Helpful information for BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED
3 participants