Minor performance improvements to SSGI (case 1367144). #5921

anisunity · 2021-10-05T19:48:36Z

https://fogbugz.unity3d.com/f/cases/edit/1367144/
Unfortunately as expected all the suggestions by the user do no improve the performance at all. I've improved the performance a bit, but it is towards for intrinsic based platforms (ps4/xbox1).

Testing status
Tested the performance improvement on console and on PC. The difference is more important on console and remains meh on PC unfortunately. tested with full/half res effect and half res denoiser. We need to run the tests mainly to validate.

github-actions · 2021-10-05T19:48:50Z

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://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, 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
/jobDefinition/.yamato%2Fall-hdrp.yml%23HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%252Fall-lightmapper.yml%2523PR_LightMapper_trunk

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.

anisunity · 2021-10-05T19:49:05Z

...der-pipelines.high-definition/Runtime/Lighting/ScreenSpaceLighting/BilateralUpsample.compute

-    float2 _DepthPyramidFirstMipLevelOffset;
-CBUFFER_END
+// LDS that store the half resolution data
+groupshared float3 gs_cacheLighting[36];


Compressing the result to 11 11 10 makes the kernel overall slower unfortunately.

anisunity · 2021-10-05T19:50:03Z

...der-pipelines.high-definition/Runtime/Lighting/ScreenSpaceLighting/BilateralUpsample.compute

