Skip to content

Commit 0270183

Browse files
authored
Fixes #4453. Regression in wide glyph rendering on all drivers (#4458)
1 parent dd12df7 commit 0270183

File tree

6 files changed

+212
-22
lines changed

6 files changed

+212
-22
lines changed
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
#nullable enable
2+
3+
using System.Text;
4+
5+
namespace UICatalog.Scenarios;
6+
7+
[ScenarioMetadata ("WideGlyphs", "Demonstrates wide glyphs with overlapped views & clipping")]
8+
[ScenarioCategory ("Unicode")]
9+
[ScenarioCategory ("Drawing")]
10+
11+
public sealed class WideGlyphs : Scenario
12+
{
13+
private Rune [,]? _codepoints;
14+
15+
public override void Main ()
16+
{
17+
// Init
18+
Application.Init ();
19+
20+
// Setup - Create a top-level application window and configure it.
21+
Window appWindow = new ()
22+
{
23+
Title = GetQuitKeyAndName (),
24+
BorderStyle = LineStyle.None
25+
};
26+
27+
// Build the array of codepoints once when subviews are laid out
28+
appWindow.SubViewsLaidOut += (s, e) =>
29+
{
30+
View? view = s as View;
31+
if (view is null)
32+
{
33+
return;
34+
}
35+
36+
// Only rebuild if size changed or array is null
37+
if (_codepoints is null ||
38+
_codepoints.GetLength (0) != view.Viewport.Height ||
39+
_codepoints.GetLength (1) != view.Viewport.Width)
40+
{
41+
_codepoints = new Rune [view.Viewport.Height, view.Viewport.Width];
42+
43+
for (int r = 0; r < view.Viewport.Height; r++)
44+
{
45+
for (int c = 0; c < view.Viewport.Width; c += 2)
46+
{
47+
_codepoints [r, c] = GetRandomWideCodepoint ();
48+
}
49+
}
50+
}
51+
};
52+
53+
// Fill the window with the pre-built codepoints array
54+
appWindow.DrawingContent += (s, e) =>
55+
{
56+
View? view = s as View;
57+
if (view is null || _codepoints is null)
58+
{
59+
return;
60+
}
61+
62+
// Traverse the Viewport, using the pre-built array
63+
for (int r = 0; r < view.Viewport.Height && r < _codepoints.GetLength (0); r++)
64+
{
65+
for (int c = 0; c < view.Viewport.Width && c < _codepoints.GetLength (1); c += 2)
66+
{
67+
Rune codepoint = _codepoints [r, c];
68+
if (codepoint != default (Rune))
69+
{
70+
view.AddRune (c, r, codepoint);
71+
}
72+
}
73+
}
74+
};
75+
76+
Line verticalLineAtEven = new Line ()
77+
{
78+
X = 10,
79+
Orientation = Orientation.Vertical,
80+
Length = Dim.Fill ()
81+
};
82+
appWindow.Add (verticalLineAtEven);
83+
84+
Line verticalLineAtOdd = new Line ()
85+
{
86+
X = 25,
87+
Orientation = Orientation.Vertical,
88+
Length = Dim.Fill ()
89+
};
90+
appWindow.Add (verticalLineAtOdd);
91+
92+
View arrangeableViewAtEven = new ()
93+
{
94+
CanFocus = true,
95+
Arrangement = ViewArrangement.Movable | ViewArrangement.Resizable,
96+
X = 30,
97+
Y = 5,
98+
Width = 15,
99+
Height = 5,
100+
BorderStyle = LineStyle.Dashed,
101+
};
102+
appWindow.Add (arrangeableViewAtEven);
103+
104+
View arrangeableViewAtOdd = new ()
105+
{
106+
CanFocus = true,
107+
Arrangement = ViewArrangement.Movable | ViewArrangement.Resizable,
108+
X = 31,
109+
Y = 11,
110+
Width = 15,
111+
Height = 5,
112+
BorderStyle = LineStyle.Dashed,
113+
};
114+
appWindow.Add (arrangeableViewAtOdd);
115+
// Run - Start the application.
116+
Application.Run (appWindow);
117+
appWindow.Dispose ();
118+
119+
// Shutdown - Calling Application.Shutdown is required.
120+
Application.Shutdown ();
121+
}
122+
123+
private Rune GetRandomWideCodepoint ()
124+
{
125+
Random random = new ();
126+
int codepoint = random.Next (0x4E00, 0x9FFF);
127+
return new Rune (codepoint);
128+
}
129+
}

