Merge pull request #6073 from smoogipoo/drawnode-subtree-draw-cleanup

Cleanup `DrawNode` drawing methods

osu.Framework.Tests/Visual/Containers/TestSceneFrontToBack.cs

@@ -113,8 +113,8 @@ protected override void Update()
             if (drawNode != null)
             {
                 labelDrawables.Text = $"boxes: {Cell(1).Children.Count * cell_count:N0}";
-                labelFrag.Text = $"samples ({nameof(DrawNode.Draw)}): {drawNode.DrawSamples:N0}";
-                labelFrag2.Text = $"samples ({nameof(DrawNode.DrawOpaqueInteriorSubTree)}): {drawNode.DrawOpaqueInteriorSubTreeSamples:N0}";
+                labelFrag.Text = $"samples (Draw): {drawNode.DrawSamples:N0}";
+                labelFrag2.Text = $"samples (DrawOpaqueInterior): {drawNode.DrawOpaqueInteriorSubTreeSamples:N0}";
             }
         }
 
@@ -151,16 +151,16 @@ public QueryingCompositeDrawableDrawNode(CompositeDrawable source)
             {
             }
 
-            internal override void DrawOpaqueInteriorSubTree(IRenderer renderer, DepthValue depthValue)
+            protected override void DrawOpaqueInterior(IRenderer renderer)
             {
                 if (renderer is VeldridRenderer)
                 {
-                    base.DrawOpaqueInteriorSubTree(renderer, depthValue);
+                    base.DrawOpaqueInterior(renderer);
                     return;
                 }
 
                 startQuery();
-                base.DrawOpaqueInteriorSubTree(renderer, depthValue);
+                base.DrawOpaqueInterior(renderer);
                 DrawOpaqueInteriorSubTreeSamples = endQuery();
             }
 
@@ -171,7 +171,7 @@ public override void ApplyState()
                 base.ApplyState();
             }
 
-            public override void Draw(IRenderer renderer)
+            protected override void Draw(IRenderer renderer)
             {
                 if (renderer is VeldridRenderer)
                 {

osu.Framework.Tests/Visual/Graphics/TestSceneShaderStorageBufferObject.cs

@@ -102,7 +102,7 @@ public override void ApplyState()
             private IShaderStorageBufferObject<ColourData>? colourBuffer;
             private IVertexBatch<ColourIndexedVertex>? vertices;
 
-            public override void Draw(IRenderer renderer)
+            protected override void Draw(IRenderer renderer)
             {
                 base.Draw(renderer);
 
@@ -192,7 +192,7 @@ public override void ApplyState()
             private ShaderStorageBufferObjectStack<ColourData>? colourBuffer;
             private IVertexBatch<ColourIndexedVertex>? vertices;
 
-            public override void Draw(IRenderer renderer)
+            protected override void Draw(IRenderer renderer)
             {
                 base.Draw(renderer);
 

osu.Framework.Tests/Visual/Graphics/TestSceneStencil.cs

@@ -131,7 +131,7 @@ public StencilDrawNode(IDrawable source)
             {
             }
 
-            public override void Draw(IRenderer renderer)
+            protected override void Draw(IRenderer renderer)
             {
                 base.Draw(renderer);
 
@@ -164,7 +164,7 @@ private void drawStencil(IRenderer renderer, DrawNode drawNode)
                     DestinationAlpha = BlendingType.One,
                 });
 
-                drawNode.Draw(renderer);
+                DrawOther(drawNode, renderer);
 
                 renderer.PopStencilInfo();
                 renderer.SetBlend(DrawColourInfo.Blending);
@@ -182,7 +182,7 @@ private void drawBackground(IRenderer renderer, DrawNode drawNode)
                     passed: StencilOperation.Keep
                 ));
 
-                drawNode.Draw(renderer);
+                DrawOther(drawNode, renderer);
 
                 renderer.PopStencilInfo();
             }

osu.Framework.Tests/Visual/Graphics/TestSceneVertexBatching.cs

@@ -88,7 +88,7 @@ public BoxWithFlushDrawNode(BoxWithFlush source)
                 {
                 }
 
