From a3b29c1f2afaa7cf6da65e08e2ae0c48a73ad3c1 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 19 Mar 2024 16:28:15 +0100 Subject: [PATCH 1/5] Remove dependency on IsGlyphFullWidth for IRM/DECSWL --- src/buffer/out/Row.cpp | 9 ++----- src/buffer/out/Row.hpp | 1 - src/buffer/out/textBuffer.cpp | 34 ++++++++++++++++++++++---- src/buffer/out/textBuffer.hpp | 3 ++- src/host/_stream.cpp | 2 +- src/host/readDataCooked.cpp | 12 ++++----- src/terminal/adapter/adaptDispatch.cpp | 29 +++++++--------------- 7 files changed, 49 insertions(+), 41 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 4722dc0dfc4..75d800f8e22 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -348,12 +348,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; @@ -365,7 +359,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 197343df6d8..fdb84d3191e 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -131,7 +131,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 5c71dd1d398..3cc29c725f4 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -563,7 +563,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); @@ -571,6 +571,33 @@ 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, til::CoordTypeMax), + .columnBegin = state.columnEnd, + }; + r.ReplaceText(restoreState); + + // Restore trailing attributes as well. + auto& rowAttr = r.Attributes(); + const auto& scratchAttr = scratch.Attributes(); + const auto restoreAttr = scratchAttr.slice(gsl::narrow(state.columnBegin), scratch.size()); + rowAttr.replace(gsl::narrow(state.columnEnd), scratch.size(), restoreAttr); + rowAttr.resize_trailing_extent(r.size()); + + 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) { @@ -1084,11 +1111,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 }, 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 419e01471e5..86d70c61bbd 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -142,7 +142,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/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/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index dfe540936d0..ba5253438a9 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); } From a15471fdfba3853b3136c74a3f0a22cc9433e71b Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 19 Mar 2024 18:09:32 +0100 Subject: [PATCH 2/5] Forgot to pick this change --- src/buffer/out/ut_textbuffer/UTextAdapterTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 { From 53305b539ae34571cec1910723ef18ac07d9f214 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 25 Mar 2024 17:03:43 +0100 Subject: [PATCH 3/5] Address feedback by j4james --- src/buffer/out/textBuffer.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 3cc29c725f4..1d822b6a0d9 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -583,17 +583,17 @@ void TextBuffer::Insert(til::CoordType row, const TextAttribute& attributes, Row // Restore trailing text from our backup in scratch. RowWriteState restoreState{ - .text = scratch.GetText(state.columnBegin, til::CoordTypeMax), + .text = scratch.GetText(state.columnBegin, state.columnLimit), .columnBegin = state.columnEnd, + .columnLimit = state.columnLimit, }; r.ReplaceText(restoreState); // Restore trailing attributes as well. auto& rowAttr = r.Attributes(); const auto& scratchAttr = scratch.Attributes(); - const auto restoreAttr = scratchAttr.slice(gsl::narrow(state.columnBegin), scratch.size()); - rowAttr.replace(gsl::narrow(state.columnEnd), scratch.size(), restoreAttr); - rowAttr.resize_trailing_extent(r.size()); + const auto restoreAttr = scratchAttr.slice(gsl::narrow(state.columnBegin), gsl::narrow(state.columnLimit)); + rowAttr.replace(gsl::narrow(state.columnEnd), gsl::narrow(state.columnEnd + restoreAttr.size()), restoreAttr); TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, row, restoreState.columnEndDirty, row + 1 })); } @@ -1112,7 +1112,7 @@ void TextBuffer::SetCurrentLineRendition(const LineRendition lineRendition, cons if (lineRendition != LineRendition::SingleWidth) { const auto fillOffset = GetLineWidth(rowIndex); - FillRect({ fillOffset, rowIndex, til::CoordTypeMax, rowIndex }, L" ", fillAttributes); + 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)); } From c45fdf284fdf672c8992af83fa4c3519a502a485 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 2 Apr 2024 20:27:33 +0200 Subject: [PATCH 4/5] Add tests, Fix offsets --- src/buffer/out/textBuffer.cpp | 11 ++-- src/host/ut_host/TextBufferTests.cpp | 91 ++++++++++++++++++++++++++-- src/inc/til/rle.h | 12 ---- 3 files changed, 93 insertions(+), 21 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 2a2c2ba9d2b..98fc4430930 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -589,10 +589,13 @@ void TextBuffer::Insert(til::CoordType row, const TextAttribute& attributes, Row r.ReplaceText(restoreState); // Restore trailing attributes as well. - auto& rowAttr = r.Attributes(); - const auto& scratchAttr = scratch.Attributes(); - const auto restoreAttr = scratchAttr.slice(gsl::narrow(state.columnBegin), gsl::narrow(state.columnLimit)); - rowAttr.replace(gsl::narrow(state.columnEnd), gsl::narrow(state.columnEnd + restoreAttr.size()), restoreAttr); + 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 })); } diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index 648b054778b..a59bfeafdcc 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); @@ -2003,13 +2004,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" @@ -2073,17 +2073,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> { From ca27ae21f3e10d10f561c007a7538192eb1e0f3d Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Wed, 3 Apr 2024 02:12:54 +0200 Subject: [PATCH 5/5] Fix spelling --- .github/actions/spelling/expect/expect.txt | 3 +++ 1 file changed, 3 insertions(+) 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