Vertex density & quad overdraw debug modes #353

adrien-de-tocqueville · 2020-05-04T12:08:56Z

Purpose of this PR

This PR is adding two fullscreen debug modes in rendering pannel (not available on mac):

Quad Overdraw: Displays an overlay that highlights pixels running multiple fragment shaders.
Vertex density: Displays an overlay that highlights pixels containing multiple vertices. Helps finding models that need LODs.

Testing status

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)

Yamato: (Select your branch):
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/HDRP%252Fvertex-density

Adds two graphics tests (3004 for quad overdraw & 3005 for vertex density) which have all standard and SG materials variations

JMargevics

Hey! PR looks good. I've checked both modes and both work as expected. Will approve (more info in favro card), but there are a few nitpicks from my side:

With Max Vertex Density <5 render mode gives inconsistent results. You can see that some vertices are assigned random colors while having a homogeneous wireframe. According to Adrien these are shader shenanigans, where GPU can recalculate random vertices.

Sliders should have tooltips as it is a bit unclear what they mean currently. Also from my experience sliders range from 0 to 2048 but "working" area was 0-100. Getting density right on the smaller object required manual editing of smaller number (2-10).

Small proposal:
To have a legend displayed in scene view, where it explains which color means which density (teal - 1 overdraw, red - 10 overdraws etc).

…nt vertices

adrien-de-tocqueville · 2020-05-13T09:48:19Z

With Max Vertex Density <5 render mode gives inconsistent results. You can see that some vertices are assigned random colors while having a homogeneous wireframe. According to Adrien these are shader shenanigans, where GPU can recalculate random vertices.

To be precise, I don't think that's random, that depends on the order of vertices in the model. It may show that reordering the vertices can lead to fewer vertex shader execution

...ty.render-pipelines.high-definition/Runtime/RenderPipeline/ShaderPass/ShaderPassForward.hlsl

sebastienlagarde

So overall the PR is good, however I am worried by the extra memory and the extra slowness we will introduce on all the debug mode. We need to add a separate debug fullscreen pass that can be stripper and If users don't implement it, then it isn't an issue. We will discuss with with Julien and we should allocated debug memory only if required.

Now allocates one fullscreen uav instead of three

sebastienlagarde · 2020-05-26T20:37:28Z

...ty.render-pipelines.high-definition/Runtime/RenderPipeline/ShaderPass/ShaderPassForward.hlsl

@@ -60,6 +60,9 @@ PackedVaryingsToPS VertTesselation(VaryingsToDS input)
 #include "Packages/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/ShaderPass/TessellationShare.hlsl"
 #endif

+#if defined(DEBUG_DISPLAY) && !defined(_DEPTHOFFSET_ON)
+[earlydepthstencil] // quad overshading debug mode write to UAVs


Do we know if attribute really work with guarded define - i.e arent they always apply even if there is guard ?
(poking @JulienIgnace-Unity / @FrancescoC-unity )

