1036: Stop using Borrow<RefCount> r=scoopr a=kvark
**Connections**
Follow-up to #931
**Description**
Takes advantage of the new `trait Resource` in the state tracker, reducing the code.
**Testing**
Not tested outside of `cargo test`
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1033: Missing label queries r=kvark a=scoopr
I missed few bits needed for labeling the resources in the errors.
Co-authored-by: Mikko Lehtonen <scoopr@iki.fi>
931: Retain labels for resources r=kvark a=scoopr
This is wip, mostly a conversation starter.
I've attempted to have some labels show up in the errors, outside of only creation errors.
Steps to do that include,
1. Retain the label in the underlying struct (here, `Buffer`). It is guarded by `debug_assertions`
2. Add the label option to the error variant, and show it
3. Add label option to the `Element::Error` variant, so that invalid ids retain the label also. Didn't guard it with `debug_assertions` yet, but probably should. `buffer_error` now takes the label option.
4. Added a query for invalid id labels
5. Add `Global::buffer_label` for returning the label from the struct, or the invalid id.
6. Change the error variants to query the `buffer_label` when creating the error
with that (and little bit of fudging to get in to the error state), I can get
```
wgpu error: Validation error
Caused by:
In CommandEncoder::copy_buffer_to_buffer
buffer (0, 1, Metal) (label: Some("Staging buffer")) is invalid
```
instead of
```
wgpu error: Validation error
Caused by:
In CommandEncoder::copy_buffer_to_buffer
buffer (0, 1, Metal) is invalid
```
Some of the hub handling etc. is a bit iffy for me, I really have no idea what I'm doing.
The point would be to massage this to the point that there would be a simple steps to follow on how to decorate all the errors with labels to whatever they refer to.
Other ideas I had, included trying to have `impl Debug for Id<BufferId>` to print out the label (from the struct, wouldn't work for invalid ids), but in that context I don't think I have access to the Global, so I'm not sure how resolve the id to anything useful. I suppose there could a whole separate singleton storage for id to label maps which could be easier to access in any context. Then the errors could just store the Id, and not have a separate label field.
Co-authored-by: Mikko Lehtonen <scoopr@iki.fi>
1023: Drop surfaces and adapters r=cwfitzgerald a=kvark
**Connections**
Not very connected
**Description**
Refactors our destruction paths a bit
**Testing**
tested on wgpu-rs
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1021: Normalize `Err(x)?` to `return Err(x.into())` r=kvark a=Kimundi
Just a tiny change based on some matrix discussion.
Co-authored-by: Marvin Löbel <loebel.marvin@gmail.com>
1018: Add context for errors originating from commads in a Pass r=kvark a=Kimundi
**Description**
This adds some error context information for some errors that can happen in a `{Render,Compute}Pass` to make error messages more clear.
Combined with https://github.com/gfx-rs/wgpu-rs/pull/613, errors in passes will look like this:
```
wgpu error: Validation error
Caused by:
In a RenderPass
In a draw command
index 3600 extends beyond limit 38
```
Co-authored-by: Marvin Löbel <loebel.marvin@gmail.com>
907: Implement OpenGL Backend For Unix Platforms r=kvark a=zicklag
**Connections**
Requires: https://github.com/gfx-rs/gfx/pull/3340 ( merged )
Should be merged first: https://github.com/gfx-rs/wgpu/pull/910
Works towards: https://github.com/gfx-rs/wgpu/issues/450
**Description**
Integrates the GFX GL backend for Unix platforms
**Testing**
Runs the basic cube and triangle examples for wgpu-rs, but not without rendering artifacts on the cube.
<!--
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: Zicklag <zicklag@katharostech.com>
1008: Add helpers to convert passes to serialized forms r=grovesNL a=kvark
**Connections**
Stuff I needed to fix in the upcoming Gecko WebGPU update.
**Description**
It allows us to use the tracing `Command` more aggressively.
**Testing**
Tested in Gecko, doesn't need testing in wgpu
Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
1007: Improve cubemap validation r=kvark a=cwfitzgerald
**Connections**
Closes#952.
**Description**
We were missing depth-size and texture dimension validation when creating Cubemap and CubemapArray views.
I split the depth-size errors so I can give a more helpful error message in both cases.
**Testing**
Tested on the skybox example when made intensionally incorrect.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
1000: Elide redundant set_pipeline calls. r=kvark a=Kimundi
**Description**
This adds an check in each `set_pipeline()` call for wether the same pipeline id has already been set previously. This should cause free performance wins for suboptimal usage of the wgpu API, while having neglible overhead otherwise.
An example scenario for where this would be useful is a game that just blindly sets all render state for each object, but has few actually different objects:
```rust
for game_obj in game_objs {
rpass.set_pipeline(...);
rpass.set_bind_group(...);
rpass.set_vertex_buffer(...);
rpass.draw(...);
}
```
**Testing**
Ideally we would have tests that check that pipeline changes have been ellided, but I'm not sure how to write them in the current codebase.
**Future work**
- We could extend this kind of redundant state change detection to most `.set_*` methods on `RenderPass`es.
- If we want to guarantee this behavior in the API, we should also document it.
Co-authored-by: Marvin Löbel <loebel.marvin@gmail.com>
998: Fix regression in device_create_swap_chain r=kvark a=qthree
**Description**
My app works fine with wgpu 0.6 but after upgrade to master branch of `wgpu-rs` it started to panic inside `Device::create_swap_chain`.
**Repro steps**
That happens when there are at least 3 windows opened right after app's startup. My [viewports](https://github.com/qthree/viewports) crate can reproduce this issue. Steps:
```
git clone -b wgpu-master https://github.com/qthree/viewports
cd viewports
RUST_BACKTRACE=1 cargo run --example wgpu --all-features
```
First run is OK, but if you drag two "virtual" windows out of native window and then restart app, it panics.
**Connections**
It's caused by the new behavior of `hub::Storage::remove` introduced in #925, it panics while trying to remove Element::Vacant.
**Proposed solution**
Previously `Storage::remove` returned `None` on `Element::Vacant` and this behavior was expected by `device::Global::device_create_swap_chain`. At least, [this code](8ce2530b69/wgpu-core/src/device/mod.rs (L3561)) doesn't look like it should panic implicitly.
After #925 `device::Global::device_create_swap_chain` was changed to use new `Storage::try_remove`, and it takes into account out of range access, but panics on Element::Vacant.
Since that's the only place where `Storage::try_remove` is used, i changed this function's body back to the body of old `Storage::remove`, and left new `Storage::remove` as is, because it's used in other places.
**Testing**
Since `Storage::try_remove` is used only in one place, `device::Global::device_create_swap_chain`, there shouldn't be any impact on another pieces of code. Also, function code is as same as on stable wgpu 0.6, so there shouldn't be any new bugs. Still, this fixed regression encountered by me, and `cargo test` of `wgpu-core` is passing.
I'm not sure if I should add unit test to cover this case, to save it from regressions in future (and what meaningful test would look like).
**Alternatives**
Don't call `Storage::try_remove` if it's first swapchain creation for surface. Probably, by checking id returned by `surface_id.to_swap_chain_id(B::VARIANT)` with `Storage::contains`. But `Storage::contains` will panic on out of range id.
Co-authored-by: qthree <qthree3@gmail.com>
990: Simplify descriptor writes r=cwfitzgerald a=kvark
**Connections**
Reverts #980 and #970
Takes advantage of https://github.com/gfx-rs/gfx/pull/3410
**Description**
Weighting all the pros and cons, I figured it's easier to adjust the Vulkan backend a little than treating it as a lowest common denominator for this issue.
**Testing**
Tested on wgpu-rs examples (on Vulkan).
Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>