[URP] DepthCopy texture as a color #5369

jonuuukas · 2021-08-17T14:33:54Z

Purpose of this PR

Why is this PR needed, what hard problem is it solving/fixing?

This PR changes the behaviour of URP's DepthCopy pass to output R32_SFloat texture instead of a native depth texture.
Why? Because Native RenderPasses support only a single depth buffer and this copy buffer usually is the main offender on why RenderPasses get split.
This should also enable some features(f.e. SSAO, Decals and etc.) to be able to utilise RenderPass API as well, as a lot of them rely on additional depth texture. This would also mean that the depth could technically be framebuffer-fetched using this newly changed texture

Testing status

Yamato is as green as it can get (it seems, not sure), but QA pass definitely should be done, as this affects the whole URP. Manually checked all of the URP test suites and ran locally on mac editor.

Comments to reviewers

Notes for the reviewers you have assigned.

…f for that

…ted later on)

…be reverted later on)" This reverts commit 80af9be.

… code

… shaders to accommodate that

…rrect one

…r ConfigureTarget

…is handled

…ecals

github-actions · 2021-08-17T14:34:09Z

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://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

URP
/.yamato%252Fall-urp.yml%2523PR_URP_2021.2

VFX
/.yamato%252Fall-vfx.yml%2523PR_VFX_2021.2

SRP Core
You could run ABV on your branch before merging your PR, but it will start A LOT of jobs. Please be responsible about it and run it only when you feel the PR is ready:
/.yamato%252F_abv.yml%2523all_project_ci_2021.2
Be aware that any modifications to the Core package impacts everyone in the Graphics repo so please discuss the PR with your lead.

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.

github-actions · 2021-08-17T14:34:21Z

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page).
See the PR template for more information.
Thank you!

manuele-bonanno

looks good! Some minor comments

manuele-bonanno · 2021-08-18T07:45:36Z

com.unity.render-pipelines.universal/Runtime/Passes/CopyDepthPass.cs

-            descriptor.depthBufferBits = 32; //TODO: do we really need this. double check;
+            // descriptor.colorFormat = RenderTextureFormat.Depth;
+            descriptor.graphicsFormat = GraphicsFormat.R32_SFloat;
+            descriptor.depthBufferBits = 0; //TODO: do we really need this. double check;


this looks weird, not sure is needed

manuele-bonanno · 2021-08-18T07:46:09Z

com.unity.render-pipelines.universal/Runtime/Passes/CopyDepthPass.cs

