Improve performance of OnDemand reflection probes [NEED C++] #2775

JulienIgnace-Unity · 2020-11-27T12:49:08Z

Purpose of this PR

Requires Ono PR: https://ono.unity3d.com/unity/unity/pull-request/123697/_/terrain/getactiveterrainsnoalloc

When using OnDemand reflection probes, unwanted performance spikes were occuring. This PR aims at improving this by doing several things:

Removed some GCAllocs in probe internal creation code
Stopped cleaning up cameras used for probes to avoid reallocating them and their volume stack (which was very costly)
Stopped updating the volume system for Lighting Override when it's not used
Disable cleaning up of terrain camera data for probe cameras (this is what requires C++ change)

This should improve https://fogbugz.unity3d.com/f/cases/1293225/

Testing status

Tested the small repro project provided in the fogbugz case.
Spikes should be much improved. There should still be smaller spikes because we do render the probe regularly (so 6 more camera renders) but antyhing else than rendering should be mostly gone.
It might be worth testing Detail Renderer (is that even supported in HDRP?) as it may have been impacted by the C++ change.
Also, checking the original user project would be good.

@Unity-Technologies/gfx-qa-hdrp here's a link to an editor with the C++ changes required to run the HDRP branch: https://bokken-artifacts-ui.bf.unity3d.com/katana-production/proj0-Build_WindowsEditor/39048765_11_01_2021_22_20_11_+0000/build/ZippedForTeamCity.zip

Disabled ApplyMaterialPropertyDrawers when rendering HDRP to avoid unwanted perf spikes

github-actions · 2020-11-27T12:49:40Z

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!

TomasKiniulis

