Fixes #4453. Regression in wide glyph rendering on all drivers (#4458)

Examples/UICatalog/Scenarios/WideGlyphs.cs

@@ -0,0 +1,129 @@
+#nullable enable
+
+using System.Text;
+
+namespace UICatalog.Scenarios;
+
+[ScenarioMetadata ("WideGlyphs", "Demonstrates wide glyphs with overlapped views & clipping")]
+[ScenarioCategory ("Unicode")]
+[ScenarioCategory ("Drawing")]
+
+public sealed class WideGlyphs : Scenario
+{
+    private Rune [,]? _codepoints;
+
+    public override void Main ()
+    {
+        // Init
+        Application.Init ();
+
+        // Setup - Create a top-level application window and configure it.
+        Window appWindow = new ()
+        {
+            Title = GetQuitKeyAndName (),
+            BorderStyle = LineStyle.None
+        };
+
+        // Build the array of codepoints once when subviews are laid out
+        appWindow.SubViewsLaidOut += (s, e) =>
+        {
+            View? view = s as View;
+            if (view is null)
+            {
+                return;
+            }
+
+            // Only rebuild if size changed or array is null
+            if (_codepoints is null || 
+                _codepoints.GetLength (0) != view.Viewport.Height || 
+                _codepoints.GetLength (1) != view.Viewport.Width)
+            {
+                _codepoints = new Rune [view.Viewport.Height, view.Viewport.Width];
+
+                for (int r = 0; r < view.Viewport.Height; r++)
+                {
+                    for (int c = 0; c < view.Viewport.Width; c += 2)
+                    {
+                        _codepoints [r, c] = GetRandomWideCodepoint ();
+                    }
+                }
+            }
+        };
+
+        // Fill the window with the pre-built codepoints array
+        appWindow.DrawingContent += (s, e) =>
+        {
+            View? view = s as View;
+            if (view is null || _codepoints is null)
+            {
+                return;
+            }
+
+            // Traverse the Viewport, using the pre-built array
+            for (int r = 0; r < view.Viewport.Height && r < _codepoints.GetLength (0); r++)
+            {
+                for (int c = 0; c < view.Viewport.Width && c < _codepoints.GetLength (1); c += 2)
+                {
+                    Rune codepoint = _codepoints [r, c];
+                    if (codepoint != default (Rune))
+                    {
+                        view.AddRune (c, r, codepoint);
+                    }
+                }
+            }
+        };
+
+        Line verticalLineAtEven = new Line ()
+        {
+            X = 10,
+            Orientation = Orientation.Vertical,
+            Length = Dim.Fill ()
+        };
+        appWindow.Add (verticalLineAtEven);
+
+        Line verticalLineAtOdd = new Line ()
+        {
+            X = 25,
+            Orientation = Orientation.Vertical,
+            Length = Dim.Fill ()
+        };
+        appWindow.Add (verticalLineAtOdd);
+
+        View arrangeableViewAtEven = new ()
+        {
+            CanFocus = true,
+            Arrangement = ViewArrangement.Movable | ViewArrangement.Resizable,
+            X = 30,
+            Y = 5,
+            Width = 15,
+            Height = 5,
+            BorderStyle = LineStyle.Dashed,
+        };
+        appWindow.Add (arrangeableViewAtEven);
+
+        View arrangeableViewAtOdd = new ()
+        {
+            CanFocus = true,
+            Arrangement = ViewArrangement.Movable | ViewArrangement.Resizable,
+            X = 31,
+            Y = 11,
+            Width = 15,
+            Height = 5,
+            BorderStyle = LineStyle.Dashed,
+        };
+        appWindow.Add (arrangeableViewAtOdd);
+        // Run - Start the application.
+        Application.Run (appWindow);
+        appWindow.Dispose ();
+
+        // Shutdown - Calling Application.Shutdown is required.
+        Application.Shutdown ();
+    }
+
+    private Rune GetRandomWideCodepoint ()
+    {
+        Random random = new ();
+        int codepoint = random.Next (0x4E00, 0x9FFF);
+        return new Rune (codepoint);
+    }
+}

Terminal.Gui/Drivers/OutputBase.cs

@@ -89,7 +89,7 @@ public abstract class OutputBase
                     {
                         if (output.Length > 0)
                         {
-                            WriteToConsole (output, ref lastCol, row, ref outputWidth);
+                            WriteToConsole (output, ref lastCol, ref outputWidth);
                         }
                         else if (lastCol == -1)
                         {
@@ -112,11 +112,8 @@ public abstract class OutputBase
                     }
 
                     Cell cell = buffer.Contents [row, col];
-                    AppendCellAnsi (cell, output, ref redrawAttr, ref _redrawTextStyle, cols, ref col);
-
-                    outputWidth++;
-
                     buffer.Contents [row, col].IsDirty = false;
+                    AppendCellAnsi (cell, output, ref redrawAttr, ref _redrawTextStyle, cols, ref col, ref outputWidth);
                 }
             }
 