-    int2 halfResTap3 = clamp(0, halfResolution + UpscaleBilateralPixels[offsetInCoordTable + 3], _HalfScreenSize.xy - 1);
-
-    // Grab the depth of all the half resolution pixels
-    float4 lowDepths = float4(LOAD_TEXTURE2D_X(_DepthTexture, asuint(_DepthPyramidFirstMipLevelOffset) + halfResTap0).x


There was a bug here due to using the first mip instead of 1 every 4 pixels

anisunity · 2021-10-05T19:50:42Z

...s.high-definition/Runtime/Lighting/ScreenSpaceLighting/ScreenSpaceGlobalIllumination.compute

    GetCountAndStart(posInput, LIGHTCATEGORY_ENV, envLightStart, envLightCount);
    totalWeight = 0.0f;

+    uint envStartFirstLane;


Force scalarization makes the kernel twice faster on ps4 as it is VGPR bound by the reflection probe code.

anisunity · 2021-10-05T19:51:19Z

....high-definition/Runtime/RenderPipeline/Raytracing/Shaders/Denoising/DiffuseDenoiser.compute

-
-        // Distribute them according a square profile
-        newSample *= denoisingRadius;
+        float2 newSample = _PointDistribution[sampleIndex] * denoisingRadius;


Improves a bit the performance to save those two texture reads.

remi-chapelain

Tested if SSGI was working the same ✔️
Tested quickly performance of the SSGI markers and found improvement between 5% and 10%

If possible, it'd be great the remove the marker SSGIConvert which apprently does not make sense anymore since the Forward Emissive rework.
Also, the marker SSGIDenoise always show zero since SSGI is using the ray tracing denoiser.

anisunity · 2021-10-12T16:02:57Z

merged, reruning the test just in case as i resolved conflicts

anisunity · 2021-10-13T12:18:37Z

mmmh, the failures are not expected i'll be investigating it.

anisunity · 2021-10-13T13:13:36Z

Fixed the test and relaunching them on yamato

anisunity · 2021-10-15T08:22:26Z

tests are green, merging the changlog and merging.

* Fixed initial decal position #5889 * Fix APV issue spewing asserts when baking with max subdiv level of 2 (#5888) * Add override checkbox. * Fix for problem when max subdiv is smaller than index voxel update size. * Revert "Merge branch 'HDRP/add-override-checkbox-for-pv' into HDRP/investigate-issue-with-faulty-index" This reverts commit e33a421, reversing changes made to 924763f. * Skip refresh ops if feature is disabled. (#5886) * Fixed the clouds not taking properly into account the fog when in distant mode and with a close far plane (case 1367993). (#5884) * Fixed the clouds not taking properly into account the fog when in distant mode and with a close far plane (case 1367993). * Fix formatting * Update CHANGELOG.md Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * To info box (#5879) * Add override checkbox. (#5878) * corrected links to HDAdditionalLightData and HDLightTypeAndShape (#5861) * updated link to expert guide (#5857) * Add comment (#5853) * [HDRP][Path Tracing] Added selection of light types for Unlit shadow mattes. #5855 * Maintain APV cells loaded if at least one scene still references that cell (#5900) * Add override checkbox. * Fix for problem when max subdiv is smaller than index voxel update size. * Revert "Merge branch 'HDRP/add-override-checkbox-for-pv' into HDRP/investigate-issue-with-faulty-index" This reverts commit e33a421, reversing changes made to 924763f. * Add a basic APV test to runtime tests (#5952) * New test * Tentative ref images * Change threshold to be non-zero. * Forgot one threshold... * Add test filter. * [HDRP] APV - Fix voxelization issues with planes at the origin (#5897) * Fix precision issues with the scene voxelization, especially with geometry at the origin. * Updated changelog * Fix renderer bounds for the plane case * disable debug * rename epsilon Co-authored-by: JulienIgnace-Unity <julien@unity3d.com> * Fix another issue with probe volume subdiv multiplier when changing to a lower subdiv level (#5893) * Add override checkbox. * Fix for problem when max subdiv is smaller than index voxel update size. * Revert "Merge branch 'HDRP/add-override-checkbox-for-pv' into HDRP/investigate-issue-with-faulty-index" This reverts commit e33a421, reversing changes made to 924763f. * Fix issue * Revert "Fix for problem when max subdiv is smaller than index voxel update size." This reverts commit 03feabd. * Fixed the volumetric clouds debug view not taking into account the exposure and leading to Nans (case 1365054). (#5940) Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Change order of fields in the cbuffer. (#5980) Co-authored-by: FrancescoC-Unity <francescoc@unity3d.com> * Fixed the dependency between transparent SSR and transparent depth prepass being implicit (case 1365915). (#5898) * Fixed the dependency between transparent SSR and transparent depth prepass being implicit (case 1365915). * review corrections Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * Updating docs for merged PRs (#5821) * PR #5459 Updated the docs changed in "Add high quality antialiasing example using the accumulation api" PR. * Doc fixes Fixed typo and changed SuperSampling "effect" to "method" Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * [Fogbugz # 1365687] Fixing depth pyramid with multicamera on hardware drs. #5902 * Fix (#5994) * release cmd (#5993) * test linux vulkan and OSX filter out. (#5989) * Fixed removal of depth buffer binding (#5910) * Fixed removal of depth buffer binding * Add object in graphic test * screenshots Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * [HDRP][Path Tracing] Added orthographic camera support #5944 * Fixed a regression in the transparent SSR color pyramid usage. (#6001) * Fixed a regression in the transparent SSR color pyramid usage. * Update CHANGELOG.md Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * [HDRP] Reenable crossfade on the HD SpeedTree 8 shader. #5957 * Force scalarization of shadow index data off (#6007) * Fixes for light anchor #5915 * Minor performance improvements to SSGI (case 1367144). #5921 * Update TestCaseFilters.asset Co-authored-by: Adrien de Tocqueville <adrien.tocqueville@unity3d.com> Co-authored-by: FrancescoC-unity <43168857+FrancescoC-unity@users.noreply.github.com> Co-authored-by: anisunity <42026998+anisunity@users.noreply.github.com> Co-authored-by: emilybrown1 <88374601+emilybrown1@users.noreply.github.com> Co-authored-by: Emmanuel Turquin <emmanuel@turquin.org> Co-authored-by: Antoine Lelievre <antoinel@unity3d.com> Co-authored-by: JulienIgnace-Unity <julien@unity3d.com> Co-authored-by: FrancescoC-Unity <francescoc@unity3d.com> Co-authored-by: Vic Cooper <63712500+Vic-Cooper@users.noreply.github.com> Co-authored-by: Kleber Garcia <kleber.garcia@unity3d.com> Co-authored-by: Tianliang Ning <paula.s.ning@gmail.com>

anisunity added the HDRP label Oct 5, 2021

anisunity requested review from a team, FrancescoC-unity and sebastienlagarde October 5, 2021 19:48

anisunity self-assigned this Oct 5, 2021

anisunity commented Oct 5, 2021

View reviewed changes

anisunity marked this pull request as ready for review October 5, 2021 19:51

FrancescoC-unity approved these changes Oct 6, 2021

View reviewed changes

remi-chapelain reviewed Oct 12, 2021

View reviewed changes

sebastienlagarde added the needs-backport-2021.2 label Oct 12, 2021

sebastienlagarde approved these changes Oct 12, 2021

View reviewed changes

Minor performance improvements to SSGI (case 1367144).

8973527

anisunity force-pushed the HDRP/fix-1367144 branch from 21b1688 to 8973527 Compare October 13, 2021 12:26

Fix tests

cc265b7

anisunity added 2 commits October 15, 2021 10:23

Merge branch 'master' into HDRP/fix-1367144

ce63965

Merge branch 'master' into HDRP/fix-1367144

8924e4c

anisunity merged commit 2732801 into master Oct 15, 2021

anisunity deleted the HDRP/fix-1367144 branch October 15, 2021 12:15

sebastienlagarde added backported-2021.2 and removed needs-backport-2021.2 labels Oct 18, 2021

sebastienlagarde pushed a commit that referenced this pull request Oct 18, 2021

Minor performance improvements to SSGI (case 1367144). #5921

a19f758

sebastienlagarde mentioned this pull request Oct 18, 2021

[HDRP Backport 21.2] Backport various PRs #6043

Merged

anisunity mentioned this pull request Oct 22, 2021

Fix regression that was introduced in a previous PR on the diffuse denoiser #6119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Minor performance improvements to SSGI (case 1367144). #5921

Minor performance improvements to SSGI (case 1367144). #5921

Uh oh!

anisunity commented Oct 5, 2021 •

edited

Loading

Uh oh!

github-actions bot commented Oct 5, 2021

Uh oh!

anisunity Oct 5, 2021

Uh oh!

anisunity Oct 5, 2021

Uh oh!

anisunity Oct 5, 2021

Uh oh!

anisunity Oct 5, 2021

Uh oh!

remi-chapelain left a comment

Uh oh!

anisunity commented Oct 12, 2021

Uh oh!

anisunity commented Oct 13, 2021

Uh oh!

anisunity commented Oct 13, 2021

Uh oh!

anisunity commented Oct 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Minor performance improvements to SSGI (case 1367144). #5921

Minor performance improvements to SSGI (case 1367144). #5921

Uh oh!

Conversation

anisunity commented Oct 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 5, 2021

Uh oh!

anisunity Oct 5, 2021

Choose a reason for hiding this comment

Uh oh!

anisunity Oct 5, 2021

Choose a reason for hiding this comment

Uh oh!

anisunity Oct 5, 2021

Choose a reason for hiding this comment

Uh oh!

anisunity Oct 5, 2021

Choose a reason for hiding this comment

Uh oh!

remi-chapelain left a comment

Choose a reason for hiding this comment

Uh oh!

anisunity commented Oct 12, 2021

Uh oh!

anisunity commented Oct 13, 2021

Uh oh!

anisunity commented Oct 13, 2021

Uh oh!

anisunity commented Oct 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anisunity commented Oct 5, 2021 •

edited

Loading