Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Merged by Bors] - Wgpu 0.15 #7356
[Merged by Bors] - Wgpu 0.15 #7356
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
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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 comment
The 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:
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 comment
The 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 comment
The 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 comment
The 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.