Skip to content

Commit

Permalink
Reimplement TextBuffer::Reflow (#15701)
Browse files Browse the repository at this point in the history
Subjectively speaking, this commit makes 3 improvements:
* Most importantly, it now would work with arbitrary Unicode text.
  (No more `IsGlyphFullWidth` or DBCS handling during reflow.)
* Due to the simpler implementation it hopefully makes review of
  future changes and maintenance simpler. (~3x less LOC.)
* It improves perf. by 1-2 orders of magnitude.
  (At 120x9001 with a full buffer I get 60ms -> 2ms.)

Unfortunately, I'm not confident that the new code replicates the old
code exactly, because I failed to understand it. During development
I simply tried to match its behavior with what I think reflow should do.

Closes #797
Closes #3088
Closes #4968
Closes #6546
Closes #6901
Closes #15964
Closes MSFT:19446208

Related to #5800 and #8000

## Validation Steps Performed
* Unit tests ✅
* Feature tests ✅
* Reflow with a scrollback ✅
* Reflowing the cursor cell causes a forced line-wrap ✅
  (Even at the end of the buffer. ✅)
* `color 8f` and reflowing retains the background color ✅
* Enter alt buffer, Resize window, Exit alt buffer ✅
  • Loading branch information
lhecker committed Sep 26, 2023
1 parent c7f30a8 commit 7474839
Show file tree
Hide file tree
Showing 20 changed files with 464 additions and 702 deletions.
8 changes: 1 addition & 7 deletions .github/actions/spelling/expect/expect.txt
Expand Up @@ -749,7 +749,6 @@ gfx
GGI
GHIJK
GHIJKL
GHIJKLM
gitfilters
gitmodules
gle
Expand Down Expand Up @@ -1000,7 +999,6 @@ listptrsize
lld
llx
LMENU
LMNOP
lnk
lnkd
lnkfile
Expand Down Expand Up @@ -1223,7 +1221,6 @@ nonspace
NOOWNERZORDER
Nop
NOPAINT
NOPQRST
noprofile
NOREDRAW
NOREMOVE
Expand Down Expand Up @@ -1498,7 +1495,6 @@ pwsz
pythonw
Qaabbcc
qos
QRSTU
QUERYOPEN
QUESTIONMARK
quickedit
Expand Down Expand Up @@ -1861,7 +1857,6 @@ testname
TESTNULL
testpass
testpasses
testtimeout
TEXCOORD
texel
TExpected
Expand Down Expand Up @@ -2019,7 +2014,6 @@ utext
UText
UTEXT
utr
UVWX
UVWXY
UVWXYZ
uwa
Expand Down Expand Up @@ -2078,7 +2072,6 @@ VTRGBTo
vtseq
vtterm
vttest
VWX
waitable
WANSUNG
WANTARROWS
Expand Down Expand Up @@ -2274,6 +2267,7 @@ YCast
YCENTER
YCount
YDPI
YLimit
YOffset
YSubstantial
YVIRTUALSCREEN
Expand Down
42 changes: 36 additions & 6 deletions src/buffer/out/Row.cpp
Expand Up @@ -152,11 +152,15 @@ til::CoordType CharToColumnMapper::GetTrailingColumnAt(ptrdiff_t offset) noexcep
return col;
}

// If given a pointer inside the ROW's text buffer, this function will return the corresponding column.
// This function in particular returns the glyph's first column.
til::CoordType CharToColumnMapper::GetLeadingColumnAt(const wchar_t* str) noexcept
{
return GetLeadingColumnAt(str - _chars);
}

// If given a pointer inside the ROW's text buffer, this function will return the corresponding column.
// This function in particular returns the glyph's last column (this matters for wide glyphs).
til::CoordType CharToColumnMapper::GetTrailingColumnAt(const wchar_t* str) noexcept
{
return GetTrailingColumnAt(str - _chars);
Expand Down Expand Up @@ -364,11 +368,16 @@ void ROW::TransferAttributes(const til::small_rle<TextAttribute, uint16_t, 1>& a

void ROW::CopyFrom(const ROW& source)
{
RowCopyTextFromState state{ .source = source };
CopyTextFrom(state);
TransferAttributes(source.Attributes(), _columnCount);
_lineRendition = source._lineRendition;
_wrapForced = source._wrapForced;

RowCopyTextFromState state{
.source = source,
.sourceColumnLimit = source.GetReadableColumnCount(),
};
CopyTextFrom(state);

TransferAttributes(source.Attributes(), _columnCount);
}

// Returns the previous possible cursor position, preceding the given column.
Expand All @@ -382,7 +391,17 @@ til::CoordType ROW::NavigateToPrevious(til::CoordType column) const noexcept
// Returns the row width if column is beyond the width of the row.
til::CoordType ROW::NavigateToNext(til::CoordType column) const noexcept
{
return _adjustForward(_clampedColumn(column + 1));
return _adjustForward(_clampedColumnInclusive(column + 1));
}

// Returns the starting column of the glyph at the given column.
// In other words, if you have 3 wide glyphs
// AA BB CC
// 01 23 45 <-- column
// then `AdjustToGlyphStart(3)` returns 2.
til::CoordType ROW::AdjustToGlyphStart(til::CoordType column) const noexcept
{
return _adjustBackward(_clampedColumn(column));
}

// Routine Description:
Expand Down Expand Up @@ -719,11 +738,12 @@ try
if (sourceColBeg < sourceColLimit)
{
charOffsets = source._charOffsets.subspan(sourceColBeg, static_cast<size_t>(sourceColLimit) - sourceColBeg + 1);
const auto charsOffset = charOffsets.front() & CharOffsetsMask;
const auto beg = size_t{ charOffsets.front() } & CharOffsetsMask;
const auto end = size_t{ charOffsets.back() } & CharOffsetsMask;
// We _are_ using span. But C++ decided that string_view and span aren't convertible.
// _chars is a std::span for performance and because it refers to raw, shared memory.
#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1).
chars = { source._chars.data() + charsOffset, source._chars.size() - charsOffset };
chars = { source._chars.data() + beg, end - beg };
}

WriteHelper h{ *this, state.columnBegin, state.columnLimit, chars };
Expand Down Expand Up @@ -939,6 +959,16 @@ til::CoordType ROW::MeasureLeft() const noexcept

til::CoordType ROW::MeasureRight() const noexcept
{
if (_wrapForced)
{
auto width = _columnCount;
if (_doubleBytePadded)
{
width--;
}
return width;
}

const auto text = GetText();
const auto beg = text.begin();
const auto end = text.end();
Expand Down
1 change: 1 addition & 0 deletions src/buffer/out/Row.hpp
Expand Up @@ -136,6 +136,7 @@ class ROW final

til::CoordType NavigateToPrevious(til::CoordType column) const noexcept;
til::CoordType NavigateToNext(til::CoordType column) const noexcept;
til::CoordType AdjustToGlyphStart(til::CoordType column) const noexcept;

void ClearCell(til::CoordType column);
OutputCellIterator WriteCells(OutputCellIterator it, til::CoordType columnBegin, std::optional<bool> wrap = std::nullopt, std::optional<til::CoordType> limitRight = std::nullopt);
Expand Down

0 comments on commit 7474839

Please sign in to comment.