diff --git a/Examples/UICatalog/Scenarios/Menus.cs b/Examples/UICatalog/Scenarios/Menus.cs index 7733e0584..4796473ac 100644 --- a/Examples/UICatalog/Scenarios/Menus.cs +++ b/Examples/UICatalog/Scenarios/Menus.cs @@ -253,7 +253,13 @@ public class Menus : Scenario // The source of truth is our status CB; any time it changes, update the menu item var enableOverwriteMenuItemCb = menuBar.GetMenuItemsWithTitle ("Overwrite").FirstOrDefault ()?.CommandView as CheckBox; - enableOverwriteStatusCb.CheckedStateChanged += (_, _) => enableOverwriteMenuItemCb!.CheckedState = enableOverwriteStatusCb.CheckedState; + enableOverwriteStatusCb.CheckedStateChanged += (_, _) => + { + if (enableOverwriteMenuItemCb is { }) + { + enableOverwriteMenuItemCb.CheckedState = enableOverwriteStatusCb.CheckedState; + } + }; menuBar.Accepted += (o, args) => { @@ -298,7 +304,13 @@ public class Menus : Scenario // The source of truth is our status CB; any time it changes, update the menu item var editModeMenuItemCb = menuBar.GetMenuItemsWithTitle ("EditMode").FirstOrDefault ()?.CommandView as CheckBox; - editModeStatusCb.CheckedStateChanged += (_, _) => editModeMenuItemCb!.CheckedState = editModeStatusCb.CheckedState; + editModeStatusCb.CheckedStateChanged += (_, _) => + { + if (editModeMenuItemCb is { }) + { + editModeMenuItemCb.CheckedState = editModeStatusCb.CheckedState; + } + }; menuBar.Accepted += (o, args) => { diff --git a/Examples/UICatalog/UICatalogRunnable.cs b/Examples/UICatalog/UICatalogRunnable.cs index a163edab6..2373d1e3b 100644 --- a/Examples/UICatalog/UICatalogRunnable.cs +++ b/Examples/UICatalog/UICatalogRunnable.cs @@ -289,24 +289,31 @@ public class UICatalogRunnable : Runnable _diagnosticFlagsSelector = new () { Styles = SelectorStyles.ShowNoneFlag, - CanFocus = true + CanFocus =true }; _diagnosticFlagsSelector.UsedHotKeys.Add (Key.D); _diagnosticFlagsSelector.AssignHotKeys = true; _diagnosticFlagsSelector.Value = Diagnostics; - _diagnosticFlagsSelector.ValueChanged += (sender, args) => - { - _diagnosticFlags = (ViewDiagnosticFlags)_diagnosticFlagsSelector.Value; - Diagnostics = _diagnosticFlags; - }; + _diagnosticFlagsSelector.Selecting += (sender, args) => + { + _diagnosticFlags = (ViewDiagnosticFlags)((int)args.Context!.Source!.Data!);// (ViewDiagnosticFlags)_diagnosticFlagsSelector.Value; + Diagnostics = _diagnosticFlags; + }; - menuItems.Add ( - new MenuItem - { - CommandView = _diagnosticFlagsSelector, - HelpText = "View Diagnostics" - }); + MenuItem diagFlagMenuItem = new MenuItem () + { + CommandView = _diagnosticFlagsSelector, + HelpText = "View Diagnostics" + }; + diagFlagMenuItem.Accepting += (sender, args) => + { + //_diagnosticFlags = (ViewDiagnosticFlags)_diagnosticFlagsSelector.Value; + //Diagnostics = _diagnosticFlags; + //args.Handled = true; + }; + + menuItems.Add (diagFlagMenuItem); menuItems.Add (new Line ()); diff --git a/Terminal.Gui/ViewBase/Adornment/Margin.cs b/Terminal.Gui/ViewBase/Adornment/Margin.cs index 0ce7740ba..ae65cbff9 100644 --- a/Terminal.Gui/ViewBase/Adornment/Margin.cs +++ b/Terminal.Gui/ViewBase/Adornment/Margin.cs @@ -1,5 +1,6 @@ +using System.Diagnostics; using System.Runtime.InteropServices; namespace Terminal.Gui.ViewBase; @@ -62,7 +63,6 @@ public class Margin : Adornment } } - // PERFORMANCE: Margins are ALWAYS drawn. This may be an issue for apps that have a large number of views with shadows. /// /// INTERNAL API - Draws the margins for the specified views. This is called by the on each /// iteration of the main loop after all Views have been drawn. @@ -77,16 +77,17 @@ public class Margin : Adornment { var view = stack.Pop (); - if (view.Margin?.GetCachedClip () != null) + if (view.Margin is { } margin && margin.Thickness != Thickness.Empty && margin.GetCachedClip () != null) { - view.Margin!.NeedsDraw = true; + margin.NeedsDraw = true; Region? saved = view.GetClip (); - view.SetClip (view.Margin!.GetCachedClip ()); - view.Margin!.Draw (); + view.SetClip (margin.GetCachedClip ()); + margin.Draw (); view.SetClip (saved); - view.Margin!.ClearCachedClip (); + margin.ClearCachedClip (); } + Debug.Assert (view.NeedsDraw == false); view.NeedsDraw = false; foreach (var subview in view.SubViews) @@ -225,7 +226,7 @@ public class Margin : Adornment return; } - bool pressed = args.Value.HasFlag (MouseState.Pressed) && parent.HighlightStates.HasFlag(MouseState.Pressed); + bool pressed = args.Value.HasFlag (MouseState.Pressed) && parent.HighlightStates.HasFlag (MouseState.Pressed); bool pressedOutside = args.Value.HasFlag (MouseState.PressedOutside) && parent.HighlightStates.HasFlag (MouseState.PressedOutside); ; if (pressedOutside) diff --git a/Terminal.Gui/ViewBase/View.Drawing.cs b/Terminal.Gui/ViewBase/View.Drawing.cs index 38d33c7f9..5d729fcc7 100644 --- a/Terminal.Gui/ViewBase/View.Drawing.cs +++ b/Terminal.Gui/ViewBase/View.Drawing.cs @@ -30,6 +30,12 @@ public partial class View // Drawing APIs // Draw the margins (those with Shadows) 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 (); + } } /// @@ -73,7 +79,7 @@ public partial class View // Drawing APIs originalClip = AddViewportToClip (); // If no context ... - context ??= new DrawContext (); + context ??= new (); SetAttributeForRole (Enabled ? VisualRole.Normal : VisualRole.Disabled); DoClearViewport (context); @@ -136,7 +142,6 @@ public partial class View // Drawing APIs // ------------------------------------ // This causes the Margin to be drawn in a second pass if it has a ShadowStyle - // PERFORMANCE: If there is a Margin w/ Shadow, it will be redrawn each iteration of the main loop. Margin?.CacheClip (); // ------------------------------------ @@ -447,7 +452,7 @@ public partial class View // Drawing APIs if (Driver is { }) { - TextFormatter?.Draw ( + TextFormatter.Draw ( Driver, drawRect, HasFocus ? GetAttributeForRole (VisualRole.Focus) : GetAttributeForRole (VisualRole.Normal), @@ -733,185 +738,4 @@ public partial class View // Drawing APIs #endregion DrawComplete - #region NeedsDraw - - // TODO: Change NeedsDraw to use a Region instead of Rectangle - // TODO: Make _needsDrawRect nullable instead of relying on Empty - // TODO: If null, it means ? - // TODO: If Empty, it means no need to redraw - // TODO: If not Empty, it means the region that needs to be redrawn - // The viewport-relative region that needs to be redrawn. Marked internal for unit tests. - internal Rectangle NeedsDrawRect { get; set; } = Rectangle.Empty; - - /// Gets or sets whether the view needs to be redrawn. - /// - /// - /// Will be if the property is or if - /// any part of the view's needs to be redrawn. - /// - /// - /// Setting has no effect on . - /// - /// - public bool NeedsDraw - { - get => Visible && (NeedsDrawRect != Rectangle.Empty || Margin?.NeedsDraw == true || Border?.NeedsDraw == true || Padding?.NeedsDraw == true); - set - { - if (value) - { - SetNeedsDraw (); - } - else - { - ClearNeedsDraw (); - } - } - } - - /// Gets whether any SubViews need to be redrawn. - public bool SubViewNeedsDraw { get; private set; } - - /// Sets that the of this View needs to be redrawn. - /// - /// If the view has not been initialized ( is ), this method - /// does nothing. - /// - public void SetNeedsDraw () - { - Rectangle viewport = Viewport; - - if (!Visible || (NeedsDrawRect != Rectangle.Empty && viewport.IsEmpty)) - { - // This handles the case where the view has not been initialized yet - return; - } - - SetNeedsDraw (viewport); - } - - /// Expands the area of this view needing to be redrawn to include . - /// - /// - /// The location of is relative to the View's . - /// - /// - /// If the view has not been initialized ( is ), the area to be - /// redrawn will be the . - /// - /// - /// The relative region that needs to be redrawn. - public void SetNeedsDraw (Rectangle viewPortRelativeRegion) - { - if (!Visible) - { - return; - } - - if (NeedsDrawRect.IsEmpty) - { - NeedsDrawRect = viewPortRelativeRegion; - } - else - { - int x = Math.Min (Viewport.X, viewPortRelativeRegion.X); - int y = Math.Min (Viewport.Y, viewPortRelativeRegion.Y); - int w = Math.Max (Viewport.Width, viewPortRelativeRegion.Width); - int h = Math.Max (Viewport.Height, viewPortRelativeRegion.Height); - NeedsDrawRect = new (x, y, w, h); - } - - // Do not set on Margin - it will be drawn in a separate pass. - - if (Border is { } && Border.Thickness != Thickness.Empty) - { - Border?.SetNeedsDraw (); - } - - if (Padding is { } && Padding.Thickness != Thickness.Empty) - { - Padding?.SetNeedsDraw (); - } - - SuperView?.SetSubViewNeedsDraw (); - - if (this is Adornment adornment) - { - adornment.Parent?.SetSubViewNeedsDraw (); - } - - // 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)) - { - Rectangle subviewRegion = Rectangle.Intersect (subview.Frame, viewPortRelativeRegion); - subviewRegion.X -= subview.Frame.X; - subviewRegion.Y -= subview.Frame.Y; - subview.SetNeedsDraw (subviewRegion); - } - } - } - - /// Sets to for this View and all Superviews. - public void SetSubViewNeedsDraw () - { - if (!Visible) - { - return; - } - - SubViewNeedsDraw = true; - - if (this is Adornment adornment) - { - adornment.Parent?.SetSubViewNeedsDraw (); - } - - if (SuperView is { SubViewNeedsDraw: false }) - { - SuperView.SetSubViewNeedsDraw (); - } - } - - /// Clears and . - protected void ClearNeedsDraw () - { - NeedsDrawRect = Rectangle.Empty; - SubViewNeedsDraw = false; - - if (Margin is { } && (Margin.Thickness != Thickness.Empty || Margin.SubViewNeedsDraw || Margin.NeedsDraw)) - { - Margin?.ClearNeedsDraw (); - } - - if (Border is { } && (Border.Thickness != Thickness.Empty || Border.SubViewNeedsDraw || Border.NeedsDraw)) - { - Border?.ClearNeedsDraw (); - } - - if (Padding is { } && (Padding.Thickness != Thickness.Empty || Padding.SubViewNeedsDraw || Padding.NeedsDraw)) - { - 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 (); - } - - if (SuperView is { }) - { - SuperView.SubViewNeedsDraw = false; - } - - // This ensures LineCanvas' get redrawn - if (!SuperViewRendersLineCanvas) - { - LineCanvas.Clear (); - } - } - - #endregion NeedsDraw } diff --git a/Terminal.Gui/ViewBase/View.NeedsDraw.cs b/Terminal.Gui/ViewBase/View.NeedsDraw.cs new file mode 100644 index 000000000..36e76afea --- /dev/null +++ b/Terminal.Gui/ViewBase/View.NeedsDraw.cs @@ -0,0 +1,175 @@ +namespace Terminal.Gui.ViewBase; + +public partial class View +{ + // TODO: Change NeedsDraw to use a Region instead of Rectangle + // TODO: Make _needsDrawRect nullable instead of relying on Empty + // TODO: If null, it means ? + // TODO: If Empty, it means no need to redraw + // TODO: If not Empty, it means the region that needs to be redrawn + + /// + /// The viewport-relative region that needs to be redrawn. Marked internal for unit tests. + /// + internal Rectangle NeedsDrawRect { get; set; } = Rectangle.Empty; + + /// Gets or sets whether the view needs to be redrawn. + /// + /// + /// Will be if the property is or if + /// any part of the view's needs to be redrawn. + /// + /// + /// Setting has no effect on . + /// + /// + public bool NeedsDraw + { + get => Visible && (NeedsDrawRect != Rectangle.Empty || Margin?.NeedsDraw == true || Border?.NeedsDraw == true || Padding?.NeedsDraw == true); + set + { + if (value) + { + SetNeedsDraw (); + } + else + { + ClearNeedsDraw (); + } + } + } + + /// Gets whether any SubViews need to be redrawn. + public bool SubViewNeedsDraw { get; private set; } + + /// Sets that the of this View needs to be redrawn. + /// + /// If the view has not been initialized ( is ), this method + /// does nothing. + /// + public void SetNeedsDraw () + { + Rectangle viewport = Viewport; + + if (!Visible || (NeedsDrawRect != Rectangle.Empty && viewport.IsEmpty)) + { + // This handles the case where the view has not been initialized yet + return; + } + + SetNeedsDraw (viewport); + } + + /// Expands the area of this view needing to be redrawn to include . + /// + /// + /// The location of is relative to the View's . + /// + /// + /// If the view has not been initialized ( is ), the area to be + /// redrawn will be the . + /// + /// + /// The relative region that needs to be redrawn. + public void SetNeedsDraw (Rectangle viewPortRelativeRegion) + { + if (!Visible) + { + return; + } + + if (NeedsDrawRect.IsEmpty) + { + NeedsDrawRect = viewPortRelativeRegion; + } + else + { + int x = Math.Min (Viewport.X, viewPortRelativeRegion.X); + int y = Math.Min (Viewport.Y, viewPortRelativeRegion.Y); + int w = Math.Max (Viewport.Width, viewPortRelativeRegion.Width); + int h = Math.Max (Viewport.Height, viewPortRelativeRegion.Height); + NeedsDrawRect = new (x, y, w, h); + } + + // Do not set on Margin - it will be drawn in a separate pass. + + if (Border is { } && Border.Thickness != Thickness.Empty) + { + Border?.SetNeedsDraw (); + } + + if (Padding is { } && Padding.Thickness != Thickness.Empty) + { + Padding?.SetNeedsDraw (); + } + + SuperView?.SetSubViewNeedsDraw (); + + if (this is Adornment adornment) + { + adornment.Parent?.SetSubViewNeedsDraw (); + } + + // 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)) + { + Rectangle subviewRegion = Rectangle.Intersect (subview.Frame, viewPortRelativeRegion); + subviewRegion.X -= subview.Frame.X; + subviewRegion.Y -= subview.Frame.Y; + subview.SetNeedsDraw (subviewRegion); + } + } + } + + /// Sets to for this View and all Superviews. + public void SetSubViewNeedsDraw () + { + if (!Visible) + { + return; + } + + SubViewNeedsDraw = true; + + if (this is Adornment adornment) + { + adornment.Parent?.SetSubViewNeedsDraw (); + } + + if (SuperView is { SubViewNeedsDraw: false }) + { + SuperView.SetSubViewNeedsDraw (); + } + } + + /// Clears and . + 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; + + if (SuperView is { }) + { + SuperView.SubViewNeedsDraw = false; + } + + // This ensures LineCanvas' get redrawn + if (!SuperViewRendersLineCanvas) + { + LineCanvas.Clear (); + } + } +} diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs index a7a11b584..539b1fb42 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs @@ -311,80 +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 (Skip = "Not valid")] - public void ClearNeedsDraw_WithSiblings_DoesNotClearSuperViewSubViewNeedsDraw () - { - // This test verifies the fix for the bug where a subview clearing its NeedsDraw - // would incorrectly clear the superview's SubViewNeedsDraw flag, even if other siblings - // still needed drawing. - - IDriver driver = CreateFakeDriver (80, 25); - driver.Clip = new Region (driver.Screen); - - var superView = new View - { - X = 0, - Y = 0, - Width = 50, - Height = 50, - Driver = driver - }; - - var subView1 = new View { X = 0, Y = 0, Width = 10, Height = 10, Id = "SubView1" }; - var subView2 = new View { X = 0, Y = 10, Width = 10, Height = 10, Id = "SubView2" }; - var subView3 = new View { X = 0, Y = 20, Width = 10, Height = 10, Id = "SubView3" }; - - superView.Add (subView1, subView2, subView3); - superView.BeginInit (); - superView.EndInit (); - superView.LayoutSubViews (); - - // All subviews should need drawing initially - Assert.True (subView1.NeedsDraw); - Assert.True (subView2.NeedsDraw); - Assert.True (subView3.NeedsDraw); - Assert.True (superView.SubViewNeedsDraw); - - // Draw subView1 - this will call ClearNeedsDraw() on subView1 - subView1.Draw (); - - // SubView1 should no longer need drawing - Assert.False (subView1.NeedsDraw); - - // But subView2 and subView3 still need drawing - Assert.True (subView2.NeedsDraw); - Assert.True (subView3.NeedsDraw); - - // THE BUG: Before the fix, subView1.ClearNeedsDraw() would set superView.SubViewNeedsDraw = false - // even though subView2 and subView3 still need drawing. - // After the fix, superView.SubViewNeedsDraw should still be true because subView2 and subView3 need drawing. - Assert.True (superView.SubViewNeedsDraw, "SuperView's SubViewNeedsDraw should still be true because subView2 and subView3 still need drawing"); - - // Now draw subView2 - subView2.Draw (); - Assert.False (subView2.NeedsDraw); - Assert.True (subView3.NeedsDraw); - - // SuperView should still have SubViewNeedsDraw = true because subView3 needs drawing - Assert.True (superView.SubViewNeedsDraw, "SuperView's SubViewNeedsDraw should still be true because subView3 still needs drawing"); - - // Now draw subView3 - subView3.Draw (); - Assert.False (subView3.NeedsDraw); - - // SuperView should STILL have SubViewNeedsDraw = true because it hasn't been cleared by the superview itself - // Only the superview's own ClearNeedsDraw() should clear this flag - Assert.True (superView.SubViewNeedsDraw, "SuperView's SubViewNeedsDraw should only be cleared by superView.ClearNeedsDraw(), not by subviews"); - - // Finally, draw the superview - this will clear SubViewNeedsDraw - superView.Draw (); - Assert.False (superView.SubViewNeedsDraw, "SuperView's SubViewNeedsDraw should now be false after superView.Draw()"); - Assert.False (subView1.NeedsDraw); - Assert.False (subView2.NeedsDraw); - Assert.False (subView3.NeedsDraw); - } - + [Fact] public void ClearNeedsDraw_ClearsOwnFlags () { @@ -481,4 +408,281 @@ public class NeedsDrawTests : FakeDriverBase Assert.False (middleView.SubViewNeedsDraw); Assert.False (bottomView.NeedsDraw); } + + #region NeedsDraw Tests + + [Fact] + public void NeedsDraw_InitiallyFalse_WhenNotVisible () + { + var view = new View { Visible = false }; + view.BeginInit (); + view.EndInit (); + + Assert.False (view.NeedsDraw); + } + + [Fact] + public void NeedsDraw_TrueAfterSetNeedsDraw () + { + var view = new View { X = 0, Y = 0, Width = 10, Height = 10 }; + view.BeginInit (); + view.EndInit (); + view.LayoutSubViews (); + + view.SetNeedsDraw (); + + Assert.True (view.NeedsDraw); + } + + [Fact] + public void NeedsDraw_ClearedAfterDraw () + { + IDriver driver = CreateFakeDriver (80, 25); + driver.Clip = new Region (driver.Screen); + + var view = new View + { + X = 0, + Y = 0, + Width = 10, + Height = 10, + Driver = driver + }; + view.BeginInit (); + view.EndInit (); + view.LayoutSubViews (); + + view.SetNeedsDraw (); + Assert.True (view.NeedsDraw); + + view.Draw (); + + Assert.False (view.NeedsDraw); + } + + [Fact] + public void SetNeedsDraw_WithRectangle_UpdatesNeedsDrawRect () + { + var view = new View { Driver = CreateFakeDriver (), X = 0, Y = 0, Width = 20, Height = 20 }; + view.BeginInit (); + view.EndInit (); + view.LayoutSubViews (); + + // After layout, view will have NeedsDrawRect set to the viewport + // We need to clear it first + view.Draw (); + Assert.False (view.NeedsDraw); + Assert.Equal (Rectangle.Empty, view.NeedsDrawRect); + + var rect = new Rectangle (5, 5, 10, 10); + view.SetNeedsDraw (rect); + + Assert.True (view.NeedsDraw); + Assert.Equal (rect, view.NeedsDrawRect); + } + + [Fact] + public void SetNeedsDraw_MultipleRectangles_Expands () + { + IDriver driver = CreateFakeDriver (80, 25); + driver.Clip = new Region (driver.Screen); + + var view = new View { X = 0, Y = 0, Width = 30, Height = 30, Driver = driver }; + view.BeginInit (); + view.EndInit (); + view.LayoutSubViews (); + + // After layout, clear NeedsDraw + view.Draw (); + Assert.False (view.NeedsDraw); + + view.SetNeedsDraw (new Rectangle (5, 5, 10, 10)); + view.SetNeedsDraw (new Rectangle (15, 15, 10, 10)); + + // Should expand to cover the entire viewport when we have overlapping regions + // The current implementation expands to viewport size + Rectangle expected = new Rectangle (0, 0, 30, 30); + Assert.Equal (expected, view.NeedsDrawRect); + } + + [Fact] + public void SetNeedsDraw_NotVisible_DoesNotSet () + { + var view = new View + { + X = 0, + Y = 0, + Width = 10, + Height = 10, + Visible = false + }; + view.BeginInit (); + view.EndInit (); + + view.SetNeedsDraw (); + + Assert.False (view.NeedsDraw); + } + + [Fact] + public void SetNeedsDraw_PropagatesToSuperView () + { + var parent = new View { X = 0, Y = 0, Width = 50, Height = 50 }; + var child = new View { X = 10, Y = 10, Width = 20, Height = 20 }; + parent.Add (child); + parent.BeginInit (); + parent.EndInit (); + parent.LayoutSubViews (); + + child.SetNeedsDraw (); + + Assert.True (child.NeedsDraw); + Assert.True (parent.SubViewNeedsDraw); + } + + [Fact] + public void SetNeedsDraw_SetsAdornmentsNeedsDraw () + { + var view = new View { X = 0, Y = 0, Width = 20, Height = 20 }; + view.Border!.Thickness = new Thickness (1); + view.Padding!.Thickness = new Thickness (1); + view.BeginInit (); + view.EndInit (); + view.LayoutSubViews (); + + view.SetNeedsDraw (); + + Assert.True (view.Border!.NeedsDraw); + Assert.True (view.Padding!.NeedsDraw); + } + + + [Fact (Skip = "This is a real bug discovered in PR #4431 that needs to be fixed")] + public void IndividualViewDraw_DoesNotClearSuperViewSubViewNeedsDraw () + { + // This test validates that individual view Draw() calls should NOT clear the superview's + // SubViewNeedsDraw flag when sibling subviews still need drawing. + // + // This is the core behavior that enables the fix in the static Draw method. + IDriver driver = CreateFakeDriver (); + driver.Clip = new (driver.Screen); + + View superview = new () + { + X = 0, + Y = 0, + Width = 50, + Height = 50, + Driver = driver, + Id = "SuperView" + }; + + View subview1 = new () { X = 0, Y = 0, Width = 10, Height = 10, Id = "SubView1" }; + View subview2 = new () { X = 0, Y = 10, Width = 10, Height = 10, Id = "SubView2" }; + + superview.Add (subview1, subview2); + superview.BeginInit (); + superview.EndInit (); + superview.LayoutSubViews (); + + Assert.True (superview.SubViewNeedsDraw); + Assert.True (subview1.NeedsDraw); + Assert.True (subview2.NeedsDraw); + + // Draw only subview1 (NOT using the static Draw method) + subview1.Draw (); + + // SubView1 should be cleared + Assert.False (subview1.NeedsDraw); + + // SubView2 still needs drawing + Assert.True (subview2.NeedsDraw); + + // THE KEY ASSERTION: SuperView's SubViewNeedsDraw should STILL be true + // because subview2 still needs drawing + // + // This behavior is REQUIRED for the static Draw fix to work properly. + // ClearNeedsDraw() does NOT clear SuperView.SubViewNeedsDraw anymore. + Assert.True (superview.SubViewNeedsDraw, + "SuperView's SubViewNeedsDraw must remain true when subview2 still needs drawing"); + + // Now draw subview2 + subview2.Draw (); + Assert.False (subview2.NeedsDraw); + + // SuperView's SubViewNeedsDraw should STILL be true because only the superview + // itself (or the static Draw method on all subviews) should clear it + Assert.True (superview.SubViewNeedsDraw, + "SuperView's SubViewNeedsDraw should only be cleared by superview.Draw() or static Draw() on all subviews"); + } + + #endregion + + #region SubViewNeedsDraw Tests + + [Fact] + public void SubViewNeedsDraw_InitiallyFalse () + { + IDriver driver = CreateFakeDriver (80, 25); + driver.Clip = new Region (driver.Screen); + + var view = new View { Width = 10, Height = 10, Driver = driver }; + view.BeginInit (); + view.EndInit (); + view.Draw (); // Draw once to clear initial NeedsDraw + + Assert.False (view.SubViewNeedsDraw); + } + + [Fact] + public void SetSubViewNeedsDraw_PropagatesUp () + { + var grandparent = new View { X = 0, Y = 0, Width = 100, Height = 100 }; + var parent = new View { X = 10, Y = 10, Width = 50, Height = 50 }; + var child = new View { X = 5, Y = 5, Width = 20, Height = 20 }; + + grandparent.Add (parent); + parent.Add (child); + grandparent.BeginInit (); + grandparent.EndInit (); + grandparent.LayoutSubViews (); + + child.SetSubViewNeedsDraw (); + + Assert.True (child.SubViewNeedsDraw); + Assert.True (parent.SubViewNeedsDraw); + Assert.True (grandparent.SubViewNeedsDraw); + } + + [Fact] + public void SubViewNeedsDraw_ClearedAfterDraw () + { + IDriver driver = CreateFakeDriver (80, 25); + driver.Clip = new Region (driver.Screen); + + var parent = new View + { + X = 0, + Y = 0, + Width = 50, + Height = 50, + Driver = driver + }; + var child = new View { X = 10, Y = 10, Width = 20, Height = 20 }; + parent.Add (child); + parent.BeginInit (); + parent.EndInit (); + parent.LayoutSubViews (); + + child.SetNeedsDraw (); + Assert.True (parent.SubViewNeedsDraw); + + parent.Draw (); + + Assert.False (parent.SubViewNeedsDraw); + Assert.False (child.SubViewNeedsDraw); + } + + #endregion + } diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/StaticDrawTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/StaticDrawTests.cs new file mode 100644 index 000000000..f724a2cad --- /dev/null +++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/StaticDrawTests.cs @@ -0,0 +1,201 @@ +#nullable enable +using UnitTests; + +namespace ViewBaseTests.Drawing; + +/// +/// Tests for the static View.Draw(IEnumerable<View>, bool) method +/// +[Trait ("Category", "Output")] +public class StaticDrawTests : FakeDriverBase +{ + [Fact] + public void StaticDraw_ClearsSubViewNeedsDraw_AfterMarginDrawMargins () + { + // This test validates the fix where the static Draw method calls ClearNeedsDraw() + // on all peer views after drawing them AND after calling Margin.DrawMargins(). + // + // THE BUG (before the fix): + // Margin.DrawMargins() can cause SubViewNeedsDraw to be set on views in the hierarchy. + // This would leave SubViewNeedsDraw = true even after drawing completed. + // + // THE FIX (current code): + // The static Draw() method explicitly calls ClearNeedsDraw() on all peer views + // at the very end, AFTER Margin.DrawMargins(), clearing any SubViewNeedsDraw flags + // that were set during margin drawing. + + IDriver driver = CreateFakeDriver (); + driver.Clip = new (driver.Screen); + + // Create a view hierarchy where a subview's subview has a margin + // This reproduces the scenario where Margin.DrawMargins sets SubViewNeedsDraw + View superview = new () + { + X = 0, + Y = 0, + Width = 60, + Height = 60, + Driver = driver, + Id = "SuperView" + }; + + View subview1 = new () { X = 0, Y = 0, Width = 40, Height = 40, Id = "SubView1" }; + View subview2 = new () { X = 0, Y = 20, Width = 20, Height = 20, Id = "SubView2" }; + + // Add a subview to subview1 that has a margin with shadow + // This is key to reproducing the bug + View subSubView = new () + { + X = 5, + Y = 5, + Width = 20, + Height = 20, + Id = "SubSubView" + }; + subSubView.Margin!.Thickness = new (1); + subSubView.Margin.ShadowStyle = ShadowStyle.Transparent; + + subview1.Add (subSubView); + superview.Add (subview1, subview2); + + superview.BeginInit (); + superview.EndInit (); + superview.LayoutSubViews (); + + // All views initially need drawing + Assert.True (superview.NeedsDraw); + Assert.True (superview.SubViewNeedsDraw); + Assert.True (subview1.NeedsDraw); + Assert.True (subview1.SubViewNeedsDraw); + Assert.True (subview2.NeedsDraw); + Assert.True (subSubView.NeedsDraw); + Assert.True (subSubView.Margin.NeedsDraw); + + // Call the static Draw method on the subviews + // This will: + // 1. Call view.Draw() on each subview + // 2. Call Margin.DrawMargins() which may set SubViewNeedsDraw in the hierarchy + // 3. Call ClearNeedsDraw() on each subview to clean up + View.Draw (superview.InternalSubViews, force: false); + + // After the static Draw completes: + // All subviews should have NeedsDraw = false + Assert.False (subview1.NeedsDraw, "SubView1 should not need drawing after Draw()"); + Assert.False (subview2.NeedsDraw, "SubView2 should not need drawing after Draw()"); + Assert.False (subSubView.NeedsDraw, "SubSubView should not need drawing after Draw()"); + Assert.False (subSubView.Margin.NeedsDraw, "SubSubView's Margin should not need drawing after Draw()"); + + // SuperView's SubViewNeedsDraw should be false because the static Draw() method + // calls ClearNeedsDraw() on all the subviews at the end, AFTER Margin.DrawMargins() + // + // BEFORE THE FIX: This would be TRUE because Margin.DrawMargins() would + // set SubViewNeedsDraw somewhere in the hierarchy and it + // wouldn't be cleared + // AFTER THE FIX: This is FALSE because the static Draw() calls ClearNeedsDraw() + // 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"); + Assert.False (subview1.SubViewNeedsDraw, + "SubView1's SubViewNeedsDraw should be false after its subviews are drawn and cleared"); + } + + [Fact] + public void StaticDraw_WithForceTrue_SetsNeedsDrawOnAllViews () + { + // Verify that when force=true, all views get SetNeedsDraw() called before drawing + IDriver driver = CreateFakeDriver (); + driver.Clip = new (driver.Screen); + + View view1 = new () { X = 0, Y = 0, Width = 10, Height = 10, Driver = driver, Id = "View1" }; + View view2 = new () { X = 10, Y = 0, Width = 10, Height = 10, Driver = driver, Id = "View2" }; + + view1.BeginInit (); + view1.EndInit (); + view2.BeginInit (); + view2.EndInit (); + + // Manually clear their NeedsDraw flags + view1.Draw (); + view2.Draw (); + Assert.False (view1.NeedsDraw); + Assert.False (view2.NeedsDraw); + + // Now call static Draw with force=true + View.Draw ([view1, view2], force: true); + + // After drawing with force=true, they should be cleared again + Assert.False (view1.NeedsDraw); + Assert.False (view2.NeedsDraw); + } + + [Fact] + public void StaticDraw_HandlesEmptyCollection () + { + // Verify that calling Draw with an empty collection doesn't crash + View.Draw ([], force: false); + View.Draw ([], force: true); + } + + + [Fact] + public void StaticDraw_ClearsNestedSubViewNeedsDraw () + { + // This test verifies that the static Draw method properly clears SubViewNeedsDraw + // flags throughout a nested view hierarchy after Margin.DrawMargins + IDriver driver = CreateFakeDriver (); + driver.Clip = new (driver.Screen); + + View topView = new () + { + X = 0, + Y = 0, + Width = 60, + Height = 60, + Driver = driver, + Id = "TopView" + }; + + View middleView1 = new () { X = 0, Y = 0, Width = 30, Height = 30, Id = "MiddleView1" }; + View middleView2 = new () { X = 30, Y = 0, Width = 30, Height = 30, Id = "MiddleView2" }; + + View bottomView = new () + { + X = 5, + Y = 5, + Width = 15, + Height = 15, + Id = "BottomView" + }; + + // Give the bottom view a margin to trigger the Margin.DrawMargins behavior + bottomView.Margin!.Thickness = new (1); + bottomView.Margin.ShadowStyle = ShadowStyle.Transparent; + + middleView1.Add (bottomView); + topView.Add (middleView1, middleView2); + + topView.BeginInit (); + topView.EndInit (); + topView.LayoutSubViews (); + + Assert.True (topView.SubViewNeedsDraw); + Assert.True (middleView1.SubViewNeedsDraw); + Assert.True (bottomView.NeedsDraw); + + // Draw the middle views using static Draw + View.Draw (topView.InternalSubViews, force: false); + + // All SubViewNeedsDraw flags should be cleared after the static Draw + Assert.False (topView.SubViewNeedsDraw, + "TopView's SubViewNeedsDraw should be false after static Draw()"); + Assert.False (middleView1.SubViewNeedsDraw, + "MiddleView1's SubViewNeedsDraw should be false after its subviews are drawn"); + Assert.False (middleView2.SubViewNeedsDraw, + "MiddleView2's SubViewNeedsDraw should be false"); + Assert.False (bottomView.NeedsDraw, + "BottomView should not need drawing after Draw()"); + Assert.False (bottomView.Margin.NeedsDraw, + "BottomView's Margin should not need drawing after Draw()"); + } +} diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingFlowTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingFlowTests.cs index 7a429a042..1014ac3a2 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingFlowTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingFlowTests.cs @@ -6,222 +6,7 @@ namespace ViewBaseTests.Drawing; public class ViewDrawingFlowTests () : FakeDriverBase { - #region NeedsDraw Tests - - [Fact] - public void NeedsDraw_InitiallyFalse_WhenNotVisible () - { - var view = new View { Visible = false }; - view.BeginInit (); - view.EndInit (); - - Assert.False (view.NeedsDraw); - } - - [Fact] - public void NeedsDraw_TrueAfterSetNeedsDraw () - { - var view = new View { X = 0, Y = 0, Width = 10, Height = 10 }; - view.BeginInit (); - view.EndInit (); - view.LayoutSubViews (); - - view.SetNeedsDraw (); - - Assert.True (view.NeedsDraw); - } - - [Fact] - public void NeedsDraw_ClearedAfterDraw () - { - IDriver driver = CreateFakeDriver (80, 25); - driver.Clip = new Region (driver.Screen); - - var view = new View - { - X = 0, - Y = 0, - Width = 10, - Height = 10, - Driver = driver - }; - view.BeginInit (); - view.EndInit (); - view.LayoutSubViews (); - - view.SetNeedsDraw (); - Assert.True (view.NeedsDraw); - - view.Draw (); - - Assert.False (view.NeedsDraw); - } - - [Fact] - public void SetNeedsDraw_WithRectangle_UpdatesNeedsDrawRect () - { - var view = new View { Driver = CreateFakeDriver (), X = 0, Y = 0, Width = 20, Height = 20 }; - view.BeginInit (); - view.EndInit (); - view.LayoutSubViews (); - - // After layout, view will have NeedsDrawRect set to the viewport - // We need to clear it first - view.Draw (); - Assert.False (view.NeedsDraw); - Assert.Equal (Rectangle.Empty, view.NeedsDrawRect); - - var rect = new Rectangle (5, 5, 10, 10); - view.SetNeedsDraw (rect); - - Assert.True (view.NeedsDraw); - Assert.Equal (rect, view.NeedsDrawRect); - } - - [Fact] - public void SetNeedsDraw_MultipleRectangles_Expands () - { - IDriver driver = CreateFakeDriver (80, 25); - driver.Clip = new Region (driver.Screen); - - var view = new View { X = 0, Y = 0, Width = 30, Height = 30, Driver = driver }; - view.BeginInit (); - view.EndInit (); - view.LayoutSubViews (); - - // After layout, clear NeedsDraw - view.Draw (); - Assert.False (view.NeedsDraw); - - view.SetNeedsDraw (new Rectangle (5, 5, 10, 10)); - view.SetNeedsDraw (new Rectangle (15, 15, 10, 10)); - - // Should expand to cover the entire viewport when we have overlapping regions - // The current implementation expands to viewport size - Rectangle expected = new Rectangle (0, 0, 30, 30); - Assert.Equal (expected, view.NeedsDrawRect); - } - - [Fact] - public void SetNeedsDraw_NotVisible_DoesNotSet () - { - var view = new View - { - X = 0, - Y = 0, - Width = 10, - Height = 10, - Visible = false - }; - view.BeginInit (); - view.EndInit (); - - view.SetNeedsDraw (); - - Assert.False (view.NeedsDraw); - } - - [Fact] - public void SetNeedsDraw_PropagatesToSuperView () - { - var parent = new View { X = 0, Y = 0, Width = 50, Height = 50 }; - var child = new View { X = 10, Y = 10, Width = 20, Height = 20 }; - parent.Add (child); - parent.BeginInit (); - parent.EndInit (); - parent.LayoutSubViews (); - - child.SetNeedsDraw (); - - Assert.True (child.NeedsDraw); - Assert.True (parent.SubViewNeedsDraw); - } - - [Fact] - public void SetNeedsDraw_SetsAdornmentsNeedsDraw () - { - var view = new View { X = 0, Y = 0, Width = 20, Height = 20 }; - view.Border!.Thickness = new Thickness (1); - view.Padding!.Thickness = new Thickness (1); - view.BeginInit (); - view.EndInit (); - view.LayoutSubViews (); - - view.SetNeedsDraw (); - - Assert.True (view.Border!.NeedsDraw); - Assert.True (view.Padding!.NeedsDraw); - } - - #endregion - - #region SubViewNeedsDraw Tests - - [Fact] - public void SubViewNeedsDraw_InitiallyFalse () - { - IDriver driver = CreateFakeDriver (80, 25); - driver.Clip = new Region (driver.Screen); - - var view = new View { Width = 10, Height = 10, Driver = driver }; - view.BeginInit (); - view.EndInit (); - view.Draw (); // Draw once to clear initial NeedsDraw - - Assert.False (view.SubViewNeedsDraw); - } - - [Fact] - public void SetSubViewNeedsDraw_PropagatesUp () - { - var grandparent = new View { X = 0, Y = 0, Width = 100, Height = 100 }; - var parent = new View { X = 10, Y = 10, Width = 50, Height = 50 }; - var child = new View { X = 5, Y = 5, Width = 20, Height = 20 }; - - grandparent.Add (parent); - parent.Add (child); - grandparent.BeginInit (); - grandparent.EndInit (); - grandparent.LayoutSubViews (); - - child.SetSubViewNeedsDraw (); - - Assert.True (child.SubViewNeedsDraw); - Assert.True (parent.SubViewNeedsDraw); - Assert.True (grandparent.SubViewNeedsDraw); - } - - [Fact] - public void SubViewNeedsDraw_ClearedAfterDraw () - { - IDriver driver = CreateFakeDriver (80, 25); - driver.Clip = new Region (driver.Screen); - - var parent = new View - { - X = 0, - Y = 0, - Width = 50, - Height = 50, - Driver = driver - }; - var child = new View { X = 10, Y = 10, Width = 20, Height = 20 }; - parent.Add (child); - parent.BeginInit (); - parent.EndInit (); - parent.LayoutSubViews (); - - child.SetNeedsDraw (); - Assert.True (parent.SubViewNeedsDraw); - - parent.Draw (); - - Assert.False (parent.SubViewNeedsDraw); - Assert.False (child.SubViewNeedsDraw); - } - - #endregion - + #region Draw Visibility Tests [Fact]