Refactored NeedsDraw and SubViewNeedsDraw logic to improve clarity and control over redraw state. Introduced SetSubViewNeedsDrawDownHierarchy for better propagation of redraw flags. Updated Margin and Adornment classes to align with the new redraw management.

Enhanced `View` class drawing logic to ensure proper ordering of margin and subview rendering, and introduced `DoDrawContent` for encapsulated content drawing. Improved comments and documentation for better maintainability.

Updated tests to reflect the new redraw management methods. Made minor formatting changes and removed redundant code for consistency and readability.

Examples/UICatalog/Scenarios/CombiningMarks.cs

@@ -10,11 +10,8 @@ public class CombiningMarks : Scenario
         Application.Init ();
         var top = new Runnable ();
 
-        top.DrawComplete += (s, e) =>
+        top.DrawingContent += (s, e) =>
         {
-            // Forces reset _lineColsOffset because we're dealing with direct draw
-            Application.TopRunnableView!.SetNeedsDraw ();
-
             var i = -1;
             top.Move (0, ++i);
             top.AddStr ("Terminal.Gui supports all combining sequences that can be rendered as an unique grapheme.");

Terminal.Gui/ViewBase/Adornment/Adornment.cs

@@ -88,7 +88,7 @@ public class Adornment : View, IDesignable
     protected override IApplication? GetApp () => Parent?.App;
 
     /// <inheritdoc />
-    protected override IDriver? GetDriver () => Parent?.Driver ?? base.GetDriver();
+    protected override IDriver? GetDriver () => Parent?.Driver ?? base.GetDriver ();
 
     // If a scheme is explicitly set, use that. Otherwise, use the scheme of the parent view.
     private Scheme? _scheme;
@@ -187,7 +187,7 @@ public class Adornment : View, IDesignable
             Thickness.Draw (Driver, ViewportToScreen (Viewport), Diagnostics, ToString ());
         }
 
-        NeedsDraw = true;
+        SetNeedsDraw ();
 
         return true;
     }

Terminal.Gui/ViewBase/Adornment/Margin.cs

@@ -79,7 +79,7 @@ public class Margin : Adornment
 
             if (view.Margin is { } margin && margin.Thickness != Thickness.Empty && margin.GetCachedClip () != null)
             {
-                margin.NeedsDraw = true;
+                margin.SetNeedsDraw ();
                 Region? saved = view.GetClip ();
                 view.SetClip (margin.GetCachedClip ());
                 margin.Draw ();
@@ -88,7 +88,7 @@ public class Margin : Adornment
             }
 
             Debug.Assert (view.NeedsDraw == false);
-            view.NeedsDraw = false;
+            view.ClearNeedsDraw ();
 
             foreach (var subview in view.SubViews)
             {

Terminal.Gui/ViewBase/View.Drawing.cs

@@ -28,17 +28,33 @@ public partial class View // Drawing APIs
             view.Draw (context);
         }
 
-        // Draw the margins (those with Shadows) last to ensure they are drawn on top of the content.
+        // Draw the margins last to ensure they are drawn on top of the content.
         Margin.DrawMargins (viewsArray);
 
         // DrawMargins may have caused some views have NeedsDraw/NeedsSubViewDraw set; clear them all.
         foreach (View view in viewsArray)
         {
             view.ClearNeedsDraw ();
-            // ClearNeedsDraw does not clear view.SuperView.SubViewsNeedDraw, so we have to do it here
+        }
-            if (view.SuperView is { })
+
+        // After all peer views have been drawn and cleared, we can now clear the SuperView's SubViewNeedsDraw flag.
+        // ClearNeedsDraw() does not clear SuperView.SubViewNeedsDraw (by design, to avoid premature clearing
+        // when siblings still need drawing), so we must do it here after ALL peers are processed.
+        // We only clear the flag if ALL the SuperView's subviews no longer need drawing.
+        View? lastSuperView = null;
+        foreach (View view in viewsArray)
+        {
+            if (view is not Adornment && view.SuperView is { } && view.SuperView != lastSuperView)
             {
-                view.SuperView.SubViewNeedsDraw = false;
+                // Check if ANY subview of this SuperView still needs drawing
+                bool anySubViewNeedsDrawing = view.SuperView.InternalSubViews.Any (v => v.NeedsDraw || v.SubViewNeedsDraw);
+
+                if (!anySubViewNeedsDrawing)
+                {
+                    view.SuperView.SubViewNeedsDraw = false;
+                }
+
+                lastSuperView = view.SuperView;
             }
         }
     }
