Fixes #5127. Anchor button delimiters to edges; fix highlight continuity

Terminal.Gui/Views/Button.cs

@@ -43,6 +43,7 @@
 /// </remarks>
 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<string> e)
     {
         base.Text = e.Value;
         TextFormatter.HotKeySpecifier = HotKeySpecifier;
+        _interiorTextFormatter.HotKeySpecifier = HotKeySpecifier;
     }
 
     /// <inheritdoc/>
     public override string Text { get => Title; set => base.Text = Title = value; }
 
     /// <inheritdoc/>
-    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;
+    }
 
     /// <inheritdoc/>
     public bool IsDefault
@@ -208,26 +218,113 @@ public bool IsDefault
     protected override void UpdateTextFormatterText ()
     {
         base.UpdateTextFormatterText ();
+        TextFormatter.Text = GetDecoratedText ();
+    }
 
-        if (NoDecorations)
+    /// <inheritdoc/>
+    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;
     }
 
     /// <inheritdoc/>
@@ -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
+        };
+    }
 }

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;
+
+/// <summary>
+///     Benchmarks for <see cref="Button"/> draw performance, focused on the cost of
+///     <c>_interiorTextFormatter</c> property assignments in <c>OnDrawingText</c>.
+/// </summary>
+/// <remarks>
+///     <para>
+///         Prior to the guards introduced in PR #5279, every call to <c>OnDrawingText</c> set all
+///         <c>_interiorTextFormatter</c> properties unconditionally, which always set
+///         <c>NeedsFormat = true</c> and forced the formatter to re-allocate on the next access —
+///         even when nothing had changed.  After the fix, unchanged values are skipped.
+///     </para>
+///     <para>
+///         <b>How to read results:</b>
+///         <list type="bullet">
+///             <item><see cref="DrawButton_Unchanged"/> represents the guard-optimized steady-state path.</item>
+///             <item><see cref="DrawButton_TextChanging"/> forces a real reformat each iteration,
+///             approximating the old unguarded behaviour.</item>
+///         </list>
+///         The allocation difference between the two methods shows the overhead that the guards eliminate.
+///     </para>
+///     <para>
+///         Run:
+///         <code>dotnet run --project Tests/Benchmarks -c Release -- --filter "*ButtonDraw*"</code>
+///     </para>
+/// </remarks>
+[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;
+
+    /// <summary>Fixed width of the button under test.</summary>
+    [Params (10, 40)]
+    public int Width { get; set; }
+
+    /// <summary>Whether the button is the default button (adds extra decoration characters).</summary>
+    [Params (false, true)]
+    public bool IsDefault { get; set; }
+
+    /// <summary>Create the application and button once per parameter combination.</summary>
+    [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 ();
+    }
+
+    /// <summary>Mark the button as needing a redraw before each iteration.</summary>
+    /// <remarks>This ensures <c>OnDrawingText</c> is always called, while keeping all
+    /// formatter property values identical to the previous draw — isolating the guard benefit.</remarks>
+    [IterationSetup]
+    public void MarkNeedsDraw ()
+    {
+        _button.SetNeedsDraw ();
+    }
+
+    /// <summary>
+    ///     Draws the button with no property changes since the last draw.
+    ///     Guards on <c>_interiorTextFormatter</c> skip all assignments, so <c>NeedsFormat</c>
+    ///     is not set and the formatter does not re-allocate.
+    /// </summary>
+    [Benchmark (Baseline = true)]
+    public void DrawButton_Unchanged ()
+    {
+        _app.LayoutAndDraw ();
+    }
+
+    /// <summary>
+    ///     Rotates the button <see cref="View.Text"/> before each draw.
+    ///     The interior text changes, so the <c>Text</c> guard fires, <c>NeedsFormat</c> is
+    ///     set, and the formatter re-allocates — approximating the old unguarded behaviour.
+    /// </summary>
+    [Benchmark]
+    public void DrawButton_TextChanging ()
+    {
+        _button.Text = _changingTexts [_textIndex++ % _changingTexts.Length];
+        _app.LayoutAndDraw ();
+    }
+
+    /// <summary>Dispose the application after all iterations.</summary>
+    [GlobalCleanup]
+    public void Cleanup ()
+    {
+        if (_session is not null)
+        {
+            _app.End (_session);
+        }
+
+        _app.Dispose ();
+    }
+}