Terminal.Gui/Drivers/OutputBase.cs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public virtual void Write (IOutputBuffer buffer)
8989
{
9090
if (output.Length > 0)
9191
{
92-
WriteToConsole (output, ref lastCol, row, ref outputWidth);
92+
WriteToConsole (output, ref lastCol, ref outputWidth);
9393
}
9494
else if (lastCol == -1)
9595
{
@@ -112,11 +112,8 @@ public virtual void Write (IOutputBuffer buffer)
112112
}
113113

114114
Cell cell = buffer.Contents [row, col];
115-
AppendCellAnsi (cell, output, ref redrawAttr, ref _redrawTextStyle, cols, ref col);
116-
117-
outputWidth++;
118-
119115
buffer.Contents [row, col].IsDirty = false;
116+
AppendCellAnsi (cell, output, ref redrawAttr, ref _redrawTextStyle, cols, ref col, ref outputWidth);
120117
}
121118
}
122119

@@ -221,7 +218,8 @@ protected void BuildAnsiForRegion (
221218
}
222219

223220
Cell cell = buffer.Contents! [row, col];
224-
AppendCellAnsi (cell, output, ref lastAttr, ref redrawTextStyle, endCol, ref col);
221+
int outputWidth = -1;
222+
AppendCellAnsi (cell, output, ref lastAttr, ref redrawTextStyle, endCol, ref col, ref outputWidth);
225223
}
226224