@@ -216,7 +232,7 @@ public partial class View // Drawing APIs
         {
             Margin.NeedsLayout = false;
             Margin?.Thickness.Draw (Driver, FrameToScreen ());
-            Margin?.Parent?.SetSubViewNeedsDraw ();
+            Margin?.Parent?.SetSubViewNeedsDrawDownHierarchy ();
         }
 
         if (SubViewNeedsDraw)
@@ -466,7 +482,7 @@ public partial class View // Drawing APIs
         }
 
         // We assume that the text has been drawn over the entire area; ensure that the subviews are redrawn.
-        SetSubViewNeedsDraw ();
+        SetSubViewNeedsDrawDownHierarchy ();
     }
 
     /// <summary>
@@ -478,6 +494,7 @@ public partial class View // Drawing APIs
     public event EventHandler? DrewText;
 
     #endregion DrawText
+
     #region DrawContent
 
     private void DoDrawContent (DrawContext? context = null)
@@ -599,7 +616,7 @@ public partial class View // Drawing APIs
             // TODO: HACK - This forcing of SetNeedsDraw with SuperViewRendersLineCanvas enables auto line join to work, but is brute force.
             if (view.SuperViewRendersLineCanvas || view.ViewportSettings.HasFlag (ViewportSettingsFlags.Transparent))
             {
-               //view.SetNeedsDraw ();
+                //view.SetNeedsDraw ();
             }
             view.Draw (context);
 

Terminal.Gui/ViewBase/View.NeedsDraw.cs

@@ -2,18 +2,18 @@
 
 public partial class View
 {
-    // TODO: Change NeedsDraw to use a Region instead of Rectangle
+    // NOTE: NeedsDrawRect is not currently used to clip drawing to only the invalidated region.
-    // TODO: Make _needsDrawRect nullable instead of relying on Empty
+    //       It is only used within SetNeedsDraw to propagate redraw requests to subviews.
-    //      TODO: If null, it means ?
+    // NOTE: Consider changing NeedsDrawRect from Rectangle to Region for more precise invalidation
-    //      TODO: If Empty, it means no need to redraw
+    //       NeedsDraw is already efficiently cached via NeedsDrawRect. It checks:
-    //      TODO: If not Empty, it means the region that needs to be redrawn
+    //       1. NeedsDrawRect (cached by SetNeedsDraw/ClearNeedsDraw)
-
+    //       2. Adornment NeedsDraw flags (each cached separately)
     /// <summary>
-    ///     The viewport-relative region that needs to be redrawn. Marked internal for unit tests.
+    ///     INTERNAL: Gets the viewport-relative region that needs to be redrawn.
     /// </summary>
-    internal Rectangle NeedsDrawRect { get; set; } = Rectangle.Empty;
+    internal Rectangle NeedsDrawRect { get; private set; } = Rectangle.Empty;
 
-    /// <summary>Gets or sets whether the view needs to be redrawn.</summary>
+    /// <summary>Gets whether the view needs to be redrawn.</summary>
     /// <remarks>
     ///     <para>
     ///         Will be <see langword="true"/> if the <see cref="NeedsLayout"/> property is <see langword="true"/> or if
@@ -23,34 +23,11 @@ public partial class View
     ///         Setting has no effect on <see cref="NeedsLayout"/>.
     ///     </para>
     /// </remarks>
-    public bool NeedsDraw
+    public bool NeedsDraw => Visible && (NeedsDrawRect != Rectangle.Empty || Margin?.NeedsDraw == true || Border?.NeedsDraw == true || Padding?.NeedsDraw == true);
-    {
-        get => Visible && (NeedsDrawRect != Rectangle.Empty || Margin?.NeedsDraw == true || Border?.NeedsDraw == true || Padding?.NeedsDraw == true);
-        set
-        {
-            if (value)
-            {
-                SetNeedsDraw ();
-            }
-            else
-            {
-                ClearNeedsDraw ();
-            }
-        }
-    }
 
-    // TODO: This property is decoupled from the actual state of the subviews (and adornments)
+    /// <summary>Sets <see cref="NeedsDraw"/> to <see langword="true"/> indicating the <see cref="Viewport"/> of this View needs to be redrawn.</summary>
-    // TODO: It is a 'cache' that is set when any subview or adornment requests a redraw
-    // TODO: As a result the code is fragile and can get out of sync. 
-    // TODO: Consider making this a computed property that checks all subviews and adornments for their NeedsDraw state
-    // TODO: But that may have performance implications.
-
-    /// <summary>Gets whether any SubViews need to be redrawn.</summary>
-    public bool SubViewNeedsDraw { get; private set; }
-
-    /// <summary>Sets that the <see cref="Viewport"/> of this View needs to be redrawn.</summary>
     /// <remarks>
-    ///     If the view has not been initialized (<see cref="IsInitialized"/> is <see langword="false"/>), this method
+    ///     If the view is not visible (<see cref="Visible"/> is <see langword="false"/>), this method
     ///     does nothing.
     /// </remarks>
     public void SetNeedsDraw ()
@@ -109,14 +86,13 @@ public partial class View
             Padding?.SetNeedsDraw ();
         }
 
