Add `view_formats` in TextureDescriptor by jinleili · Pull Request #3237 · gfx-rs/wgpu

jinleili · 2022-11-26T12:59:44Z

Checklist

Run cargo clippy.
Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
Add change to CHANGELOG.md. See simple instructions inside file.

Connections
WebGPU spec: https://www.w3.org/TR/webgpu/#dom-gputexturedescriptor-viewformats

Description

Closes #3030

Testing
Tested on Metal, Vulkan(--features vulkan-portability) and Dx12 backend.

codecov-commenter · 2022-11-26T13:28:22Z

Codecov Report

Merging #3237 (8bbccdd) into master (4400ff8) will decrease coverage by 0.00%.
The diff coverage is 57.81%.

@@            Coverage Diff             @@
##           master    #3237      +/-   ##
==========================================
- Coverage   64.64%   64.63%   -0.01%     
==========================================
  Files          86       86              
  Lines       42722    42768      +46     
==========================================
+ Hits        27616    27645      +29     
- Misses      15106    15123      +17

Impacted Files	Coverage Δ
wgpu-core/src/present.rs	`0.00% <0.00%> (ø)`
wgpu-core/src/resource.rs	`25.13% <ø> (ø)`
wgpu/src/lib.rs	`51.04% <33.33%> (-0.03%)`	⬇️
wgpu/src/backend/direct.rs	`56.98% <40.00%> (+0.11%)`	⬆️
wgpu-core/src/device/mod.rs	`66.73% <57.14%> (-0.07%)`	⬇️
wgpu-types/src/lib.rs	`87.45% <60.52%> (-0.62%)`	⬇️
wgpu-core/src/command/clear.rs	`89.58% <100.00%> (ø)`
wgpu-core/src/command/transfer.rs	`75.14% <100.00%> (ø)`
wgpu-core/src/init_tracker/texture.rs	`93.65% <100.00%> (ø)`
wgpu-core/src/hub.rs	`60.98% <0.00%> (-0.16%)`	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

crowlKats · 2022-11-26T15:10:05Z

Could the deno webgpu part be implemented fully/properly?

cwfitzgerald

Thank you for tackling this one!

The code is fine, but there are two extra parts we need (besides the deno stuff).

First can you add a test to make sure the feature works (read a unorm texture as srgb and validate it does the conversion - and vise versa)

We also need a downlevel feature for this, as gles doesn't support texture views. And afaik we can't polyfill this feature in any way.

wgpu-types/src/lib.rs

wgpu-core/src/resource.rs

wgpu-core/src/device/mod.rs

jinleili · 2022-11-27T03:04:05Z

@crowlKats I'm sorry, I don't know how to properly implement view_formats in deno_webgpu. deno_webgpu's webgpu.idl doesn't have viewFormats field, is it enough to add viewFormats field to deno_webgpu::texture::CreateTextureArgs?

crowlKats · 2022-11-27T05:34:28Z

no, its slightly more than just that. but no worries, i will do it then

wgpu-types/src/lib.rs

khyperia · 2022-12-27T17:19:56Z

Hi! I'm just a random user of wgpu, so take this whole comment with a grain of salt. Proper sRGB support in the webgpu backend of wgpu is blocked on specifying the view format of the Surface's current_texture (the swapchain image). In webgpu, this is done by setting GPUCanvasConfiguration.viewFormats to the formats you'd like to view the current_texture as, then specifying that format in create_view. (I guess this GPUCanvasConfiguration.viewFormats field is a sort of backdoor to pass in view_formats into the system's internal create_texture for the current_texture?)

https://www.w3.org/TR/webgpu/#canvas-configuration

In wgpu, the GPUCanvasConfiguration is created here, and I suppose mapped.view_formats() should be called, similar to what this PR is already doing in the device_create_texture function in web.rs

wgpu/wgpu/src/backend/web.rs

Lines 1037 to 1041 in aa46e82

    
           let mut mapped = 
        
               web_sys::GpuCanvasConfiguration::new(&device.0, map_texture_format(config.format)); 
        
           mapped.usage(config.usage.bits()); 
        
           mapped.alpha_mode(alpha_mode); 
        
           surface.0.configure(&mapped);

I suppose SurfaceConfiguration should get a new view_formats: Vec<TextureFormat> field, similar to how TextureDescriptor got that field in this current PR? I haven't looked into how other backends other than webgpu handle the view format of the surface current_texture, and if they support alternate views at all, and how they're configured/enabled.

If you could implement this in this PR or a followup, that would be amazing, but please ping me if you don't plan on doing so and I can try looking into implementing it!

jinleili · 2022-12-28T00:56:06Z

The implementation of this PR is not yet accepted and may need to be modified. I have plans to implement view_formats in SurfaceConfiguration after this PR.

cwfitzgerald

Another round, we're very close!

wgpu-hal/src/gles/adapter.rs

wgpu-core/src/device/mod.rs

wgpu-types/src/lib.rs

wgpu/src/lib.rs

wgpu/tests/shader_view_format/mod.rs

cwfitzgerald

Really good stuff, we're almost ready to go!

wgpu-types/src/lib.rs

wgpu/src/backend/direct.rs

