gpuweb icon indicating copy to clipboard operation
gpuweb copied to clipboard

Resurrect canvas srgb formats

Open kainino0x opened this issue 3 years ago • 4 comments

We just removed these in #2522 with the intent of just using viewFormat to get -srgb texture views into the canvas.

But I'm proposing adding them back because on some systems (current D3D12, Vulkan without extension), enabling srgb viewFormats are likely more costly than just returning an srgb format. Internally, it may need to be reinterpreted, but hopefully sometimes it doesn't.

Implementation cost should be small (hopefully) and this is slightly more convenient for developers also.

https://github.com/gpuweb/gpuweb/discussions/2537#discussioncomment-2062936 https://github.com/gpuweb/gpuweb/pull/2540#discussion_r794095160


Preview | Diff

kainino0x avatar Jan 29 '22 04:01 kainino0x

Previews, as seen when this build job started (54c401f7ec920a242eb38b201044a2385e4875df): WebGPU | IDL WGSL Explainer

github-actions[bot] avatar Jan 29 '22 04:01 github-actions[bot]

Previews, as seen when this build job started (f9bbe88c3f9abd9ccd9b27d8cf2ab62199b2aacf): WebGPU | IDL WGSL Explainer

github-actions[bot] avatar Feb 07 '22 23:02 github-actions[bot]

We just removed these in #2522 with the intent of just using viewFormat to get -srgb texture views into the canvas.

But I'm proposing adding them back because on some systems (current D3D12, Vulkan without extension), enabling srgb viewFormats are likely more costly than just returning an srgb format. Internally, it may need to be reinterpreted, but hopefully sometimes it doesn't.

Implementation cost should be small (hopefully) and this is slightly more convenient for developers also.

#2537 (reply in thread) #2540 (comment)

Preview | Diff

Implementation cost may not be small. As far as I know there are no "-srgb" formats in Viz yet.

jchen10 avatar Feb 08 '22 07:02 jchen10

That's a fair point. I was thinking we could essentially start by just taking a texture view, but of course that's not possible because getCurrentTexture returns a texture.

Maybe we should postpone this change then.

kainino0x avatar Feb 09 '22 02:02 kainino0x