Skip to content

Conversation

ellioman
Copy link
Contributor

@ellioman ellioman commented Jan 19, 2021

Purpose of this PR

This PR introduces various improvements and features to SSAO.

Open issues

  • Android GLES3: Instability with the test results often showing capturing half of the screen. I've disabled this platform in the test filters and will create a issue for it. I will link to the case in the test filters file once the issue has been created.
  • Linux Vulkan: Issue 1305639 is still open and affects the new test scenes. They are filtered out until the case is fixed.

Quality of downscaled SSAO improved

Screen Shot 2021-01-19 at 16 50 21

Performance

  • SSAO final Texture is now R8 instead of ARGB32
  • Performance improved in the SSAO Passes, shaving almost a 1ms on an iPhone 8.

Bugfixes

  • Fixes an issue where Depth Prepass was not added when SSAO was set to Depth Only.

Main changes

  • DepthNormal Passes are now split into separate .hlsl files for each shader type
  • DepthNormal passes now take the NormalMaps/Detail/Parallax into account if present
  • Unlit shaders now receive SSAO
  • Particle Shaders now receive SSAO (Depth & DepthNormal passes added in [2021.2] Particle Shaders: Adding Depth & Depth Normals passes #3141)
  • Intensity is no longer a slider but a float field

Other changes

  • Added a OutputTextureFeature renderer feature and shaders to the test project for the depth and normals scenes
  • Frame Timings settings were disabled for Android because of issue 1316285
  • Blit Type setting is now set to Always because of an issue with two cameras and a full screen quad (under investigation)

Improved test scenes and setup for SSAO

Previous scenes have been removed and new ones have been added.

Removed test scenes

  • 140_SSAO_DepthOnly_Projection
  • 141_SSAO_DepthOnly_Orthographic
  • 143_SSAO_DepthNormals_Orthographic
  • 144_SSAO_RenderToBackBuffer
  • 145_SSAO_Deferred_Projection
  • 146_SSAO_Deferred_Orthographic
  • 147_SSAO_Deferred_RenderToBackBuffer

New test scene setup (Tests Projection & Orthographic cameras)

  • 200_DepthDeferred
  • 200_DepthForward
  • 201_DepthNormalsDeferred
  • 201_DepthNormalsForward
  • 202_SSAO_Depth
  • 203_SSAO_DepthNorma
  • 204_SSAO_Deferred
  • 205_SSAO_BackBuffer

The new scenes now test

  • Lit Shader (Without Normal maps, with normal maps, alpha clipped, normalmap + detail + parallax)
  • ComplexLit (Without Normal maps, with normal maps, alpha clipped, normalmap + detail + parallax)
  • SimpleLit (Without Normal maps, with normal maps, alpha clipped)
  • BakedLit (Without Normal maps, with normal maps, alpha clipped)
  • Unlit
  • Particles Lit (Without Normal maps, with normal maps, alpha clipped)
  • Particles Simple Lit (Without Normal maps, with normal maps, alpha clipped)
  • Particles Unlit (Without Normal maps, with normal maps, alpha clipped)
  • SpeedTree 7 (Without Normal maps, with normal maps)
  • SpeedTree 8 (Without Normal maps, with normal maps)
  • Terrain
  • Terrain Waving Grass
  • Three different ShaderGraph shaders

QA

What has been tested

  • Tested in Editor (OS X 10.14.6) with the new test scenes, made sure all pass
  • Tested a build on an iPhone 8 (iOS 14.3) with one of the new test scenes
  • Tested a build on Galaxy S10+ (Mali-G76 MP12) with GLES3 and Vulkan
  • Tested a build on Galaxy Note10+ 5G (Adreno 640) with GLES3 and Vulkan

What needs testing

  • Test if there are any regressions with these changes, especially on mobile platforms & API's
  • Test performance difference

Performance Docs

Yamato

https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/universal%252Fssao-normalmaps-and-improvements

kaychang-unity and others added 30 commits September 29, 2020 17:20
Removed now unecessary gbuffer option in SSAO renderer feature.
…ked occlusion).

