diff --git a/Terminal.Gui/Views/Button.cs b/Terminal.Gui/Views/Button.cs index 3e4d1b61bb..76cddab16a 100644 --- a/Terminal.Gui/Views/Button.cs +++ b/Terminal.Gui/Views/Button.cs @@ -43,6 +43,7 @@ /// public class Button : View, IDesignable, IAcceptTarget { + private readonly TextFormatter _interiorTextFormatter = new (); private readonly Rune _leftBracket; private readonly Rune _leftDefault; private readonly Rune _rightBracket; @@ -71,6 +72,10 @@ public Button () _leftDefault = Glyphs.LeftDefaultIndicator; _rightDefault = Glyphs.RightDefaultIndicator; + _interiorTextFormatter.Alignment = Alignment.Center; + _interiorTextFormatter.VerticalAlignment = Alignment.Center; + _interiorTextFormatter.HotKeySpecifier = HotKeySpecifier; + Height = Dim.Auto (DimAutoStyle.Text); Width = Dim.Auto (DimAutoStyle.Text); @@ -167,13 +172,18 @@ private void Button_TitleChanged (object? sender, EventArgs e) { base.Text = e.Value; TextFormatter.HotKeySpecifier = HotKeySpecifier; + _interiorTextFormatter.HotKeySpecifier = HotKeySpecifier; } /// public override string Text { get => Title; set => base.Text = Title = value; } /// - public override Rune HotKeySpecifier { get => base.HotKeySpecifier; set => TextFormatter.HotKeySpecifier = base.HotKeySpecifier = value; } + public override Rune HotKeySpecifier + { + get => base.HotKeySpecifier; + set => _interiorTextFormatter.HotKeySpecifier = TextFormatter.HotKeySpecifier = base.HotKeySpecifier = value; + } /// public bool IsDefault @@ -208,26 +218,113 @@ public bool IsDefault protected override void UpdateTextFormatterText () { base.UpdateTextFormatterText (); + TextFormatter.Text = GetDecoratedText (); + } - if (NoDecorations) + /// + protected override bool OnDrawingText (DrawContext? context) + { + if (NoDecorations || Driver is null) { - TextFormatter.Text = Text; + return base.OnDrawingText (context); } - else if (IsDefault) + + Rectangle drawRect = new (ContentToScreen (Point.Empty), GetContentSize ()); + + if (drawRect.Width < 2 || drawRect.Height < 1) { - TextFormatter.Text = $"{_leftBracket}{_leftDefault} {Text} {_rightDefault}{_rightBracket}"; + return base.OnDrawingText (context); } - else + + Rectangle interiorRect = new (drawRect.X + 1, drawRect.Y, drawRect.Width - 2, drawRect.Height); + Attribute normalAttr = HasFocus ? GetAttributeForRole (VisualRole.Focus) : GetAttributeForRole (VisualRole.Normal); + Attribute hotAttr = HasFocus ? GetAttributeForRole (VisualRole.HotFocus) : GetAttributeForRole (VisualRole.HotNormal); + string interiorText = GetInteriorText (); + + // Mirror all TextFormatter settings onto _interiorTextFormatter. Guard each assignment + // so NeedsFormat is not set when a value is unchanged. + if (_interiorTextFormatter.Text != interiorText) { - if (NoPadding) - { - TextFormatter.Text = $"{_leftBracket}{Text}{_rightBracket}"; - } - else - { - TextFormatter.Text = $"{_leftBracket} {Text} {_rightBracket}"; - } + _interiorTextFormatter.Text = interiorText; + } + + if (_interiorTextFormatter.Alignment != TextAlignment) + { + _interiorTextFormatter.Alignment = TextAlignment; + } + + if (_interiorTextFormatter.VerticalAlignment != VerticalTextAlignment) + { + _interiorTextFormatter.VerticalAlignment = VerticalTextAlignment; + } + + if (_interiorTextFormatter.Direction != TextDirection) + { + _interiorTextFormatter.Direction = TextDirection; + } + + if (_interiorTextFormatter.PreserveTrailingSpaces != PreserveTrailingSpaces) + { + _interiorTextFormatter.PreserveTrailingSpaces = PreserveTrailingSpaces; + } + + if (_interiorTextFormatter.MultiLine != TextFormatter.MultiLine) + { + _interiorTextFormatter.MultiLine = TextFormatter.MultiLine; + } + + if (_interiorTextFormatter.WordWrap != TextFormatter.WordWrap) + { + _interiorTextFormatter.WordWrap = TextFormatter.WordWrap; + } + + if (_interiorTextFormatter.TabWidth != TextFormatter.TabWidth) + { + _interiorTextFormatter.TabWidth = TextFormatter.TabWidth; + } + + if (_interiorTextFormatter.PreserveTabs != TextFormatter.PreserveTabs) + { + _interiorTextFormatter.PreserveTabs = TextFormatter.PreserveTabs; + } + + if (_interiorTextFormatter.ConstrainToWidth != interiorRect.Width) + { + _interiorTextFormatter.ConstrainToWidth = interiorRect.Width; + } + + if (_interiorTextFormatter.ConstrainToHeight != interiorRect.Height) + { + _interiorTextFormatter.ConstrainToHeight = interiorRect.Height; } + + Region? interiorDrawRegion = (interiorRect.Width > 0 && !string.IsNullOrEmpty (interiorText)) + ? _interiorTextFormatter.GetDrawRegion (interiorRect) + : null; + + context?.AddDrawnRegion (new Region (drawRect)); + + int delimiterRow = GetDelimiterRow (drawRect, interiorDrawRegion); + + // Fill the entire content area with the focus/normal attribute to ensure continuous + // highlight across all rows and across the bracket columns. + Driver.SetAttribute (normalAttr); + Driver.FillRect (drawRect); + + Driver.Move (drawRect.X, delimiterRow); + Driver.AddRune (_leftBracket); + + Driver.Move (drawRect.X + drawRect.Width - 1, delimiterRow); + Driver.AddRune (_rightBracket); + + if (interiorDrawRegion is not null) + { + _interiorTextFormatter.Draw (Driver, interiorRect, normalAttr, hotAttr, Rectangle.Empty); + } + + SetSubViewNeedsDrawDownHierarchy (); + + return true; } /// @@ -237,4 +334,51 @@ public bool EnableForDesign () return true; } + + // GetDecoratedText (called by UpdateTextFormatterText for Dim.Auto sizing) and GetInteriorText + // (called by OnDrawingText for rendering) must remain in sync when modifying button text formatting. + private string GetDecoratedText () + { + if (NoDecorations) + { + return Text; + } + + return $"{_leftBracket}{GetInteriorText ()}{_rightBracket}"; + } + + private string GetInteriorText () + { + if (IsDefault) + { + return $"{_leftDefault} {Text} {_rightDefault}"; + } + + if (NoPadding) + { + return Text; + } + + return $" {Text} "; + } + + private int GetDelimiterRow (Rectangle drawRect, Region? interiorDrawRegion) + { + if (interiorDrawRegion is not null) + { + Rectangle interiorBounds = interiorDrawRegion.GetBounds (); + + if (!interiorBounds.IsEmpty) + { + return interiorBounds.Y; + } + } + + return VerticalTextAlignment switch + { + Alignment.End => drawRect.Y + drawRect.Height - 1, + Alignment.Center => drawRect.Y + (drawRect.Height - 1) / 2, + _ => drawRect.Y + }; + } } diff --git a/Tests/Benchmarks/Views/ButtonDrawBenchmark.cs b/Tests/Benchmarks/Views/ButtonDrawBenchmark.cs new file mode 100644 index 0000000000..01a24884b4 --- /dev/null +++ b/Tests/Benchmarks/Views/ButtonDrawBenchmark.cs @@ -0,0 +1,126 @@ +using BenchmarkDotNet.Attributes; +using Terminal.Gui.App; +using Terminal.Gui.Drivers; +using Terminal.Gui.Drawing; +using Terminal.Gui.ViewBase; +using Terminal.Gui.Views; + +namespace Terminal.Gui.Benchmarks.Views; + +/// +/// Benchmarks for draw performance, focused on the cost of +/// _interiorTextFormatter property assignments in OnDrawingText. +/// +/// +/// +/// Prior to the guards introduced in PR #5279, every call to OnDrawingText set all +/// _interiorTextFormatter properties unconditionally, which always set +/// NeedsFormat = true and forced the formatter to re-allocate on the next access — +/// even when nothing had changed. After the fix, unchanged values are skipped. +/// +/// +/// How to read results: +/// +/// represents the guard-optimized steady-state path. +/// forces a real reformat each iteration, +/// approximating the old unguarded behaviour. +/// +/// The allocation difference between the two methods shows the overhead that the guards eliminate. +/// +/// +/// Run: +/// dotnet run --project Tests/Benchmarks -c Release -- --filter "*ButtonDraw*" +/// +/// +[MemoryDiagnoser] +[BenchmarkCategory ("Views", "Button")] +public class ButtonDrawBenchmark +{ + // Four interior texts of equal display width so layout stays stable between iterations. + private static readonly string [] _changingTexts = ["_OK", "_No", "_Go", "_Up"]; + + private IApplication _app = null!; + private Button _button = null!; + private int _textIndex; + private SessionToken? _session; + + /// Fixed width of the button under test. + [Params (10, 40)] + public int Width { get; set; } + + /// Whether the button is the default button (adds extra decoration characters). + [Params (false, true)] + public bool IsDefault { get; set; } + + /// Create the application and button once per parameter combination. + [GlobalSetup] + public void Setup () + { + _app = Application.Create (); + _app.Init (DriverRegistry.Names.ANSI); + _app.Driver!.SetScreenSize (Width, 1); + + Runnable runnable = new () { Width = Width, Height = 1 }; + _session = _app.Begin (runnable); + + _button = new () + { + Text = "_OK", + X = 0, + Y = 0, + Width = Width, + Height = 1, + IsDefault = IsDefault, + ShadowStyle = ShadowStyles.None + }; + + runnable.Add (_button); + + // Warm-up draw so caches and JIT paths are primed before measurement. + _app.LayoutAndDraw (); + } + + /// Mark the button as needing a redraw before each iteration. + /// This ensures OnDrawingText is always called, while keeping all + /// formatter property values identical to the previous draw — isolating the guard benefit. + [IterationSetup] + public void MarkNeedsDraw () + { + _button.SetNeedsDraw (); + } + + /// + /// Draws the button with no property changes since the last draw. + /// Guards on _interiorTextFormatter skip all assignments, so NeedsFormat + /// is not set and the formatter does not re-allocate. + /// + [Benchmark (Baseline = true)] + public void DrawButton_Unchanged () + { + _app.LayoutAndDraw (); + } + + /// + /// Rotates the button before each draw. + /// The interior text changes, so the Text guard fires, NeedsFormat is + /// set, and the formatter re-allocates — approximating the old unguarded behaviour. + /// + [Benchmark] + public void DrawButton_TextChanging () + { + _button.Text = _changingTexts [_textIndex++ % _changingTexts.Length]; + _app.LayoutAndDraw (); + } + + /// Dispose the application after all iterations. + [GlobalCleanup] + public void Cleanup () + { + if (_session is not null) + { + _app.End (_session); + } + + _app.Dispose (); + } +} diff --git a/Tests/UnitTestsParallelizable/Views/ButtonDrawingTests.cs b/Tests/UnitTestsParallelizable/Views/ButtonDrawingTests.cs new file mode 100644 index 0000000000..65b0ed602b --- /dev/null +++ b/Tests/UnitTestsParallelizable/Views/ButtonDrawingTests.cs @@ -0,0 +1,121 @@ +using UnitTests; + +namespace ViewsTests; + +public class ButtonDrawingTests (ITestOutputHelper output) : TestDriverBase +{ + // Copilot + [Theory] + [InlineData (false)] + [InlineData (true)] + public void FixedWidth_Anchors_Delimiters_To_Edges (bool isDefault) + { + const int width = 10; + string expected = isDefault + ? $"{Glyphs.LeftBracket} {Glyphs.LeftDefaultIndicator} OK {Glyphs.RightDefaultIndicator} {Glyphs.RightBracket}" + : $"{Glyphs.LeftBracket} OK {Glyphs.RightBracket}"; + + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + app.Driver!.SetScreenSize (width, 1); + + Runnable runnable = new () { Width = width, Height = 1 }; + app.Begin (runnable); + + Button button = new () + { + Text = "_OK", + X = 0, + Y = 0, + Width = width, + Height = 1, + IsDefault = isDefault, + ShadowStyle = null + }; + + runnable.Add (button); + app.LayoutAndDraw (); + + DriverAssert.AssertDriverContentsWithFrameAre (expected, output, app.Driver); + } + + // Copilot + [Fact] + public void Focused_FixedWidth_Button_MultiRow_Highlight_Is_Continuous () + { + const int width = 10; + const int height = 3; + + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + app.Driver!.SetScreenSize (width, height); + + Runnable runnable = new () { Width = width, Height = height }; + app.Begin (runnable); + + Button button = new () + { + Text = "_OK", + X = 0, + Y = 0, + Width = width, + Height = height, + ShadowStyle = null + }; + + runnable.Add (button); + button.SetFocus (); + app.LayoutAndDraw (); + + // All rows must carry the Focus attribute — verifies the highlight is continuous + // across rows above and below the text (not just the delimiter row). + DriverAssert.AssertDriverAttributesAre ( + """ + 0000000000 + 0000100000 + 0000000000 + """, + output, + app.Driver, + button.GetAttributeForRole (VisualRole.Focus), + button.GetAttributeForRole (VisualRole.HotFocus) + ); + } + + // Copilot + [Fact] + public void Focused_FixedWidth_Button_Highlight_Is_Continuous () + { + const int width = 10; + + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + app.Driver!.SetScreenSize (width, 1); + + Runnable runnable = new () { Width = width, Height = 1 }; + app.Begin (runnable); + + Button button = new () + { + Text = "_OK", + X = 0, + Y = 0, + Width = width, + Height = 1, + ShadowStyle = null + }; + + runnable.Add (button); + button.SetFocus (); + app.LayoutAndDraw (); + + DriverAssert.AssertDriverContentsWithFrameAre ($"{Glyphs.LeftBracket} OK {Glyphs.RightBracket}", output, app.Driver); + DriverAssert.AssertDriverAttributesAre ( + "0000100000", + output, + app.Driver, + button.GetAttributeForRole (VisualRole.Focus), + button.GetAttributeForRole (VisualRole.HotFocus) + ); + } +}