-
Notifications
You must be signed in to change notification settings - Fork 855
[URP] Remove unnecessary CopyDepth pass before post processing #6097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -519,6 +519,12 @@ public override void Setup(ScriptableRenderContext context, ref RenderingData re | |||||
createColorTexture = createDepthTexture; | ||||||
} | ||||||
|
||||||
bool requiresDepthCopyPass = !requiresDepthPrepass | ||||||
&& requiresDepthTexture | ||||||
&& createDepthTexture; | ||||||
// Post process passes that require depth texture (Motion Blur / DoF) as shader read resource will need to access depth information from main geometry pass if there is no depth copy | ||||||
// and depth pre-pass enqueued | ||||||
bool postProcessingUseDepthTextureMainPass = !requiresDepthCopyPass && !requiresDepthPrepass && cameraHasPostProcessingWithDepth; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
bool postProcessingUseDepthTExtureMainPass = (!requiresDepthPrepass && requiresDepthTexture && createDepthTexture) && !requiresDepthPrepass && cameraHasPostProcessingWithDepth; |
||||||
// Configure all settings require to start a new camera stack (base camera only) | ||||||
if (cameraData.renderType == CameraRenderType.Base) | ||||||
{ | ||||||
|
@@ -531,7 +537,7 @@ public override void Setup(ScriptableRenderContext context, ref RenderingData re | |||||
|
||||||
// Doesn't create texture for Overlay cameras as they are already overlaying on top of created textures. | ||||||
if (intermediateRenderTexture) | ||||||
CreateCameraRenderTarget(context, ref cameraTargetDescriptor, useDepthPriming); | ||||||
CreateCameraRenderTarget(context, ref cameraTargetDescriptor, useDepthPriming, postProcessingUseDepthTextureMainPass); | ||||||
} | ||||||
else | ||||||
{ | ||||||
|
@@ -541,9 +547,6 @@ public override void Setup(ScriptableRenderContext context, ref RenderingData re | |||||
|
||||||
cameraData.renderer.useDepthPriming = useDepthPriming; | ||||||
|
||||||
bool requiresDepthCopyPass = !requiresDepthPrepass | ||||||
&& (requiresDepthTexture || cameraHasPostProcessingWithDepth) | ||||||
&& createDepthTexture; | ||||||
bool copyColorPass = renderingData.cameraData.requiresOpaqueTexture || renderPassInputs.requiresColorTexture; | ||||||
|
||||||
if ((DebugHandler != null) && DebugHandler.IsActiveForCamera(ref cameraData)) | ||||||
|
@@ -684,13 +687,13 @@ public override void Setup(ScriptableRenderContext context, ref RenderingData re | |||||
#endif | ||||||
|
||||||
// handle multisample depth resolve by setting the appropriate store actions if supported | ||||||
if (requiresDepthCopyPass && cameraTargetDescriptor.msaaSamples > 1 && RenderingUtils.MultisampleDepthResolveSupported(useRenderPassEnabled)) | ||||||
if ((requiresDepthCopyPass || postProcessingUseDepthTextureMainPass) && cameraTargetDescriptor.msaaSamples > 1 && RenderingUtils.MultisampleDepthResolveSupported(useRenderPassEnabled)) | ||||||
{ | ||||||
bool isCopyDepthAfterTransparent = m_CopyDepthPass.renderPassEvent == RenderPassEvent.AfterRenderingTransparents; | ||||||
|
||||||
// we could StoreAndResolve when the depth copy is after opaque, but performance wise doing StoreAndResolve of depth targets is more expensive than a simple Store + following depth copy pass on Apple GPUs, | ||||||
// because of the extra resolve step. So, unless we are copying the depth after the transparent pass, just Store the depth target. | ||||||
if (isCopyDepthAfterTransparent && !copyColorPass) | ||||||
if ((isCopyDepthAfterTransparent || postProcessingUseDepthTextureMainPass) && !copyColorPass) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpicking: you have in multiple places isCopyDEpthAfterTransparent || postProcessingUseDepthTextureMainPass maybe you could give have a variable with some meaning for this to improve readability. The logic of store actions is a little bit convoluted, but I guess the plan is to improve it with RenderGraph anyway, so no big deal. |
||||||
{ | ||||||
if (opaquePassDepthStoreAction == RenderBufferStoreAction.Store) | ||||||
opaquePassDepthStoreAction = RenderBufferStoreAction.StoreAndResolve; | ||||||
|
@@ -762,8 +765,9 @@ public override void Setup(ScriptableRenderContext context, ref RenderingData re | |||||
RenderBufferStoreAction transparentPassColorStoreAction = cameraTargetDescriptor.msaaSamples > 1 && lastCameraInTheStack ? RenderBufferStoreAction.Resolve : RenderBufferStoreAction.Store; | ||||||
RenderBufferStoreAction transparentPassDepthStoreAction = RenderBufferStoreAction.DontCare; | ||||||
|
||||||
// If CopyDepthPass pass event is scheduled on or after AfterRenderingTransparent, we will need to store the depth buffer or resolve (store for now until latest trunk has depth resolve support) it for MSAA case | ||||||
if (requiresDepthCopyPass && m_CopyDepthPass.renderPassEvent >= RenderPassEvent.AfterRenderingTransparents) | ||||||
// If CopyDepthPass pass event is scheduled on or after AfterRenderingTransparent, or if post processing passes will read from main geometry pass' depth texture, | ||||||
// we will need to store the depth buffer or resolve it for MSAA case | ||||||
if (postProcessingUseDepthTextureMainPass || (requiresDepthCopyPass && m_CopyDepthPass.renderPassEvent >= RenderPassEvent.AfterRenderingTransparents)) | ||||||
{ | ||||||
transparentPassDepthStoreAction = RenderBufferStoreAction.Store; | ||||||
|
||||||
|
@@ -796,7 +800,7 @@ public override void Setup(ScriptableRenderContext context, ref RenderingData re | |||||
{ | ||||||
// if resolving to screen we need to be able to perform sRGBConvertion in post-processing if necessary | ||||||
bool doSRGBConvertion = resolvePostProcessingToCameraTarget; | ||||||
postProcessPass.Setup(cameraTargetDescriptor, m_ActiveCameraColorAttachment, resolvePostProcessingToCameraTarget, m_ActiveCameraDepthAttachment, colorGradingLut, applyFinalPostProcessing, doSRGBConvertion); | ||||||
postProcessPass.Setup(cameraTargetDescriptor, m_ActiveCameraColorAttachment, resolvePostProcessingToCameraTarget, postProcessingUseDepthTextureMainPass, m_ActiveCameraDepthAttachment, m_DepthTexture, colorGradingLut, applyFinalPostProcessing, doSRGBConvertion); | ||||||
EnqueuePass(postProcessPass); | ||||||
} | ||||||
|
||||||
|
@@ -850,7 +854,7 @@ public override void Setup(ScriptableRenderContext context, ref RenderingData re | |||||
// stay in RT so we resume rendering on stack after post-processing | ||||||
else if (applyPostProcessing) | ||||||
{ | ||||||
postProcessPass.Setup(cameraTargetDescriptor, m_ActiveCameraColorAttachment, false, m_ActiveCameraDepthAttachment, colorGradingLut, false, false); | ||||||
postProcessPass.Setup(cameraTargetDescriptor, m_ActiveCameraColorAttachment, false, postProcessingUseDepthTextureMainPass, m_ActiveCameraDepthAttachment, m_DepthTexture, colorGradingLut, false, false); | ||||||
EnqueuePass(postProcessPass); | ||||||
} | ||||||
|
||||||
|
@@ -1002,7 +1006,7 @@ private RenderPassInputSummary GetRenderPassInputs(ref RenderingData renderingDa | |||||
return inputSummary; | ||||||
} | ||||||
|
||||||
void CreateCameraRenderTarget(ScriptableRenderContext context, ref RenderTextureDescriptor descriptor, bool primedDepth) | ||||||
void CreateCameraRenderTarget(ScriptableRenderContext context, ref RenderTextureDescriptor descriptor, bool primedDepth, bool postProcessingUseDepthTextureMainPass) | ||||||
{ | ||||||
CommandBuffer cmd = CommandBufferPool.Get(); | ||||||
using (new ProfilingScope(null, Profiling.createCameraRenderTarget)) | ||||||
|
@@ -1035,7 +1039,7 @@ void CreateCameraRenderTarget(ScriptableRenderContext context, ref RenderTexture | |||||
depthDescriptor.autoGenerateMips = false; | ||||||
depthDescriptor.bindMS = depthDescriptor.msaaSamples > 1 && !SystemInfo.supportsMultisampleAutoResolve && (SystemInfo.supportsMultisampledTextures != 0); | ||||||
|
||||||
if (depthDescriptor.msaaSamples > 1 && RenderingUtils.MultisampleDepthResolveSupported(useRenderPassEnabled) && m_CopyDepthMode == CopyDepthMode.AfterTransparents) | ||||||
if (depthDescriptor.msaaSamples > 1 && RenderingUtils.MultisampleDepthResolveSupported(useRenderPassEnabled) && (m_CopyDepthMode == CopyDepthMode.AfterTransparents || postProcessingUseDepthTextureMainPass)) | ||||||
depthDescriptor.bindMS = false; | ||||||
|
||||||
depthDescriptor.colorFormat = RenderTextureFormat.Depth; | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to improve depth binding in URP so we would unify the depth buffer and the shader resource with a single ID, similar to how we handle color? I feel otherwise this is quite confusing for user that would like to do the same thing. I guess this is because the depth resolve issues and the fact that we don't have secondary id?