With the PR there is 40ms improvement, but discussing with Julien it seems that majority of ms time is still taken by "CullScriptable" on user project(https://forum.unity.com/threads/hdrp-enable-camera-is-really-slow.988695/). It still uses up to 175ms which causes frame stutters.

Attaching my findings:
EmptyProject with terrain enabled:

PR DeepProfile

trunk master DeepProfile

PR without DeepProfile

trunk master without DeepProfile

… into hd/improve-ondemand-probe-perf # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs

Co-authored-by: Sebastien Lagarde <sebastien@unity3d.com>

* Not checking NdotL since it's not really valid for area lights (We have multiple valid light directions, not one) * Changelog Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

#3035) Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Fixed ShaderGraph decal draw order * Updated changelog Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Fixed access to invalid Contexts references after exiting playmode. * Fixed comparison gizmo after playmode. * Fixes from PR feedback

… conserving specular color option for the specular input parametrization (similar to case 1257050) (#3060)

…amera (#3063)

…phics into hd/improve-ondemand-probe-perf # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/Lighting/ProbeVolume/ProbeVolumeLighting.cs # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Camera/HDCamera.cs

TomasKiniulis

Looks good now! Testing the user project TryCull() method goes down from 180ms to 20ms!
trunk master

PR

Adding a lot more additional details on terrain does not cause a significant spike anymore.

… into hd/improve-ondemand-probe-perf # Conflicts: # com.unity.render-pipelines.high-definition/CHANGELOG.md # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs

… into hd/improve-ondemand-probe-perf

sebastienlagarde · 2021-04-20T19:42:00Z

fogbugzz: https://fogbugz.unity3d.com/f/cases/1324062/

sebastienlagarde · 2021-04-21T08:17:00Z

Currently waiting for this C++ PR to land: https://ono.unity3d.com/unity/unity/pull-request/123697/_/terrain/getactiveterrainsnoalloc

… into hd/improve-ondemand-probe-perf # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs

JulienIgnace-Unity added 2 commits November 27, 2020 13:41

Removed some GCAlloc when creating probes

e4c171d

Stop updating volume system for Lighting Override when not necessary.

76f58c0

Disabled ApplyMaterialPropertyDrawers when rendering HDRP to avoid unwanted perf spikes

github-actions bot added the HDRP label Nov 27, 2020

JulienIgnace-Unity requested review from a team, alelievr and sebastienlagarde November 27, 2020 12:51

JulienIgnace-Unity marked this pull request as draft November 27, 2020 13:08

alelievr approved these changes Nov 27, 2020

View reviewed changes

TomasKiniulis suggested changes Nov 27, 2020

View reviewed changes

sebastienlagarde added the need-backport-2021 label Dec 16, 2020

JulienIgnace-Unity and others added 13 commits December 16, 2020 16:03

Merge branch 'master' of https://github.com/Unity-Technologies/Graphics…

53bdbf0

… into hd/improve-ondemand-probe-perf # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs

Removed C++ change dependency

d15aa3e

Used new API to disable terrain data deletion for reflection cameras.

2fe900c

Fixed Render Graph immediate mode. (#3033)

77bae01

Co-authored-by: Sebastien Lagarde <sebastien@unity3d.com>

Fix issue with shadow mask and area lights (#3019)

47bd6c2

* Not checking NdotL since it's not really valid for area lights (We have multiple valid light directions, not one) * Changelog Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

Fix issue with capture callback (now includes post processing results) (

88daab3

#3035) Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

[HDRP] Fix decal draw order for ShaderGraph decal materials (#3018)

4313b48

* Fixed ShaderGraph decal draw order * Updated changelog Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

Fixed various Look Dev issues after exiting Playmode (#2956)

a9c4c27

* Fixed access to invalid Contexts references after exiting playmode. * Fixed comparison gizmo after playmode. * Fixes from PR feedback

Merge branch 'master' into hd/bugfix

d358740

Merge branch 'master' into hd/bugfix

11b5984

StackLit: Fix SG surface option property block to only display energy…

a22cfa4

… conserving specular color option for the specular input parametrization (similar to case 1257050) (#3060)

Fixed missing BeginCameraRendering call for custom render mode of a C…

a3c36a5

…amera (#3063)

JulienIgnace-Unity marked this pull request as ready for review January 12, 2021 13:10

github-actions bot added the SRP label Jan 12, 2021

JulienIgnace-Unity changed the base branch from master to hd/bugfix January 12, 2021 13:21

Fix wrong merge

d932606

TomasKiniulis approved these changes Jan 12, 2021

View reviewed changes

Base automatically changed from hd/bugfix to master January 14, 2021 10:52

sebastienlagarde removed the needs-backport-2021.1 label Mar 11, 2021

JulienIgnace-Unity added 4 commits March 22, 2021 10:31

Merge branch 'master' of https://github.com/Unity-Technologies/Graphics…

3aa5fa5

… into hd/improve-ondemand-probe-perf # Conflicts: # com.unity.render-pipelines.high-definition/CHANGELOG.md # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs

Fixed C++ API change

8d89719

Merge branch 'master' of https://github.com/Unity-Technologies/Graphics…

3a3c002

… into hd/improve-ondemand-probe-perf

Use new no alloc API to retrieve active terrain list.

858b661

JulienIgnace-Unity added 2 commits April 29, 2021 10:11

Merge branch 'master' of https://github.com/Unity-Technologies/Graphics…

332af8b

… into hd/improve-ondemand-probe-perf # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs

Fixed API change

711d170

sebastienlagarde approved these changes Apr 30, 2021

View reviewed changes

Merge branch 'master' of https://github.com/Unity-Technologies/Graphics…

83cb5aa

… into hd/improve-ondemand-probe-perf # Conflicts: # com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs

sebastienlagarde merged commit f52392a into master May 7, 2021

sebastienlagarde deleted the hd/improve-ondemand-probe-perf branch May 7, 2021 12:57

adrien-de-tocqueville mentioned this pull request Oct 25, 2021

Fix Sky override not taken into account #6127

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve performance of OnDemand reflection probes [NEED C++] #2775

Improve performance of OnDemand reflection probes [NEED C++] #2775

Uh oh!

JulienIgnace-Unity commented Nov 27, 2020 •

edited

Loading

Uh oh!

github-actions bot commented Nov 27, 2020

Uh oh!

TomasKiniulis left a comment

Uh oh!

TomasKiniulis left a comment •

edited

Loading

Uh oh!

sebastienlagarde commented Apr 20, 2021

Uh oh!

sebastienlagarde commented Apr 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Improve performance of OnDemand reflection probes [NEED C++] #2775

Improve performance of OnDemand reflection probes [NEED C++] #2775

Uh oh!

Conversation

JulienIgnace-Unity commented Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR

Testing status

Uh oh!

github-actions bot commented Nov 27, 2020

Uh oh!

TomasKiniulis left a comment

Choose a reason for hiding this comment

Uh oh!

TomasKiniulis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebastienlagarde commented Apr 20, 2021

Uh oh!

sebastienlagarde commented Apr 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

JulienIgnace-Unity commented Nov 27, 2020 •

edited

Loading

TomasKiniulis left a comment •

edited

Loading