Selaa lähdekoodia

Fix SubViewNeedsDraw handling and improve test coverage

Refactored `View` class to ensure `SuperView.SubViewNeedsDraw`
is managed correctly. Added logic to prevent clearing the flag
prematurely when sibling subviews still require drawing.

Introduced a new `SubViewNeedsDraw` property with a private
setter and added `TODO` comments for potential future
improvements, such as making it a computed property.

Updated and added tests in `NeedsDrawTests` and `StaticDrawTests`
to validate the corrected behavior and prevent regressions.
Re-enabled a previously skipped test after fixing the related bug.
Tig 1 viikko sitten
vanhempi
sitoutus
96151201f6

+ 5 - 0
Terminal.Gui/ViewBase/View.Drawing.cs

@@ -35,6 +35,11 @@ public partial class View // Drawing APIs
         foreach (View view in viewsArray)
         foreach (View view in viewsArray)
         {
         {
             view.ClearNeedsDraw ();
             view.ClearNeedsDraw ();
+            // ClearNeedsDraw does not clear view.SuperView.SubViewsNeedDraw, so we have to do it here
+            if (view.SuperView is { })
+            {
+                view.SuperView.SubViewNeedsDraw = false;
+            }
         }
         }
     }
     }
 
 

+ 20 - 4
Terminal.Gui/ViewBase/View.NeedsDraw.cs

@@ -39,6 +39,12 @@ public partial class View
         }
         }
     }
     }
 
 
+    // TODO: This property is decoupled from the actual state of the subviews (and adornments)
+    // 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>
     /// <summary>Gets whether any SubViews need to be redrawn.</summary>
     public bool SubViewNeedsDraw { get; private set; }
     public bool SubViewNeedsDraw { get; private set; }
 
 
@@ -161,10 +167,20 @@ public partial class View
 
 
         SubViewNeedsDraw = false;
         SubViewNeedsDraw = false;
 
 
-        if (SuperView is { })
-        {
-            SuperView.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
         // This ensures LineCanvas' get redrawn
         if (!SuperViewRendersLineCanvas)
         if (!SuperViewRendersLineCanvas)

+ 2 - 2
Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs

@@ -311,7 +311,7 @@ public class NeedsDrawTests : FakeDriverBase
         Assert.Equal (new (1, 1, 5, 5), view.Viewport);
         Assert.Equal (new (1, 1, 5, 5), view.Viewport);
         Assert.Equal (new (1, 1, 5, 5), view.NeedsDrawRect);
         Assert.Equal (new (1, 1, 5, 5), view.NeedsDrawRect);
     }
     }
-    
+
     [Fact]
     [Fact]
     public void ClearNeedsDraw_ClearsOwnFlags ()
     public void ClearNeedsDraw_ClearsOwnFlags ()
     {
     {
@@ -557,7 +557,7 @@ public class NeedsDrawTests : FakeDriverBase
     }
     }
 
 
 
 
-    [Fact (Skip = "This is a real bug discovered in PR #4431 that needs to be fixed")]
+    [Fact]
     public void IndividualViewDraw_DoesNotClearSuperViewSubViewNeedsDraw ()
     public void IndividualViewDraw_DoesNotClearSuperViewSubViewNeedsDraw ()
     {
     {
         // This test validates that individual view Draw() calls should NOT clear the superview's
         // This test validates that individual view Draw() calls should NOT clear the superview's

+ 3 - 3
Tests/UnitTestsParallelizable/ViewBase/Draw/StaticDrawTests.cs

@@ -95,9 +95,9 @@ public class StaticDrawTests : FakeDriverBase
         //                at the very end, cleaning up any SubViewNeedsDraw flags set
         //                at the very end, cleaning up any SubViewNeedsDraw flags set
         //                by Margin.DrawMargins()
         //                by Margin.DrawMargins()
         Assert.False (superview.SubViewNeedsDraw,
         Assert.False (superview.SubViewNeedsDraw,
-            "SuperView's SubViewNeedsDraw should be false after all subviews are drawn and cleared");
+                      "superview's SubViewNeedsDraw should be false after static Draw(). All subviews were drawn in the call to View.Draw");
         Assert.False (subview1.SubViewNeedsDraw,
         Assert.False (subview1.SubViewNeedsDraw,
-            "SubView1's SubViewNeedsDraw should be false after its subviews are drawn and cleared");
+                      "SubView1's SubViewNeedsDraw should be false after its subviews are drawn and cleared");
     }
     }
 
 
     [Fact]
     [Fact]
@@ -188,7 +188,7 @@ public class StaticDrawTests : FakeDriverBase
 
 
         // All SubViewNeedsDraw flags should be cleared after the static Draw
         // All SubViewNeedsDraw flags should be cleared after the static Draw
         Assert.False (topView.SubViewNeedsDraw,
         Assert.False (topView.SubViewNeedsDraw,
-            "TopView's SubViewNeedsDraw should be false after static Draw()");
+            "TopView's SubViewNeedsDraw should be false after static Draw(). All subviews were drawn in the call to View.Draw");
         Assert.False (middleView1.SubViewNeedsDraw,
         Assert.False (middleView1.SubViewNeedsDraw,
             "MiddleView1's SubViewNeedsDraw should be false after its subviews are drawn");
             "MiddleView1's SubViewNeedsDraw should be false after its subviews are drawn");
         Assert.False (middleView2.SubViewNeedsDraw,
         Assert.False (middleView2.SubViewNeedsDraw,