diff --git a/Terminal.Gui/ViewBase/View.Drawing.cs b/Terminal.Gui/ViewBase/View.Drawing.cs index 5d729fcc7..cd3bd6905 100644 --- a/Terminal.Gui/ViewBase/View.Drawing.cs +++ b/Terminal.Gui/ViewBase/View.Drawing.cs @@ -35,6 +35,11 @@ public partial class View // Drawing APIs 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 { }) + { + view.SuperView.SubViewNeedsDraw = false; + } } } diff --git a/Terminal.Gui/ViewBase/View.NeedsDraw.cs b/Terminal.Gui/ViewBase/View.NeedsDraw.cs index 36e76afea..7af94f287 100644 --- a/Terminal.Gui/ViewBase/View.NeedsDraw.cs +++ b/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. + /// Gets whether any SubViews need to be redrawn. public bool SubViewNeedsDraw { get; private set; } @@ -161,10 +167,20 @@ public partial class View 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 if (!SuperViewRendersLineCanvas) diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs index 539b1fb42..eac7c4ee0 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs +++ b/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.NeedsDrawRect); } - + [Fact] 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 () { // This test validates that individual view Draw() calls should NOT clear the superview's diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/StaticDrawTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/StaticDrawTests.cs index f724a2cad..95ce3b3a9 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Draw/StaticDrawTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/StaticDrawTests.cs @@ -95,9 +95,9 @@ public class StaticDrawTests : FakeDriverBase // at the very end, cleaning up any SubViewNeedsDraw flags set // by Margin.DrawMargins() 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, - "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] @@ -188,7 +188,7 @@ public class StaticDrawTests : FakeDriverBase // All SubViewNeedsDraw flags should be cleared after the static Draw 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, "MiddleView1's SubViewNeedsDraw should be false after its subviews are drawn"); Assert.False (middleView2.SubViewNeedsDraw,