Skip to content

[HDRP][Fix] Case 1158661: Hardware mode for dynamic resolution breaks editor and game views #1652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 15, 2020

Conversation

NetteVI
Copy link
Contributor

@NetteVI NetteVI commented Aug 20, 2020

Checklist for PR maker

  • Have you added a backport label (if needed)? For example, the need-backport-* label. After you backport the PR, the label changes to backported-*.
  • Have you updated the changelog? Each package has a CHANGELOG.md file.
  • Have you updated or added the documentation for your PR? When you add a new feature, change a property name, or change the behavior of a feature, it's best practice to include related documentation changes in the same PR. If you do add documentation, make sure to add the relevant Graphics Docs team member as a reviewer of the PR. If you are not sure which person to add, see the Docs team contacts sheet.
  • Have you added a graphic test for your PR (if needed)? When you add a new feature, or discover a bug that tests don't cover, please add a graphic test.

Purpose of this PR

Fixes the visual artifacts and issues with hardware dynamic resolution, and re-enables it for Metal.

  • Specifically, this PR adds an overload of RTHandleSystem::SetReferenceSize that takes in the size of the final full resolution (without dyn-res applied) so that when hardware dynamic resolution is enabled, rtHandleScale stores the final resolution / maxSize. This is necessary to upscale to the final resolution correctly during the final pass, instead of upscaling to the maxSize of the render texture which causes the scaling artifacts that were previously seen when resizing the game view.

  • This PR also prevents the Lanczos upsampling shader function from dividing by 0, which previously caused numerous black line artifacts, especially noticeable when resizing.
    image

  • Finally, this also ensures that the RTHandleSystem always populates the m_AutoSizedRTsArray when setting a new hardware dynamic resolution state so that render textures are always recreated with the correct dynamic resolution state (this fixes case 1158661: https://fogbugz.unity3d.com/f/cases/1158661/ where the Scene View wasn't turning dynamic resolution off).


Testing status

NOTE: There was an existing issue with DX12 that causes the editor to crash when hardware dynamic resolution is enabled. That's fixed in this trunk PR: https://ono.unity3d.com/unity/unity/pull-request/111190/_/graphics/dynamic-resolution/case-1158661 and tests should be run against the custom revision: adc71429b87e

Manual Tests: What did you do?

  • Opened test project + Run graphic tests locally
  • Built a player
  • Checked new UI names with UX convention
  • Tested UI multi-edition + Undo/Redo + Prefab overrides + Alignment in Preset
  • C# and shader warnings (supress shader cache to see them)
  • Checked new resources path for the reloader (in developer mode, you have a button at end of resources that check the paths)
  • Other:

Yamato: (Select your branch):
https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics/tree/HDRP%252Fhardware-dynamic-resolution

@FrancescoC-unity
Copy link
Contributor

I pushed some small changes

Copy link
Contributor

@FrancescoC-unity FrancescoC-unity left a comment

Choose a reason for hiding this comment

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

Actually scratch this, it is an issue that was there before too.

@FrancescoC-unity
Copy link
Contributor

FrancescoC-unity commented Sep 4, 2020

NOTE I see some TAA issue, I am investigating a bit to see if exclusive to this PR or previous issue.

EDIT: This PR just changes the behaviour of a bug that was already there, so this PR can go on.

@NetteVI NetteVI marked this pull request as ready for review September 8, 2020 16:42
@sebastienlagarde sebastienlagarde merged commit 124a614 into master Sep 15, 2020
@sebastienlagarde sebastienlagarde deleted the HDRP/hardware-dynamic-resolution branch September 15, 2020 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants