From 844179b1bdb18aa499cb112a4379b13fe91049a3 Mon Sep 17 00:00:00 2001 From: Tig Date: Sun, 27 Oct 2024 23:18:38 -0700 Subject: [PATCH] ConsoleDriver now uses Region for Clip. Still only rectangular regions. All tests pass. --- Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs | 30 ++++++++++++------- .../CursesDriver/CursesDriver.cs | 3 +- .../ConsoleDrivers/FakeDriver/FakeDriver.cs | 2 +- Terminal.Gui/ConsoleDrivers/NetDriver.cs | 6 ++-- Terminal.Gui/ConsoleDrivers/WindowsDriver.cs | 4 +-- Terminal.Gui/View/View.Drawing.Clipping.cs | 13 ++++---- Terminal.Gui/View/View.Drawing.Primitives.cs | 8 +---- Terminal.Gui/View/View.Drawing.cs | 6 +--- Terminal.Gui/Views/Menu/Menu.cs | 4 +-- Terminal.Gui/Views/ScrollBarView.cs | 2 +- Terminal.Gui/Views/TabView.cs | 2 +- UICatalog/Scenarios/AllViewsTester.cs | 1 + UnitTests/ConsoleDrivers/ClipRegionTests.cs | 6 ++-- UnitTests/View/Draw/DrawTests.cs | 10 +++---- UnitTests/View/ViewTests.cs | 8 ++--- UnitTests/Views/ContextMenuTests.cs | 4 +-- UnitTests/Views/MenuBarTests.cs | 2 +- 17 files changed, 57 insertions(+), 54 deletions(-) diff --git a/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs b/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs index 696941ff3..d86a5f147 100644 --- a/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs +++ b/Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs @@ -23,14 +23,14 @@ public abstract class ConsoleDriver /// Gets the location and size of the terminal screen. internal Rectangle Screen => new (0, 0, Cols, Rows); - private Rectangle _clip; + private Region? _clip = null; /// /// Gets or sets the clip rectangle that and are subject /// to. /// /// The rectangle describing the of region. - public Rectangle Clip + public Region? Clip { get => _clip; set @@ -40,8 +40,13 @@ public abstract class ConsoleDriver return; } + _clip = value; + // Don't ever let Clip be bigger than Screen - _clip = Rectangle.Intersect (Screen, value); + if (_clip is { }) + { + _clip.Intersect (Screen); + } } } @@ -130,6 +135,8 @@ public abstract class ConsoleDriver return; } + Rectangle clipRect = Clip!.GetBounds (); + if (validLocation) { rune = rune.MakePrintable (); @@ -217,14 +224,14 @@ public abstract class ConsoleDriver { Contents [Row, Col].Rune = rune; - if (Col < Clip.Right - 1) + if (Col < clipRect.Right - 1) { Contents [Row, Col + 1].IsDirty = true; } } else if (runeWidth == 2) { - if (Col == Clip.Right - 1) + if (Col == clipRect.Right - 1) { // We're at the right edge of the clip, so we can't display a wide character. // TODO: Figure out if it is better to show a replacement character or ' ' @@ -234,7 +241,7 @@ public abstract class ConsoleDriver { Contents [Row, Col].Rune = rune; - if (Col < Clip.Right - 1) + if (Col < clipRect.Right - 1) { // Invalidate cell to right so that it doesn't get drawn // TODO: Figure out if it is better to show a replacement character or ' ' @@ -264,7 +271,7 @@ public abstract class ConsoleDriver { Debug.Assert (runeWidth <= 2); - if (validLocation && Col < Clip.Right) + if (validLocation && Col < clipRect.Right) { lock (Contents!) { @@ -314,9 +321,11 @@ public abstract class ConsoleDriver public void ClearContents () { Contents = new Cell [Rows, Cols]; + //CONCURRENCY: Unsynchronized access to Clip isn't safe. // TODO: ClearContents should not clear the clip; it should only clear the contents. Move clearing it elsewhere. - Clip = Screen; + Clip = new (Screen); + _dirtyLines = new bool [Rows]; lock (Contents) @@ -375,7 +384,8 @@ public abstract class ConsoleDriver /// The Rune used to fill the rectangle public void FillRect (Rectangle rect, Rune rune = default) { - rect = Rectangle.Intersect (rect, Clip); + // BUGBUG: This should be a method on Region + rect = Rectangle.Intersect (rect, Clip?.GetBounds () ?? Screen); lock (Contents!) { for (int r = rect.Y; r < rect.Y + rect.Height; r++) @@ -427,7 +437,7 @@ public abstract class ConsoleDriver /// public bool IsValidLocation (int col, int row) { - return col >= 0 && row >= 0 && col < Cols && row < Rows && Clip.Contains (col, row); + return col >= 0 && row >= 0 && col < Cols && row < Rows && Clip!.Contains (col, row); } /// diff --git a/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs b/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs index 3c86d0082..1d95d5531 100644 --- a/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs +++ b/Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs @@ -101,7 +101,8 @@ internal class CursesDriver : ConsoleDriver { // Not a valid location (outside screen or clip region) // Move within the clip region, then AddRune will actually move to Col, Row - Curses.move (Clip.Y, Clip.X); + Rectangle clipRect = Clip.GetBounds (); + Curses.move (clipRect.Y, clipRect.X); } } diff --git a/Terminal.Gui/ConsoleDrivers/FakeDriver/FakeDriver.cs b/Terminal.Gui/ConsoleDrivers/FakeDriver/FakeDriver.cs index 97d219cdb..99fdeacf7 100644 --- a/Terminal.Gui/ConsoleDrivers/FakeDriver/FakeDriver.cs +++ b/Terminal.Gui/ConsoleDrivers/FakeDriver/FakeDriver.cs @@ -456,7 +456,7 @@ public class FakeDriver : ConsoleDriver } // CONCURRENCY: Unsynchronized access to Clip is not safe. - Clip = new (0, 0, Cols, Rows); + Clip = new (new (0, 0, Cols, Rows)); } public override void UpdateCursor () diff --git a/Terminal.Gui/ConsoleDrivers/NetDriver.cs b/Terminal.Gui/ConsoleDrivers/NetDriver.cs index f787dfd43..837a4c7f7 100644 --- a/Terminal.Gui/ConsoleDrivers/NetDriver.cs +++ b/Terminal.Gui/ConsoleDrivers/NetDriver.cs @@ -1227,12 +1227,12 @@ internal class NetDriver : ConsoleDriver catch (IOException) { // CONCURRENCY: Unsynchronized access to Clip is not safe. - Clip = new (0, 0, Cols, Rows); + Clip = new (new (0, 0, Cols, Rows)); } catch (ArgumentOutOfRangeException) { // CONCURRENCY: Unsynchronized access to Clip is not safe. - Clip = new (0, 0, Cols, Rows); + Clip = new (new (0, 0, Cols, Rows)); } } else @@ -1241,7 +1241,7 @@ internal class NetDriver : ConsoleDriver } // CONCURRENCY: Unsynchronized access to Clip is not safe. - Clip = new (0, 0, Cols, Rows); + Clip = new (new (0, 0, Cols, Rows)); } #endregion diff --git a/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs b/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs index d6027a6f6..c8912d9af 100644 --- a/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs +++ b/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs @@ -1424,7 +1424,7 @@ internal class WindowsDriver : ConsoleDriver _outputBuffer = new WindowsConsole.ExtendedCharInfo [Rows * Cols]; // CONCURRENCY: Unsynchronized access to Clip is not safe. - Clip = new (0, 0, Cols, Rows); + Clip = new (new (0, 0, Cols, Rows)); _damageRegion = new WindowsConsole.SmallRect { @@ -1844,7 +1844,7 @@ internal class WindowsDriver : ConsoleDriver { _outputBuffer = new WindowsConsole.ExtendedCharInfo [Rows * Cols]; // CONCURRENCY: Unsynchronized access to Clip is not safe. - Clip = new (0, 0, Cols, Rows); + Clip = new (new (0, 0, Cols, Rows)); _damageRegion = new WindowsConsole.SmallRect { diff --git a/Terminal.Gui/View/View.Drawing.Clipping.cs b/Terminal.Gui/View/View.Drawing.Clipping.cs index 36aa5e1b8..cb9c4c54b 100644 --- a/Terminal.Gui/View/View.Drawing.Clipping.cs +++ b/Terminal.Gui/View/View.Drawing.Clipping.cs @@ -1,4 +1,5 @@ -namespace Terminal.Gui; +#nullable enable +namespace Terminal.Gui; public partial class View { @@ -18,17 +19,17 @@ public partial class View /// The current screen-relative clip region, which can be then re-applied by setting /// . /// - public Rectangle SetClip () + public Region? SetClip () { if (Driver is null) { - return Rectangle.Empty; + return null; } - Rectangle previous = Driver.Clip; + Region previous = Driver.Clip ?? new (Application.Screen); // Clamp the Clip to the entire visible area - Rectangle clip = Rectangle.Intersect (ViewportToScreen (Viewport with { Location = Point.Empty }), previous); + Rectangle clip = Rectangle.Intersect (ViewportToScreen (Viewport with { Location = Point.Empty }), previous.GetBounds()); if (ViewportSettings.HasFlag (ViewportSettings.ClipContentOnly)) { @@ -37,7 +38,7 @@ public partial class View clip = Rectangle.Intersect (clip, visibleContent); } - Driver.Clip = clip; + Driver.Clip = new (clip);// !.Complement(clip); return previous; } diff --git a/Terminal.Gui/View/View.Drawing.Primitives.cs b/Terminal.Gui/View/View.Drawing.Primitives.cs index 24ddfcb6b..4f8ed4c45 100644 --- a/Terminal.Gui/View/View.Drawing.Primitives.cs +++ b/Terminal.Gui/View/View.Drawing.Primitives.cs @@ -116,17 +116,11 @@ public partial class View return; } - // Get screen-relative coords + Region prevClip = SetClip (); Rectangle toClear = ViewportToScreen (rect); - - Rectangle prevClip = Driver.Clip; - - Driver.Clip = Rectangle.Intersect (prevClip, ViewportToScreen (Viewport with { Location = new (0, 0) })); - Attribute prev = SetAttribute (new (color ?? GetNormalColor ().Background)); Driver.FillRect (toClear); SetAttribute (prev); - Driver.Clip = prevClip; } diff --git a/Terminal.Gui/View/View.Drawing.cs b/Terminal.Gui/View/View.Drawing.cs index c94e39695..4ce67fcf3 100644 --- a/Terminal.Gui/View/View.Drawing.cs +++ b/Terminal.Gui/View/View.Drawing.cs @@ -33,7 +33,7 @@ public partial class View // Drawing APIs // By default, we clip to the viewport preventing drawing outside the viewport // We also clip to the content, but if a developer wants to draw outside the viewport, they can do // so via settings. SetClip honors the ViewportSettings.DisableVisibleContentClipping flag. - Rectangle prevClip = SetClip (); + Region prevClip = SetClip (); DoClearViewport (Viewport); DoDrawText (Viewport); @@ -251,8 +251,6 @@ public partial class View // Drawing APIs // Get screen-relative coords Rectangle toClear = ViewportToScreen (Viewport with { Location = new (0, 0) }); - Rectangle prevClip = Driver.Clip; - if (ViewportSettings.HasFlag (ViewportSettings.ClearContentOnly)) { Rectangle visibleContent = ViewportToScreen (new Rectangle (new (-Viewport.X, -Viewport.Y), GetContentSize ())); @@ -262,8 +260,6 @@ public partial class View // Drawing APIs Attribute prev = SetAttribute (GetNormalColor ()); Driver.FillRect (toClear); SetAttribute (prev); - - Driver.Clip = prevClip; SetNeedsDraw (); } diff --git a/Terminal.Gui/Views/Menu/Menu.cs b/Terminal.Gui/Views/Menu/Menu.cs index de9b47de0..5b7887a75 100644 --- a/Terminal.Gui/Views/Menu/Menu.cs +++ b/Terminal.Gui/Views/Menu/Menu.cs @@ -409,8 +409,8 @@ internal sealed class Menu : View DrawAdornments (); RenderLineCanvas (); - Rectangle savedClip = Driver.Clip; - Driver.Clip = new (0, 0, Driver.Cols, Driver.Rows); + Region savedClip = Driver.Clip; + Driver.Clip = new (new (0, 0, Driver.Cols, Driver.Rows)); SetAttribute (GetNormalColor ()); for (int i = Viewport.Y; i < _barItems!.Children.Length; i++) diff --git a/Terminal.Gui/Views/ScrollBarView.cs b/Terminal.Gui/Views/ScrollBarView.cs index 12e53e2bf..02fc2b355 100644 --- a/Terminal.Gui/Views/ScrollBarView.cs +++ b/Terminal.Gui/Views/ScrollBarView.cs @@ -769,7 +769,7 @@ public class ScrollBarView : View // I'm forced to do this here because the Clear method is // changing the color attribute and is different of this one - Driver.FillRect (Driver.Clip); + Driver.FillRect (Driver.Clip.GetBounds()); e.Cancel = true; } diff --git a/Terminal.Gui/Views/TabView.cs b/Terminal.Gui/Views/TabView.cs index bf22e4ece..f98e82ebf 100644 --- a/Terminal.Gui/Views/TabView.cs +++ b/Terminal.Gui/Views/TabView.cs @@ -310,7 +310,7 @@ public class TabView : View { if (Tabs.Any ()) { - Rectangle savedClip = SetClip (); + Region savedClip = SetClip (); _tabsBar.Draw (); _contentView.SetNeedsDraw (); _contentView.Draw (); diff --git a/UICatalog/Scenarios/AllViewsTester.cs b/UICatalog/Scenarios/AllViewsTester.cs index 251017611..4bd3e7868 100644 --- a/UICatalog/Scenarios/AllViewsTester.cs +++ b/UICatalog/Scenarios/AllViewsTester.cs @@ -200,6 +200,7 @@ public class AllViewsTester : Scenario _hostPane = new () { + Id = "_hostPane", X = Pos.Right (_adornmentsEditor), Y = Pos.Bottom (_settingsPane), Width = Dim.Width (_layoutEditor), diff --git a/UnitTests/ConsoleDrivers/ClipRegionTests.cs b/UnitTests/ConsoleDrivers/ClipRegionTests.cs index 73eff741d..26d6fbe1d 100644 --- a/UnitTests/ConsoleDrivers/ClipRegionTests.cs +++ b/UnitTests/ConsoleDrivers/ClipRegionTests.cs @@ -42,7 +42,7 @@ public class ClipRegionTests Assert.Equal ((Rune)' ', driver.Contents [0, 0].Rune); // Setup the region with a single rectangle, fill screen with 'x' - driver.Clip = new Rectangle (5, 5, 5, 5); + driver.Clip = new (new Rectangle (5, 5, 5, 5)); driver.FillRect (new Rectangle (0, 0, driver.Rows, driver.Cols), 'x'); Assert.Equal ((Rune)' ', driver.Contents [0, 0].Rune); Assert.Equal ((Rune)' ', driver.Contents [4, 9].Rune); @@ -66,7 +66,7 @@ public class ClipRegionTests Application.Init (driver); // Define a clip rectangle - driver.Clip = Rectangle.Empty; + driver.Clip = new (Rectangle.Empty); // negative Assert.False (driver.IsValidLocation (4, 5)); @@ -111,7 +111,7 @@ public class ClipRegionTests Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows)); // Define a clip rectangle - driver.Clip = new Rectangle (5, 5, 5, 5); + driver.Clip = new(new Rectangle(5, 5, 5, 5)); // positive Assert.True (driver.IsValidLocation (5, 5)); diff --git a/UnitTests/View/Draw/DrawTests.cs b/UnitTests/View/Draw/DrawTests.cs index fa09c6ea5..2dabd9d9c 100644 --- a/UnitTests/View/Draw/DrawTests.cs +++ b/UnitTests/View/Draw/DrawTests.cs @@ -741,7 +741,7 @@ public class DrawTests (ITestOutputHelper _output) return; - void Top_LayoutComplete (object? sender, LayoutEventArgs e) { Application.Driver!.Clip = container.Frame; } + void Top_LayoutComplete (object? sender, LayoutEventArgs e) { Application.Driver!.Clip = new (container.Frame); } } [Fact] @@ -944,13 +944,13 @@ public class DrawTests (ITestOutputHelper _output) view.Border.Thickness = new (1); view.BeginInit (); view.EndInit (); - Assert.Equal (view.Frame, Application.Driver?.Clip); + Assert.Equal (view.Frame, Application.Driver?.Clip?.GetBounds ()); // Act view.SetClip (); // Assert - Assert.Equal (expectedClip, Application.Driver?.Clip); + Assert.Equal (expectedClip, Application.Driver?.Clip?.GetBounds()); view.Dispose (); } @@ -977,14 +977,14 @@ public class DrawTests (ITestOutputHelper _output) view.Border.Thickness = new (1); view.BeginInit (); view.EndInit (); - Assert.Equal (view.Frame, Application.Driver?.Clip); + Assert.Equal (view.Frame, Application.Driver?.Clip.GetBounds ()); view.Viewport = view.Viewport with { X = 1, Y = 1 }; // Act view.SetClip (); // Assert - Assert.Equal (expectedClip, Application.Driver?.Clip); + Assert.Equal (expectedClip, Application.Driver?.Clip.GetBounds ()); view.Dispose (); } diff --git a/UnitTests/View/ViewTests.cs b/UnitTests/View/ViewTests.cs index b50aa3ca4..63f76d83c 100644 --- a/UnitTests/View/ViewTests.cs +++ b/UnitTests/View/ViewTests.cs @@ -14,8 +14,8 @@ public class ViewTests (ITestOutputHelper output) view.DrawingContent += (s, e) => { - Rectangle savedClip = Application.Driver!.Clip; - Application.Driver!.Clip = new (1, 1, view.Viewport.Width, view.Viewport.Height); + Region savedClip = Application.Driver!.Clip; + Application.Driver!.Clip = new (new (1, 1, view.Viewport.Width, view.Viewport.Height)); for (var row = 0; row < view.Viewport.Height; row++) { @@ -78,8 +78,8 @@ public class ViewTests (ITestOutputHelper output) view.DrawingContent += (s, e) => { - Rectangle savedClip = Application.Driver!.Clip; - Application.Driver!.Clip = new (1, 1, view.Viewport.Width, view.Viewport.Height); + Region savedClip = Application.Driver!.Clip; + Application.Driver!.Clip = new (new (1, 1, view.Viewport.Width, view.Viewport.Height)); for (var row = 0; row < view.Viewport.Height; row++) { diff --git a/UnitTests/Views/ContextMenuTests.cs b/UnitTests/Views/ContextMenuTests.cs index 74369fc64..bbfd0ed77 100644 --- a/UnitTests/Views/ContextMenuTests.cs +++ b/UnitTests/Views/ContextMenuTests.cs @@ -132,7 +132,7 @@ public class ContextMenuTests (ITestOutputHelper output) { ((FakeDriver)Application.Driver!).SetBufferSize (20, 15); - Assert.Equal (new Rectangle (0, 0, 20, 15), Application.Driver?.Clip); + Assert.Equal (new Rectangle (0, 0, 20, 15), Application.Driver?.Clip.GetBounds ()); TestHelpers.AssertDriverContentsWithFrameAre ("", output); var top = new Toplevel { X = 2, Y = 2, Width = 15, Height = 4 }; @@ -268,7 +268,7 @@ public class ContextMenuTests (ITestOutputHelper output) { ((FakeDriver)Application.Driver!).SetBufferSize (20, 15); - Assert.Equal (new Rectangle (0, 0, 20, 15), Application.Driver?.Clip); + Assert.Equal (new Rectangle (0, 0, 20, 15), Application.Driver?.Clip.GetBounds ()); TestHelpers.AssertDriverContentsWithFrameAre ("", output); // Don't use Dialog here as it has more layout logic. Use Window instead. diff --git a/UnitTests/Views/MenuBarTests.cs b/UnitTests/Views/MenuBarTests.cs index 47b234b61..74cea6ff7 100644 --- a/UnitTests/Views/MenuBarTests.cs +++ b/UnitTests/Views/MenuBarTests.cs @@ -698,7 +698,7 @@ public class MenuBarTests (ITestOutputHelper output) Dialog.DefaultShadow = ShadowStyle.None; Button.DefaultShadow = ShadowStyle.None; - Assert.Equal (new (0, 0, 40, 15), Application.Driver?.Clip); + Assert.Equal (new (0, 0, 40, 15), Application.Driver?.Clip.GetBounds()); TestHelpers.AssertDriverContentsWithFrameAre (@"", output); List items = new ()