Skip to content

Arm: Optimize away unnecessary clear empty RenderPass (Vulkan) in 2DRP. #4149

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public Render2DLightingPass(Renderer2DData rendererData, Material blitMaterial,
m_Renderer2DData = rendererData;
m_BlitMaterial = blitMaterial;
m_SamplingMaterial = samplingMaterial;
overrideCameraClear = true;

m_CameraSortingLayerBoundsIndex = GetCameraSortingLayerBoundsIndex();
}
Expand Down Expand Up @@ -235,12 +236,18 @@ private int DrawLayerBatches(
var blendStylesCount = m_Renderer2DData.lightBlendStyles.Length;
using (new ProfilingScope(cmd, m_ProfilingDrawRenderers))
{
// Clear the target only when the first layer is rendered and this is the base camera (not camera overlay)
bool needsClear = ((startIndex == 0) && (renderingData.cameraData.renderType == CameraRenderType.Base));
RenderBufferStoreAction initialStoreAction;
if (msaaEnabled)
initialStoreAction = resolveDuringBatch < startIndex ? RenderBufferStoreAction.Resolve : RenderBufferStoreAction.StoreAndResolve;
else
initialStoreAction = RenderBufferStoreAction.Store;
cmd.SetRenderTarget(colorAttachment, RenderBufferLoadAction.Load, initialStoreAction, depthAttachment, RenderBufferLoadAction.Load, initialStoreAction);
cmd.SetRenderTarget(colorAttachment, RenderBufferLoadAction.DontCare, initialStoreAction, depthAttachment, RenderBufferLoadAction.DontCare, initialStoreAction);
if (needsClear)
{
CoreUtils.ClearRenderTarget(cmd, ClearFlag.All, CoreUtils.ConvertSRGBToActiveColorSpace(renderingData.cameraData.camera.backgroundColor));
}

for (var i = startIndex; i < startIndex + batchesDrawn; i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ public Color clearColor
/// </summary>
protected internal ProfilingSampler profilingSampler { get; set; }
internal bool overrideCameraTarget { get; set; }
// Use this flag to override the clear logic in ScriptableRenderer and assume responsibility of manually clearing the pass's targets (Color & Depth).
internal bool overrideCameraClear { get; set; }
internal bool isBlitRenderPass { get; set; }

internal bool useNativeRenderPass { get; set; }
Expand Down Expand Up @@ -253,6 +255,7 @@ public ScriptableRenderPass()
m_ClearFlag = ClearFlag.None;
m_ClearColor = Color.black;
overrideCameraTarget = false;
overrideCameraClear = false;
isBlitRenderPass = false;
profilingSampler = new ProfilingSampler($"Unnamed_{nameof(ScriptableRenderPass)}");
useNativeRenderPass = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ void ExecuteRenderPass(ScriptableRenderContext context, ScriptableRenderPass ren
void SetRenderPassAttachments(CommandBuffer cmd, ScriptableRenderPass renderPass, ref CameraData cameraData)
{
Camera camera = cameraData.camera;
ClearFlag cameraClearFlag = GetCameraClearFlag(ref cameraData);
ClearFlag cameraClearFlag = renderPass.overrideCameraClear ? ClearFlag.None : GetCameraClearFlag(ref cameraData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in 2D renderer you never care about camera clear flags? In this case I’d rather suggest to add logic for 2D renderer to override functionality of GetCameraClearFlags instead of adding internal API to ScriptableRenderPass.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, any feedback on the injected passes issue?. As it stands, either with the current approach or if I override the camera clear flags to NONE in 2DRenderer, an earlier pass happening before Render2DLightingPass would render to a non-cleared camera RT and we would later clear such target overriding whatever was in there. I think the way to go here is to check whether there are ScriptableRenderFeatures queued and decide whether to allow clearing or not based on that. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// Invalid configuration - use current attachment setup
// Note: we only check color buffers. This is only technically correct because for shadowmaps and depth only passes
Expand Down