Gaenerally the preprocessor is run before the attributes are evaluated for compilation (made small example with [branch] here http://shader-playground.timjones.io/82ad51ba693770094363e31f8b3bafe6 you can see in the DXC output how the branch contains the hint or not depending on the define value), so I think this should work fine, though never tried myself with forcing early depth stencil.

sebastienlagarde · 2020-05-26T20:42:51Z

com.unity.render-pipelines.high-definition/Runtime/Material/SharedRTManager.cs

@@ -113,6 +116,8 @@ public void InitSharedBuffers(GBufferManager gbufferManager, RenderPipelineSetti
                // In case of deferred, we must be in sync with NormalBuffer.hlsl and lit.hlsl files and setup the correct buffers
                m_NormalRT = gbufferManager.GetNormalBuffer(0); // Normal + Roughness
            }
+
+            m_DebugDisplayUAV = RTHandles.Alloc(Vector2.one, TextureXR.slices, colorFormat: GraphicsFormat.R32_UInt, dimension: TextureXR.dimension, enableRandomWrite: true, useDynamicScale: true, name: "DebugDisplayUAV");


@JulienIgnace-Unity we really need to do a pass on memory (note poking you also for render graph).
Currently we allocate a lot of memory even if we don't used thing, I know render graph can help here. But right now situation is still not good.
For the memory above for example 1) we shouldn't allocated in release or if we don't require debug, 2) we could reuse m_ContactShadowBuffer memory (which shouldn't be allocated if we disble shadow).

We will still merge this PR with this, but we really need to do a better memory management

Agreed but the question is: Do we want to spend time now doing this for the current code path are should we "just" wait for Render Graph to be enabled by default? Since we target Render Graph for the end of the month I'd say we wait.

sebastienlagarde

Waiting for a comment of Julien / Francesco about the #define on early stencil, otherwise it is ok.
But we will need to do a pass on memory allocation later. Situation isn't good.

FrancescoC-unity · 2020-05-27T07:12:32Z

com.unity.render-pipelines.high-definition/Runtime/Debug/DebugDisplay.hlsl

+// These modes are exclusive so we make only one fullscreen allocation for both.
+// For vertex density, it stores the number of vertex projected in each pixel.
+// For quad overdraw, each 2x2 quad of the UAV contains the overdraw count in top-left pixel and the locked quad in the bottom-right pixel. The two other pixels of the quad are unused.
+RW_TEXTURE2D_X(uint, _DebugDisplayUAV) :register(u2);


@sebastienlagarde I don't remember the details of our platforms implementation, but for the explicit registers in the uav did it need to be starting from after render target count or it is fine as soon as the register is after the render target count?

yes, I have think about that as well, guess the code may not work on PS4.
on PS4 we will need to bing on 2 + number of RT allocated. We will check in a second time (I tracked this).

com.unity.render-pipelines.high-definition/Runtime/Material/SharedRTManager.cs

sebastienlagarde · 2020-05-27T17:53:18Z

Note: not sure yet but the PR could affect Vulkan (guess this could be related to the early stencil?). I have re-run vulkan to be sure.

adrien-de-tocqueville · 2020-07-27T15:02:21Z

Compilation error on metal

error: use of undeclared identifier 'input'
u_xlati6 = int(input.SV_PrimitiveID0) + 0x1;

Looks like a bug when the shader gets converted from HLSL to MSL.
SV_PrimitiveID is necessary for the quad overdraw debug mode only

Bug when accessing ShaderVariablesDebugDisplay cbuffer in tessellation stage

com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/ShaderPass/VertMesh.hlsl

sebastienlagarde · 2020-09-17T08:06:37Z

com.unity.render-pipelines.high-definition/Documentation~/whats-new-10-0.md

@@ -112,6 +112,11 @@ For more information, see the Lighting panel section in the [HDRP debug window](

 The Render Pipeline Debug window now has a new Volume panel which you can use to visualize the Volume components that affect a specific Camera. For each Volume that contributes to the final interpolated value, this panel shows the value of each property and whether or not it is overridden. It also calculates the Volume's influence percentage using the Volume's weight and blend distance. For more information, see the Volume panel section in the [HDRP debug window](Render-Pipeline-Debug-Window.md#VolumePanel).

+#### Quad Overdraw and Vertex Density
+
+Quad Overdraw highlights GPU quads running multiple fragment shaders, which is mainly caused by small or thin triangles while Vertex Density displays pixels running multiple vertex shaders.


please add that it is not supported on metal

it's already there in the line under

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

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

JulienIgnace-Unity

RenderFullScreenDebug needs to be static. Otherwise lgtm.
Is it expected that we have so many scenes, screenshots and prefab modified? Should this change any test at all?

adrien-de-tocqueville · 2020-09-17T13:57:26Z

Is it expected that we have so many scenes, screenshots and prefab modified? Should this change any test at all?

There are two new tests but nothing else have been modified

JulienIgnace-Unity · 2020-09-17T14:09:45Z

My bad, did not see most of them are in the new scenes. There is just one file for the DebugLightLayer scene then that should probably not be changed (probably not a big deal)

lambda to static; fix with msaa

adrien-de-tocqueville · 2020-09-17T14:31:12Z

There is just one file for the DebugLightLayer

It's because I changed the script that sets the debug mode for the tests to be more consistent

PaulDemeulenaere · 2020-10-06T16:09:37Z

Note, this change fails with PSSL as well : SV_PrimitiveID isn't available at fragment stage.
We are fixing this asap, see https://github.com/Unity-Technologies/Graphics/compare/HDRP/disable-quad-overdraw-ps4

adrien-de-tocqueville added 7 commits April 28, 2020 15:30

Quad overdraw

a25272a

Vertex density

925c519

doc

3fbae89

Merge branch 'HDRP/staging' into HDRP/vertex-density

e7c370b

Auto assign register

b5f78f5

Use other uav slots

b0e1727

depth test pixel displacement

914e099

adrien-de-tocqueville added the HDRP label May 4, 2020

adrien-de-tocqueville requested review from sebastienlagarde and a team May 4, 2020 12:08

JMargevics approved these changes May 13, 2020

View reviewed changes

Changed sliders range. Updated doc. Fixed viewport clipping for dista…

e0855a0

…nt vertices

Base automatically changed from HDRP/staging to master May 19, 2020 23:34

adrien-de-tocqueville changed the base branch from master to HDRP/staging May 20, 2020 10:55

sebastienlagarde requested a review from JulienIgnace-Unity May 22, 2020 18:04

sebastienlagarde reviewed May 26, 2020

View reviewed changes

...ty.render-pipelines.high-definition/Runtime/RenderPipeline/ShaderPass/ShaderPassForward.hlsl Outdated Show resolved Hide resolved

sebastienlagarde reviewed May 26, 2020

View reviewed changes

Reduced allocated rendertarget count

e8d84bd

Now allocates one fullscreen uav instead of three

sebastienlagarde reviewed May 26, 2020

View reviewed changes

sebastienlagarde approved these changes May 26, 2020

View reviewed changes

FrancescoC-unity reviewed May 27, 2020

View reviewed changes

JulienIgnace-Unity reviewed May 27, 2020

View reviewed changes

com.unity.render-pipelines.high-definition/Runtime/Material/SharedRTManager.cs Outdated Show resolved Hide resolved

Return RTHandle

00180b5

sebastienlagarde approved these changes May 27, 2020

View reviewed changes

Merge branch 'HDRP/staging' into 'HDRP/vertex-density'

696a101

adrien-de-tocqueville added 2 commits June 2, 2020 12:03

Merge branch 'HDRP/staging' into HDRP/vertex-density

60445ea

Vulkan fix test

e7f1e75

adrien-de-tocqueville added 4 commits August 3, 2020 12:39

Merge branch 'HDRP/staging' into HDRP/vertex-density

ee759a1

Disable quad overdraw on metal

7e727d6

Update doc

51c7d29

Workaround for another metal compiler bug

ae3c333

Bug when accessing ShaderVariablesDebugDisplay cbuffer in tessellation stage

sebastienlagarde reviewed Aug 5, 2020

View reviewed changes

com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/ShaderPass/VertMesh.hlsl Outdated Show resolved Hide resolved

adrien-de-tocqueville added 9 commits August 5, 2020 11:21

Try something else

6d67545

At least now it compiles

16f859b

Completely disable these modes on metal

ca89e99

...

fd8689f

Merge branch 'HDRP/staging' into HDRP/vertex-density

fb344c3

Merge branch 'HDRP/staging' into HDRP/vertex-density

0cade0d

Update whats new and upgrade guide

3bf1bb7

rendergraph

8a88832

Enable tests for rendegraph

90d3934

sebastienlagarde reviewed Sep 17, 2020

View reviewed changes

com.unity.render-pipelines.high-definition/Runtime/Debug/DebugDisplay.cs Outdated Show resolved Hide resolved

Remove debug entry

cf4a016

JulienIgnace-Unity reviewed Sep 17, 2020

View reviewed changes

com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs Outdated Show resolved Hide resolved

JulienIgnace-Unity requested changes Sep 17, 2020

View reviewed changes

Fixes for rendergraph

a1c370a

lambda to static; fix with msaa

Merge branch 'HDRP/staging' into HDRP/vertex-density

af69c92

JulienIgnace-Unity approved these changes Sep 21, 2020

View reviewed changes

sebastienlagarde merged commit e8acbb2 into HDRP/staging Sep 21, 2020

sebastienlagarde deleted the HDRP/vertex-density branch September 21, 2020 07:51

sebastienlagarde mentioned this pull request Sep 22, 2020

Merge Hdrp/staging #1917

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vertex density & quad overdraw debug modes #353

Vertex density & quad overdraw debug modes #353

adrien-de-tocqueville commented May 4, 2020 •

edited

JMargevics left a comment

adrien-de-tocqueville commented May 13, 2020 •

edited

sebastienlagarde left a comment

sebastienlagarde May 26, 2020

FrancescoC-unity May 27, 2020

sebastienlagarde May 26, 2020

JulienIgnace-Unity May 27, 2020

sebastienlagarde left a comment

FrancescoC-unity May 27, 2020

sebastienlagarde May 27, 2020

sebastienlagarde commented May 27, 2020

adrien-de-tocqueville commented Jul 27, 2020

sebastienlagarde Sep 17, 2020

adrien-de-tocqueville Sep 17, 2020

JulienIgnace-Unity left a comment

adrien-de-tocqueville commented Sep 17, 2020

JulienIgnace-Unity commented Sep 17, 2020

adrien-de-tocqueville commented Sep 17, 2020 •

edited

PaulDemeulenaere commented Oct 6, 2020 •

edited

Vertex density & quad overdraw debug modes #353

Vertex density & quad overdraw debug modes #353

Conversation

adrien-de-tocqueville commented May 4, 2020 • edited

Purpose of this PR

Testing status

JMargevics left a comment

Choose a reason for hiding this comment

adrien-de-tocqueville commented May 13, 2020 • edited

sebastienlagarde left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienlagarde left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienlagarde commented May 27, 2020

adrien-de-tocqueville commented Jul 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JulienIgnace-Unity left a comment

Choose a reason for hiding this comment

adrien-de-tocqueville commented Sep 17, 2020

JulienIgnace-Unity commented Sep 17, 2020

adrien-de-tocqueville commented Sep 17, 2020 • edited

PaulDemeulenaere commented Oct 6, 2020 • edited

adrien-de-tocqueville commented May 4, 2020 •

edited

adrien-de-tocqueville commented May 13, 2020 •

edited

adrien-de-tocqueville commented Sep 17, 2020 •

edited

PaulDemeulenaere commented Oct 6, 2020 •

edited