-        SuperView?.SetSubViewNeedsDraw ();
+        SuperView?.SetSubViewNeedsDrawDownHierarchy ();
 
         if (this is Adornment adornment)
         {
-            adornment.Parent?.SetSubViewNeedsDraw ();
+            adornment.Parent?.SetSubViewNeedsDrawDownHierarchy ();
         }
 
-        // There was multiple enumeration error here, so calling new snapshot collection - probably a stop gap
         foreach (View subview in InternalSubViews.Snapshot ())
         {
             if (subview.Frame.IntersectsWith (viewPortRelativeRegion))
@@ -129,8 +105,51 @@ public partial class View
         }
     }
 
-    /// <summary>Sets <see cref="SubViewNeedsDraw"/> to <see langword="true"/> for this View and all Superviews.</summary>
+    /// <summary>INTERNAL: Clears <see cref="NeedsDraw"/> and <see cref="SubViewNeedsDraw"/> for this view and all SubViews.</summary>
-    public void SetSubViewNeedsDraw ()
+    /// <remarks>
+    ///     See <see cref="SubViewNeedsDraw"/> is a cached value that is set when any subview or adornment requests a redraw.
+    ///     It may not always be in sync with the actual state of the subviews.
+    /// </remarks>
+    internal void ClearNeedsDraw ()
+    {
+        NeedsDrawRect = Rectangle.Empty;
+
+        Margin?.ClearNeedsDraw ();
+        Border?.ClearNeedsDraw ();
+        Padding?.ClearNeedsDraw ();
+
+        foreach (View subview in InternalSubViews.Snapshot ())
+        {
+            subview.ClearNeedsDraw ();
+        }
+
+        SubViewNeedsDraw = false;
+
+        // This ensures LineCanvas' get redrawn
+        if (!SuperViewRendersLineCanvas)
+        {
+            LineCanvas.Clear ();
+        }
+    }
+
+    // NOTE: SubViewNeedsDraw is decoupled from the actual state of the subviews (and adornments).
+    //       It is a performance optimization to avoid having to traverse all subviews and adornments to check if any need redraw.
+    //       As a result the code is fragile and can get out of sync; care must be taken to ensure it is set and cleared correctly.
+    /// <summary>
+    ///     INTERNAL: Gets whether any SubViews need to be redrawn.
+    /// </summary>
+    /// <remarks>
+    ///     See <see cref="SubViewNeedsDraw"/> is a cached value that is set when any subview or adornment requests a redraw.
+    ///     It may not always be in sync with the actual state of the subviews.
+    /// </remarks>
+    internal bool SubViewNeedsDraw { get; private set; }
+
+    /// <summary>INTERNAL: Sets <see cref="SubViewNeedsDraw"/> to <see langword="true"/> for this View and all Superviews.</summary>
+    /// <remarks>
+    ///     See <see cref="SubViewNeedsDraw"/> is a cached value that is set when any subview or adornment requests a redraw.
+    ///     It may not always be in sync with the actual state of the subviews.
+    /// </remarks>
+    internal void SetSubViewNeedsDrawDownHierarchy ()
     {
         if (!Visible)
         {
@@ -141,51 +160,12 @@ public partial class View
 
         if (this is Adornment adornment)
         {
-            adornment.Parent?.SetSubViewNeedsDraw ();
+            adornment.Parent?.SetSubViewNeedsDrawDownHierarchy ();
         }
 
         if (SuperView is { SubViewNeedsDraw: false })
         {
-            SuperView.SetSubViewNeedsDraw ();
+            SuperView.SetSubViewNeedsDrawDownHierarchy ();
-        }
-    }
-
-    /// <summary>Clears <see cref="NeedsDraw"/> and <see cref="SubViewNeedsDraw"/>.</summary>
-    protected void ClearNeedsDraw ()
-    {
-        NeedsDrawRect = Rectangle.Empty;
-
-        Margin?.ClearNeedsDraw ();
-        Border?.ClearNeedsDraw ();
-        Padding?.ClearNeedsDraw ();
-
-        // There was multiple enumeration error here, so calling new snapshot collection - probably a stop gap
-        foreach (View subview in InternalSubViews.Snapshot ())
-        {
-            subview.ClearNeedsDraw ();
-        }
-
-        SubViewNeedsDraw = false;
-
-        // DO NOT clear SuperView.SubViewNeedsDraw here!
-        // The SuperView is responsible for clearing its own SubViewNeedsDraw flag.
-        // Previously this code cleared it:
-        //if (SuperView is { })
-        //{
-        //    SuperView.SubViewNeedsDraw = false;
-        //}
-        // This caused a bug where drawing one subview would incorrectly clear the SuperView's
-        // SubViewNeedsDraw flag even when sibling subviews still needed drawing.
-        //
-        // The SuperView will clear its own SubViewNeedsDraw after all its subviews are drawn,
-        // either via:
-        // 1. The superview's own Draw() method calling ClearNeedsDraw()
-        // 2. The static View.Draw(peers) method calling ClearNeedsDraw() on all peers
-
-        // This ensures LineCanvas' get redrawn
-        if (!SuperViewRendersLineCanvas)
-        {
-            LineCanvas.Clear ();
         }
     }
 }

