-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Upgrade wgpu #830
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
Upgrade wgpu #830
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got this to compile using the latest releases of wgpu
and wgpu_glyph
.
However, I am getting some shader validation errors once I try to run the tour
.
Running `target/debug/tour`
wgpu error: Validation Error
Caused by:
In Device::create_render_pipeline
note: label = `iced_wgpu::quad pipeline`
error matching VERTEX shader requirements against the pipeline
shader global ResourceBinding { group: 0, binding: 0 } is not available in the layout pipeline layout
buffer structure size 80, added to one element of an unbound array, if it's the last field, ended up greater than the given `min_binding_size`
let me take a look, thanks for the patch so hopefully not too bad to fix |
talked to wgpu team and had me make an issue: gfx-rs/naga#882 will be waiting on that. I was suggested to just pad it up to the size it wants, but if i do that the uniform size doesn't match, and then if i pad the actual byte array it gives the original error again, so going to look into it further. Seems to be something introduced in 0.8.1 |
As was mentioned in that issue, the error wasn't super helpful but structs in uniforms need to align to their largest member, in this case mat4x4 which aligns to 16: https://gpuweb.github.io/gpuweb/wgsl/#alignment-and-size. Modified to fix padding for quad, also noticed triangle was doing the same thing, but it was already aligned to 16 so the current padding could just be removed. I'm not sure what issue that caused previously, but everything runs correctly on my system without it so looks like its ok to me |
Thanks for fixing that @Dispersia! 🎉 I believe this should probably fix #511 too. However, I still get a panic caused by a validation error on macOS Catalina:
|
Appreciate you testing on mac! I'll make sure to try and cover all of the other platforms then (since I don't have a mac yet, unfortunately). Created an issue here: gfx-rs/naga#888, unsure how metal solves this problem |
I just tested your branch on Linux/X11. The wgpu integration example works fine but the solar system one crashes on startup.
|
interesting, that was passing before 0.8 release but i guess new validation broke it, which is good! However, looking into that pipeline, there's actually several things im having a hard time understanding what it's doing, the main thing is the uniform, it's a dynamically sized transform, but isn't defined as an array in the shader. I'm not sure how glsl is even handling that currently, but once i figure out the intent I'll try to get a fix for that. Thanks for testing it! |
I have reverted 2d549d8, as removing the padding in
I think I have also fixed the issue reported by @kaimast in 4cbc345. Let me know! All the examples seem to work on my end now. |
ha, you would think it would be obvious that i removed padding and then had alignment issues, but oh well haha. Thanks for the patch, tested on x11 and windows and all examples seem good to me too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again everyone! I think we can merge this 🎉
Draft PR to upgrade wgpu and use wgsl so cross can stop being compiled for android, includes temporary patches until fix for staging belt goes out.
Fixes #511.
Closes #523.