Optimisation in SSAO code to calculate view vector in (camera centered) world space.
Various bug fixes to pass most test scenes in UniversalGraphicsTest.
…d of the function (mathematically same result).
… instances are used when deferred renderer is enabled).

Adjusted internal logic that control allocation of RT in CopyDepthPass to match DepthOnlyPass and DepthNormalsOnlyPass.
Added some comments in StencilDeferred.shader.
- in editor mode, the deferred renderer now ignores UniversalRenderPipeline->MixedLighting flag and use mixed lighting shader variants. This matches forward renderer behaviour.
- refactored how main directional light is handled vs additional directional lights
…repeatedly bound for rendering and avoid clearing it more than once (some code set depthSlice to -1, some other to 0).

Instead, RenderTargetHandle now always set depthSlice to -1 to be consistent with all XR compatible code.
Made normal normalization in fragment shader platform specific (coherent with existing code).
…ot have additional lights enabled.

Fixed accurateGbufferNormal shader variants to be stripped on Vulkan devices, as Android Vulkan may need to fallback to accurateGbufferNormals in some situations.
Fixed incorrect calculation of render-target size.
Renamed _DEFERRED_ADDITIONAL_LIGHT_SHADOWS into _DEFERRED_LIGHT_SHADOWS as _ADDITIONAL part if irrelevant for deferred renderer.
…fer pass shaders.

Do not disable mixed lighting keyword in deferred to match behaviour of ForwardLights.
…cause there is no gbuffer pass for SM2.0 fallback shaders).
@ernestasKupciunas ernestasKupciunas requested a review from a team March 30, 2021 12:05
Copy link
Contributor

@erikabar erikabar left a comment

Choose a reason for hiding this comment

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

Android performance is similar or better with this PR. https://docs.google.com/spreadsheets/d/1BwQxOJXhzk4X6GeSvPqUlbuzKSFYtH_N_oIpxKsvtbg/edit#gid=1452499847&range=A1:D1

iOS will be tested by mobile team.

no regressions found in win/macOS

@aleksandrasdzikia aleksandrasdzikia requested review from aleksandrasdzikia and removed request for a team March 31, 2021 10:01
Copy link

@broepstorff broepstorff left a comment

Choose a reason for hiding this comment

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

Looks good from the terrain side of things. Just a few code cleanup requests

-Brad (I'm a new Graphics engineer hire on the Terrain team)

Copy link

@aleksandrasdzikia aleksandrasdzikia left a comment

Choose a reason for hiding this comment

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

Looks good, no issues found. Ran graphics test projects from UniversalGraphicsTest_Foundation, UniversalGraphicsTest_Terrain, UniversalGraphicsTest_Lighting (Scenes from 200 to 205). Also, ran URP Examples to check that there are no new issues. No visual or functionality issues.

Also there's a slight improvement on iOS performance, tests done by @ernestasKupciunas : https://docs.google.com/spreadsheets/d/149ac28KGkTC3G-AtvwrPVs8v57B_eo0he8FcYT05HmA/edit#gid=0

Devices used: OnePlus Nord, Galaxy Note10 USA, Huawei P20, Huawei P40 lite, iPhone SE.

@ellioman ellioman marked this pull request as ready for review April 6, 2021 07:33
@ellioman ellioman requested a review from a team as a code owner April 6, 2021 07:33
ellioman added 3 commits April 6, 2021 17:16
# Conflicts:
#	TestProjects/UniversalGraphicsTest_Foundation/Assets/Test/TestFilters/TestCaseFilters.asset
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md
#	com.unity.render-pipelines.universal/Shaders/BakedLit.shader
@phi-lira phi-lira merged commit ef69e29 into master Apr 7, 2021
@phi-lira phi-lira deleted the universal/ssao-normalmaps-and-improvements branch April 7, 2021 07:18
unity-emilk pushed a commit that referenced this pull request Dec 21, 2022
**URP Only Changes**
- Add `DepthNormal` on builtin Unlit output
- Add Ambient Occlusion on Unlit in URP

This is relative to this [change](#3174) landed in _2021.2.0a16_ at 95b83c506a187934af0898d9e5498a08ba231649
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.

10 participants