Tests/UnitTestsParallelizable/Drawing/Lines/StraightLineTests.cs

@@ -1,6 +1,6 @@
 using Xunit.Abstractions;
 
-namespace UnitTests.Parallelizable.Drawing.Lines;
+namespace DrawingTests.Lines;
 
 public class StraightLineTests (ITestOutputHelper output)
 {

Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs

@@ -69,7 +69,7 @@ public class NeedsDrawTests : FakeDriverBase
         view.BeginInit ();
         Assert.True (view.NeedsDraw);
 
-        view.NeedsDraw = false;
+        view.ClearNeedsDraw ();
 
         view.BeginInit ();
         Assert.False (view.NeedsDraw); // Because layout is still needed
@@ -94,7 +94,7 @@ public class NeedsDrawTests : FakeDriverBase
 
         view = new () { Width = 2, Height = 2, BorderStyle = LineStyle.Single };
         view.BeginInit ();
-        view.NeedsDraw = false;
+        view.ClearNeedsDraw ();
         view.EndInit ();
         Assert.True (view.NeedsDraw);
     }
@@ -145,7 +145,7 @@ public class NeedsDrawTests : FakeDriverBase
         Assert.True (view.NeedsDraw);
         Assert.False (view.NeedsLayout);
 
-        view.NeedsDraw = false;
+        view.ClearNeedsDraw ();
 
         // SRL won't change anything since the view frame wasn't changed. However, Layout has not been called
         view.SetRelativeLayout (new (10, 10));
@@ -199,7 +199,7 @@ public class NeedsDrawTests : FakeDriverBase
         superView.Layout ();
         Assert.True (superView.NeedsDraw);
 
-        superView.NeedsDraw = false;
+        superView.ClearNeedsDraw ();
         superView.SetRelativeLayout (new (10, 10));
         Assert.True (superView.NeedsDraw);
     }
@@ -647,7 +647,7 @@ public class NeedsDrawTests : FakeDriverBase
         grandparent.EndInit ();
         grandparent.LayoutSubViews ();
 
-        child.SetSubViewNeedsDraw ();
+        child.SetSubViewNeedsDrawDownHierarchy ();
 
         Assert.True (child.SubViewNeedsDraw);
         Assert.True (parent.SubViewNeedsDraw);