-
-
Couldn't load subscription status.
- Fork 4.2k
[Merged by Bors] - Wgpu 0.15 #7356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d773188
8676ca0
529c7bf
b88283f
f876b09
5d07a94
7485256
9cd74e1
540b384
9a61633
1f58da9
37332bc
3c59032
1a3735e
b0abf7b
cb120f9
5e83642
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -338,6 +338,8 @@ fn prepare_view_targets( | |
| format: main_texture_format, | ||
| usage: TextureUsages::RENDER_ATTACHMENT | ||
| | TextureUsages::TEXTURE_BINDING, | ||
| // TODO: Consider changing this if main_texture_format is not sRGB | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are the implications here? The comment doesn't elaborate enough for me to understand its purpose. It seems to imply that something assumes a default that will be an sRGB format? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The wgpu documentation suggests this is about potentially requesting a different kind of texture view than the texture format of the surface texture. So I suppose we would do something like: if !main_texture_format.describe().srgb {
main_texture_format.add_srgb_suffix()
}Or, we could even do it unconditionally as the same format would be returned anyway: https://docs.rs/wgpu-types/0.15.0/src/wgpu_types/lib.rs.html#2608 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I commented this with a focus on understanding the wgpu-side and calling it. But for our side I feel like we should only need to bother about srgb for the final surface, but I know that the deband dithering needs to be applied where quantisation happens which is unfortunately at the output of the main lighting pass, and then if we’re converting to srgb at some later point like in the final blit to the surface texture, we’d probably need to apply it there too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Afaik this should only be an issue on WebGPU (and maybe Nvidia Wayland?) where sRGB surfaces aren't supported. You need to use an sRGB texture view on the surfaces for the output colours to look right, but I was running into Vulkan validation errors which seemed to imply that the whole pipeline needs to have the same sRGB texture view as the surface it outputs to. (which isn't possible, as texture views are currently limited in wgpu to the same format as the texture, just with sRGB toggled) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The validation issues might also be due to a wgpu bug. There's a fix for it (gfx-rs/wgpu#3432), but I haven't tested it yet. |
||
| view_formats: &[], | ||
| }; | ||
| MainTargetTextures { | ||
| a: texture_cache | ||
|
|
@@ -370,6 +372,7 @@ fn prepare_view_targets( | |
| dimension: TextureDimension::D2, | ||
| format: main_texture_format, | ||
| usage: TextureUsages::RENDER_ATTACHMENT, | ||
| view_formats: &[], | ||
| }, | ||
| ) | ||
| .default_view | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.