Experimental GL backend: Do not use larger-than-screen textures for blur buffers #674
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
Blur-texture sampling has been changed to `CLAMP_TO_EDGE` in commit 4b0ff37 and to using the buffer textures at screen position instead of texture origin in commit 89c18af. When using the above approach, expanding the buffer textures by the same amount as the damage region is not needed anymore, as we cannot render more than the screen region anyways. Having larger-than-screen buffer textures might lead to a slight darkening at the upper and right edges since we don't necessarily trigger the `CLAMP_TO_EDGE` condition in the intermediate steps. This becomes apparent when using dual-kawase at large blur-strengths with light backgrounds. These changes do not affect the general approach of rendering a larger-than-window region with the blur to accommodate the necessary increase in damage region. Related: 6d646b5
Codecov Report
@@ Coverage Diff @@
## next #674 +/- ##
==========================================
+ Coverage 39.39% 39.47% +0.07%
==========================================
Files 46 46
Lines 9613 9586 -27
==========================================
- Hits 3787 3784 -3
+ Misses 5826 5802 -24
|
@tryone144 I think since we are clamping to edge this will be fine? |
|
We should be fine. We are clamping to the edge of the textures, we copy a larger-than-window region as the base for blurring, and we only copy the blurred window region to the screen buffer. I just wanted to make sure I didn't have a fundamental flaw in my understanding on how we blur and what the mentioned commit tried to achieve. |
|
@tryone144 Yes, your understanding is the same as mine. |
yshui
approved these changes
Aug 24, 2021
|
Thanks! |
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.
Blur-texture sampling has been changed to
CLAMP_TO_EDGEin commit 4b0ff37 and to using the buffer textures at screen position instead of texture origin in commit 89c18af.When using the above approach, expanding the buffer textures by the same amount as the damage region is not needed anymore, as we cannot render more than the screen region anyways. Having larger-than-screen buffer textures might lead to a slight darkening at the upper and right edges since we don't necessarily trigger the
CLAMP_TO_EDGEcondition in the intermediate steps. This becomes apparent when using dual-kawase at large blur-strengths with light backgrounds.These changes do not affect the general approach of rendering a larger-than-window region with the blur to accommodate the necessary increase in damage region.
Fixes #673