{{ message }}
[URP] Upscaling Filters & FXAA Scaling Logic#6178
Merged
Merged
Conversation
gmitrano-unity
force-pushed
the
urp/flexible-fxaa
branch
from
ae032a7 to
850f478
Compare
gmitrano-unity
changed the title
gmitrano-unity
force-pushed
the
urp/flexible-fxaa
branch
from
850f478 to
61283b8
Compare
gmitrano-unity
force-pushed
the
urp/flexible-fxaa
branch
from
b2752e0 to
43a567c
Compare
Contributor
|
Adding Sandy and Manuele for coverage on this PR. |
This change moves the FXAA HLSL implementation into the common post processing shader file. This will make it easier to execute the FXAA shader logic outside of the FinalPost pass which is a prerequisite for FSR. This change also modifies the FXAA Load helper function to make it use point sampling instead of linear sampling on GLES. This should yield more consistent behavior between GLES and non-GLES environments.
This change adds a new property to the pipeline asset that allows users to control which filter is used when upscaling is performed. The current implementation supports selecting either bilinear or nearest-neighbor. Additional methods of filtering will be added in future changes.
This change adds a new enum called ImageScaling which enumerates all possible image scaling scenarios. This helps make the scaling related conditional logic easier to read.
This change splits the upscaling filter selected by the user from the one used within URP's implementation to allow for a "meta filter" called Auto. This new filter automatically switches between bilinear and nearest-neighbor filtering based on the current rendering environment's ability to perform integer scaling.
This change renames the _FILTER_POINT macro to _POINT_SAMPLING to improve consistency with future FSR changes.
This change updates the FXAA common shader functions to support 2d array input textures which are used in XR's single pass instanced mode.
gmitrano-unity
force-pushed
the
urp/flexible-fxaa
branch
2 times, most recently
from
d53ab85 to
9f388a7
Compare
This commit adds new test cases for the upscaling filters that were added in a previous change.
gmitrano-unity
force-pushed
the
urp/flexible-fxaa
branch
from
9f388a7 to
7642199
Compare
manuele-bonanno
suggested changes
Nov 24, 2021
manuele-bonanno
left a comment
manuele-bonanno
left a comment
Contributor
There was a problem hiding this comment.
looks good to me. The only requested change would be to replace the GetTemporaryRT with RTHandles (just landed in master) since we deprecated temporaryRT allocs
This change updates the upscaling setup logic to use an RTHandle instead of a temporary render target.
Updated the URP changelog to reflect the upscaling filter changes and the FXAA scaling fix.
This change resolves a black screen issue on XR platforms (tested in the mock HMD) which was caused by missing support for draw procedural in the upscaling setup shader.
phi-lira
reviewed
Dec 3, 2021
phi-lira
left a comment
phi-lira
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Made some comments but nothing blocking the PR.
How do you plan to land docs?
This commit reverts the auto-format changes made in PostProcessData.asset.
This commit renames upscaleSetupPs -> upscaleSetupPS to make the naming consistent with other shaders.
This commit addresses various pieces of review feedback such as missing docs, renames, and data visibility. It also fixes a draw procedural related multi-compile option.
This change adds a comment block in the upscaling setup shader that describes the cases where it's used and why.
phi-lira
approved these changes
Dec 3, 2021
phi-lira
left a comment
phi-lira
left a comment
Contributor
There was a problem hiding this comment.
LGTM the remaining feedback can be addressed in the next PR if you wish.
This change renames the extra scaling blit pass to better communicate that it can be used in both upscaling and downscaling scenarios.
This change adds additional information to the changelog notes for the upscaling filters.
phi-lira
requested review from
a team
and removed request for
a team and
sandy-carter-unity
Contributor
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Purpose of this PR
This change adds a new "upscaling filter" property to the pipeline asset that allows users to control which filtering method is used when upscaling occurs within URP. In the current implementation, nearest-neighbor and bilinear filtering are supported. In the future, other upscaling methods such as FidelityFX Super Resolution (FSR) could be supported as well.
This change also adds a new upscaling setup shader which performs the FXAA logic when scaling is enabled. This will likely have a performance cost, but it improves quality since FXAA wasn't designed to be used during scaling. The image below shows the original logic at 77% scale on the left and the modified logic with 77% scale on the right:
This new pass could also be used to perform color space conversion logic for FSR if it's added in the future.
Testing status
Manual tests on Viking Village URP with FXAA on and off + nearest-neighbor vs bilinear filtering on Windows and Android
Comments to reviewers
This change does not modify any of the Renderer2D logic. How should the upscaling filter property interact with the pixel perfect filtering support in the 2d renderer?
Notes for QA
The changes in this PR will be verified for quality as part of the larger FSR PR here: #6262
Remaining Tasks