@@ -221,7 +218,8 @@ public abstract class OutputBase
                 }
 
                 Cell cell = buffer.Contents! [row, col];
-                AppendCellAnsi (cell, output, ref lastAttr, ref redrawTextStyle, endCol, ref col);
+                int outputWidth = -1;
+                AppendCellAnsi (cell, output, ref lastAttr, ref redrawTextStyle, endCol, ref col, ref outputWidth);
             }
 
             // Add newline at end of row if requested
@@ -241,7 +239,8 @@ public abstract class OutputBase
     /// <param name="redrawTextStyle">The current text style for optimization.</param>
     /// <param name="maxCol">The maximum column, used for wide character handling.</param>
     /// <param name="currentCol">The current column, updated for wide characters.</param>
-    protected void AppendCellAnsi (Cell cell, StringBuilder output, ref Attribute? lastAttr, ref TextStyle redrawTextStyle, int maxCol, ref int currentCol)
+    /// <param name="outputWidth">The current output width, updated for wide characters.</param>
+    protected void AppendCellAnsi (Cell cell, StringBuilder output, ref Attribute? lastAttr, ref TextStyle redrawTextStyle, int maxCol, ref int currentCol, ref int outputWidth)
     {
         Attribute? attribute = cell.Attribute;
 
@@ -256,11 +255,13 @@ public abstract class OutputBase
         // Add the grapheme
         string grapheme = cell.Grapheme;
         output.Append (grapheme);
+        outputWidth++;
 
         // Handle wide grapheme
         if (grapheme.GetColumns () > 1 && currentCol + 1 < maxCol)
         {
             currentCol++; // Skip next cell for wide character
+            outputWidth++;
         }
     }
 
@@ -280,7 +281,7 @@ public abstract class OutputBase
         return output.ToString ();
     }
 