227225
// Add newline at end of row if requested
@@ -241,7 +239,8 @@ protected void BuildAnsiForRegion (
241239
/// <param name="redrawTextStyle">The current text style for optimization.</param>
242240
/// <param name="maxCol">The maximum column, used for wide character handling.</param>
243241
/// <param name="currentCol">The current column, updated for wide characters.</param>
244-
protected void AppendCellAnsi (Cell cell, StringBuilder output, ref Attribute? lastAttr, ref TextStyle redrawTextStyle, int maxCol, ref int currentCol)
242+
/// <param name="outputWidth">The current output width, updated for wide characters.</param>
243+
protected void AppendCellAnsi (Cell cell, StringBuilder output, ref Attribute? lastAttr, ref TextStyle redrawTextStyle, int maxCol, ref int currentCol, ref int outputWidth)
245244
{
246245
Attribute? attribute = cell.Attribute;
247246

@@ -256,11 +255,13 @@ protected void AppendCellAnsi (Cell cell, StringBuilder output, ref Attribute? l
256255
// Add the grapheme
257256
string grapheme = cell.Grapheme;
258257
output.Append (grapheme);
258+
outputWidth++;
259259

260260
// Handle wide grapheme
261261
if (grapheme.GetColumns () > 1 && currentCol + 1 < maxCol)
262262
{
263263
currentCol++; // Skip next cell for wide character
264+
outputWidth++;
264265
}
265266
}
266267

@@ -280,7 +281,7 @@ public string ToAnsi (IOutputBuffer buffer)
280281
return output.ToString ();
281282
}
282283

283-
private void WriteToConsole (StringBuilder output, ref int lastCol, int row, ref int outputWidth)
284+
private void WriteToConsole (StringBuilder output, ref int lastCol, ref int outputWidth)
284285
{
285286
if (IsLegacyConsole)
286287
{

Terminal.Gui/ViewBase/View.Drawing.cs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public void Draw (DrawContext? context = null)
127127
// because they may draw outside the viewport.
128128
SetClip (originalClip);
129129
originalClip = AddFrameToClip ();
130-
DoRenderLineCanvas ();
130+
DoRenderLineCanvas (context);
131131

132132
// ------------------------------------
133133
// Re-draw the border and padding subviews
@@ -672,16 +672,17 @@ public void DrawSubViews (DrawContext? context = null)
672672

673673
#region DrawLineCanvas
674674

675-
private void DoRenderLineCanvas ()
675+
private void DoRenderLineCanvas (DrawContext? context)
676676
{
677+
// TODO: Add context to OnRenderingLineCanvas
677678
if (!NeedsDraw || OnRenderingLineCanvas ())
678679
{
679680
return;
680681
}
681682

682683
// TODO: Add event
683684

684-
RenderLineCanvas ();
685+
RenderLineCanvas (context);
685686
}
686687

687688
/// <summary>
@@ -709,7 +710,8 @@ private void DoRenderLineCanvas ()
709710
/// <see cref="LineCanvas"/> of this view's subviews will be rendered. If <see cref="SuperViewRendersLineCanvas"/> is
710711
/// false (the default), this method will cause the <see cref="LineCanvas"/> to be rendered.
711712
/// </summary>
712-
public void RenderLineCanvas ()
713+
/// <param name="context"></param>
714+
public void RenderLineCanvas (DrawContext? context)
713715
{
714716
if (Driver is null)
715717
{
@@ -728,6 +730,9 @@ public void RenderLineCanvas ()
728730

729731
// TODO: #2616 - Support combining sequences that don't normalize
730732
AddStr (p.Value.Value.Grapheme);
733+
734+
// Add each drawn cell to the context
735+
context?.AddDrawnRectangle (new Rectangle (p.Key, new (1, 1)) );
731736
}
732737
}
733738

@@ -759,9 +764,6 @@ private void DoDrawComplete (DrawContext? context)
759764
// Exclude the Border and Padding from the clip
760765
ExcludeFromClip (Border?.Thickness.AsRegion (Border.FrameToScreen ()));
761766
ExcludeFromClip (Padding?.Thickness.AsRegion (Padding.FrameToScreen ()));
762-
763-
// QUESTION: This makes it so that no nesting of transparent views is possible, but is more correct?
764-
context = new DrawContext ();
765767
}
766768
else
767769
{

Terminal.Gui/Views/GraphView/LegendAnnotation.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#nullable disable
2-
namespace Terminal.Gui.Views;
2+
namespace Terminal.Gui.Views;
33

44
/// <summary>
55
/// Used by <see cref="GraphView"/> to render smbol definitions in a graph, e.g. colors and their meanings
@@ -46,14 +46,16 @@ public void Render (GraphView graph)
4646
if (!IsInitialized)
4747
{
4848
// BUGBUG: We should be getting a visual role here?
49-
SetScheme (new() { Normal = Application.Driver?.GetAttribute () ?? Attribute.Default });
49+
SetScheme (new () { Normal = Application.Driver?.GetAttribute () ?? Attribute.Default });
5050
graph.Add (this);
5151
}
5252

5353
if (BorderStyle != LineStyle.None)
5454
{
55+
// BUGBUG: View code should never call Draw directly. This
56+
// BUGBUG: needs to be refactored to decouple.
5557
DrawAdornments ();
56-
RenderLineCanvas ();
58+
RenderLineCanvas (null);
5759
}
5860

5961
var linesDrawn = 0;

Tests/UnitTestsParallelizable/Drivers/OutputBaseTests.cs

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#nullable enable
1+
#nullable enable
22

33
namespace DriverTests;
44

@@ -154,6 +154,62 @@ public void Write_Virtual_Or_NonVirtual_Uses_WriteToConsole_And_Clears_Dirty_Fla
154154
Assert.Equal (new Point (2, 0), output.GetCursorPosition ());
155155
}
156156

157+
[Theory]
158+
[InlineData (true)]
159+
[InlineData (false)]
160+
public void Write_Virtual_Or_NonVirtual_Uses_WriteToConsole_And_Clears_Dirty_Flags_Mixed_Graphemes (bool isLegacyConsole)
161+
{
162+
// Arrange
163+
// FakeOutput exposes this because it's in test scope
164+
var output = new FakeOutput { IsLegacyConsole = isLegacyConsole };
165+
IOutputBuffer buffer = output.LastBuffer!;
166+
buffer.SetSize (3, 1);
167+
168+
// Write '🦮' at col 0 and 'A' at col 3; leave col 1 untouched (not dirty)
169+
buffer.Move (0, 0);
170+
buffer.AddStr ("🦮A");
171+
172+
// Confirm some dirtiness before to write
173+
Assert.True (buffer.Contents! [0, 0].IsDirty);
174+
Assert.False (buffer.Contents! [0, 1].IsDirty);
175+
Assert.True (buffer.Contents! [0, 2].IsDirty);
176+
177+
// Act
178+
output.Write (buffer);
179+
180+
Assert.Contains ("🦮", output.Output);
181+
Assert.Contains ("A", output.Output);
182+
183+
// Dirty flags cleared for the written cells
184+
Assert.False (buffer.Contents! [0, 0].IsDirty);
185+
Assert.False (buffer.Contents! [0, 1].IsDirty);
186+
Assert.False (buffer.Contents! [0, 2].IsDirty);
187+
188+
Assert.Equal (new (0, 0), output.GetCursorPosition ());
189+
190+
// Now write 'X' at col 1 which replaces with the replacement character the col 0
191+
buffer.Move (1, 0);
192+
buffer.AddStr ("X");
193+
194+
// Confirm dirtiness state before to write
195+
Assert.True (buffer.Contents! [0, 0].IsDirty);
196+
Assert.True (buffer.Contents! [0, 1].IsDirty);
197+
Assert.True (buffer.Contents! [0, 2].IsDirty);
198+
199+
output.Write (buffer);
200+
201+
Assert.Contains ("�", output.Output);
202+
Assert.Contains ("X", output.Output);
203+
204+
// Dirty flags cleared for the written cells
205+
Assert.False (buffer.Contents! [0, 0].IsDirty);
206+
Assert.False (buffer.Contents! [0, 1].IsDirty);
207+
Assert.False (buffer.Contents! [0, 2].IsDirty);
208+
209+
// Verify SetCursorPositionImpl was invoked by WriteToConsole (cursor set to a written column)
210+
Assert.Equal (new (0, 0), output.GetCursorPosition ());
211+
}
212+
157213
[Theory]
158214
[InlineData (true)]
159215
[InlineData (false)]

Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawTextAndLineCanvasTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ public void RenderLineCanvas_DrawsLines ()
240240
Point screenPos = new Point (15, 15);
241241
view.LineCanvas.AddLine (screenPos, 5, Orientation.Horizontal, LineStyle.Single);
242242

243-
view.RenderLineCanvas ();
243+
view.RenderLineCanvas (null);
244244

245245
// Verify the line was drawn (check for horizontal line character)
246246
for (int i = 0; i < 5; i++)
@@ -272,7 +272,7 @@ public void RenderLineCanvas_ClearsAfterRendering ()
272272

273273
Assert.NotEqual (Rectangle.Empty, view.LineCanvas.Bounds);
274274

275-
view.RenderLineCanvas ();
275+
view.RenderLineCanvas (null);
276276

277277
// LineCanvas should be cleared after rendering
278278
Assert.Equal (Rectangle.Empty, view.LineCanvas.Bounds);
@@ -302,7 +302,7 @@ public void RenderLineCanvas_WithSuperViewRendersLineCanvas_DoesNotClear ()
302302

303303
Rectangle boundsBefore = view.LineCanvas.Bounds;
304304

305-
view.RenderLineCanvas ();
305+
view.RenderLineCanvas (null);
306306

307307
// LineCanvas should NOT be cleared when SuperViewRendersLineCanvas is true
308308
Assert.Equal (boundsBefore, view.LineCanvas.Bounds);

0 commit comments

Comments
 (0)