Skip to content

Conversation

FrancescoC-unity
Copy link
Contributor

@FrancescoC-unity FrancescoC-unity commented Jul 6, 2021

Gonna be a vaguely long PR description, so

TL;DR:

  • Temporal Upscaling algorithm for dynamic resolution.
  • Take into account jittering vector when computing filter weights for TAA high quality.
  • Reduce flickering by a bit (no need to remove the jitter on the reprojection coordinates)
  • Exposing a base blend factor parameter to more/less history (as advanced setting), this can help reduce ghosting in favour of more aliasing or viceversa.
  • Removed some ringing from sharpening filter.
  • Optimized also DLSS path by sampling the original resolution input textures (depth and motion vectors) instead of having to upscale them (their upscale doesn't contribute quality, I checked with Kleber and it was for simplicity).

Temporal Upscaling

Added an option to perform temporal upscaling as Dynamic resolution upscaling filter. Similarly to DLSS it happens in place of TAA.

Specifically this pass is a modification of the TAA shader, most notable differences is to map the incoming input into an output resolution, the resampling strategy is a very sharp gaussian filter (see For Honor XFest presentation) and a confidence filter at the end for accumulating the history only on pixels we are confident about.
Generally there is a bit more of details, but the gist is the above.

The variance clipping corners are also a bit contracted as acting on lower resolutions will lead to possibly more ghosting.

Visual results

Full resolution images are available upon requests.

The following 2 images are with a zoom into the image
I suggest opening in a new tab, also note that even though representative, slight quality improvements have been made since I took the images (so actual result is slightly better)

Comparison

Comparison2

Comparison including also DLSS with 50% scale (25% amount of pixels). DLSS quality is still better (I suspect mostly due to better sharpening), but the quality is in that ballpark according to first tests with QA.

124583688-a1045680-de53-11eb-8e84-b49cb57d5814.mp4

Native 4k (sadly compressed)
SampleScene_20210702_1811422_Native

Catmull-Rom 70% (50% amount of pixels)
SampleScene_20210702_1811776_Dynamic Resolution - 70 - Catmull Rom

DLSS 70% (50% amount of pixels)
SampleScene_20210702_1812413_Dynamic Resolution - 70 - DLSS

TAAU 70% (50% amount of pixels)
SampleScene_20210702_1811930_Dynamic Resolution - 70 - TAA-U

Uncompressed images https://drive.google.com/drive/u/1/folders/130RVIBqFfpuwKyiWb6ipPK76203x9OO4

I have many more images if desired.

Random silly one with very low scale to have a more dramatic comparison
image

Performance impact

The cost of the pass is very similar to TAA high quality, the actual cost depends on how big the output resolution is. It is roughly similar (though a bit cheaper) than running said TAA high quality on the destination output.

PS4 Pro

VERY BASIC tests on template (I don't have a pro devkit, this was provided by Jans)

GPU COST Native 4K TAAU 70% (~50% pixels) TAAU 50% (~25% pixels)
~60ms (15 FPS) ~36ms (~28FPS) ~22ms (~46FPS)

PC

Of course PC numbers are what they are, but @remi-chapelain helped getting some numbers. The PC in question is fairly beefy, so more numbers are needed to be collected.

The template shot above

Native 4K 70% 50%
Catmull-Rom 39.2ms (~25.5FPS) 15ms (~66.6 FPS) 8.75ms (~114FPS)
TAA Upscale 39.2ms (~25.5FPS) 15.3ms (~65.3 FPS) 10.56ms (~94.7FPS)
DLSS 39.2ms (~25.5FPS) ~16.3 (~61.3 FPS) ~10.85ms (~92 FPS)

Spaceship benchmark (1440p, 4K hopefully to come. Here differences are less dramatic as bottlenecks with lower start resolution shift).

Native 1440p 70% 50%
Catmull-Rom 11.5ms (86.6FPS) 7.98ms (125.3FPS) 7.72ms (129.5FPS)
TAA Upscale 11.5ms (86.6FPS) 8.26 (121.1 FPS) 7.6 (131.5FPS)
DLSS 11.5ms (86.6FPS) 8.74 (114.4 FPS) 7.61 (131.4 FPS)

Known issues / missing things

  • Currently Exclude from TAA stencil bit don't work in this pass, even if excluded from TAA it will go through TAAU.
  • Missing expanded documentation and guidelines (e.g. speed rejection with low percentages kinda work bad)
  • A bit more exploration on confidence factor can be done.
  • A test.

Generally another iteration wouldn't necessarily hurt. I'd like to get some feedback first, hence the (Experimental)

Normal TAA changes

This PR also introduces a couple of TAA improvements.

Fix TAA High quality filtering
Finally taking jittering into account, on screenshots here might be subtle, if interested I suggest opening in separate tabs and switch back and forth.

Before
image
After
image

Expose base blend factor

To control how much history is taken each frame, the trade off is aliasing vs ghosting.

(NOTE: Ghosting is exaggerated in gifs, this way just to show the parameter impact)

blendfactor.mp4

Such parameter is only in advanced parameters since I don't expect non-power user need to touch this.

Reducing ringing in sharpening filter

Before
OldSharpen

After
NewSharpen


Both for TAAU and normal TAA we can and should do an optimization pass (now that we can sample stencil in compute, we can move TAA to compute and leverage LDS) which should simplify a few things as well.

@github-actions
Copy link

github-actions bot commented Jul 6, 2021

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2

SRP Core
You could run ABV on your branch before merging your PR, but it will start A LOT of jobs. Please be responsible about it and run it only when you feel the PR is ready:
/.yamato%252F_abv.yml%2523all_project_ci_2021.2
Be aware that any modifications to the Core package impacts everyone in the Graphics repo so please discuss the PR with your lead.

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@github-actions github-actions bot added the SRP label Jul 6, 2021
float2 prevUV = input.texcoord - motionVector;

CTYPE history = GetFilteredHistory(_InputHistoryTexture, prevUV, _HistorySharpening, _TaaHistorySize);
CTYPE history = GetFilteredHistory(_InputHistoryTexture, prevUV, _HistorySharpening, _TaaHistorySize, _RTHandleScaleForTAAHistory);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a small comment explaining why we need a separate scale here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added on top where the scales are defined

@FrancescoC-unity
Copy link
Contributor Author

FrancescoC-unity commented Jul 8, 2021

Fixed few issues and addressed review comments

Now running yamato

EDIT: Ouch saw that custom pass reference changed, so need to re-run.

# Conflicts:
#	TestProjects/HDRP_Tests/Assets/ReferenceImages/Linear/OSXEditor/Metal/None/9702_CustomPass_API.png
#	TestProjects/HDRP_Tests/Assets/ReferenceImages/Linear/WindowsEditor/Direct3D11/None/9702_CustomPass_API.png
#	TestProjects/HDRP_Tests/Assets/ReferenceImages/Linear/WindowsEditor/Direct3D12/None/9702_CustomPass_API.png
#	TestProjects/HDRP_Tests/Assets/ReferenceImages/Linear/WindowsEditor/Vulkan/None/9702_CustomPass_API.png
Copy link
Contributor

@remi-chapelain remi-chapelain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the performance tests I already did in the PR description and the general visual pass on the quality of the upsample pair-testing with @FrancescoC-unity,

Here's what I tested:

  • Changing resolution when using the filter ❌
  • Tested with DLSS that it disabled correctly the Fallback post AA ✔️
  • Tampering with HDRP Asset when using the filter ✔️
  • Tested behavior with VFX Graph and moving objects in general ✔️
  • Disabling custom frame settings (especially post process and AA) ✔️
  • Made a small script to modulate screen percentage to stay at a constant FPS at 4k ✔️
  • Various post process effects like DoF, Motion blur, Fog ⚠️
  • MSAA at various level ✔️
  • Forward / Deferred ✔️
  • Color buffer and PP buffer format R11G11B10 and R16G16B16A16 ✔️

Also, couldn't test building and changing resolution there because of this recent issue in trunk

Issues founds:

Few concerns :

  • Documentation, I won't block this PR for now, but now that we have a bunch of these filters, it can be useful for users to have some sort of pros and cons of each in the documentation to help them pick a filter depending on the context. For now, in the DRS HDRP page, we don't talk about any of them.
  • Base Blend factor, If I understood it correctly, this is the same type of parameter as we have in Volumetric Clouds and SSR with PBR Algorithm.
    In those effects, they are called "Temporal Accumulation" and "Accumulation Factor" respectively. If they do basically the same thing, could we unify this term to make it clear to users that it's the same principle behind ?

@FrancescoC-unity
Copy link
Contributor Author

Nope different issue, the one mentioned by Kleber got since fixed.
We discussed this, can be easily fixed will do when I get the chance.

Probably due to history of exposure being reset as well, I vaguely remember this getting fixed by another PR. It is something I think we should see if still there after I merge master and if it is file a separate bug related to it.

This is instead just very bad :D Will fix.

  • Documentation, I won't block this PR for now, but now that we have a bunch of these filters, it can be useful for users to have some sort of pros and cons of each in the documentation to help them pick a filter depending on the context. For now, in the DRS HDRP page, we don't talk about any of them.

As I mentioned in the PR, we do definitively need Docs, this PR however is likely to land with the option hidden as I need to sort the Exclude from TAA issue separately. Will likely write docs when I have the chance to come back to this PR in full force.

  • Base Blend factor, If I understood it correctly, this is the same type of parameter as we have in Volumetric Clouds and SSR with PBR Algorithm.
    In those effects, they are called "Temporal Accumulation" and "Accumulation Factor" respectively. If they do basically the same thing, could we unify this term to make it clear to users that it's the same principle behind ?

Nope, fairly different. TAA blend factor is more complex than those two in that it is modified by external parameters too instead of been used as is. I think they are still fairly different and I think similarly that the word base is really important here given that the value will likely be changed significantly per pixel.

# Conflicts:
#	com.unity.render-pipelines.core/Runtime/Common/GlobalDynamicResolutionSettings.cs
#	com.unity.render-pipelines.high-definition/CHANGELOG.md
#	com.unity.render-pipelines.high-definition/Runtime/PostProcessing/Shaders/DepthOfFieldCoC.compute
#	com.unity.render-pipelines.high-definition/Runtime/PostProcessing/Shaders/DoFCircleOfConfusion.compute
#	com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Camera/HDCamera.cs
@FrancescoC-unity
Copy link
Contributor Author

@remi-chapelain I fixed the white flash and the black screen, can't repro the other issue, can you take another look when you get the chance?

@remi-chapelain remi-chapelain self-requested a review July 23, 2021 13:45
Copy link
Contributor

@remi-chapelain remi-chapelain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made another quick pass and updated my previous comment, one more issue was found and fixed and the rest seems good for shipping to me !

Only issue that isn't fixed is the artefact when the resolution is changed (from low to high) when DRS is already very low not in forced percentage mode.
I reported an linked issue with DLSS that will hopefully fix it later.

I'd like documentation to be updated with pros and cons about other filter to help user picking but this can happen in a second pass on DRS as a whole.

@FrancescoC-unity
Copy link
Contributor Author

Yup, documentation will happen as soon as this land, there is also a bit of guideline needed for TAAU specifically so will write it later.

@sebastienlagarde sebastienlagarde requested a review from kecho July 26, 2021 10:48
@sebastienlagarde sebastienlagarde marked this pull request as ready for review July 26, 2021 10:48
FrancescoC-unity and others added 5 commits July 26, 2021 17:26
# Conflicts:
#	com.unity.render-pipelines.high-definition/CHANGELOG.md
#	com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.PostProcess.cs
@sebastienlagarde sebastienlagarde merged commit 06358ff into master Jul 27, 2021
@sebastienlagarde sebastienlagarde deleted the HDRP/taau-and-more-taa-options branch July 27, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants