From 4fd15c993719104641cf4ae16be0e4d28887b4f2 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Wed, 10 Apr 2024 17:12:40 +0200 Subject: [PATCH] Remove dependency on IsGlyphFullWidth for IRM/DECSWL (#16903) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This gets rid off the implicit dependency on `IsGlyphFullWidth` for the IRM and DECSWL/DECDWL/DECDHL implementations. ## Validation Steps Performed In pwsh: * ``"`e[31mab`e[m`b`e[4h`e[32m$('*'*10)`e[m`e[4l"`` prints a red "a", 10 green "*" and a red "b" βœ… * ``"`e[31mab`e[m`b`e[4h`e[32m$('*'*1000)`e[m`e[4l"`` prints a red "a" and a couple lines of green "*" βœ… * ``"`e[31mf$('o'*70)`e[m`e#6`e#5"`` the right half of the row is erased βœ… --- .github/actions/spelling/expect/expect.txt | 3 + src/buffer/out/Row.cpp | 9 +- src/buffer/out/Row.hpp | 1 - src/buffer/out/textBuffer.cpp | 37 +++++++- src/buffer/out/textBuffer.hpp | 3 +- .../out/ut_textbuffer/UTextAdapterTests.cpp | 2 +- src/host/_stream.cpp | 2 +- src/host/readDataCooked.cpp | 12 +-- src/host/ut_host/TextBufferTests.cpp | 91 ++++++++++++++++++- src/inc/til/rle.h | 12 --- src/terminal/adapter/adaptDispatch.cpp | 29 ++---- 11 files changed, 142 insertions(+), 59 deletions(-) diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index 1ce77a78ad5..e458086ce89 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -1,3 +1,4 @@ +aaaaabbb aabbcc ABANDONFONT abbcc @@ -92,6 +93,8 @@ backported backstory barbaz Bazz +bbb +bbccb BBDM bbwe bcount diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 8e318da41ef..4793b86dac9 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -349,12 +349,6 @@ void ROW::_init() noexcept #pragma warning(push) } -void ROW::TransferAttributes(const til::small_rle& attr, til::CoordType newWidth) -{ - _attr = attr; - _attr.resize_trailing_extent(gsl::narrow(newWidth)); -} - void ROW::CopyFrom(const ROW& source) { _lineRendition = source._lineRendition; @@ -366,7 +360,8 @@ void ROW::CopyFrom(const ROW& source) }; CopyTextFrom(state); - TransferAttributes(source.Attributes(), _columnCount); + _attr = source.Attributes(); + _attr.resize_trailing_extent(_columnCount); } // Returns the previous possible cursor position, preceding the given column. diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index d26fc87e7ed..317c935edce 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -132,7 +132,6 @@ class ROW final til::CoordType GetReadableColumnCount() const noexcept; void Reset(const TextAttribute& attr) noexcept; - void TransferAttributes(const til::small_rle& attr, til::CoordType newWidth); void CopyFrom(const ROW& source); til::CoordType NavigateToPrevious(til::CoordType column) const noexcept; diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 713cf2a09a9..bb2be49fd1f 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -562,7 +562,7 @@ til::point TextBuffer::NavigateCursor(til::point position, til::CoordType distan // This function is intended for writing regular "lines" of text as it'll set the wrap flag on the given row. // You can continue calling the function on the same row as long as state.columnEnd < state.columnLimit. -void TextBuffer::Write(til::CoordType row, const TextAttribute& attributes, RowWriteState& state) +void TextBuffer::Replace(til::CoordType row, const TextAttribute& attributes, RowWriteState& state) { auto& r = GetMutableRowByOffset(row); r.ReplaceText(state); @@ -570,6 +570,36 @@ void TextBuffer::Write(til::CoordType row, const TextAttribute& attributes, RowW TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, row, state.columnEndDirty, row + 1 })); } +void TextBuffer::Insert(til::CoordType row, const TextAttribute& attributes, RowWriteState& state) +{ + auto& r = GetMutableRowByOffset(row); + auto& scratch = GetScratchpadRow(); + + scratch.CopyFrom(r); + + r.ReplaceText(state); + r.ReplaceAttributes(state.columnBegin, state.columnEnd, attributes); + + // Restore trailing text from our backup in scratch. + RowWriteState restoreState{ + .text = scratch.GetText(state.columnBegin, state.columnLimit), + .columnBegin = state.columnEnd, + .columnLimit = state.columnLimit, + }; + r.ReplaceText(restoreState); + + // Restore trailing attributes as well. + if (const auto copyAmount = restoreState.columnEnd - restoreState.columnBegin; copyAmount > 0) + { + auto& rowAttr = r.Attributes(); + const auto& scratchAttr = scratch.Attributes(); + const auto restoreAttr = scratchAttr.slice(gsl::narrow(state.columnBegin), gsl::narrow(state.columnBegin + copyAmount)); + rowAttr.replace(gsl::narrow(restoreState.columnBegin), gsl::narrow(restoreState.columnEnd), restoreAttr); + } + + TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, row, restoreState.columnEndDirty, row + 1 })); +} + // Fills an area of the buffer with a given fill character(s) and attributes. void TextBuffer::FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes) { @@ -1083,11 +1113,8 @@ void TextBuffer::SetCurrentLineRendition(const LineRendition lineRendition, cons // And if it's no longer single width, the right half of the row should be erased. if (lineRendition != LineRendition::SingleWidth) { - const auto fillChar = L' '; const auto fillOffset = GetLineWidth(rowIndex); - const auto fillLength = gsl::narrow(GetSize().Width() - fillOffset); - const OutputCellIterator fillData{ fillChar, fillAttributes, fillLength }; - row.WriteCells(fillData, fillOffset, false); + FillRect({ fillOffset, rowIndex, til::CoordTypeMax, rowIndex + 1 }, L" ", fillAttributes); // We also need to make sure the cursor is clamped within the new width. GetCursor().SetPosition(ClampPositionWithinLine(cursorPosition)); } diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 79872cea0c4..2e1d0532215 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -107,7 +107,8 @@ class TextBuffer final til::point NavigateCursor(til::point position, til::CoordType distance) const; // Text insertion functions - void Write(til::CoordType row, const TextAttribute& attributes, RowWriteState& state); + void Replace(til::CoordType row, const TextAttribute& attributes, RowWriteState& state); + void Insert(til::CoordType row, const TextAttribute& attributes, RowWriteState& state); void FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes); OutputCellIterator Write(const OutputCellIterator givenIt); diff --git a/src/buffer/out/ut_textbuffer/UTextAdapterTests.cpp b/src/buffer/out/ut_textbuffer/UTextAdapterTests.cpp index ee879f4f677..427e90e6dac 100644 --- a/src/buffer/out/ut_textbuffer/UTextAdapterTests.cpp +++ b/src/buffer/out/ut_textbuffer/UTextAdapterTests.cpp @@ -41,7 +41,7 @@ class UTextAdapterTests RowWriteState state{ .text = L"abc 𝒢𝒷𝒸 abc ネコけゃん", }; - buffer.Write(0, TextAttribute{}, state); + buffer.Replace(0, TextAttribute{}, state); VERIFY_IS_TRUE(state.text.empty()); static constexpr auto s = [](til::CoordType beg, til::CoordType end) -> til::point_span { diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index 8d884e7136e..e030cec03c7 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -125,7 +125,7 @@ void _writeCharsLegacyUnprocessed(SCREEN_INFORMATION& screenInfo, const std::wst auto cursorPosition = textBuffer.GetCursor().GetPosition(); state.columnBegin = cursorPosition.x; - textBuffer.Write(cursorPosition.y, textBuffer.GetCurrentAttributes(), state); + textBuffer.Replace(cursorPosition.y, textBuffer.GetCurrentAttributes(), state); cursorPosition.x = state.columnEnd; if (wrapAtEOL && state.columnEnd >= state.columnLimit) diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index 4aa458b1cdf..6028acf074c 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -1192,13 +1192,13 @@ try buffer.front() = L'β”Œ'; buffer.back() = L'┐'; state.text = buffer; - textBuffer.Write(contentRect.top - 1, attributes, state); + textBuffer.Replace(contentRect.top - 1, attributes, state); // bottom line β””β”€β”€β”€β”˜ buffer.front() = L'β””'; buffer.back() = L'β”˜'; state.text = buffer; - textBuffer.Write(contentRect.bottom, attributes, state); + textBuffer.Replace(contentRect.bottom, attributes, state); // middle lines β”‚ β”‚ buffer.assign(widthSizeT, L' '); @@ -1207,7 +1207,7 @@ try for (til::CoordType y = contentRect.top; y < contentRect.bottom; ++y) { state.text = buffer; - textBuffer.Write(y, attributes, state); + textBuffer.Replace(y, attributes, state); } } @@ -1391,7 +1391,7 @@ void COOKED_READ_DATA::_popupHandleCommandNumberInput(Popup& popup, const wchar_ .columnBegin = popup.contentRect.right - CommandNumberMaxInputLength, .columnLimit = popup.contentRect.right, }; - _screenInfo.GetTextBuffer().Write(popup.contentRect.top, _screenInfo.GetPopupAttributes(), state); + _screenInfo.GetTextBuffer().Replace(popup.contentRect.top, _screenInfo.GetPopupAttributes(), state); } } @@ -1474,7 +1474,7 @@ void COOKED_READ_DATA::_popupDrawPrompt(const Popup& popup, const UINT id) const .columnBegin = popup.contentRect.left, .columnLimit = popup.contentRect.right, }; - _screenInfo.GetTextBuffer().Write(popup.contentRect.top, _screenInfo.GetPopupAttributes(), state); + _screenInfo.GetTextBuffer().Replace(popup.contentRect.top, _screenInfo.GetPopupAttributes(), state); } void COOKED_READ_DATA::_popupDrawCommandList(Popup& popup) const @@ -1533,7 +1533,7 @@ void COOKED_READ_DATA::_popupDrawCommandList(Popup& popup) const buffer.append(width, L' '); state.text = buffer; - _screenInfo.GetTextBuffer().Write(y, attr, state); + _screenInfo.GetTextBuffer().Replace(y, attr, state); } cl.dirtyHeight = height; diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index 33660d876a8..8f837985b70 100644 --- a/src/host/ut_host/TextBufferTests.cpp +++ b/src/host/ut_host/TextBufferTests.cpp @@ -147,7 +147,8 @@ class TextBufferTests TEST_METHOD(TestBurrito); TEST_METHOD(TestOverwriteChars); - TEST_METHOD(TestRowReplaceText); + TEST_METHOD(TestReplace); + TEST_METHOD(TestInsert); TEST_METHOD(TestAppendRTFText); @@ -2005,13 +2006,12 @@ void TextBufferTests::TestOverwriteChars() #undef complex1 } -void TextBufferTests::TestRowReplaceText() +void TextBufferTests::TestReplace() { static constexpr til::size bufferSize{ 10, 3 }; static constexpr UINT cursorSize = 12; const TextAttribute attr{ 0x7f }; TextBuffer buffer{ bufferSize, attr, cursorSize, false, _renderer }; - auto& row = buffer.GetMutableRowByOffset(0); #define complex L"\U0001F41B" @@ -2075,17 +2075,98 @@ void TextBufferTests::TestRowReplaceText() .columnBegin = t.input.columnBegin, .columnLimit = t.input.columnLimit, }; - row.ReplaceText(actual); + buffer.Replace(0, attr, actual); VERIFY_ARE_EQUAL(t.expected.text, actual.text); VERIFY_ARE_EQUAL(t.expected.columnEnd, actual.columnEnd); VERIFY_ARE_EQUAL(t.expected.columnBeginDirty, actual.columnBeginDirty); VERIFY_ARE_EQUAL(t.expected.columnEndDirty, actual.columnEndDirty); - VERIFY_ARE_EQUAL(t.expectedRow, row.GetText()); + VERIFY_ARE_EQUAL(t.expectedRow, buffer.GetRowByOffset(0).GetText()); } #undef complex } +void TextBufferTests::TestInsert() +{ + static constexpr til::size bufferSize{ 10, 3 }; + static constexpr UINT cursorSize = 12; + static constexpr TextAttribute attr1{ 0x11111111, 0x00000000 }; + static constexpr TextAttribute attr2{ 0x22222222, 0x00000000 }; + static constexpr TextAttribute attr3{ 0x33333333, 0x00000000 }; + TextBuffer buffer{ bufferSize, attr1, cursorSize, false, _renderer }; + + struct Test + { + const wchar_t* description; + struct + { + std::wstring_view text; + til::CoordType columnBegin = 0; + til::CoordType columnLimit = 0; + TextAttribute attr; + } input; + struct + { + std::wstring_view text; + til::CoordType columnEnd = 0; + til::CoordType columnBeginDirty = 0; + til::CoordType columnEndDirty = 0; + } expected; + std::wstring_view expectedRow; + }; + + static constexpr std::array tests{ + Test{ + L"Not enough space -> early exit", + { L"aaa", 5, 5, attr1 }, + { L"aaa", 5, 5, 5 }, + L" ", + }, + Test{ + L"Too much to fit", + { L"aaaaabbb", 0, 5, attr1 }, + { L"bbb", 5, 0, 5 }, + L"aaaaa ", + }, + Test{ + L"Wide char intersects limit", + { L"bbbbπŸ˜„", 0, 5, attr2 }, + { L"πŸ˜„", 5, 0, 5 }, + L"bbbb ", + }, + Test{ + L"Insert middle", + { L"cc", 2, 5, attr3 }, + { L"", 4, 2, 4 }, + L"bbccb ", + }, + }; + + for (const auto& t : tests) + { + Log::Comment(t.description); + RowWriteState actual{ + .text = t.input.text, + .columnBegin = t.input.columnBegin, + .columnLimit = t.input.columnLimit, + }; + buffer.Insert(0, t.input.attr, actual); + VERIFY_ARE_EQUAL(t.expected.text, actual.text); + VERIFY_ARE_EQUAL(t.expected.columnEnd, actual.columnEnd); + VERIFY_ARE_EQUAL(t.expected.columnBeginDirty, actual.columnBeginDirty); + VERIFY_ARE_EQUAL(t.expected.columnEndDirty, actual.columnEndDirty); + VERIFY_ARE_EQUAL(t.expectedRow, buffer.GetRowByOffset(0).GetText()); + } + + auto& scratch = buffer.GetScratchpadRow(); + scratch.ReplaceAttributes(0, 5, attr2); + scratch.ReplaceAttributes(2, 4, attr3); + + const auto& expectedAttr = scratch.Attributes(); + const auto& actualAttr = buffer.GetRowByOffset(0).Attributes(); + VERIFY_ARE_EQUAL(expectedAttr, actualAttr); +} + void TextBufferTests::TestAppendRTFText() { { diff --git a/src/inc/til/rle.h b/src/inc/til/rle.h index 99bdf734c45..b91b11b8a34 100644 --- a/src/inc/til/rle.h +++ b/src/inc/til/rle.h @@ -1042,18 +1042,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" #ifdef __WEX_COMMON_H__ namespace WEX::TestExecution { - template - class VerifyOutputTraits<::til::basic_rle> - { - using rle_vector = ::til::basic_rle; - - public: - static WEX::Common::NoThrowString ToString(const rle_vector& object) - { - return WEX::Common::NoThrowString(object.to_string().c_str()); - } - }; - template class VerifyCompareTraits<::til::basic_rle, ::til::basic_rle> { diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index cdf50148ef3..818261cf112 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -5,7 +5,6 @@ #include "adaptDispatch.hpp" #include "../../renderer/base/renderer.hpp" -#include "../../types/inc/GlyphWidth.hpp" #include "../../types/inc/Viewport.hpp" #include "../../types/inc/utils.hpp" #include "../../inc/unicode.hpp" @@ -119,26 +118,17 @@ void AdaptDispatch::_WriteToBuffer(const std::wstring_view string) } } - if (_modes.test(Mode::InsertReplace)) - { - // If insert-replace mode is enabled, we first measure how many cells - // the string will occupy, and scroll the target area right by that - // amount to make space for the incoming text. - const OutputCellIterator it(state.text, attributes); - auto measureIt = it; - while (measureIt && measureIt.GetCellDistance(it) < state.columnLimit) - { - ++measureIt; - } - const auto row = cursorPosition.y; - const auto cellCount = measureIt.GetCellDistance(it); - _ScrollRectHorizontally(textBuffer, { cursorPosition.x, row, state.columnLimit, row + 1 }, cellCount); - } - state.columnBegin = cursorPosition.x; const auto textPositionBefore = state.text.data(); - textBuffer.Write(cursorPosition.y, attributes, state); + if (_modes.test(Mode::InsertReplace)) + { + textBuffer.Insert(cursorPosition.y, attributes, state); + } + else + { + textBuffer.Replace(cursorPosition.y, attributes, state); + } const auto textPositionAfter = state.text.data(); // TODO: A row should not be marked as wrapped just because we wrote the last column. @@ -190,8 +180,7 @@ void AdaptDispatch::_WriteToBuffer(const std::wstring_view string) // Notify UIA of new text. // It's important to do this here instead of in TextBuffer, because here you - // have access to the entire line of text, whereas TextBuffer writes it one - // character at a time via the OutputCellIterator. + // have access to the entire line of text. textBuffer.TriggerNewTextNotification(string); }