[Merged by Bors] - Fix missing sRGB conversion for dithering non-HDR pipelines #6707
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aevyrie
added
A-Rendering
aevyrie
changed the title
superdump
approved these changes
Nov 21, 2022
Aceeri
approved these changes
Nov 21, 2022
mockersf
approved these changes
Nov 21, 2022
inodentry
reviewed
Nov 22, 2022
| output_rgb = output_rgb + screen_space_dither(in.frag_coord.xy); | ||
| // This conversion back to linear space is required because our output texture format is | ||
| // SRGB; the GPU will assume our output is linear and will apply an SRGB conversion. | ||
| output_rgb = pow(output_rgb, vec3<f32>(2.2)); |
There was a problem hiding this comment.
Quick (possibly stupid) question: could this back-and-forth conversion be avoided by choosing a different output texture format?
(i don't really know about how the output texture format is chosen in the first place, if srgb is a requirement then i guess that is not an option)
There was a problem hiding this comment.
Nope, great question. The answer seems to be "yes, but we can't do that" for a variety of reasons I don't fully understand. That would be my preference for sure though.
alice-i-cecile
added
the
S-Ready-For-Final-Review
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Nov 22, 2022
# Objective - Fixes #6706 Zoom in on the shadow in the following images: ## Current bevy/main ### HDR On - correct  ### HDR Off - incorrect  ## This PR ### HDR On - correct  ### HDR Off - corrected  ## Close-up comparison ### New  ### Old  ## Solution - It turns out there was an outright missing sRGB conversion for dithering non-HDR cameras. - I also tried using a precise sRGB conversion, but it had no apparent effect on the final image. --- ## Changelog - Fix deband dithering intensity for non-HDR pipelines.
|
Build failed: |
|
bors retry |
bors bot
pushed a commit
that referenced
this pull request
Nov 22, 2022
# Objective - Fixes #6706 Zoom in on the shadow in the following images: ## Current bevy/main ### HDR On - correct  ### HDR Off - incorrect  ## This PR ### HDR On - correct  ### HDR Off - corrected  ## Close-up comparison ### New  ### Old  ## Solution - It turns out there was an outright missing sRGB conversion for dithering non-HDR cameras. - I also tried using a precise sRGB conversion, but it had no apparent effect on the final image. --- ## Changelog - Fix deband dithering intensity for non-HDR pipelines.
|
Build failed: |
|
bors plz 🥺 bors retry |
bors bot
pushed a commit
that referenced
this pull request
Nov 22, 2022
# Objective - Fixes #6706 Zoom in on the shadow in the following images: ## Current bevy/main ### HDR On - correct  ### HDR Off - incorrect  ## This PR ### HDR On - correct  ### HDR Off - corrected  ## Close-up comparison ### New  ### Old  ## Solution - It turns out there was an outright missing sRGB conversion for dithering non-HDR cameras. - I also tried using a precise sRGB conversion, but it had no apparent effect on the final image. --- ## Changelog - Fix deband dithering intensity for non-HDR pipelines.
|
Build failed: |
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Nov 22, 2022
# Objective - Fixes #6706 Zoom in on the shadow in the following images: ## Current bevy/main ### HDR On - correct  ### HDR Off - incorrect  ## This PR ### HDR On - correct  ### HDR Off - corrected  ## Close-up comparison ### New  ### Old  ## Solution - It turns out there was an outright missing sRGB conversion for dithering non-HDR cameras. - I also tried using a precise sRGB conversion, but it had no apparent effect on the final image. --- ## Changelog - Fix deband dithering intensity for non-HDR pipelines.
bors
bot
changed the title
cart
pushed a commit
that referenced
this pull request
Nov 30, 2022
# Objective - Fixes #6706 Zoom in on the shadow in the following images: ## Current bevy/main ### HDR On - correct  ### HDR Off - incorrect  ## This PR ### HDR On - correct  ### HDR Off - corrected  ## Close-up comparison ### New  ### Old  ## Solution - It turns out there was an outright missing sRGB conversion for dithering non-HDR cameras. - I also tried using a precise sRGB conversion, but it had no apparent effect on the final image. --- ## Changelog - Fix deband dithering intensity for non-HDR pipelines.
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
…ne#6707) # Objective - Fixes bevyengine#6706 Zoom in on the shadow in the following images: ## Current bevy/main ### HDR On - correct  ### HDR Off - incorrect  ## This PR ### HDR On - correct  ### HDR Off - corrected  ## Close-up comparison ### New  ### Old  ## Solution - It turns out there was an outright missing sRGB conversion for dithering non-HDR cameras. - I also tried using a precise sRGB conversion, but it had no apparent effect on the final image. --- ## Changelog - Fix deband dithering intensity for non-HDR pipelines.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Objective
Zoom in on the shadow in the following images:
Current bevy/main
HDR On - correct
HDR Off - incorrect
This PR
HDR On - correct
HDR Off - corrected
Close-up comparison
New
Old
Solution
Changelog