1240: Fix issue with multiple queued mappings where the last one is not cancelled r=kvark a=Imberflur
**Connections**
Follow up from #1239
**Description**
In #1239 I missed the case where there are multiple rounds of mapping/unmapping with the last map not being cancelled. In practice it doesn't occur very often, but I did encounter it with settings/a scene that created a heavy GPU load.
Sorry for not catching this earlier!
**Testing**
Using `wgpu-rs` capture example, after the first `map_async` I added in an `unmap` followed by another `map_async`.
<!--
Non-trivial functional changes would need to be tested through:
- [wgpu-rs](https://github.com/gfx-rs/wgpu-rs) - test the examples.
- [wgpu-native](https://github.com/gfx-rs/wgpu-native/) - check the generated C header for sanity.
Ideally, a PR needs to link to the draft PRs in these projects with relevant modifications.
See https://github.com/gfx-rs/wgpu/pull/666 for an example.
If you can add a unit/integration test here in `wgpu`, that would be best.
-->
Co-authored-by: Imbris <imbrisf@gmail.com>
1239: Avoid panic when requesting to unmap a buffer that is pending mapping r=kvark a=Imberflur
**Connections**
Fix for #1238
**Description**
This might not be the cleanest fix but it is quite minimal. I'm not sure what the exact intent is with the organization of things and with the async status enum, would be happy to hear critiques.
**Testing**
Tested against example in #1238
<!--
Non-trivial functional changes would need to be tested through:
- [wgpu-rs](https://github.com/gfx-rs/wgpu-rs) - test the examples.
- [wgpu-native](https://github.com/gfx-rs/wgpu-native/) - check the generated C header for sanity.
Ideally, a PR needs to link to the draft PRs in these projects with relevant modifications.
See https://github.com/gfx-rs/wgpu/pull/666 for an example.
If you can add a unit/integration test here in `wgpu`, that would be best.
-->
Co-authored-by: Imbris <imbrisf@gmail.com>
1237: Move `#[error]` attributes after the corresponding `#[derive]` r=kvark a=Aaron1011
**Connections**
See https://github.com/rust-lang/rust/issues/79202
**Description**
Fixes Nightly future-incompat warnings
**Testing**
There are no behavior changes intended.
Co-authored-by: Aaron Hill <aa1ronham@gmail.com>
* const in ShaderStage that represents vertex and fragment access
We can store `ShaderStage`s as const values or members of const structs in all individual cases, but for example `ShaderStage::VERTEX | ShaderStage::FRAGMENT` cannot be stored in a const. This is not very elegant, but would be convenient.
* Correct VERTEX_FRAGMENT definition
VERTEX_FRAGMENT definition now follows directly from the bit representation of `ShaderStage::VERTEX` and `Shaderstage::FRAGMENT` such that it does not need to be independently maintained.
1231: Added TextureFormatFeatures::filterable which may overwrite TextureSampleType::Float.filterable r=kvark a=Wumpf
**Description**
The expectation is that with `wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES` enabled, any texture format which is sampled as float may be (linearly) filtered if so supported by the adapter, not only those that the webgpu specification denotes as filterable.
This arguably causes a bit of a mess in the way we distinguish `sample_type` and `(guaranteed_)format_features`: The float sample type (via spec) essentially integrates a feature, namely if it is filterable or not. Unlike the actual format _type_ which is just a property of the format, this property may or may not be available depending on the device.
So the type of `sample_type` stays statically defined whereas the Float.filterable property suddenly becomes overwritten by a format_feature which may or may not be present. Couldn't come up with a cleaner solution so far.
**Testing**
Tested in project (blub) depending on R32F filtering - passing now what was previously causing
```
wgpu error: Validation Error
Caused by:
In Device::create_bind_group
note: label = `BindGroup: Narrow Range filter 1`
texture binding 1 expects sample type = Float { filterable: true }, but given a view with format = R32Float
```
(Blub still can't run on latest wgpu because of other reported issues)
Co-authored-by: Andreas Reich <r_andreas2@web.de>
1227: Derive more things for primitive and multisampling states r=kvark a=kvark
**Connections**
Related to https://github.com/gfx-rs/wgpu-rs/issues/769
**Description**
Adding a few derives and fixing a doc comment.
**Testing**
CI gets it
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1226: Update gfx and naga to gfx-12 r=kvark a=kvark
**Connections**
Picks up https://github.com/gfx-rs/gfx/pull/3650Fixes#1225
**Description**
Also fixes our playtests, interestingly. One of the changes here was that now `Analysis` is needed for everything, including the SPV-out backend of Naga. And we can only get it via validating the `naga::Module`.
What we used to was: if validation isn't enabled, we'd carry the module around, and then still try to produce a SPIR-V from it, if there is no original SPIR-V. This precise thing was happening in the tests. However, we had `dummy` backend depending on the `wgpu-core/cross` feature, which means the playtests were actually built and run with spirv-cross enabled!
Another factor here is the introduction of `ShaderModule::flags` instead of a single boolean, which was done in #1093. The problem here was that the playtest RON files weren't updated accordingly, they still had `experimental_translation: true` in them, which got ignored now.
So with this combination of events, the playtests ended up generating SPIR-V from Naga modules, without validation(!), and passing the SPIR-V to gfx-rs and SPIRV-Cross... Which still worked, as we'd expect, but definitely not want here.
So this PR fixes everything. It makes it required to have the validation for a module, and we force-validate if there is no SPV source to fall back on. And it disables "cross" feature implicitly enabled in testing.
**Testing**
Tested on wgpu-rs examples.
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1224: Update the blend API to upstream r=kvark a=kvark
**Connections**
Matches https://github.com/gpuweb/gpuweb/pull/1134
**Description**
Makes blending state ON/OFF explicit.
**Testing**
Simple enough!
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1220: Update naga to gfx-11, make spirv-cross optional r=kvark a=kvark
**Connections**
https://github.com/gfx-rs/gfx/pull/3642https://github.com/gfx-rs/gfx/pull/3641
**Description**
Gets us the latest Naga updates, plus the ability to make spirv-cross optional (on Linux and Windows only for now).
**Testing**
Will be tested on wgpu-rs
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1219: Validate texture bindings r=cwfitzgerald a=kvark
**Connections**
It was mentioned/requested somewhere, but I can't find the place...
**Description**
Fairly straightforward - just check the view properties against the bind group layout.
**Testing**
Tested on wgpu-rs examples
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1210: Reset the bindings on the push constant change r=wumpf a=kvark
**Connections**
Fixes#1207
(unconfirmed!)
**Description**
I was able to replay the trace in #1207 without validation errors, so I looked deeper into what might have gone wrong.
The code isolated in #1194 looks great. However, the calling code's handling of push constants might have changed a bit.
And I see that the trace uses push constants, so here is what happened, I think:
- layout A was set, expecting bind group layouts X and Z
- bind groups X and Y where bound
- layout B was set, expecting bind group layout X and Y. We see that Y group can now be bound, so we do this. The old logic in this case wasn't considering the push constants in any way, it would only consider them if bind group layouts didn't change, erroneously.
- layout B has different push constant ranges...
So it's a one-line fix now, which I'm hoping is correct.
**Testing**
Not tested
@Wumpf would you be able to check this?
Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
1209: Handle player window resizing gracefully r=kvark a=kvark
**Connections**
Deprecates #794 and #793
**Description**
We were often getting this kind of errors when replaying on Linux:
>VALIDATION [VUID-VkSwapchainCreateInfoKHR-imageExtent-01274 (2094043421)] : Validation Error: [ VUID-VkSwapchainCreateInfoKHR-imageExtent-01274 ] Object 0: handle = 0x56194a357250, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0x7cd0911d | vkCreateSwapchainKHR() called with imageExtent = (1980,1080), which is outside the bounds returned by vkGetPhysicalDeviceSurfaceCapabilitiesKHR(): currentExtent = (800,600), minImageExtent = (800,600), maxImageExtent = (800,600). The Vulkan spec states: imageExtent must be between minImageExtent and maxImageExtent, inclusive, where minImageExtent and maxImageExtent are members of the VkSurfaceCapabilitiesKHR structure returned by vkGetPhysicalDeviceSurfaceCapabilitiesKHR for the surface (https://www.khronos.org/registry/vulkan/specs/1.2-extensions/html/vkspec.html#VUID-VkSwapchainCreateInfoKHR-imageExtent-01274)
object info: (type: DEVICE, hndl: 94666619187792)
It's annoying to work around!
**Testing**
Tested on replaying wgpu-rs stuff.
Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
1206: Convert `PrimitiveState::cull_mode` to `Option<Face>` r=kvark a=yzsolt
**Connections**
Closes#1192
**Description**
`wgpu::CullMode` was an enum with a `None` variant, which would be more idiomatic as an `Option` in Rust.
**Testing**
- `wgpu-rs` builds with the appropriate changes
- `wgpu-native` needs https://github.com/gfx-rs/wgpu-native/pull/71 merged before it can be updated
Co-authored-by: Zsolt Bölöny <bolony.zsolt@gmail.com>
1205: Update naga to gfx-10, add push constants validation r=kvark a=kvark
**Connections**
Picks up https://github.com/gfx-rs/gfx/pull/3632 with a bunch of Naga stuff
**Description**
Update!
**Testing**
Not much
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1190: Fix crash on zero init of buffer with no more ref count r=kvark a=Wumpf
**Description**
Previously, if a buffer would no longer have any reference, the zero init step crashed when trying to unwrap its ref_count.
**Testing**
Tested successfully on 54a0f4ff13 where this problem would pop up
Co-authored-by: Andreas Reich <r_andreas2@web.de>
1191: Buffer zero init test for binding (and use in a compute shader) r=kvark a=Wumpf
**Connections**
Buffer zero init #1159
Buffer zero crash on unused buffer fix#1190
**Description**
Tried to create a player ron file repro case for #1190 but didn't work out quite well (test keeps buffers alive even when I tried to go through an indirection via buffer copies). But with a bit of cleanup the test still felt very useful since it gives more coverage to buffer zero init.
Generally zero init is hard to test since the way to get the result is to map it which is one particular case of requiring zero init. But there's a few ways like in this test here where we look at what modifications a compute shader did to a buffer - since the buffer was not explicitly initialized, the outcome would be different if the compute shader would have seen a buffer without zero init.
Given how few tests there are right now this ofc implicitly tests a bunch of other things ;)
Co-authored-by: Andreas Reich <r_andreas2@web.de>
1201: Don't check the index format for non-indexed calls r=kvark a=kvark
**Connections**
Fixes#1200
**Description**
Avoid the check that shouldn't be done.
**Testing**
Not tested, but should work.
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1198: Fix command allocator race condition with maintenance r=kvark a=kvark
**Connections**
Fixes#1196
**Description**
This was a recent change, where I thought it would be more pure to just prepare the pool but not create anything right away. It looked more elegant, but now we see it was flowed.
**Testing**
Not really tested the concurrent aspect of it, but it should work.
Perhaps, we can hook up Loom in the future for this task?
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1194: Isolate binding compatibility logic into a separate module r=cwfitzgerald a=kvark
**Connections**
Fixes#1188
**Description**
The actual problem was the following: when a pipeline changed, we used to go through the binding entries and re-bind only those that *became* compatible. However, we were missing ones that were already compatible, but were not activated because they were behind some previously incompatible ones.
So it's not a very complex fix. However, working with the binding code had me concerned that it wasn't isolated enough. It got quite a few issues discovered in it over the time, which only proven the point that it was way too fragile. So I took this opportunity to bring us closer to the world I want wgpu to be in: the world of semi-independent testable components.
This PR makes the binding compatibility checker one of these components. It's fully isolated, has unit tests, fairly straightforward to reason about, and integrates nicely with the render pass logic (without too many generics!). It's more profitable in the long run, I think, to build this kind of architecture versus integration-testing the thing (even at `wgpu` playtest level).
There is less magic overall now: no binding iterator, no fancy chaining, no complex matching. The logic is straightforward, hopefully. And the 40 LOC saved is a good indication of it.
**Testing**
Tested on wgpu-rs examples.
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1187: validate for strip_index_format being used with non-strip topology r=kvark a=Wumpf
**Description**
Hit lack of this validation when porting a project to newest wgpu-rs: Accidentally set the strip_index_format for a TriangleList pipeline which left me with a Vulkan validation error (since wgpu decides to set `restart_index` in presence of a `strip_index_format`)
```
VALIDATION [VUID-VkPipelineInputAssemblyStateCreateInfo-topology-00428 (-1077750125)] : Validation Error: [ VUID-VkPipelineInputAssemblyStateCreateInfo-topology-00428 ] Object 0: handle = 0x1ebbfa512a8, type = VK_OBJECT_TYPE_DEVICE; | MessageID
= 0xbfc2d693 | vkCreateGraphicsPipelines() pCreateInfo[0]: topology is VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST and primitiveRestartEnable is VK_TRUE. It is invalid. The Vulkan spec states: If topology is VK_PRIMITIVE_TOPOLOGY_POINT_LIST, VK_PRIMITIVE_TOPOLOGY_LINE_LIST, VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST, VK_PRIMITIVE_TOPOLOGY_LINE_LIST_WITH_ADJACENCY, VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST_WITH_ADJACENCY or VK_PRIMITIVE_TOPOLOGY_PATCH_LIST, primitiveRestartEnable must be VK_FALSE (https://vulkan.lunarg.com/doc/view/1.2.141.0/windows/1.2-extensions/vkspec.html#VUID-VkPipelineInputAssemblyStateCreateInfo-topology-00428)
object info: (type: DEVICE, hndl: 2112044208808)
```
(arguably wgpu could be more clever about this and just not set `restart_index`, but that leaves the user code in a non-sense state)
**Testing**
One-off test triggering the error through wgpu-rs.
Co-authored-by: Andreas Reich <r_andreas2@web.de>
1183: Improve docs language r=cwfitzgerald a=danielzgtg
**Description**
- s/fact culling/face culling/g
- s/are draw /are drawn /g
**Testing**
- There are no code change.
- The documentation should be less confusing
Co-authored-by: Daniel Tang <danielzgtg.opensource@gmail.com>
