1135: Make all textures support usage COPY_SRC and COPY_DST r=kvark a=fintelia
**Connections**
N/A
**Description**
In WebGPU, all textures should be allowed to have usages COPY_SRC and COPY_DST. However, right now when Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES is enabled any features that the backend says can't be blitted to/from will not gain these capabilities. In particular, compressed textures may not support blitting while still allowing access via copies to/from buffers.
You can also see the defaults used when TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES is not requested:
8c00cbcc8e/wgpu-types/src/lib.rs (L1224-L1342)
**Testing**
I ran a [patched version of the wgpu-rs cube example](https://github.com/fintelia/wgpu-rs/tree/cube-bc4/examples/cube).
Co-authored-by: Jonathan Behrens <fintelia@gmail.com>
1112: Expose adapter specific formats features via Extension, support for storage read+write r=kvark a=Wumpf
**Connections**
Draft until https://github.com/gfx-rs/gfx/pull/3559 landed (then update and comment two lines in)
New & improved replacement for #1109
PR for wgpu-rs to be opened once there's some confidence that this is that it should look like. [Preview](https://github.com/gfx-rs/wgpu-rs/compare/master...Wumpf:texture-format-feature-query?expand=1)
**Description**
Adapter exposes now per texture format specific features. Feature struct defined as allowed usages + flags. Flags describe only storage texture usage so far.
Query not used for validation unless `TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES` feature is enabled on a device. Otherwise uses hardcoded feature set guaranteed by webgpu.
Storage read/write binding can then be used as a consequence (storage atomic has no effect yet).
**Testing**
Manual testing with my fluid sim project through wgpu-rs which covers storage r/w for a few different formats and storage in general for formats that don't support this usage by default at all.
Ran tests and a couple of samples to see if anything broke with the feature disabled.
Co-authored-by: Andreas Reich <r_andreas2@web.de>
1124: Fix range tracker merging r=kvark a=kvark
**Connections**
Fixes#1123
**Description**
The merge routine was not properly updating the end pointer in some branches.
**Testing**
A unit test is added with the data extracted from the user case.
Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
1115: Request buffer FAST_DEVICE_ACCESS only with some non MAP or COPY usage r=kvark a=fintelia
**Connections**
Fixes#1113
**Description**
On Linux AMD (and likely elsewhere) buffers with FAST_DEVICE_ACCESS are very slow to access when mapped. This makes sure that upload/download buffers don't request it unless those buffers are leveraging MAPPABLE_PRIMARY_BUFFERS for some non-map/copy usage.
**Testing**
I reran the failing reproduction case in the linked issue.
Co-authored-by: Jonathan Behrens <fintelia@gmail.com>
1103: Fix Assorted Low-Hanging Validation and Error Message Issues r=kvark a=cwfitzgerald
**Connections**
Closes#1085Closes#393Closes#1053
**Description**
These commits are independent and should be reviewed individually. Combined into a single PR to reduce noise.
Overview of what was done:
- Add validation for empty texture and buffer usage flags. (#393)
- Add allowed texture usage flags to `format.describe()` (#1085)
Validate new textures follow the allowed usage flags.
- Properly validates vertex and buffers are bound. (#1053)
Improves error messages when no vertex buffer is bound. (Before it said the limit was 0, now it says something is unbound)
- Improve the vertex buffer overrun messages by keeping track of which slot has the smallest index.
**Testing**
Tested on examples by artificially creating the situation I am trying to validate, as well as running clean examples to make sure they pass validation.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
1096: Fix build for wasm32 target r=kvark a=VincentFTS
**Connections**
#115
**Description**
Fix Wgpu build for Wasm target
**Testing**
It’s just a first step toward wasm support in Wgpu-rs, a PR in Wgpu-rs will come later.
Co-authored-by: Vincent Jousse <contact@ftsoftware.fr>
1093: Update naga and gfx, move the shader validation into a shader descriptor bit r=cwfitzgerald a=kvark
**Connections**
Includes https://github.com/gfx-rs/gfx/pull/3533 and a bunch of Naga things.
**Description**
Updates Naga with the new WGSL syntax, lots of fixes.
Large update of the GL backend.
**Testing**
Tested on some wgpu-rs examples.
Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
1077: Remove extern "C" from set_index_buffer r=kvark a=DevOrc
**Connections**
Fixes an issue that is blocking gfx-rs/wgpu-native#61
**Description**
Moves the ffi definition of wgpu_render_bundle_set_index_buffer / wgpu_render_pass_set_index_buffer to wgpu-native. This is needed because wgpu-native has its own version of IndexFormat that is different than the wgpu_types version.
**Testing**
My projects with wgpu-native work
Co-authored-by: Noah Charlton <ncharlton002@gmail.com>
1080: Fix the validation of vertex buffer sizes r=kvark a=JCapucho
**Connections**
None that i know of
**Description**
~~The vertex buffer size (in vertices) was being divided by stride causing the limit to be lower than it was supposed to be.~~
This bug wasn't triggered earlier because if the stride was 0 it wouldn't perform any calculation and the stride was only set when a set pipeline command was received and the `VertexState` `inputs` were already created so the following commands would work:
```
SetPipeline with 1 vertex buffer
SetVertexBuffer with only 4 vertices
Draw 6 vertices
```
This would have passed validation while this wouldn't
```
SetPipeline with 1 vertex buffer of stride 8
SetVertexBuffer with 4 vertices
SetPipeline with 1 vertex buffer of stride 8
SetVertexBuffer with 4 vertices
Draw 3 vertices
```
Now all draw calls have proper vertex validation and not only after the `inputs` are populated
**Testing**
This change was tested after debugging an issue with the draw calls failing in a specific order in [veloren](https://gitlab.com/veloren/veloren/-/tree/imbris/wgpu-master-rebased)
Co-authored-by: Capucho <jcapucho7@gmail.com>
The purpose of the PR is to support Naga modules everywhere.
As a requirement, it updates the gfx-rs version used.
Most of the logic is dedicated towards building a shader interface,
where previously we just used naga's IR. Now we have our own mini-IR.
1047: Update bind group layout API to match upstream r=cwfitzgerald a=kvark
**Connections**
Follows https://github.com/gpuweb/gpuweb/pull/1076, https://github.com/gpuweb/gpuweb/pull/1223 (https://github.com/gpuweb/gpuweb/issues/1164), https://github.com/gpuweb/gpuweb/pull/1255, and https://github.com/gpuweb/gpuweb/pull/1256
**Description**
Aligns our API closer to the latest changes in WebGPU upstream. We technically don't have to do this, but I believe in the end it would be best if our API gets close to upstream.
Note: this is a sensitive change for the users, everybody will get their code broken. So please take a look at the API and see if something is missing or needs improvement, so that we don't have to go through the changes again afterwards.
**Testing**
Doesn't really need testing. Partially covered by the existing playtest.
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
1051: Register init-destination buffer in the pending writes r=kvark a=kvark
**Connections**
Fixes#1049
**Description**
At some recent point we introduced the map of destination buffers for copies, and I missed the case where buffers are mapped at creation.
**Testing**
Tested on the test-case in #1049
Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
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>
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>
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>
982: Fix debug markers in render passes (#981) r=kvark a=kazimuth
Hey, that was easy. Didn't change anything in the debug groups markers / command pass markers, since afaict they work correctly.
I didn't add a test here because I'm not sure how to use the `player` testing apparatus, but I figure since I'm just adding a line of code that's already there for compute passes ([here](https://github.com/gfx-rs/wgpu/blob/master/wgpu-core/src/command/compute.rs#L452)) it should be alright. Let me know if there's anything else I should add.
Co-authored-by: James Gilles <jameshgilles@gmail.com>
966: Immediate resource destruction and freeing r=cwfitzgerald a=kvark
**Connections**
Fixes#964
**Description**
We are making it so a buffer or a texture can have their native resources freed while they are still referenced, so without waiting for GC.
In addition, the PR adds a few missing cases where error IDs should have been handled, like at render pass encoding.
**Testing**
Tested on wgpu-rs examples, see https://github.com/gfx-rs/wgpu-rs/pull/591
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
* Pass [u8] instead of [u32] for push constants
Use 2-instruction aligner
Fixed alignment
Fixed offsets
Added alignment checks and fixed size
Use semantic constants
* Replaced all magic number with constant
* Fixed type cast
963: Add repr(C) on mapping structs r=trivial a=kvark
**Connections**
Required for https://phabricator.services.mozilla.com/D92636
**Description**
Allows C clients to avoid redefining those structs.
**Testing**
No need
Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
938: Remove erroneous extra feature check r=kvark a=cwfitzgerald
**Connections**
None, found during investigation of DI issue.
**Description**
This line of code just shows up twice in a row, checking the same feature and setting the same feature.
**Testing**
Untested, but shouldn't affect anything.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
* Allow dropping of error resources
* Unregister error resources without panic
Panic when removing Vacant element or when index is out of range
* address review comments
Purpose for this is to add more context to an error.
create_render_pipeline error is improved, by showing the action
(creating render pipeline) and the user provided label, to aid with
the investigation.
909: Add Quad play test r=kvark a=DevOrc
**Connections**
#807
**Description**
Adds a play test that renders a white quad to a texture. The texture is then copied to a buffer and checked for accuracy with a file of expected bytes (All 0xFF).
**Testing**
Cargo test succeeds
Co-authored-by: Noah Charlton <ncharlton002@gmail.com>
903: fix for mixed STORAGE_STORE and STORAGE_LOAD r=kvark a=kocsis1david
**Connections**
None
**Description**
A buffer that contains dynamic data is used for both uniforms and storage read. This was a problem for wgpu and it gave a `PendingTransition` error.
**Testing**
It's a trivial fix, but I don't know if there's an example for it in wgpu-rs that can be used for testing.
It seems that the PR "Sync changes from mozilla-central" is already solving the same issue.
Co-authored-by: Dávid Kocsis <kocsis1david@windowslive.com>
* Allow copying from depth textures
* Rename TextureFormat::is_depth_format to just is_depth
* Only allow Depth32Float format for copying, and only as source
895: Improve validation in B2B copies and RenderCommands r=kvark a=kunalmohan
**Connections**
_Link to the issues addressed by this PR, or dependent PRs in other repositories_
**Description**
_Describe what problem this is solving, and how it's solved._
Validation for the following have been added-
- Validate all parameters in B2B copy even if `copy_size == 0`.
- Check for copy operations issued within same buffer.
- `RenderPass.setViewport()`
- `RenderPass.setScissorRect()`
**Testing**
_Explain how this change is tested._
Tested with CTS in Servo. More tests pass now. Will test on wgpu-rs now.
<!--
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: Kunal Mohan <kunalmohan99@gmail.com>
845: Finish error model refactor r=kvark a=GabrielMajeri
**Connections**
I think this is the last part of #638. I've reviewed all the remaining `unwrap`s and `assert`s in the code, and these should be the last ones left which ought to return errors (the remaining ones seem to uphold internal invariants).
**Description**
Implements error handling for various conditions, which are then returned to the caller. Including, but not limited to:
- running out of memory when creating a command pool
- running out of memory when creating a frame buffer for a render pass
- invalid dimensions when creating a texture
**Testing**
Tested with core and player.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
838: wait for buffer to be done in the player r=cwfitzgerald a=kvark
**Connections**
Fixes our code enough to replay #834 without issues. Doesn't help to solve the original problem though.
**Description**
There are two things in here:
1. don't deduplicate the BGLs if we are not generating new IDs at this layer. This helps Servo/Gecko/player. cc @kunalmohan
2. have an option in `buffer_destroy` to *actually* kill it, at the cost of blocking on GPU sometimes. This is required for the player, since the very next command may try to reuse the ID.
**Testing**
Tested on the trace in #834
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
839: Fix push constant pipeline invalidation r=kvark a=cwfitzgerald
**Connections**
Fixes#821
**Description**
We need to invalidate everything if push constants change. This code is kinda ugly but this is try three and it's the least terrible I got.
**Testing**
Pinging @Wumpf :)
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
832: Return errors from device functions r=kvark a=GabrielMajeri
**Connections**
Part of #638
**Description**
Lots of changes, but they should be easily reviewable commit-by-commit.
- Error types for most of the fallible resource creation errors in `device/mod.rs`
- Removed assertions and replaced them with error types
- All of the `BufferMap`, `BufferNotMapped` etc. errors were united in a single `BufferAccessError`, since it was pretty weird to have so many overlapping error types.
- Removed all `unwrap`s of `gfx-hal` errors - they are now returned to the caller.
**Testing**
Checked with core and tested with `wgpu-rs` (see https://github.com/gfx-rs/wgpu-rs/pull/469)
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
833: Switch pipeline flag to mutation of depth/stencil r=startoaster a=kvark
**Connections**
Reported on the matrix
**Description**
If the pass mutates depth/stencil, but the pipeline doesn't, it's not a bug!
**Testing**
on it...
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
831: Carry RefCount in future suspects in order to prevent early removal r=cwfitzgerald a=kvark
**Connections**
Fixes #830
**Description**
see https://github.com/gfx-rs/wgpu/issues/830#issuecomment-662825775
**Testing**
Tested on harmony + Sponza
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
825: Add depth clamping support r=cwfitzgerald a=kvark
**Connections**
Implements https://github.com/gpuweb/gpuweb/pull/900
**Description**
Depth clamping is useful for shadow mapping and stuff. We'd want to use it in our `shadow` example.
**Testing**
Untested, should work though :)
Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
* Replace &[T] in descriptor structs with Cow<[T]>
* Changed label fields to use Cow<'a, str>
* Deduplicate types across the tracing API
Introduce the ToStatic trait
* Deduplicate trace::RenderBundleDescriptor
* Remove ugly ToOwned bounds
* Simplifiy Action variants
* Make Clone bounds on type defs less verbose
* Fix a error in play.rs
* Remove ToStatic, and Make Action<'a> borrow descriptors instead
816: Error types for buffer mapping API r=kvark a=GabrielMajeri
**Connections**
Part of #638
**Description**
Adds safe error handling for buffer mapping/unmapping functions.
**Testing**
Checked with core, and ran tests with player.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
813: Add buffer creation error type r=kvark a=GabrielMajeri
**Connections**
Part of #638
**Description**
Adds an error type for the buffer creation functions.
**Testing**
Checked with core and ran test on player.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
815: Remove error logging from `binding_model` r=kvark a=GabrielMajeri
**Connections**
Part of #638
**Description**
Removes some `log::error!` calls from `binding_model.rs`, instead encoding the messages in the error type.
**Testing**
Checked with core.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
812: Error type for `WaitIdle` r=kvark a=GabrielMajeri
**Connections**
Part of #638
**Description**
Adds an error type for the `wait_idle` function.
**Testing**
Checked with core.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
798: Fixed storage buffer counting towards storage textures for binding validation r=kvark a=Wumpf
See title.
(broke my group layout creation since my compute shaders need quite a few storage buffers _and_ storage textures bound... ;-))
Co-authored-by: Andreas Reich <r_andreas2@web.de>
796: Safe error handling for command API r=kvark a=GabrielMajeri
**Connections**
Part of #638
**Description**
Initially started to remove assertions from the `bundle.rs` file, but ended up implementing safe error handling for the whole `command` module.
**Testing**
Checked with core and player.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
795: Move `RenderPass*AttachmentDescriptor` types into core r=kvark a=GabrielMajeri
**Connections**
Follow up to #791.
**Description**
I had the impression that these two types were already shared between `wgpu-core` and `wgpu-rs`, considering they were generic. Further investigation reveals that not to be the case.
This PR moves these types into `wgpu-core`, since they're specific to this crate.
**Testing**
Checked with core, player and `wgpu-rs`.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
789: Panic on using a pending buffer r=kvark a=kvark
**Connections**
Validates the use case of https://github.com/gfx-rs/wgpu-rs/issues/436
**Description**
We didn't detect if a buffer had pending *initial* mapping, previously, on submit.
**Testing**
Untested
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
791: Unify `wgpu-core` and `wgpu-rs` types r=kvark a=GabrielMajeri
**Connections**
Closes#689.
**Description**
Moves a lot of types from `wgpu-rs` which were duplicated in `wgpu-core` to `wgpu-types`.
**Testing**
Checked with core, player and `wgpu-rs`.
Corresponding `wgpu-rs` PR: https://github.com/gfx-rs/wgpu-rs/pull/437
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
790: core: make DeviceType repr(u8) r=kvark a=DavidPeicho
**Connections**
This PR will make gfx-rs/wgpu-native#43 mergeable.
**Description**
`DeviceType` wasn't made to be used in FFI. The enum is now set to `#[repr(u8)]`.
**Testing**
It's not tested 💯
Co-authored-by: David Peicho <david.peicho@gmail.com>
787: Remove non-exhaustive markers r=kvark a=GabrielMajeri
Based on the previous [discussion](https://github.com/gfx-rs/wgpu/pull/774#issuecomment-657171354), this PR removes the `NonExhaustive` structure and any `#[non_exhaustive]` attribute since it's too early to decide how to ensure forward compatibility.
**Testing**
Checked the `core`, `types` and `player` crates.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
773: Error handling for data transfer API r=kvark a=GabrielMajeri
**Connections**
Work on the error model described in #376
**Description**
Removes assertions from the transfer functions, instead returning a custom error type.
**Testing**
Checked with `player` and `wgpu-rs`: https://github.com/gfx-rs/wgpu-rs/pull/430
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
769: Implement and Validate all WebGPU Limits r=kvark a=cwfitzgerald
**Connections**
None?
**Description**
This adds all the limits that webgpu has currently. It also validates the values.
The main piece of code here is `BindingTypeMaxCountValidator` and `PerStageBindingTypeCounter` which provides the interface for figuring out the maximum per-stage counts in the pipeline. For each `BindGroupLayout` a `BindingTypeMaxCountValidator`is put together during creation using all of the bindings. Then the `BindingTypeMaxCountValidator`s are combined into a single max. This is then validated against the max limits.
Each commit should be independently testable and are grouped by responsibility.
**Testing**
I modified the wgpu-rs example framework to ask for extremely reduced limits and then tested various examples to verify it properly accepted/rejected based on the actual limit.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
763: Remove raw pointers from the render pipelines API r=kvark a=GabrielMajeri
**Connections**
Rust-ification of API, as part of #689
**Description**
The objective is to get rid of raw pointers in the render pipeline descriptor and associated structures.
**Testing**
Checked with `player` and the `trace` feature, and with `wgpu-rs` with the changes in https://github.com/gfx-rs/wgpu-rs/pull/425
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
754: Implement MultiDrawIndirect Extensions r=kvark a=cwfitzgerald
**Connections**
Closes#742.
**Description**
These extensions, especially when combined with binding indexing, allow the creation of extremely cpu-efficient gpu powered pipelines.
Adds two extensions allowing various types of multi-draw-indirect
- MULTI_DRAW_INDIRECT (giving `multi_draw_indirect` and `multi_draw_indexed_indirect`)
- MULTI_DRAW_INDIRECT_COUNT (giving `multi_draw_indirect_count` and `multi_draw_indexed_indirect_count`)
This adds what I believe to be an extra restriction on the `*count` family of functions when compared to the underlying api. The buffer _must_ be large enough to draw `max_count` draws, even if that many draws are never drawn. This makes these operations no more unsafe than indirect would be, which is currently safe.
I did not implement these for renderbundles, but there's no reason they couldn't work, so those branches are marked with `unimplemented` as opposed to `unreachable`.
Additional Changes:
- Added some validation to the normal `draw_*_indirect` functions to prevent buffer overruns.
- The DX12 gfx-hal backend requires the strides to _always_ be non-zero, so I modified the normal indirect draws to use explicit strides.
- Made device limits and features `pub(crate)` as they need to be checked in random places in the code.
**Testing**
The change was tested using a modified version of wgpu-rs's texture-array example using a variety of permutations. I have not been able to test regular MULTI_DRAW_INDIRECT on mac, but I see no reason why that wouldn't work.
https://github.com/gfx-rs/wgpu-rs/pull/414
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
752: gfx-memory update r=trivial a=kvark
**Connections**
Switches gfx-extras to https://github.com/gfx-rs/gfx-extras/pull/18Fixes#750
**Description**
Includes important correctness fixes.
**Testing**
Tested on wgpu-rs examples on macOS.
Also, by @cwfitzgerald having the repro case of #750
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
747: Custom implement Debug for RenderCommand and ComputeCommand r=kvark a=kunalmohan
This would avoid unnecessarily long debug logs for Render and Compute passes to some extent. I am not sure if it would be helpful to print `dynamic_offsets` and `string_data` under `BasePass` without the content of related `Compute/Render Command`.
<!--**Connections**
_Link to the issues addressed by this PR, or dependent PRs in other repositories_
**Description**
_Describe what problem this is solving, and how it's solved._
This would avoid
**Testing**
_Explain how this change is tested._
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: Kunal Mohan <kunalmohan99@gmail.com>
740: First Phase of Tracing Transition r=kvark a=cwfitzgerald
## Connections
First step in the implementation of #491. https://github.com/gfx-rs/wgpu-rs/pull/395
## Description
This adds the tracing crate, implements a tracing "layer" for chrome tracing, and instruments every entrypoint into wgpu.
Tracing is added as a main dependency. A feature is added called `subscriber` which guards the tracing and default logger implementation, as that adds 3 dependencies.
The main macro is there to make creating a span a simple one line process. This macro will come in useful in the next couple stages. Use of this macro is used unqualified with it imported into scope as that style allows IntelliJ ides to actually find the macro.
I also removed a really annoying warning that was driving me crazy.
This PR does not make sure the logging output from tracing is up to snuff, that will be done when logging output and conversion is the priority.
Both commits should compile individually, so shouldn't need to be squashed.
## Testing
This PR was tested with the wgpu-rs PR on various examples, as well as my personal project.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
743: Derive Serialize and Deserialize for HostMap r=kvark a=kunalmohan
**Connections**
_Link to the issues addressed by this PR, or dependent PRs in other repositories_
**Description**
_Describe what problem this is solving, and how it's solved._
Also a switch from `f32::MAX` to `std::f32::MAX`
**Testing**
_Explain how this change is tested._
<!--
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: Kunal Mohan <kunalmohan99@gmail.com>
745: Update to latest gfx-extras commit r=kvark a=rukai
Brings the fix for https://github.com/gfx-rs/wgpu-rs/issues/363 into wgpu
Co-authored-by: Lucas Kent <rubickent@gmail.com>
739: Remove Peek-Poke r=cwfitzgerald a=kvark
**Connections**
Related to https://github.com/gfx-rs/wgpu/issues/738
Related to https://github.com/djg/peek-poke/issues/10
**Description**
As of #726 , the buffers have a minimum binding size that has to include the shader struct size. It, therefore, can't be zero.
We can remove the hacks we had previously and switch fully to the idiomatic `Option<NonZeroU64>`.
Peek-poke doesn't `NonZeroU64` and friends, so this made me reconsider the user of it entirely. Today, render bundles as well as the Player already represent command streams using a much rustier method. I tried to move everything to this method now, and I think this is going to work much better, and safer.
**Testing**
wgpu-rs works - https://github.com/gfx-rs/wgpu-rs/pull/396
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
736: Validate sampler type in `create_bind_group` r=kvark a=GabrielMajeri
**Connections**
Fixes#588
**Description**
* Makes `create_bind_group` return a `Result`, and adds a new `binding_model::BindGroupError` enum.
* Converts _some_ assertions from `create_bind_group` into `Err`s
* Validates that the type of sampler declared in the bind group layout (comparison or not) is the same as the one of the actual sampler.
**Testing**
Tested locally on the `shadow` example in `wgpu-rs`. ~Will follow up with a PR in that repo with the updated API.~ https://github.com/gfx-rs/wgpu-rs/pull/398
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
There was a lot of highly unsafe use of serialization based on peek-poke that we
weren't entirely happy with. It's replaced by just serializing the passes now.
Also, switch BufferSize to Option<NonZero>.
735: Set descriptor set name when creating bind group r=kvark a=GabrielMajeri
**Description**
There is a [`TODO` in the code](a02a566841/wgpu-core/src/device/mod.rs (L1428)) for setting the descriptor set name when creating a bind group. It was blocked on https://github.com/gfx-rs/gfx-extras/pull/5
**Testing**
Tested with examples from `wgpu-rs`.
Co-authored-by: Gabriel Majeri <gabriel.majeri6@gmail.com>
732: Dummy workspace crate r=cwfitzgerald a=kvark
Basically, it makes sure that doing `cargo check` locally in the workspace is all you need.
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
728: Documentation Pass r=kvark a=cwfitzgerald
## Connections
https://github.com/gfx-rs/wgpu-rs/issues/378.
## Description
The number one thing that people want to see in wgpu is better documentation, so this is a first step towards that goal. It unifies the documentation and fills it out so most things have documentation that is at least marginally helpful.
Notable changes to existing documentation:
- Removes "a" and "the" at the beginning of short descriptions.
- Always use the phrasing `Describes a...` for descriptors that create objects directly.
- Always use a period at the end of short descriptions.
## Testing
No code was changed, but constant monitoring of cargo doc output in wgpu-rs was used to keep everything completely consistent.
## TODO
- [x] wgpu-rs PR (https://github.com/gfx-rs/wgpu-rs/pull/380)
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
726: Basic support for minBufferBindingSize r=cwfitzgerald a=kvark
**Connections**
Has basic (partial) implementation of https://github.com/gpuweb/gpuweb/pull/678
wgpu-rs update - https://github.com/gfx-rs/wgpu-rs/pull/377
**Description**
This change allows users to optionally specify the expected minimum binding size for buffers. We are then validating this against both the pipelines and bind groups.
If it's not provided, we'll need to validate at draw time - this PR doesn't do this (focus on API changes first).
It also moves out the `read_spirv`, since wgpu-types wasn't the right home for it ever.
**Testing**
Tested on wgpu-rs examples
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
719: Implement debug marker support r=kvark a=krupitskas
**Connections**
Closes https://github.com/gfx-rs/wgpu/issues/697
**Description**
Looks like because I've once pushed forward reset branch to my master, previous pull request https://github.com/gfx-rs/wgpu/pull/713
show that there no commits and it automatically was closed :/
**Testing**
Not tested yet
Co-authored-by: Nikita Krupitskas <krupitskas@icloud.com>
722: Refactor tracking of device last completed submission r=kvark a=kvark
**Connections**
Fixes https://github.com/gfx-rs/wgpu-rs/issues/358
**Description**
We used to track the next submission index in `device.life_guard.submission_index` atomic. This PR changes that to point to the last *done* submission, and also introduces a non-atomic field to keep track of the current/next submission.
This allows us to avoid waiting on the frame semaphore on presentation if the relevant submission is done by then.
**Testing**
Not tested!
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
717: Implement Capability/Extension Split r=kvark a=cwfitzgerald
## Connections
Follow up to a discussion we had on #wgpu:matrix.org.
## Description
Splits capabilities and extensions so that extensions are things you ask for and possibly change behavior and capabilities are passively enabled when their extension (if any) are enabled.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
712: RODS part 2 r=cwfitzgerald a=kvark
**Connections**
Unblocks https://github.com/gfx-rs/wgpu-rs/issues/359
Is a follow-up to RODS part 1 - #685
**Description**
There is a few things in here.
Thing 1: Stronger assertions on the load/store ops of the depth & stencil.
### Thing 2: rewritten tracking of render attachments
Background: from the usage tracker point of view, each subresource can be in either "extend" mode, where it accumulates a single combined usage, or in the "replace" mode, where it goes from one usage to another, producing the relevant transitions on the way.
The problem turned out to come from the fact that the render pass attachments were always tracked in "replace" mode. This is needed because their track don't have a single state: render pass itself encodes a transition of attachments. However, it also means that there was no way to "extend" the usage in RODS scenarios...
So I could see two ways to address this:
- re-achitecture the tracking a bit more in general, representing it as a sequence of merges.
- introduce the "prepend()" tracking operator that's *only* used for render pass attachments
I opted for the latter as it seems much less intrusive. The render pass attachments accumulate their usage like everything else in the "extend mode". But right before we are inserting the transitions (between the active command buffer and the pass), we turn the tracking of the attachments from "extend" into "replace" mode by installing the "first" usage according to what we expect it to be.
### Thing 3: missing API for RODS bind groups
The original RODS design missed a problem with Vulkan image layouts. When creating a bind group, one has to specify what layout the image will be in. We always used `ShaderReadOnlyOptimal` until now for texture views. However, in RODS scenarios this has to be `DepthStencilReadOnlyOptimal`. Luckily, it's compatible with sampling from the shader, but we still need to know about this when creating the bind group.
**Testing**
Tested on the modified water example provided in https://github.com/gfx-rs/wgpu-rs/issues/359#issuecomment-642167269
Added a few tests to the buffer implementation of the new `prepend()` operator.
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
715: Implement SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING r=kvark a=cwfitzgerald
## Connections
Closes#483. Extends support for #106.
## Description
This forwards the descriptor indexing features gfx-hal up to wgpu.
Note this PR also changes the name of the `TEXTURE_BINDING_ARRAY` to `SAMPLED_TEXTURE_BINDING_ARRAY` to be more consistent with extensions.
## Testing
Texture-array example was simply extended to use descriptor indexing. This works on all platforms that the feature is supported. (thanks @stararawn)
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
This is a major change in how the bundles are implemented. Instead of
transparently injecting them into the pass command stream, we are now
treating bundles as first-class API objects and API tracing them
accordingly. The bundle contains a normalized command stream that is
very easy to inject into a native command buffer multiple times.
711: Implement Descriptor Array Extension r=kvark a=cwfitzgerald
## Connections
Blocked on https://github.com/gfx-rs/gfx/pull/3269 and does some funky git overrides to get CI to return meaningful results what will be removed once that PR lands and is published.
## Description
This PR implements the `TEXTURE_BINDING_ARRAY` native extension. This allows users to specify a uniform sized array of textures for use in shaders.
As a corollary, this PR rustifies the Bind Group and Bind Group Layout interface. Two main actions were taken when doing this:
- Types that were able to be shared among wgt, wgc, and wgpu-rs were moved into wgt.
- Types that had references to other wgpu-rs specific structures were duplicated into wgc with wgpu-rs structures replaced with the appropriate ID. Notes were added to the wgc types that they were duplicated directly from wgpu-rs.
From what I can tell, this resulted in a significant reduction in code complexity when dealing with bind groups, favoring strong types over runtime assertions.
Naga validation of arrays of textures was not implemented.
Documentation was added to extensions to help users understand what underlying tech was being relied on as well as the platforms it should be expected to work on. I think this pattern should be implemented across the board for extensions as it makes them much more user friendly to use.
## Testing
There is an example included in the wgpu-rs PR which was used for testing this feature. It worked on DX12, Vulkan, and Metal (MSL > 2.0), as was expected.
Additionally the other examples were run and are still verified to run.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
707: Implement Bounds Checking on Buffers to Buffer and Texture Copies r=kvark a=cwfitzgerald
**Connections**
Closes#362, closes#554
This addresses issues found by @gretchenfrage when accidentally writing off the end of a texture causing a DEVICE_LOST error.
**Description**
Adds bounds checks to write_texture and write_buffer, and copy_texture_to_texture and friends. The bounds checking functions themselves follow guidance from the webgpu standard.
This doesn't make wgpu 100% to all the checks required by the standard, but takes care of the ones related to bounds and buffer overruns.
**Testing**
I tested this against all the examples, including intentionally making mistakes in texture copies, with this successfully catching those errors.
**Review Notes**
This code should be picked through fairly closely, as it's very likely there's some typos due to the close-but-not-quite repetition that these tests require. There's a bunch of LOC, but they are fairly boring and should be easy to understand.
I have tried to give assert messages that are as descriptive as possible, but if there is a message that could be changed to be more clear, let me know.
One thing that could change is the location of the functions. I left them where I originally wrote them, before I realized that the functions on queue also needed bounds checking, but they could have a better home elsewhere, maybe even their own file.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
704: Pipeline layout validation r=cwfitzgerald a=kvark
**Connections**
Implements a solid part of #269
Starts converting the function to return results, related to #638
cc @GabrielMajeri
**Description**
This change matches shader bindings against the pipeline layout. It's *mostly* complete, minus some bugs and not handling the `storage_texture_format` properly.
The risk here is that Naga reflection may have bugs, or our validation may have bugs, and we don't want to break the user content while this is in flux. So the PR introduces an internal `WGPU_SHADER_VALIDATION` environment variable. Switching it to "0" skips Naga shader parsing completely and allows the users to unsafely use the API.
Another aspect of the PR is that some of the functions now return `Result`. The way I see us proceeding is that any errors that we don't expect users to handle should result in panics when `wgpu` is used natively (i.e. not from a browser). These panics would happen in the "direct" backend of wgpu-rs (as well as in wgpu-native), but the `Result` would not be exposed to wgpu-rs, so that it matches the Web behavior.
At the same time, browser implementations (Gecko and Servo) will check the result on their GPU process and implement the WebGPU error model accordingly. This means `wgpu-core` can be super Rusty and safe.
**Testing**
Running on wgpu-rs examples. Most of them fail to get parsed by Naga, but `boids` succeeds and passes validation 🎉
Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
667: Add asserts to validate that a resource exists when used r=kvark a=DavidPeicho
Hi,
As discussed in #610, this is mostly for C/C++ users. Some questions:
* Would it be possible to display the ID of the resource that failed? I don't know what you think about implementing the `Display` trait on the `Id` struct?
* Do you see other places needing for checks?
Thanks!
Co-authored-by: David Peicho <david.peicho@gmail.com>
668: Return failures to the user in swap_chain_get_next_texture, rather than transparently reconfiguring. r=kvark a=AlphaModder
TODO:
- [x] Change `Global::swap_chain_get_next_texture` in `wgpu-core`.
- [x] Update `wgpu_swap_chain_get_next_texture` in `wgpu-native`. (https://github.com/gfx-rs/wgpu-native/pull/32)
- [x] Wrap `SwapChainOutput`/`SwapChainStatus` in a nice enum in `wgpu-rs`. (https://github.com/gfx-rs/wgpu-rs/pull/323)
- [ ] Update `wgpu_bindings` (?)
Co-authored-by: AlphaModder <quasiflux@gmail.com>
* "Use the whole buffer" is !0, not 0
Fixes#654
Applies to BufferBinding, set_vertex_buffer, set_index_buffer
* Add BufferSize type alias
* Make BufferSize a transparent type
Add a custom serialization "buddy" type
Use BufferSize::WHOLE instead of crate::WHOLE_SIZE
* Move SerBufferSize into device::trace mod
Co-authored-by: Paul Kernfeld <paulkernfeld@gmail.com>
- Clean up after the pending writes on destroy.
- Fix temporary buffer creation.
- Fix internal thread initialization by the command allocator.
- Clean up player event_loop usage.
645: Add a loom test for RefCount r=kvark a=paulkernfeld
A first effort at gfx-rs/wgpu-rs#96
loom testing is gated behind cfg(loom)
Co-authored-by: Paul Kernfeld <paulkernfeld@gmail.com>
