-
Notifications
You must be signed in to change notification settings - Fork 858
[Universal] MSAA store actions optimizations for forward renderer Drawobjects pass #4161
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 24 commits
b4b6edf
5c53798
6233c18
1a77378
03504e3
a6f59ed
af2ecba
1a9ce7f
1ee51a6
8e95a89
f2e7ec3
cc6422b
0b2d3a1
701f703
f07f748
de91e70
060261b
cd25137
1bd8a24
4e300f6
9761545
887eb25
6b1315f
facbf42
507291d
71e9011
b46f66a
23694e6
39b1394
497777f
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 |
|---|---|---|
|
|
@@ -150,6 +150,16 @@ public RenderTargetIdentifier depthAttachment | |
| get => m_DepthAttachment; | ||
| } | ||
|
|
||
| public RenderBufferStoreAction[] colorStoreActions | ||
| { | ||
| get => m_ColorStoreActions; | ||
| } | ||
|
|
||
| public RenderBufferStoreAction depthStoreAction | ||
| { | ||
| get => m_DepthStoreAction; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// The input requirements for the <c>ScriptableRenderPass</c>, which has been set using <c>ConfigureInput</c> | ||
| /// </summary> | ||
|
|
@@ -169,6 +179,9 @@ public Color clearColor | |
| get => m_ClearColor; | ||
| } | ||
|
|
||
| RenderBufferStoreAction[] m_ColorStoreActions = new RenderBufferStoreAction[] { RenderBufferStoreAction.Store }; | ||
| RenderBufferStoreAction m_DepthStoreAction = RenderBufferStoreAction.Store; | ||
|
Comment on lines
+183
to
+184
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. I wonder why not backing the entire attachemnt setup with something like https://docs.unity3d.com/ScriptReference/Rendering.RenderTargetBinding.html I also find it weird that we provide StoreActions but don't expose LoadAction. If we don't think in terms of API completeness I feel like very soon someone might ask for Load actions as well. 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. in engine load actions are inferred based on the store actions setup, and this change or every API implementation. The only proper way right now to set load/store actions without having to fight built-in logic is to use the RenderPass API. So while exposing the load actions here is easy, I am not sure they will have much effect in terms of determining the final store action used by the device. So there would be a lot of work needed to allow that. If it is important can be done as a future task, but from my experience the engine does a good job in figuring out the load actions and the main problems were introduced by wrong stores, so not sure the effort to have an explicit Load action API is worth it (also considering RenderPasses will do that) I agree having only stores exposed in the API makes it feel limited, but if we want load actions too then it might involve lot of extra work. Future PRs could address it if really needed. WDYT? 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. Ok. sounds good to me and fair justification. |
||
|
|
||
| /// <summary> | ||
| /// A ProfilingSampler for the entire render pass. Used as a profiling name by <c>ScriptableRenderer</c> when executing the pass. | ||
| /// Default is <c>Unnamed_ScriptableRenderPass</c>. | ||
|
|
@@ -201,6 +214,8 @@ public ScriptableRenderPass() | |
| renderPassEvent = RenderPassEvent.AfterRenderingOpaques; | ||
| m_ColorAttachments = new RenderTargetIdentifier[] {BuiltinRenderTextureType.CameraTarget, 0, 0, 0, 0, 0, 0, 0}; | ||
| m_DepthAttachment = BuiltinRenderTextureType.CameraTarget; | ||
| m_ColorStoreActions = new RenderBufferStoreAction[] { RenderBufferStoreAction.Store, 0, 0, 0, 0, 0, 0, 0 }; | ||
| m_DepthStoreAction = RenderBufferStoreAction.Store; | ||
| m_ClearFlag = ClearFlag.None; | ||
| m_ClearColor = Color.black; | ||
| overrideCameraTarget = false; | ||
|
|
@@ -229,6 +244,25 @@ public void ConfigureInput(ScriptableRenderPassInput passInput) | |
| m_Input = passInput; | ||
| } | ||
|
|
||
| public void ConfigureColorStoreAction(RenderBufferStoreAction storeAction, uint attachmentIndex = 0) | ||
| { | ||
| m_ColorStoreActions[attachmentIndex] = storeAction; | ||
| } | ||
|
|
||
| public void ConfigureColorStoreActions(RenderBufferStoreAction[] storeActions) | ||
|
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. Missing public API |
||
| { | ||
| int count = Math.Min(storeActions.Length, m_ColorStoreActions.Length); | ||
| for (uint i = 0; i < count; ++i) | ||
| { | ||
| m_ColorStoreActions[i] = storeActions[i]; | ||
| } | ||
| } | ||
|
|
||
| public void ConfigureDepthStoreAction(RenderBufferStoreAction storeAction) | ||
| { | ||
| m_DepthStoreAction = storeAction; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Configures render targets for this render pass. Call this instead of CommandBuffer.SetRenderTarget. | ||
| /// This method should be called inside Configure. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -402,6 +402,9 @@ static class RenderPassBlock | |
| public static readonly int AfterRendering = 3; | ||
| } | ||
|
|
||
| private StoreActionsOptimization m_StoreActionsOptimizationSetting = StoreActionsOptimization.Auto; | ||
| private static bool m_UseOptimizedStoreActions = false; | ||
|
|
||
| const int k_RenderPassBlockCount = 4; | ||
|
|
||
| List<ScriptableRenderPass> m_ActiveRenderPassQueue = new List<ScriptableRenderPass>(32); | ||
|
|
@@ -423,6 +426,14 @@ static class RenderPassBlock | |
| static RenderTargetIdentifier[] m_ActiveColorAttachments = new RenderTargetIdentifier[] {0, 0, 0, 0, 0, 0, 0, 0 }; | ||
| static RenderTargetIdentifier m_ActiveDepthAttachment; | ||
|
|
||
| private static RenderBufferStoreAction[] m_ActiveColorStoreActions = new RenderBufferStoreAction[] | ||
| { | ||
| RenderBufferStoreAction.Store, RenderBufferStoreAction.Store, RenderBufferStoreAction.Store, RenderBufferStoreAction.Store, | ||
| RenderBufferStoreAction.Store, RenderBufferStoreAction.Store, RenderBufferStoreAction.Store, RenderBufferStoreAction.Store | ||
| }; | ||
|
|
||
| private static RenderBufferStoreAction m_ActiveDepthStoreAction = RenderBufferStoreAction.Store; | ||
|
|
||
| static AttachmentDescriptor[] m_ActiveColorAttachmentDescriptors = new AttachmentDescriptor[] | ||
| { | ||
| RenderingUtils.emptyAttachment, RenderingUtils.emptyAttachment, RenderingUtils.emptyAttachment, | ||
|
|
@@ -477,6 +488,9 @@ public ScriptableRenderer(ScriptableRendererData data) | |
| useRenderPassEnabled = data.useNativeRenderPass; | ||
| Clear(CameraRenderType.Base); | ||
| m_ActiveRenderPassQueue.Clear(); | ||
|
|
||
| m_StoreActionsOptimizationSetting = UniversalRenderPipeline.asset.storeActionsOptimization; | ||
| m_UseOptimizedStoreActions = m_StoreActionsOptimizationSetting != StoreActionsOptimization.Store; | ||
|
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. StoreActionsOptimization.Store should also remove store optimization from other passes like post-processing imo. 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. not sure I understand what you mean? 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. PostProcessingPass.cs has some optimizations to load and store with SetRenderTarget. F.ex, I think we never store depth after post but some users asked for it. I was asking if these settings should affect those optimizations we already have there. 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. yes that sounds like a good idea. This PR is focusing on the opaque/transparent passes on top of introducing the new API. But extending the optimization options to other passes in future PRs sounds trivial (should be a matter of making sure that the new explicit actions API is called by the postproc pass) |
||
| } | ||
|
|
||
| public void Dispose() | ||
|
|
@@ -802,6 +816,10 @@ protected void AddRenderPasses(ref RenderingData renderingData) | |
| if (activeRenderPassQueue[i] == null) | ||
| activeRenderPassQueue.RemoveAt(i); | ||
| } | ||
|
|
||
| // if any pass was injected, the "automatic" store optimization policy will disable the optimized load actions | ||
| if (count > 0 && m_StoreActionsOptimizationSetting == StoreActionsOptimization.Auto) | ||
| m_UseOptimizedStoreActions = false; | ||
| } | ||
|
|
||
| void ClearRenderingState(CommandBuffer cmd) | ||
|
|
@@ -1193,9 +1211,10 @@ void SetRenderPassAttachments(CommandBuffer cmd, ScriptableRenderPass renderPass | |
| else | ||
| { | ||
| // Only setup render target if current render pass attachments are different from the active ones | ||
manuele-bonanno marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (passColorAttachment != m_ActiveColorAttachments[0] || passDepthAttachment != m_ActiveDepthAttachment || finalClearFlag != ClearFlag.None) | ||
| if (passColorAttachment != m_ActiveColorAttachments[0] || passDepthAttachment != m_ActiveDepthAttachment || finalClearFlag != ClearFlag.None || | ||
| renderPass.colorStoreActions[0] != m_ActiveColorStoreActions[0] || renderPass.depthStoreAction != m_ActiveDepthStoreAction) | ||
| { | ||
| SetRenderTarget(cmd, passColorAttachment, passDepthAttachment, finalClearFlag, finalClearColor); | ||
| SetRenderTarget(cmd, passColorAttachment, passDepthAttachment, finalClearFlag, finalClearColor, renderPass.colorStoreActions[0], renderPass.depthStoreAction); | ||
|
|
||
| #if ENABLE_VR && ENABLE_XR_MODULE | ||
| if (cameraData.xr.enabled) | ||
|
|
@@ -1243,6 +1262,11 @@ internal static void SetRenderTarget(CommandBuffer cmd, RenderTargetIdentifier c | |
| for (int i = 1; i < m_ActiveColorAttachments.Length; ++i) | ||
| m_ActiveColorAttachments[i] = 0; | ||
|
|
||
| m_ActiveColorStoreActions[0] = RenderBufferStoreAction.Store; | ||
| m_ActiveDepthStoreAction = RenderBufferStoreAction.Store; | ||
| for (int i = 1; i < m_ActiveColorStoreActions.Length; ++i) | ||
| m_ActiveColorStoreActions[i] = RenderBufferStoreAction.Store; | ||
|
|
||
| m_ActiveDepthAttachment = depthAttachment; | ||
|
|
||
| RenderBufferLoadAction colorLoadAction = ((uint)clearFlag & (uint)ClearFlag.Color) != 0 ? | ||
|
|
@@ -1255,6 +1279,39 @@ internal static void SetRenderTarget(CommandBuffer cmd, RenderTargetIdentifier c | |
| depthAttachment, depthLoadAction, RenderBufferStoreAction.Store, clearFlag, clearColor); | ||
| } | ||
|
|
||
| internal static void SetRenderTarget(CommandBuffer cmd, RenderTargetIdentifier colorAttachment, RenderTargetIdentifier depthAttachment, ClearFlag clearFlag, Color clearColor, RenderBufferStoreAction colorStoreAction, RenderBufferStoreAction depthStoreAction) | ||
| { | ||
| m_ActiveColorAttachments[0] = colorAttachment; | ||
| for (int i = 1; i < m_ActiveColorAttachments.Length; ++i) | ||
| m_ActiveColorAttachments[i] = 0; | ||
|
|
||
| m_ActiveColorStoreActions[0] = colorStoreAction; | ||
| m_ActiveDepthStoreAction = depthStoreAction; | ||
| for (int i = 1; i < m_ActiveColorStoreActions.Length; ++i) | ||
| m_ActiveColorStoreActions[i] = RenderBufferStoreAction.Store; | ||
|
Comment on lines
+1317
to
+1324
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. Hooking up on the previous idea, if we used RenderTargetBinding we could configure it here once. |
||
|
|
||
| m_ActiveDepthAttachment = depthAttachment; | ||
|
|
||
| RenderBufferLoadAction colorLoadAction = ((uint)clearFlag & (uint)ClearFlag.Color) != 0 ? | ||
| RenderBufferLoadAction.DontCare : RenderBufferLoadAction.Load; | ||
|
|
||
| RenderBufferLoadAction depthLoadAction = ((uint)clearFlag & (uint)ClearFlag.Depth) != 0 ? | ||
| RenderBufferLoadAction.DontCare : RenderBufferLoadAction.Load; | ||
|
|
||
| // if we shouldn't use optimized store actions then fall back to the conservative safe (un-optimal!) route and just store everything | ||
| if (!m_UseOptimizedStoreActions) | ||
| { | ||
| if (colorStoreAction != RenderBufferStoreAction.StoreAndResolve) | ||
| colorStoreAction = RenderBufferStoreAction.Store; | ||
| if (depthStoreAction != RenderBufferStoreAction.StoreAndResolve) | ||
| depthStoreAction = RenderBufferStoreAction.Store; | ||
| } | ||
|
|
||
|
|
||
| SetRenderTarget(cmd, colorAttachment, colorLoadAction, colorStoreAction, | ||
| depthAttachment, depthLoadAction, depthStoreAction, clearFlag, clearColor); | ||
| } | ||
|
|
||
| static void SetRenderTarget( | ||
| CommandBuffer cmd, | ||
| RenderTargetIdentifier colorAttachment, | ||
|
|
@@ -1280,7 +1337,7 @@ static void SetRenderTarget( | |
| // XRTODO: Revisit the logic. Why treat CameraTarget depth specially? | ||
| if (depthAttachment == BuiltinRenderTextureType.CameraTarget) | ||
| { | ||
| SetRenderTarget(cmd, colorAttachment, colorLoadAction, colorStoreAction, clearFlags, clearColor); | ||
| CoreUtils.SetRenderTarget(cmd, colorAttachment, colorLoadAction, colorStoreAction, depthLoadAction, depthStoreAction, clearFlags, clearColor); | ||
| } | ||
| else | ||
| { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.