-                public override void Draw(IRenderer renderer)
+                protected override void Draw(IRenderer renderer)
                 {
                     base.Draw(renderer);
                     renderer.FlushCurrentBatch(null);

osu.Framework/Graphics/Audio/WaveformGraph.cs

@@ -317,7 +317,7 @@ public override void ApplyState()
 
             private IVertexBatch<TexturedVertex2D>? vertexBatch;
 
-            public override void Draw(IRenderer renderer)
+            protected override void Draw(IRenderer renderer)
             {
                 base.Draw(renderer);
 

osu.Framework/Graphics/BufferedDrawNode.cs

@@ -80,7 +80,7 @@ public override void ApplyState()
         /// <returns>A version representing this <see cref="DrawNode"/>'s state.</returns>
         protected virtual long GetDrawVersion() => InvalidationID;
 
-        public sealed override void Draw(IRenderer renderer)
+        protected sealed override void Draw(IRenderer renderer)
         {
             if (!SharedData.IsInitialised)
                 SharedData.Initialise(renderer);
@@ -101,7 +101,7 @@ public sealed override void Draw(IRenderer renderer)
                         renderer.PushOrtho(screenSpaceDrawRectangle);
                         renderer.Clear(new ClearInfo(backgroundColour));
 
-                        Child.Draw(renderer);
+                        DrawOther(Child, renderer);
 
                         renderer.PopOrtho();
                     }

osu.Framework/Graphics/Containers/CompositeDrawable_DrawNode.cs

@@ -9,7 +9,6 @@
 using osuTK;
 using osu.Framework.Graphics.Colour;
 using System;
-using System.Runtime.CompilerServices;
 using osu.Framework.Graphics.Effects;
 using osu.Framework.Graphics.Rendering;
 using osu.Framework.Graphics.Rendering.Vertices;
@@ -177,7 +176,7 @@ private void updateQuadBatch(IRenderer renderer)
                     quadBatch = renderer.CreateQuadBatch<TexturedVertex2D>(100, 1000);
             }
 
-            public override void Draw(IRenderer renderer)
+            protected override void Draw(IRenderer renderer)
             {
                 updateQuadBatch(renderer);
 
@@ -201,7 +200,7 @@ public override void Draw(IRenderer renderer)
                 if (Children != null)
                 {
                     for (int i = 0; i < Children.Count; i++)
-                        Children[i].Draw(renderer);
+                        DrawOther(Children[i], renderer);
                 }
 
                 if (maskingInfo != null)
@@ -211,21 +210,11 @@ public override void Draw(IRenderer renderer)
                     renderer.PopQuadBatch();
             }
 
-            internal override void DrawOpaqueInteriorSubTree(IRenderer renderer, DepthValue depthValue)
+            protected override void DrawOpaqueInterior(IRenderer renderer)
             {
-                DrawChildrenOpaqueInteriors(renderer, depthValue);
-                base.DrawOpaqueInteriorSubTree(renderer, depthValue);
-            }
+                base.DrawOpaqueInterior(renderer);
 
-            /// <summary>
-            /// Performs <see cref="DrawOpaqueInteriorSubTree"/> on all children of this <see cref="CompositeDrawableDrawNode"/>.
-            /// </summary>
-            /// <param name="renderer">The renderer to draw with.</param>
-            /// <param name="depthValue">The previous depth value.</param>
-            [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            protected virtual void DrawChildrenOpaqueInteriors(IRenderer renderer, DepthValue depthValue)
-            {
-                bool canIncrement = depthValue.CanIncrement;
+                bool canIncrement = renderer.BackbufferDepth.CanIncrement;
 
                 // Assume that if we can't increment the depth value, no child can, thus nothing will be drawn.
                 if (canIncrement)
@@ -244,7 +233,7 @@ protected virtual void DrawChildrenOpaqueInteriors(IRenderer renderer, DepthValu
                 if (Children != null)
                 {
                     for (int i = Children.Count - 1; i >= 0; i--)
-                        Children[i].DrawOpaqueInteriorSubTree(renderer, depthValue);
+                        DrawOtherOpaqueInterior(Children[i], renderer);
                 }
 
                 // Assume that if we can't increment the depth value, no child can, thus nothing will be drawn.
@@ -258,6 +247,9 @@ protected virtual void DrawChildrenOpaqueInteriors(IRenderer renderer, DepthValu
                 }
             }
 
+            protected internal override bool CanDrawOpaqueInterior => true;
+            protected override bool HasOwnOpaqueInterior => false;
+
             protected override void Dispose(bool isDisposing)
             {
                 base.Dispose(isDisposing);

osu.Framework/Graphics/DrawNode.cs

@@ -40,8 +40,8 @@ public class DrawNode : IDisposable
         private long referenceCount;
 
         /// <summary>
-        /// The depth at which drawing should take place.
-        /// This is written to from the front-to-back pass and used in both passes.
+        /// The depth at which this <see cref="DrawNode"/> is to be drawn.
+        /// Only used if front-to-back is enabled.
         /// </summary>
         private float drawDepth;
 
@@ -74,60 +74,45 @@ public virtual void ApplyState()
         /// Subclasses must invoke <code>base.Draw()</code> prior to drawing vertices.
         /// </remarks>
         /// <param name="renderer">The renderer to draw with.</param>
-        public virtual void Draw(IRenderer renderer)
+        protected virtual void Draw(IRenderer renderer)
         {
             renderer.SetBlend(DrawColourInfo.Blending);
-
-            // This is the back-to-front (BTF) pass. The back-buffer depth test function used is GL_LESS.
-            // The depth test will fail for samples that overlap the opaque interior of this <see cref="DrawNode"/> and any <see cref="DrawNode"/>s above this one.
-            renderer.SetDrawDepth(drawDepth);
+            renderer.BackbufferDepth.Set(drawDepth);
         }
 
         /// <summary>
-        /// Draws the opaque interior of this <see cref="DrawNode"/> and all <see cref="DrawNode"/>s further down the scene graph, invoking <see cref="DrawOpaqueInterior"/> if <see cref="CanDrawOpaqueInterior"/>
-        /// indicates that an opaque interior can be drawn for each relevant <see cref="DrawNode"/>.
+        /// Draws the opaque interior of this <see cref="DrawNode"/> to the screen, if <see cref="CanDrawOpaqueInterior"/> indicates that an opaque interior can be drawn.
+        /// The opaque interior must be a fully-opaque, non-blended area of this <see cref="DrawNode"/>, clipped to the current masking area via <code>DrawClipped()</code>.
+        /// See <see cref="Sprites.SpriteDrawNode"/> for an example implementation.
         /// </summary>
         /// <remarks>
-        /// This is the front-to-back pass. The back-buffer depth test function used is GL_LESS.<br />
-        /// During this pass, the opaque interior is drawn BELOW ourselves. For this to occur, <see cref="drawDepth"/> is temporarily incremented and then decremented after drawing is complete.
-        /// Other <see cref="DrawNode"/>s behind ourselves receive the incremented depth value before doing the same themselves, allowing early-z to take place during this pass.
+        /// Subclasses must invoke <code>base.DrawOpaqueInterior()</code> prior to drawing vertices.
         /// </remarks>
         /// <param name="renderer">The renderer to draw with.</param>
-        /// <param name="depthValue">The previous depth value.</param>
-        internal virtual void DrawOpaqueInteriorSubTree(IRenderer renderer, DepthValue depthValue)
+        protected virtual void DrawOpaqueInterior(IRenderer renderer)
         {
-            if (!depthValue.CanIncrement || !CanDrawOpaqueInterior)
-            {
-                // The back-to-front pass requires the depth value.
-                drawDepth = depthValue;
-                return;
-            }
-
-            // For an incoming depth value D, the opaque interior is drawn at depth D+e and the content is drawn at depth D.
-            // As such, when the GL_LESS test function is applied, the content will always pass the depth test for the same DrawNode (D < D+e).
-
-            // Increment the depth.
-            float previousDepthValue = depthValue;
-            drawDepth = depthValue.Increment();
-
-            DrawOpaqueInterior(renderer);
+            if (HasOwnOpaqueInterior)
+                renderer.BackbufferDepth.Increment();
+        }
 
-            // Decrement the depth.
-            drawDepth = previousDepthValue;
+        protected internal static void DrawOther(DrawNode node, IRenderer renderer)
+        {
+            node.Draw(renderer);
         }
 
         /// <summary>
-        /// Draws the opaque interior of this <see cref="DrawNode"/> to the screen.
-        /// The opaque interior must be a fully-opaque, non-blended area of this <see cref="DrawNode"/>, clipped to the current masking area via <code>DrawClipped()</code>.
-        /// See <see cref="Sprites.SpriteDrawNode"/> for an example implementation.
+        /// Draws the opaque interior of a <see cref="DrawNode"/>.
         /// </summary>
-        /// <remarks>
-        /// Subclasses must invoke <code>base.DrawOpaqueInterior()</code> prior to drawing vertices.
-        /// </remarks>
-        /// <param name="renderer">The renderer to draw with.</param>
-        protected virtual void DrawOpaqueInterior(IRenderer renderer)
+        protected internal static void DrawOtherOpaqueInterior(DrawNode node, IRenderer renderer)
         {
-            renderer.SetDrawDepth(drawDepth);
+            // For an incoming depth value D, the content is to be drawn at depth D and the opaque interior is to be drawn at depth D+e.
+            // When the GL_LESS test function is applied, the content will therefore always pass the depth test for the same DrawNode (D < D+e)
+            node.drawDepth = renderer.BackbufferDepth;
+
+            if (!renderer.BackbufferDepth.CanIncrement || !node.CanDrawOpaqueInterior)
+                return;
+
+            node.DrawOpaqueInterior(renderer);
         }
 
         /// <summary>
@@ -136,6 +121,12 @@ protected virtual void DrawOpaqueInterior(IRenderer renderer)
         /// </summary>
         protected internal virtual bool CanDrawOpaqueInterior => false;
 
+        /// <summary>
+        /// Whether this <see cref="DrawNode"/> has its own opaque interior.
+        /// If <c>false</c>, a child of the <see cref="DrawNode"/> may be the source of the opaque interior.
+        /// </summary>
+        protected virtual bool HasOwnOpaqueInterior => CanDrawOpaqueInterior;
+
         /// <summary>
         /// Increments the reference count of this <see cref="DrawNode"/>, blocking <see cref="Dispose()"/> until the count reaches 0.
         /// Invoke <see cref="Dispose()"/> to remove the reference.

osu.Framework/Graphics/Drawable_ProxyDrawable.cs

@@ -88,11 +88,17 @@ public ProxyDrawNode(ProxyDrawable proxyDrawable)
                 {
                 }
 
-                internal override void DrawOpaqueInteriorSubTree(IRenderer renderer, DepthValue depthValue)
-                    => getCurrentFrameSource()?.DrawOpaqueInteriorSubTree(renderer, depthValue);
+                protected override void DrawOpaqueInterior(IRenderer renderer)
+                {
+                    if (getCurrentFrameSource() != null)
+                        DrawOtherOpaqueInterior(getCurrentFrameSource(), renderer);
+                }
 
-                public override void Draw(IRenderer renderer)
-                    => getCurrentFrameSource()?.Draw(renderer);
+                protected override void Draw(IRenderer renderer)
+                {
+                    if (getCurrentFrameSource() != null)
+                        DrawOther(getCurrentFrameSource(), renderer);
+                }
 
                 protected internal override bool CanDrawOpaqueInterior => getCurrentFrameSource()?.CanDrawOpaqueInterior ?? false;
 

osu.Framework/Graphics/Lines/Path_DrawNode.cs

@@ -50,7 +50,7 @@ public override void ApplyState()
                 pathShader = Source.pathShader;
             }
 
-            public override void Draw(IRenderer renderer)
+            protected override void Draw(IRenderer renderer)
             {
                 base.Draw(renderer);
 

osu.Framework/Graphics/Rendering/DepthValue.cs

@@ -55,6 +55,16 @@ internal void Reset()
             count = 0;
         }
 
+        /// <summary>
+        /// Sets the depth to a given value. After this, the depth value can no longer be incremented.
+        /// </summary>
+        /// <param name="value">The new depth value.</param>
+        public void Set(float value)
+        {
+            count = max_count;
+            depth = value;
+        }
+
         /// <summary>
         /// Whether the depth value can be incremented.
         /// </summary>

osu.Framework/Graphics/Rendering/Dummy/DummyRenderer.cs

@@ -41,9 +41,9 @@ public sealed class DummyRenderer : IRenderer
         public WrapMode CurrentWrapModeS => WrapMode.None;
         public WrapMode CurrentWrapModeT => WrapMode.None;
         public bool IsMaskingActive => false;
-        public float BackbufferDrawDepth => 0;
         public bool UsingBackbuffer => false;
         public Texture WhitePixel { get; }
+        DepthValue IRenderer.BackbufferDepth { get; } = new DepthValue();
 
         public bool IsInitialised { get; private set; }
 
@@ -220,10 +220,6 @@ void IRenderer.SetUniform<T>(IUniformWithValue<T> uniform)
         {
         }
 
-        void IRenderer.SetDrawDepth(float drawDepth)
-        {
-        }
-
         IVertexBatch<TexturedVertex2D> IRenderer.DefaultQuadBatch => new DummyVertexBatch<TexturedVertex2D>();
 
         void IRenderer.PushQuadBatch(IVertexBatch<TexturedVertex2D> quadBatch)