-
Notifications
You must be signed in to change notification settings - Fork 250
Flesh out compute example #360
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
Conversation
Thank you for your PR! You should try building this on-top of #358, I was having a similar issue until I updated the nightly version. |
0431f10
to
912d35a
Compare
Just a copy of wgpu-rs's 'hello-compute' example: https://github.com/gfx-rs/wgpu-rs/tree/v0.6/examples/hello-compute
912d35a
to
2aaf73e
Compare
Yes, that did it, thanks. I also fixed the tests. But... I'm getting a segfault on the compute example (BTW the compute example isn't being tested, shall I add it to the tests?). The segfault seems to come from Vulkan, I suppose that backend could be an automatic choice for my machine? I should note that the compute example works fine when run from the wgpu repo. Here's what minimal info can be gleaned from
And I hunted down the offending block to this in
|
Yes please, thank you. cc @Jasper-Bekkers Are you able to tell if this is an upstream bug? |
I'll check it out - I suspect that we're probably just running into a validation error instead, so it would be good to run with validation enabled to double check. |
I'm getting some validation errors here at least #362 seems related
|
That makes sense, thanks. How can I run the validator myself to check if I've fixed it? Also, I added the compute test and now I see I suppose we have to install Vulkan in CI? https://github.com/EmbarkStudios/rust-gpu/pull/360/checks?check_run_id=1652063534#step:8:422 |
In the wgpu samples it should be enabled by default, but it would require you to have the VulkanSDK installed for them to run.
We probably just shouldn't run that sample on CI since there are no GPUs in the CI machines for now. |
Thanks. I'm on Arch and just installing You're right about the tests, it's not a simple matter of just installing Vulkan in CI, I assumed that because they have tests in gfx-rs/qgpu-rs then they use Vulkan in CI, but they don't. Though I wonder what you think about https://github.com/google/swiftshader It seems in theory shaders can be run without a GPU. Compute shaders are suitable for testing because they output simpler structures, not graphics, so it could be worth it to get coverage on this otherwise untested area of the code? |
I believe at least the validation error that @Jasper-Bekkers included is a bug in how we're generating the code so it requires fixes on our end, if there are other validation errors please do post them here.
We have an open issue about it here #136, and currently some support is lacking on the swift shader side. I think as long as we're ensuring the code compiles, that's fine for now. |
Ok, I'll leave this PR open until the code generation issue is fixed, is there an issue that we can track about that? I'll append the full validation report at the end of this comment. I'll disable the new test on this PR as well. It could be trivially re-enabled once #136 is done.
|
Yes it's #362 |
I'm still interested in getting this working. I'm just quite out of my depth with the internals that generate the spirv code. Any tips for where to get started with that? |
We've merged an updated version of this PR here #623 so closing this |
Just a copy of wgpu-rs's 'hello-compute' example: https://github.com/gfx-rs/wgpu-rs/tree/v0.6/examples/hello-compute
I'm not sure if this is even possible at the moment? Not to mention that this is my first time using rust-gpu (which is such a great project). I get a compiler error: