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?
This commit is contained in:
Tonttu
2025-03-13 19:15:49 +02:00
committed by GitHub
parent 7aae0c2ad5
commit b09b4e78bd
3 changed files with 40 additions and 11 deletions

View File

@@ -217,7 +217,7 @@ public abstract class ConsoleDriver : IConsoleDriver
if (Contents [Row, Col - 1].CombiningMarks.Count > 0)
{
// Just add this mark to the list
Contents [Row, Col - 1].CombiningMarks.Add (rune);
Contents [Row, Col - 1].AddCombiningMark (rune);
// Ignore. Don't move to next column (let the driver figure out what to do).
}
@@ -240,7 +240,7 @@ public abstract class ConsoleDriver : IConsoleDriver
else
{
// It didn't normalize. Add it to the Cell to left's CM list
Contents [Row, Col - 1].CombiningMarks.Add (rune);
Contents [Row, Col - 1].AddCombiningMark (rune);
// Ignore. Don't move to next column (let the driver figure out what to do).
}
@@ -298,7 +298,7 @@ public abstract class ConsoleDriver : IConsoleDriver
else if (!Clip.Contains (Col, Row))
{
// Our 1st column is outside the clip, so we can't display a wide character.
Contents [Row, Col+1].Rune = Rune.ReplacementChar;
Contents [Row, Col + 1].Rune = Rune.ReplacementChar;
}
else
{

View File

@@ -164,7 +164,7 @@ public class OutputBuffer : IOutputBuffer
if (Contents [Row, Col - 1].CombiningMarks.Count > 0)
{
// Just add this mark to the list
Contents [Row, Col - 1].CombiningMarks.Add (rune);
Contents [Row, Col - 1].AddCombiningMark (rune);
// Ignore. Don't move to next column (let the driver figure out what to do).
}
@@ -187,7 +187,7 @@ public class OutputBuffer : IOutputBuffer
else
{
// It didn't normalize. Add it to the Cell to left's CM list
Contents [Row, Col - 1].CombiningMarks.Add (rune);
Contents [Row, Col - 1].AddCombiningMark (rune);
// Ignore. Don't move to next column (let the driver figure out what to do).
}

View File

@@ -1,4 +1,6 @@
namespace Terminal.Gui;
#nullable enable
namespace Terminal.Gui;
/// <summary>
/// Represents a single row/column in a Terminal.Gui rendering surface (e.g. <see cref="LineCanvas"/> and
@@ -23,12 +25,12 @@ public record struct Cell (Attribute? Attribute = null, bool IsDirty = false, Ru
get => _rune;
set
{
CombiningMarks.Clear ();
_combiningMarks?.Clear ();
_rune = value;
}
}
private List<Rune> _combiningMarks;
private List<Rune>? _combiningMarks;
/// <summary>
/// The combining marks for <see cref="Rune"/> that when combined makes this Cell a combining sequence. If
@@ -38,10 +40,37 @@ public record struct Cell (Attribute? Attribute = null, bool IsDirty = false, Ru
/// Only valid in the rare case where <see cref="Rune"/> is a combining sequence that could not be normalized to a
/// single Rune.
/// </remarks>
internal List<Rune> CombiningMarks
internal IReadOnlyList<Rune> CombiningMarks
{
get => _combiningMarks ?? [];
private set => _combiningMarks = value ?? [];
// PERFORMANCE: Downside of the interface return type is that List<T> struct enumerator cannot be utilized, i.e. enumerator is allocated.
// If enumeration is used heavily in the future then might be better to expose the List<T> Enumerator directly via separate mechanism.
get
{
// Avoid unnecessary list allocation.
if (_combiningMarks == null)
{
return Array.Empty<Rune> ();
}
return _combiningMarks;
}
}
/// <summary>
/// Adds combining mark to the cell.
/// </summary>
/// <param name="combiningMark">The combining mark to add to the cell.</param>
internal void AddCombiningMark (Rune combiningMark)
{
_combiningMarks ??= [];
_combiningMarks.Add (combiningMark);
}
/// <summary>
/// Clears combining marks of the cell.
/// </summary>
internal void ClearCombiningMarks ()
{
_combiningMarks?.Clear ();
}
/// <inheritdoc/>