[Fogbugz # 1365368] Fixing debug views for drs #5948

kecho · 2021-10-06T18:18:18Z

Purpose of this PR

Fixing all the broken debug views for DRS.
These have been broken for a long time.

https://fogbugz.unity3d.com/f/cases/1365368/

Testing status

Tested on dx12, PC with DRS on, at force resolution percentage of 20.

Feature	Software DRS	Hardware DRS	DLSS + Software DRS	DLSS + Software DRS	TAAU + Hardware DRS	TAAU + Hardware DRS
Material debug menu	Pass	Pass	Pass	Pass	Pass	Pass
Lighting debug menu (ray tracing options not tested)	Pass	Pass	Color Pyramid broken (unrelated, need a new PR)	Color Pyramid broken (unrelated, need a new PR)	Pass	Pass
Rendering debug menu	Pass	Pass	Pass	Pass	Pass	Pass

Full list of debug options in the bug.

Comment to QA

There are 2 bugs that have been discovered.

First one is with Color pyramid. There is a fogbugz tracked and ill attack it next
There is a problem when in dx12, hardware drs (any filter / algorithm). Must start with dual camera, forced drs, and with drs enabled on the camera. Hit play, then disable drs on the camera. Notice that the resolution is corrupted. This is mostly a bug with the forced option. Let me know if this qualifies as a bug and please qa try to repro it.

Apart from these issues Everything looked peachy. QA feel free to add to the test matrix above.

github-actions · 2021-10-06T18:18:32Z

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.

FrancescoC-unity · 2021-10-25T13:39:53Z

com.unity.render-pipelines.high-definition/Runtime/Debug/ClearDebugBuffer.compute

+[numthreads(64, 1, 1)]
+void clearMain(uint dispatchThreadID : SV_DispatchThreadID)
+{
+    const uint totalPixels = (uint)_DebugViewportSize.x * (uint) _DebugViewportSize.y;


(Apologies for late looking at it, I came back with 100s of emails, lots of stuff get lost in the catchup)

Why do we need a specific clear like this rather than an hardware clear?

There is no hardware clear for this. The clear used to be in software, always.
THe clear before was a uav in a pixel shader, and unless your pixel tiles aligned perfectly with your resolution it was fine. Because of DRS they dont, thus you get some strange race conditions.

As far as I know there isnt a hardware clear for a buffer. Unless of course we change _FullScreenDebugBuffer to a texture.

Ah nvm, seeing pixels mentioned and the name _FullScreenDebugBuffer I thought we were clearing the debug texture not the quad debug buffer.

FrancescoC-unity · 2021-10-25T13:40:46Z

com.unity.render-pipelines.high-definition/Runtime/Debug/DebugFullScreen.shader

@@ -305,7 +305,9 @@ Shader "Hidden/HDRP/DebugFullScreen"
                }
                if (_FullScreenDebugMode == FULLSCREENDEBUGMODE_CONTACT_SHADOWS)
                {
-                    uint contactShadowData = LOAD_TEXTURE2D_X(_ContactShadowTexture, (uint2)input.positionCS.xy).r;
+                    int w, h, e, l;
+                    _ContactShadowTexture.GetDimensions(0, w, h, e, l);


Why is this a special case getting the dimensions? Contact shadow texture is going to have same size as any screen sized buffer

FrancescoC-unity · 2021-10-25T13:44:53Z

com.unity.render-pipelines.high-definition/Runtime/Debug/DebugFullScreen.shader

@@ -214,7 +214,7 @@ Shader "Hidden/HDRP/DebugFullScreen"
                }
                if ( _FullScreenDebugMode == FULLSCREENDEBUGMODE_SCREEN_SPACE_SHADOWS)
                {
-                    float4 color = LOAD_TEXTURE2D_X(_DebugFullScreenTexture, (uint2)input.positionCS.xy);
+                    float4 color = SAMPLE_TEXTURE2D_X(_DebugFullScreenTexture, s_point_clamp_sampler, input.texcoord);


So this change relies on the fact that we do call this after resetting the RTHandle scales I assume? Where is that happening?

FrancescoC-unity · 2021-10-25T13:53:03Z

com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.Debug.cs


                builder.SetRenderFunc(
                    (FullScreenDebugPassData data, RenderGraphContext ctx) =>
                    {
+
+                        ctx.cmd.SetComputeBufferParam(data.clearBufferCS, data.clearBufferCSKernel, "_FullScreenDebugBuffer", data.debugBuffer);


Should create a constant in HDStringConstants , but I see it has been like this for other pre-existing usage too. Can change later

remi-chapelain

I just tried this PR and encountered an issue when showing the HDRP Debugger when DLSS was on. (By the way, this also happen on master). This errors prevent from testing the PR.

972a80a8e20105bd9662b5b77ce53fc1.mp4

kecho · 2021-11-03T14:56:27Z

@remi-chapelain absolutely weird. Could you try on a different project? / New project? I think I encountered this once and went away. It's definitely a bug, but I don't want to block this pr.
Feel free to create a new bug with this problem. If the PR is still after the workaround above blocked then this is a 'lets drop everything' issue and gotta fix it asap.

remi-chapelain · 2021-11-03T16:21:07Z

Yep, logged it, also repro on master.
However, the PR will be blocked either way since we can't test debug mode with DLSS :|

remi-chapelain

Much better :). Most of the debug view are fixed. ✔️
Tested Software / Hardware. Also built a player in dev mode ✔️
All tests were done in force resolution at 20% to exacerbate possible problems.
I found a few exceptions though:

Rendering

Full Screen Debug Mode > Lens Flare Data Driven fails ONLY WITH DLSS (black screen)

Decals

Display Atlas: Position and Scale of the displayed atlas is not the same with DRS on (repro)

Lighting

Tile/Cluster/Material Feature Variants Debug: this one I'm not entirely sure what to expect, it's just we are seeing the cluster / tile at input resolution leaving really big rectangles (repro)
SkyReflection / Cookie Atlas / Planar Reflection Atlas / Local Volumetric Fog Atlas: Same issue than with Decals atlas, atlas position and scale is not right for those 4 (repro)

Also, can you mark this one as fixed since this commit fixes the problem ?

kecho · 2021-11-15T19:28:35Z

@remi-chapelain Done. The new cluster debug view should look like this

The gray menu at the bottom will be scaled. The tiles however must remain resolution dependant. Sorry this took a while I had to rewrite a lot of pieces so it would work with DRS properly.

Tested with DRS hardware & software.

I also fixed the debug views from the atlases so they dont get offset.

remi-chapelain

We're getting there.

Among the things reported, there's only one thing that appears to be not working.
Rendering > Full Screen Debug Mode > LensFlare.
With DLSS there's still black, with DRS, it's still scaled down.

06ea1a1dff9141506e52df74720af89c.mp4

I used the lensFlareSamples from HDRP Package to test it (interiorScene)

kecho · 2021-11-23T18:47:02Z

We're getting there.

Among the things reported, there's only one thing that appears to be not working. Rendering > Full Screen Debug Mode > LensFlare. With DLSS there's still black, with DRS, it's still scaled down.

06ea1a1dff9141506e52df74720af89c.mp4
I used the lensFlareSamples from HDRP Package to test it (interiorScene)

Fixed! lmk if this works

remi-chapelain

Last issue fixed ! ✔️

* Fixing overdraw view by passing the correct sizes, removing super hacky clear of _FullScreenDebugBuffer * Null access check for dlss * FIxing vertex density view for DRS debugging * Fixing motion vector debug view for all drs configs * Fixing color log debug view * Fixing Color Log, Qnan and CoC debug views for DRS software/Hardware + DLSS * Fixing light tile combine with DRS debug views * Undoing changes in post process, not needed anymore * Fixing contact shadows and screen space shadows * Fixing material, depth pyramid and other debug modes * Fixing material categorization * Fixing issues with remaining full screen * Adding comment for clarity * Removing hacky GetDimensions * Moving to string constant * changelog * Forgot to add support for XR pixels * Making dlss debug table not resync its contents, this avoids bad state propagated up the UI * Fixing lens flares and nvidia debug views * Fixing debug overlay of shadows * Fix debug view of clustering & tiles (debug cateogry menu) * Fixing lens flare debug view

* Fixed references to probes not cleared when unloading a scene #5945 * [Fogbugz # 1365368] Fixing debug views for drs #5948 * [HDRP] Stacklit RTR: Fix issues with RTR reflections from stacklit materials #6103 * [HDRP] Lit coat ssr-rtr: Fix various issues with using SSR lighting with IBL fallback (case 1380351) #6106 * [HDRP] StackLit coat ssr-rtr light hierarchy and IBL fallback fixes. #6107 * AxF ssr-rtr: Implement refined light reflection hierarchy with separate weight for coat and base lobe to correctly fallback from SSR light to light reflection probes and also implement the same coat-traced light "reuse" as Lit for bottom lobe when roughnesses are similar. (#6108) This also cleans up the overlapping contributing reflection probes / dual normal setup problem in EvaluateBSDF_Env(). Based on previously discussed #4968. * Fix issue with reflection probe normalization via APV (#6140) * fix issue * - * [not ready] HDRP Scene Template: Bumped up IET framework version to 2.1 and better default layout for 1080p monitors (#6153) * bumped up iet framework to 2.1, created new 1080p and 4k friendly layout * Merge branch 'master' into hdrp-template-bumpup-iet-framework-version Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * [HDRP] Fix custom pass utils in XR #6271 * Update reference screenshots Co-authored-by: Adrien de Tocqueville <adrien.tocqueville@unity3d.com> Co-authored-by: Kleber Garcia <kleber.garcia@unity3d.com> Co-authored-by: slunity <37302815+slunity@users.noreply.github.com> Co-authored-by: FrancescoC-unity <43168857+FrancescoC-unity@users.noreply.github.com> Co-authored-by: pierre-unity <39901544+pierre-unity@users.noreply.github.com> Co-authored-by: Antoine Lelievre <antoinel@unity3d.com>

github-actions bot added the HDRP label Oct 6, 2021

kecho requested review from sebastienlagarde and FrancescoC-unity October 6, 2021 21:39

kecho changed the title ~~[WIP][Fogbugz # 1365368] Fixing debug views for drs~~ [Fogbugz # 1365368] Fixing debug views for drs Oct 6, 2021

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

sebastienlagarde requested a review from a team October 12, 2021 18:18

sebastienlagarde approved these changes Oct 19, 2021

View reviewed changes

kecho force-pushed the HDRP/FixingDebugViewsForDRS branch from 7834cb0 to b71b228 Compare October 25, 2021 13:36

FrancescoC-unity reviewed Oct 25, 2021

View reviewed changes

kecho requested a review from FrancescoC-unity October 25, 2021 19:16

kecho marked this pull request as ready for review October 25, 2021 19:17

remi-chapelain requested changes Nov 3, 2021

View reviewed changes

remi-chapelain self-requested a review November 8, 2021 13:29

remi-chapelain requested changes Nov 8, 2021

View reviewed changes

kecho force-pushed the HDRP/FixingDebugViewsForDRS branch 2 times, most recently from e1e3ca9 to 66e454b Compare November 15, 2021 19:27

kecho requested a review from remi-chapelain November 15, 2021 19:28

remi-chapelain requested changes Nov 18, 2021

View reviewed changes

kecho requested a review from remi-chapelain November 23, 2021 18:46

remi-chapelain approved these changes Nov 24, 2021

View reviewed changes

kecho force-pushed the HDRP/FixingDebugViewsForDRS branch from 01279f6 to 54e944e Compare November 24, 2021 16:19

Merge branch 'master' into HDRP/FixingDebugViewsForDRS

c737992

sebastienlagarde merged commit e3a57aa into master Nov 25, 2021

sebastienlagarde deleted the HDRP/FixingDebugViewsForDRS branch November 25, 2021 13:10

sebastienlagarde added backported-2021.2 and removed needs-backport-2021.2 labels Dec 8, 2021

sebastienlagarde pushed a commit that referenced this pull request Dec 8, 2021

[Fogbugz # 1365368] Fixing debug views for drs #5948

181334c

sebastienlagarde mentioned this pull request Dec 8, 2021

[HDRP] Backport various PRs #6528

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fogbugz # 1365368] Fixing debug views for drs #5948

[Fogbugz # 1365368] Fixing debug views for drs #5948

kecho commented Oct 6, 2021 •

edited

github-actions bot commented Oct 6, 2021

FrancescoC-unity Oct 25, 2021

kecho Oct 25, 2021

FrancescoC-unity Oct 25, 2021

FrancescoC-unity Oct 25, 2021

FrancescoC-unity Oct 25, 2021

FrancescoC-unity Oct 25, 2021

remi-chapelain left a comment

kecho commented Nov 3, 2021

remi-chapelain commented Nov 3, 2021

remi-chapelain left a comment •

edited

kecho commented Nov 15, 2021

remi-chapelain left a comment

kecho commented Nov 23, 2021

remi-chapelain left a comment

[Fogbugz # 1365368] Fixing debug views for drs #5948

[Fogbugz # 1365368] Fixing debug views for drs #5948

Conversation

kecho commented Oct 6, 2021 • edited

Purpose of this PR

Testing status

Comment to QA

github-actions bot commented Oct 6, 2021

FrancescoC-unity Oct 25, 2021

Choose a reason for hiding this comment

kecho Oct 25, 2021

Choose a reason for hiding this comment

FrancescoC-unity Oct 25, 2021

Choose a reason for hiding this comment

FrancescoC-unity Oct 25, 2021

Choose a reason for hiding this comment

FrancescoC-unity Oct 25, 2021

Choose a reason for hiding this comment

FrancescoC-unity Oct 25, 2021

Choose a reason for hiding this comment

remi-chapelain left a comment

Choose a reason for hiding this comment

kecho commented Nov 3, 2021

remi-chapelain commented Nov 3, 2021

remi-chapelain left a comment • edited

Choose a reason for hiding this comment

kecho commented Nov 15, 2021

remi-chapelain left a comment

Choose a reason for hiding this comment

kecho commented Nov 23, 2021

remi-chapelain left a comment

Choose a reason for hiding this comment

kecho commented Oct 6, 2021 •

edited

remi-chapelain left a comment •

edited