Skip to content

Commit b09b4e7

Browse files
authored
Fix Cell.CombiningMarks property getter rune list allocation (#3980)
* Skip rune list allocation from accessing Cell.CombiningMarks property The getter would every time allocate a new list when the backing field was not assigned, which is most of the time. * Fix comment about performance I accidentally word. Is this dangerous?
1 parent 7aae0c2 commit b09b4e7

File tree

3 files changed

+40
-11
lines changed

3 files changed

+40
-11
lines changed

Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ public void AddRune (Rune rune)
217217
if (Contents [Row, Col - 1].CombiningMarks.Count > 0)
218218
{
219219
// Just add this mark to the list
220-
Contents [Row, Col - 1].CombiningMarks.Add (rune);
220+
Contents [Row, Col - 1].AddCombiningMark (rune);
221221

222222
// Ignore. Don't move to next column (let the driver figure out what to do).
223223
}
@@ -240,7 +240,7 @@ public void AddRune (Rune rune)
240240
else
241241
{
242242
// It didn't normalize. Add it to the Cell to left's CM list
243-
Contents [Row, Col - 1].CombiningMarks.Add (rune);
243+
Contents [Row, Col - 1].AddCombiningMark (rune);
244244

245245
// Ignore. Don't move to next column (let the driver figure out what to do).
246246
}
@@ -298,7 +298,7 @@ public void AddRune (Rune rune)
298298
else if (!Clip.Contains (Col, Row))
299299
{
300300
// Our 1st column is outside the clip, so we can't display a wide character.
301-
Contents [Row, Col+1].Rune = Rune.ReplacementChar;
301+
Contents [Row, Col + 1].Rune = Rune.ReplacementChar;
302302
}
303303
else
304304
{

Terminal.Gui/ConsoleDrivers/V2/OutputBuffer.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public void AddRune (Rune rune)
164164
if (Contents [Row, Col - 1].CombiningMarks.Count > 0)
165165
{
166166
// Just add this mark to the list
167-
Contents [Row, Col - 1].CombiningMarks.Add (rune);
167+
Contents [Row, Col - 1].AddCombiningMark (rune);
168168

169169
// Ignore. Don't move to next column (let the driver figure out what to do).
170170
}
@@ -187,7 +187,7 @@ public void AddRune (Rune rune)
187187
else
188188
{
189189
// It didn't normalize. Add it to the Cell to left's CM list
190-
Contents [Row, Col - 1].CombiningMarks.Add (rune);
190+
Contents [Row, Col - 1].AddCombiningMark (rune);
191191

192192
// Ignore. Don't move to next column (let the driver figure out what to do).
193193
}

Terminal.Gui/Drawing/Cell.cs

+35-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace Terminal.Gui;
1+
#nullable enable
2+
3+
namespace Terminal.Gui;
24

35
/// <summary>
46
/// Represents a single row/column in a Terminal.Gui rendering surface (e.g. <see cref="LineCanvas"/> and
@@ -23,12 +25,12 @@ public Rune Rune
2325
get => _rune;
2426
set
2527
{
26-
CombiningMarks.Clear ();
28+
_combiningMarks?.Clear ();
2729
_rune = value;
2830
}
2931
}
3032

31-
private List<Rune> _combiningMarks;
33+
private List<Rune>? _combiningMarks;
3234

3335
/// <summary>
3436
/// The combining marks for <see cref="Rune"/> that when combined makes this Cell a combining sequence. If
@@ -38,10 +40,37 @@ public Rune Rune
3840
/// Only valid in the rare case where <see cref="Rune"/> is a combining sequence that could not be normalized to a
3941
/// single Rune.
4042
/// </remarks>
41-
internal List<Rune> CombiningMarks
43+
internal IReadOnlyList<Rune> CombiningMarks
44+
{
45+
// PERFORMANCE: Downside of the interface return type is that List<T> struct enumerator cannot be utilized, i.e. enumerator is allocated.
46+
// If enumeration is used heavily in the future then might be better to expose the List<T> Enumerator directly via separate mechanism.
47+
get
48+
{
49+
// Avoid unnecessary list allocation.
50+
if (_combiningMarks == null)
51+
{
52+
return Array.Empty<Rune> ();
53+
}
54+
return _combiningMarks;
55+
}
56+
}
57+
58+
/// <summary>
59+
/// Adds combining mark to the cell.
60+
/// </summary>
61+
/// <param name="combiningMark">The combining mark to add to the cell.</param>
62+
internal void AddCombiningMark (Rune combiningMark)
63+
{
64+
_combiningMarks ??= [];
65+
_combiningMarks.Add (combiningMark);
66+
}
67+
68+
/// <summary>
69+
/// Clears combining marks of the cell.
70+
/// </summary>
71+
internal void ClearCombiningMarks ()
4272
{
43-
get => _combiningMarks ?? [];
44-
private set => _combiningMarks = value ?? [];
73+
_combiningMarks?.Clear ();
4574
}
4675

4776
/// <inheritdoc/>

0 commit comments

Comments
 (0)