Expose adapter specific formats features via Extension, support for storage read+write #1112

Wumpf · 2020-12-30T11:34:00Z

Connections
Draft until gfx-rs/gfx#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

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.

TextureFormatFeatures is now a struct of two u32 flags. Query happens only at texture creation time

monocodus

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 82 warnings, 0 errors.

wgpu-core/src/device/mod.rs

kvark · 2021-01-04T20:32:29Z

wgpu-core/src/instance.rs

@@ -132,7 +133,8 @@ impl<B: GfxBackend> Adapter<B> {

        let mut features = wgt::Features::default()
            | wgt::Features::MAPPABLE_PRIMARY_BUFFERS
-            | wgt::Features::PUSH_CONSTANTS;
+            | wgt::Features::PUSH_CONSTANTS
+            | wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES;


I wonder if we actually need this feature. I.e. can we pretend it's always supported? On the Web, we can just return the statically known features (until WebGPU gets a proper extension).

That thought crossed my mind as well but what kept me from it is

(most important) one might develop a cross platform application (e.g. web + desktop) and unknowingly break its portability if this is not an explicit opt-in. Imho a huge selling point of wgpu(-rs) is that it formulates a feature set that will work on a very wide variety of devices

other things in the limits struct need to be requested explicitly as well and default to the webgpu specification (e.g. how many storage bindings are supported)

if and when WebGPU gets an extension this may fit more nicely as a "free-for-all tier"

That's a very good point! Ideally I'd love to make it so you can't use a feature that you haven't either discovered or requested. This should apply to format features as well. So what you are saying makes total sense, and is much appreciated!

We can also go a slightly different route and consider the format features to be "basic" unless the user asks for them. This would make the same code portable across the web and non-web.

consider the format features to be "basic" unless the user asks for them

You're thinking that the user can request particular texture features ahead of time? That would make sense in regard to how the limit struct works and keeps all the missing-feature surprises to the device selection rather than some random texture binding.

Or did you mean something else? :)
Otherwise, the way it's done here would already be a manual opt-in by the user, only that it's a bit vague what one ends up with :/

No, I wasn't think about "ahead of time" kind of thing as we do for limits. I was thinking more simple: if the user didn't call get_texture_features (or whatever it's called), then they can only assume the base guaranteed feature set for this format specifically (and we validate that).

kvark · 2021-01-04T20:35:37Z

wgpu-core/src/instance.rs

+            flags |= wgt::TextureFormatFeatureFlags::STORAGE_ATOMICS;
+        }
+        // TODO: hal update
+        //if texture_format_properties.contains(hal::format::ImageFeature::STORAGE_READ_WRITE) {


can now be implemented thanks to you!

😊
once we managed to upgrade to newest gfx that is :)

wgpu-types/src/lib.rs

+ some cleanup/fix indentations

monocodus

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 82 warnings, 0 errors.

… atm

Wumpf · 2021-01-06T17:19:46Z

Support for read+write textures is all operational but will always fail validation right now (see commented out code with TODO note in get_texture_format_features) until gfx/hal is updated (necessary changes there have landed!). Update is blocked right now by zakarumych/gpu-alloc#33

monocodus

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 82 warnings, 0 errors.

kvark

Very nice, thank you!
bors r+

kvark · 2021-01-06T18:56:57Z

I was going back and forth on some pieces, thinking if they could be written better... but no, it's all really good 🕶️ .

bors · 2021-01-06T19:08:33Z

Build succeeded:

@elartista

1140: Enable storage read/write format feature r=kvark a=Wumpf **Connections** #1112 **Description** Use `wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE` in `get_texture_format_features`. This has been waiting for a gfx update which was enabled by @elartista 's workaround in #1136 \o/ Co-authored-by: Andreas Reich <[email protected]>

713: Expose texture format feature query (and update to latest wgpu) r=kvark a=Wumpf Exposes texture format feature query from gfx-rs/wgpu#1112 Updating wgpu-core led to breaking change of moving get_swap_chain_preferred_format from device to adapter. Co-authored-by: Andreas Reich <[email protected]>

713: Expose texture format feature query (and update to latest wgpu) r=kvark a=Wumpf Exposes texture format feature query from gfx-rs#1112 Updating wgpu-core led to breaking change of moving get_swap_chain_preferred_format from device to adapter. Co-authored-by: Andreas Reich <[email protected]>

Wumpf added 4 commits December 29, 2020 17:14

wip adapter specific format features & rw storage

0ccad9a

validation against adapter specific format features

4207e57

TextureFormatFeatures is now a struct of two u32 flags. Query happens only at texture creation time

Expose adapter_get_texture_format_features

c17b2dc

map depth/stencil feature to RENDER_ATTACHMENT

757d12a

Wumpf mentioned this pull request Dec 30, 2020

Add STORAGE_TEXTURE_ACCESS_READ_WRITE native extension #1109

Closed

Wumpf marked this pull request as draft December 30, 2020 11:38

monocodus bot suggested changes Dec 30, 2020

View reviewed changes

kvark reviewed Jan 4, 2021

View reviewed changes

wgpu format features are again part of TextureFormatInfo

e29d17a

+ some cleanup/fix indentations

monocodus bot suggested changes Jan 5, 2021

View reviewed changes

Added note on why hal update needed for STORAGE_READ_WRITE is blocked…

30c96bf

… atm

Wumpf marked this pull request as ready for review January 6, 2021 17:19

monocodus bot suggested changes Jan 6, 2021

View reviewed changes

kvark approved these changes Jan 6, 2021

View reviewed changes

bors bot merged commit 8c00cbc into gfx-rs:master Jan 6, 2021

Wumpf deleted the adapter-specific-format-feature branch January 6, 2021 19:29

Wumpf mentioned this pull request Jan 10, 2021

Enable storage read/write format feature #1140

Merged

Wumpf mentioned this pull request Jan 12, 2021

Expose texture format feature query (and update to latest wgpu) gfx-rs/wgpu-rs#713

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose adapter specific formats features via Extension, support for storage read+write #1112

Expose adapter specific formats features via Extension, support for storage read+write #1112

Wumpf commented Dec 30, 2020

monocodus bot left a comment

kvark Jan 4, 2021

Wumpf Jan 4, 2021

kvark Jan 4, 2021

Wumpf Jan 5, 2021

kvark Jan 6, 2021

kvark Jan 4, 2021

Wumpf Jan 4, 2021

monocodus bot left a comment

Wumpf commented Jan 6, 2021

monocodus bot left a comment

kvark left a comment

kvark commented Jan 6, 2021

bors bot commented Jan 6, 2021

Expose adapter specific formats features via Extension, support for storage read+write #1112

Expose adapter specific formats features via Extension, support for storage read+write #1112

Conversation

Wumpf commented Dec 30, 2020

monocodus bot left a comment

Choose a reason for hiding this comment

kvark Jan 4, 2021

Choose a reason for hiding this comment

Wumpf Jan 4, 2021

Choose a reason for hiding this comment

kvark Jan 4, 2021

Choose a reason for hiding this comment

Wumpf Jan 5, 2021

Choose a reason for hiding this comment

kvark Jan 6, 2021

Choose a reason for hiding this comment

kvark Jan 4, 2021

Choose a reason for hiding this comment

Wumpf Jan 4, 2021

Choose a reason for hiding this comment

monocodus bot left a comment

Choose a reason for hiding this comment

Wumpf commented Jan 6, 2021

monocodus bot left a comment

Choose a reason for hiding this comment

kvark left a comment

Choose a reason for hiding this comment

kvark commented Jan 6, 2021

bors bot commented Jan 6, 2021