Mesh shaders - initial wgpu hal changes #7089
Conversation
Please enter a commit message to explain why this merge is necessary,
SupaMaggie70Incorporated
changed the title
|
Going to look at your proposal in a minute, but there's nothing really controversial in here, seems pretty straight forward. The spicy part is validation :) |
|
It is saying it's failing some benchmarks in the CI, is that a concern? Otherwise, should I mark this as a non-draft PR? Thanks! |
Yeah the error message is about a violation of a wgpu-hal precondition about features being violated, so there's something going on with wgpu-hal's features. After that is fixed, feel free to mark as ready-to-go |
wgpu-hal/src/vulkan/adapter.rs
Outdated
| .multiview_mesh_shader( | ||
| needed | ||
| && multiview | ||
| && phd_features.mesh_shader.unwrap().multiview_mesh_shader == 1, |
There was a problem hiding this comment.
Not sure if this is the best way to do this. I had to change stuff around so that whether this was supported was visible in this context. Worst comes to worst, we can forget multiview in mesh shaders for now. The main problem is that apparently llvmpipe supports mesh shaders and multiview but not multiview in mesh shaders.
There was a problem hiding this comment.
Lets just forget it, and we'll tackle it as a separate feature.
There was a problem hiding this comment.
Removed this
The CI pooped itself, hopefully this fixes that. Will probably be undone either way.
|
@cwfitzgerald Sorry to bug you again. CI says there are 30 checks which haven’t completed, but are expected and waiting to be reported. But then it also says there are no checks to be done. Do I have to do something to kickstart the checking process? For some of the previous commits, it hasn’t needed any prompting. Otherwise, I think this PR is ready to be merged :) |
CI runs on the result of merging with trunk, so if there are conflicts with the merge, it can't run CI - so if you fix those conflicts, the CI will run. |
|
Alright, will add this to my list to review properly before we land! |
wgpu-hal/src/vulkan/adapter.rs
Outdated
| .multiview_mesh_shader( | ||
| needed | ||
| && multiview | ||
| && phd_features.mesh_shader.unwrap().multiview_mesh_shader == 1, |
There was a problem hiding this comment.
Lets just forget it, and we'll tackle it as a separate feature.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Done from web out of impatience
Made CI less angry by fixing formatting(hopefully). This commit was also done from GitHub web.
|
Going to re-review this on Monday |
Alright, let’s get this landed! |
Connections
#3018
Description
Introduces basic mesh shading functionality to wgpu-hal
Testing
This change doesn't itself contain any tests. However, the functionality is tested and an example is provided on a separate branch of my fork.
Checklist
cargo fmt.taplo format.cargo clippy. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.Comments
This is one of my first PRs, so I probably did something wrong with regard to git or some other convention. Also, this is a very early draft PR. I just want some initial feedback.