@@ -43,14 +43,15 @@ public void Setup(RenderTargetHandle source, RenderTargetHandle destination)
        public override void OnCameraSetup(CommandBuffer cmd, ref RenderingData renderingData)
        {
            var descriptor = renderingData.cameraData.cameraTargetDescriptor;
-            descriptor.colorFormat = RenderTextureFormat.Depth;
-            descriptor.depthBufferBits = 32; //TODO: do we really need this. double check;
+            // descriptor.colorFormat = RenderTextureFormat.Depth;


the commented out line can be removed

manuele-bonanno · 2021-08-18T07:47:29Z

com.unity.render-pipelines.universal/Runtime/Passes/DepthOnlyPass.cs

@@ -31,6 +31,7 @@ public DepthOnlyPass(RenderPassEvent evt, RenderQueueRange renderQueueRange, Lay
            base.profilingSampler = new ProfilingSampler(nameof(DepthOnlyPass));
            m_FilteringSettings = new FilteringSettings(renderQueueRange, layerMask);
            renderPassEvent = evt;
+            useNativeRenderPass = false;


why is this pass not RP compatible anymore?

this actually get's set later as there is a possibility that with depth prepass we are going to have the additional depth as depth. however, if we can use it as a color texture - it will get set to true

…t buffer in shader

phi-lira · 2021-09-21T07:45:25Z

com.unity.render-pipelines.universal/Runtime/Passes/FinalBlitPass.cs

-                        ClearFlag.None,
+                        ClearFlag.Depth,


I don't follow why we are changing this. Could this maybe introduce regression for users that might read depend on depth after final blit pass?

phi-lira · 2021-09-21T07:51:48Z

Adding @PaulDemeulenaere and @thomas-zeng for area review.

manuele-bonanno · 2021-09-23T07:48:25Z

please also update the changelog

thomas-zeng

Approve for the XR related changes. Please address other reviewer's feedback :)

PaulDemeulenaere

Yamato is green on VFX: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/universal%252Fdepth-copy-as-color/.yamato%252Fall-vfx.yml%2523VFX_trunk/9097612/job

PaulDemeulenaere · 2021-10-06T14:38:25Z

com.unity.visualeffectgraph/Shaders/VFXParticleCommon.template

@@ -271,7 +271,7 @@ void frag(ps_input i
 		outSelection = float4(_ObjectId, _PassValue, 1.0, 1.0);
 	#elif VFX_PASSDEPTH == VFX_PASSDEPTH_ACTUAL
 		#ifndef WRITE_MSAA_DEPTH
-			dummy = (float4)0;
+		    dummy = float4(i.VFX_VARYING_POSCS.z, 0,0,0);


I'm not sure why we need to do this change but I think ™️ this value wasn't actually outputted, thus, it should be fine.

This reverts commit a65bb59.

jonuuukas added 14 commits July 29, 2021 11:02

switching _CameraDepthTexture to R32Float and accommodating some stuf…

a2e4eb6

…f for that

some changes to the Foundation Project for Repro project (to be rever…

80af9be

…ted later on)

Revert "some changes to the Foundation Project for Repro project (to …

d1cef12

…be reverted later on)" This reverts commit 80af9be.

disable fbfetch on DbufferDeferred decals

d39e96d

adding IsTransientAttachment to ScriptableRenderPass/NativeRenderPass…

c149ec9

… code

making depthOnly pass compatible to render color texture and changing…

933f089

… shaders to accommodate that

Merge branch 'master' into universal/depth-copy-as-color

624f0a1

fix after updating trunk

d887392

merge fix - change descriptor for DepthOnly configureTarget to the co…

6accb1c

…rrect one

enabling depthbuffer for depth copy and adding additional override fo…

ee6e7d4

…r ConfigureTarget

formatting fixes

399ee75

terrain shader passes depthonly pass fix to output clippos

68d3938

adding depth copy for renderpass as well and changing how fake depth …

4f9cbd7

…is handled

some cleanup and refactoring and enabling Renderpass for SS forward d…

165f4d5

…ecals

jonuuukas requested review from a team as code owners August 17, 2021 14:33

jonuuukas marked this pull request as draft August 17, 2021 14:34

github-actions bot added SRP testing universal labels Aug 17, 2021

manuele-bonanno approved these changes Aug 18, 2021

View reviewed changes

jonuuukas added 2 commits August 19, 2021 15:47

adding _OUTPUT_DEPTH keyword to CopyDepthPass to select correct outpu…

fd4df44

…t buffer in shader

Merge branch 'master' into universal/depth-copy-as-color

2510a18

lukaschod mentioned this pull request Aug 24, 2021

[URP] Decal fixes #5384

Merged

jonuuukas added 2 commits August 24, 2021 11:53

adding depth output for VFXGraph depth only passes

5dec840

changing the semantic for outDepthColor in VFXPasses.template

c4ddd12

phi-lira approved these changes Sep 21, 2021

View reviewed changes

phi-lira requested a review from PaulDemeulenaere September 21, 2021 07:51

phi-lira requested a review from thomas-zeng September 21, 2021 07:51

thomas-zeng approved these changes Oct 5, 2021

View reviewed changes

phi-lira marked this pull request as ready for review October 6, 2021 14:32

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

PaulDemeulenaere approved these changes Oct 6, 2021

View reviewed changes

phi-lira changed the base branch from master to universal/staging October 6, 2021 15:09

phi-lira merged commit a65bb59 into universal/staging Oct 6, 2021

phi-lira deleted the universal/depth-copy-as-color branch October 6, 2021 15:10

phi-lira mentioned this pull request Oct 6, 2021

Universal Staging #5943

Merged

phi-lira added a commit that referenced this pull request Oct 11, 2021

Revert "[URP] DepthCopy texture as a color (#5369)"

e9b4985

This reverts commit a65bb59.

phi-lira restored the universal/depth-copy-as-color branch October 11, 2021 09:57

jonuuukas mentioned this pull request Oct 12, 2021

Universal/depth copy as color #5987

Closed

lukaschod mentioned this pull request Oct 27, 2021

[2022.1][URP] Decal fixes #5961

Merged

lukaschod mentioned this pull request Nov 11, 2021

[2022.1][URP] Decal fixes #6239

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[URP] DepthCopy texture as a color #5369

[URP] DepthCopy texture as a color #5369

Uh oh!

jonuuukas commented Aug 17, 2021 •

edited

Loading

Uh oh!

github-actions bot commented Aug 17, 2021 •

edited

Loading

Uh oh!

github-actions bot commented Aug 17, 2021

Uh oh!

manuele-bonanno left a comment

Uh oh!

manuele-bonanno Aug 18, 2021

Uh oh!

manuele-bonanno Aug 18, 2021

Uh oh!

manuele-bonanno Aug 18, 2021

Uh oh!

jonuuukas Oct 7, 2021

Uh oh!

phi-lira Sep 21, 2021

Uh oh!

phi-lira commented Sep 21, 2021

Uh oh!

manuele-bonanno commented Sep 23, 2021

Uh oh!

thomas-zeng left a comment

Uh oh!

PaulDemeulenaere left a comment

Uh oh!

PaulDemeulenaere Oct 6, 2021 •

edited

Loading

Uh oh!

Uh oh!

[URP] DepthCopy texture as a color #5369

[URP] DepthCopy texture as a color #5369

Uh oh!

Conversation

jonuuukas commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR

Why is this PR needed, what hard problem is it solving/fixing?

Testing status

Yamato is as green as it can get (it seems, not sure), but QA pass definitely should be done, as this affects the whole URP. Manually checked all of the URP test suites and ran locally on mac editor.

Comments to reviewers

Uh oh!

github-actions bot commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 17, 2021

Uh oh!

manuele-bonanno left a comment

Choose a reason for hiding this comment

Uh oh!

manuele-bonanno Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

manuele-bonanno Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

manuele-bonanno Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

jonuuukas Oct 7, 2021

Choose a reason for hiding this comment

Uh oh!

phi-lira Sep 21, 2021

Choose a reason for hiding this comment

Uh oh!

phi-lira commented Sep 21, 2021

Uh oh!

manuele-bonanno commented Sep 23, 2021

Uh oh!

thomas-zeng left a comment

Choose a reason for hiding this comment

Uh oh!

PaulDemeulenaere left a comment

Choose a reason for hiding this comment

Uh oh!

PaulDemeulenaere Oct 6, 2021 • 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!

Uh oh!

jonuuukas commented Aug 17, 2021 •

edited

Loading

github-actions bot commented Aug 17, 2021 •

edited

Loading

PaulDemeulenaere Oct 6, 2021 •

edited

Loading