wgpu/src/util/device.rs

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>

cwfitzgerald · 2023-01-18T21:03:35Z

Thank you for all the hard work!

jinleili force-pushed the view_formats branch 2 times, most recently from 735a35f to bf9c1c6 Compare November 26, 2022 13:22

jinleili requested a review from crowlKats as a code owner November 26, 2022 13:34

cwfitzgerald requested changes Nov 26, 2022

View reviewed changes

wgpu-types/src/lib.rs Outdated Show resolved Hide resolved

wgpu-types/src/lib.rs Outdated Show resolved Hide resolved

wgpu-core/src/resource.rs Outdated Show resolved Hide resolved

wgpu-core/src/device/mod.rs Outdated Show resolved Hide resolved

cwfitzgerald reviewed Nov 27, 2022

View reviewed changes

wgpu-types/src/lib.rs Outdated Show resolved Hide resolved

cwfitzgerald reviewed Nov 27, 2022

View reviewed changes

wgpu-types/src/lib.rs Outdated Show resolved Hide resolved

jinleili force-pushed the view_formats branch 6 times, most recently from f4dd6fc to 45ea4ca Compare December 1, 2022 05:13

crowlKats mentioned this pull request Dec 3, 2022

Further spec compliance #2170

Closed

36 tasks

jinleili force-pushed the view_formats branch from 0ce9099 to 6fde981 Compare December 7, 2022 12:07

jinleili force-pushed the view_formats branch from 6fde981 to 577a492 Compare December 24, 2022 00:31

cwfitzgerald requested changes Jan 4, 2023

View reviewed changes

jinleili force-pushed the view_formats branch 2 times, most recently from d9db149 to 5cd7541 Compare January 11, 2023 08:49

cwfitzgerald requested changes Jan 12, 2023

View reviewed changes

wgpu-types/src/lib.rs Outdated Show resolved Hide resolved

wgpu-types/src/lib.rs Outdated Show resolved Hide resolved

wgpu/src/backend/direct.rs Outdated Show resolved Hide resolved

wgpu/src/util/device.rs Outdated Show resolved Hide resolved

jinleili and others added 5 commits January 13, 2023 10:28

Add view_formats in TextureDescriptor

f55db4f

Fix build deno failure

aeb2c88

Update wgpu-types/src/lib.rs

0a08124

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>

implement deno part

e85f1b1

Change Option<Vec<TextureFormat>> to & [TextureFormat]

924e216

jinleili and others added 14 commits January 13, 2023 10:28

Set view_formats on web target

22868cc

Add a reinterpret_srgb_ness test

ecfd971

Define tolerance data to improve test

11329c2

Set VIEW_FORMATS downlevel_flag for test

0775f29

Add the missing field view_formats in copy_overflow_z test

c64a03f

Update CHANGELOG

fe57a26

Update wgpu-core/src/device/mod.rs

3d62698

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>

rebase upstream/master

b8d2178

Remove duplicate definitions of TextureDescriptor

2078468

Make TextureDescriptor pass doctest

bac83f6

Remove DownlevelFlags::VIEW_FORMATS from gles/adapter.rs

8c491b8

Update wgpu-types/src/lib.rs

9dd7bd1

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>

Update wgpu-types/src/lib.rs

b00e4cd

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>

impl map_label_and_view_formats

247dc5c

jinleili force-pushed the view_formats branch from 9550278 to 247dc5c Compare January 13, 2023 02:32

Make pass doctest again

8bbccdd

jinleili force-pushed the view_formats branch from 1c274a2 to 8bbccdd Compare January 13, 2023 02:50

DCNick3 mentioned this pull request Jan 13, 2023

Restructure the rendering pipeline to do the blending the "wrong way" DCNick3/shin#6

Closed

cwfitzgerald approved these changes Jan 18, 2023

View reviewed changes

cwfitzgerald merged commit 0849e78 into gfx-rs:master Jan 18, 2023

cwfitzgerald mentioned this pull request Jan 18, 2023

Allow vulkan to change view formats #3399

Merged

2 tasks

jinleili deleted the view_formats branch January 19, 2023 02:06

This was referenced Jan 20, 2023

Implement view_formats for SurfaceConfiguration #3409

Merged

vk: improve view_formats setting #3412

Merged

teoxoy mentioned this pull request Mar 3, 2023

[dx12] Use typeless formats for textures that might be viewed as srgb or non-srgb #3555

Merged

3 tasks

Conversation

jinleili commented Nov 26, 2022 • edited by cwfitzgerald Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Nov 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

crowlKats commented Nov 26, 2022

Uh oh!

cwfitzgerald left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jinleili commented Nov 27, 2022

Uh oh!

crowlKats commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

khyperia commented Dec 27, 2022

Uh oh!

jinleili commented Dec 28, 2022

Uh oh!

cwfitzgerald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cwfitzgerald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cwfitzgerald commented Jan 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jinleili commented Nov 26, 2022 •

edited by cwfitzgerald

Loading

codecov-commenter commented Nov 26, 2022 •

edited

Loading

cwfitzgerald left a comment •

edited

Loading

crowlKats commented Nov 27, 2022 •

edited

Loading