Skip to content

Conversation

FrancescoC-unity
Copy link
Contributor

There were two issues:

  1. Taa was using the RTHandleScale for current history system, while it needed to use the RTHandleScale for previous history system.

  2. Because the history system does not go through the HW dynamic system, therefore we should preserve the RTHandleScale computation.

Before fixing these issues the TAA will produce flickering halos around objects because history was sampled completely wrong.

TAAFlicker

With the fix

TAAFlickerFix

What have I tested: The repro case (note it mentions Gamecore, but it repros on dx12 too), both manual change of res and automatic script to change dyn res fraction.

Note: There is another hw dynamic res bug fixed by this PR #1652 , to workaround the issue you will see, after pressing play, change res -> see scene appearing small on corner -> force screen res to 100 -> remove force res and try again. This seems to work as a workaround. Otherwise just merge that branch into this.

@github-actions
Copy link

github-actions bot commented Sep 4, 2020

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page)
and cancel any jobs that started on Yamato.
See the PR template for more information.
Thank you!

Copy link
Contributor

@TomasKiniulis TomasKiniulis left a comment

Choose a reason for hiding this comment

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

Checked repro project with different Dynamic Resolution settings on Dx12, Xbox, Vulkan. Issue does not occur anymore.
m2F0BPjBcK

@sebastienlagarde
Copy link
Contributor

@FrancescoC-unity does it need backport to 7 / 8 ? or it is only the new TAA?

@sebastienlagarde sebastienlagarde merged commit f32a5a9 into HDRP/staging Sep 8, 2020
@sebastienlagarde sebastienlagarde deleted the HDRP/fix-taa-with-hw-dynamic-res branch September 8, 2020 07:27
@FrancescoC-unity
Copy link
Contributor Author

@FrancescoC-unity does it need backport to 7 / 8 ? or it is only the new TAA?

Already backported (you merged it friday)

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.

3 participants