-    private void WriteToConsole (StringBuilder output, ref int lastCol, int row, ref int outputWidth)
+    private void WriteToConsole (StringBuilder output, ref int lastCol, ref int outputWidth)
     {
         if (IsLegacyConsole)
         {

Terminal.Gui/ViewBase/View.Drawing.cs

@@ -127,7 +127,7 @@ public partial class View // Drawing APIs
             // because they may draw outside the viewport.
             SetClip (originalClip);
             originalClip = AddFrameToClip ();
-            DoRenderLineCanvas ();
+            DoRenderLineCanvas (context);
 
             // ------------------------------------
             // Re-draw the border and padding subviews
@@ -672,8 +672,9 @@ public partial class View // Drawing APIs
 
     #region DrawLineCanvas
 
-    private void DoRenderLineCanvas ()
+    private void DoRenderLineCanvas (DrawContext? context)
     {
+        // TODO: Add context to OnRenderingLineCanvas
         if (!NeedsDraw || OnRenderingLineCanvas ())
         {
             return;
@@ -681,7 +682,7 @@ public partial class View // Drawing APIs
 
         // TODO: Add event
 
-        RenderLineCanvas ();
+        RenderLineCanvas (context);
     }
 
     /// <summary>
@@ -709,7 +710,8 @@ public partial class View // Drawing APIs
     ///     <see cref="LineCanvas"/> of this view's subviews will be rendered. If <see cref="SuperViewRendersLineCanvas"/> is
     ///     false (the default), this method will cause the <see cref="LineCanvas"/> to be rendered.
     /// </summary>
-    public void RenderLineCanvas ()
+    /// <param name="context"></param>
+    public void RenderLineCanvas (DrawContext? context)
     {
         if (Driver is null)
         {
@@ -728,6 +730,9 @@ public partial class View // Drawing APIs
 
                     // TODO: #2616 - Support combining sequences that don't normalize
                     AddStr (p.Value.Value.Grapheme);
+
+                    // Add each drawn cell to the context
+                    context?.AddDrawnRectangle (new Rectangle (p.Key, new (1, 1)) );
                 }
             }
 
@@ -759,9 +764,6 @@ public partial class View // Drawing APIs
                 // Exclude the Border and Padding from the clip
                 ExcludeFromClip (Border?.Thickness.AsRegion (Border.FrameToScreen ()));
                 ExcludeFromClip (Padding?.Thickness.AsRegion (Padding.FrameToScreen ()));
-
-                // QUESTION: This makes it so that no nesting of transparent views is possible, but is more correct?
-                context = new DrawContext ();
             }
             else
             {

Terminal.Gui/Views/GraphView/LegendAnnotation.cs

@@ -1,5 +1,5 @@
 #nullable disable
-namespace Terminal.Gui.Views;
+namespace Terminal.Gui.Views;
 
 /// <summary>
 ///     Used by <see cref="GraphView"/> to render smbol definitions in a graph, e.g. colors and their meanings
@@ -52,8 +52,10 @@ public class LegendAnnotation : View, IAnnotation
 
         if (BorderStyle != LineStyle.None)
         {
+            // BUGBUG: View code should never call Draw directly. This
+            // BUGBUG: needs to be refactored to decouple.
             DrawAdornments ();
-            RenderLineCanvas ();
+            RenderLineCanvas (null);
         }
 
         var linesDrawn = 0;

Tests/UnitTestsParallelizable/Drivers/OutputBaseTests.cs

@@ -1,4 +1,4 @@
-#nullable enable
+#nullable enable
 
 namespace DriverTests;
 
@@ -154,6 +154,62 @@ public class OutputBaseTests
         Assert.Equal (new Point (2, 0), output.GetCursorPosition ());
     }
 
+    [Theory]
+    [InlineData (true)]
+    [InlineData (false)]
+    public void Write_Virtual_Or_NonVirtual_Uses_WriteToConsole_And_Clears_Dirty_Flags_Mixed_Graphemes (bool isLegacyConsole)
+    {
+        // Arrange
+        // FakeOutput exposes this because it's in test scope
+        var output = new FakeOutput { IsLegacyConsole = isLegacyConsole };
+        IOutputBuffer buffer = output.LastBuffer!;
+        buffer.SetSize (3, 1);
+
+        // Write '🦮' at col 0 and 'A' at col 3; leave col 1 untouched (not dirty)
+        buffer.Move (0, 0);
+        buffer.AddStr ("🦮A");
+
+        // Confirm some dirtiness before to write
+        Assert.True (buffer.Contents! [0, 0].IsDirty);
+        Assert.False (buffer.Contents! [0, 1].IsDirty);
+        Assert.True (buffer.Contents! [0, 2].IsDirty);
+
+        // Act
+        output.Write (buffer);
+
+        Assert.Contains ("🦮", output.Output);
+        Assert.Contains ("A", output.Output);
+
+        // Dirty flags cleared for the written cells
+        Assert.False (buffer.Contents! [0, 0].IsDirty);
+        Assert.False (buffer.Contents! [0, 1].IsDirty);
+        Assert.False (buffer.Contents! [0, 2].IsDirty);
+
+        Assert.Equal (new (0, 0), output.GetCursorPosition ());
+
+        // Now write 'X' at col 1 which replaces with the replacement character the col 0
+        buffer.Move (1, 0);
+        buffer.AddStr ("X");
+
+        // Confirm dirtiness state before to write
+        Assert.True (buffer.Contents! [0, 0].IsDirty);
+        Assert.True (buffer.Contents! [0, 1].IsDirty);
+        Assert.True (buffer.Contents! [0, 2].IsDirty);
+
+        output.Write (buffer);
+
+        Assert.Contains ("<22>", output.Output);
+        Assert.Contains ("X", output.Output);
+
+        // Dirty flags cleared for the written cells
+        Assert.False (buffer.Contents! [0, 0].IsDirty);
+        Assert.False (buffer.Contents! [0, 1].IsDirty);
+        Assert.False (buffer.Contents! [0, 2].IsDirty);
+
+        // Verify SetCursorPositionImpl was invoked by WriteToConsole (cursor set to a written column)
+        Assert.Equal (new (0, 0), output.GetCursorPosition ());
+    }
+
     [Theory]
     [InlineData (true)]
     [InlineData (false)]

Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawTextAndLineCanvasTests.cs

@@ -240,7 +240,7 @@ public class ViewDrawTextAndLineCanvasTests () : FakeDriverBase
         Point screenPos = new Point (15, 15);
         view.LineCanvas.AddLine (screenPos, 5, Orientation.Horizontal, LineStyle.Single);
 
-        view.RenderLineCanvas ();
+        view.RenderLineCanvas (null);
 
         // Verify the line was drawn (check for horizontal line character)
         for (int i = 0; i < 5; i++)
@@ -272,7 +272,7 @@ public class ViewDrawTextAndLineCanvasTests () : FakeDriverBase
 
         Assert.NotEqual (Rectangle.Empty, view.LineCanvas.Bounds);
 
-        view.RenderLineCanvas ();
+        view.RenderLineCanvas (null);
 
         // LineCanvas should be cleared after rendering
         Assert.Equal (Rectangle.Empty, view.LineCanvas.Bounds);
@@ -302,7 +302,7 @@ public class ViewDrawTextAndLineCanvasTests () : FakeDriverBase
 
         Rectangle boundsBefore = view.LineCanvas.Bounds;
 
-        view.RenderLineCanvas ();
+        view.RenderLineCanvas (null);
 
         // LineCanvas should NOT be cleared when SuperViewRendersLineCanvas is true
         Assert.Equal (boundsBefore, view.LineCanvas.Bounds);