From cb2f347c2f6937adf272c54dbf74b23d1c3e1047 Mon Sep 17 00:00:00 2001 From: PankajBhojwani Date: Fri, 20 Aug 2021 15:36:25 -0700 Subject: [PATCH] Fix text selection while new lines are being printed when history buffer is full (#10749) When our text buffer is full, newlines cause the buffer to scroll underneath the viewport (rather than the viewport moving down). This was causing selections made during text output to scroll down. To solve this, when we increment the circular buffer, we decrement the y-coordinates of the current selections by 1. We also invalidate the previous selection rects. Closes #10319 --- src/cascadia/TerminalCore/Terminal.cpp | 25 ++++++ .../ControlInteractivityTests.cpp | 78 +++++++++++++++++++ src/renderer/base/renderer.cpp | 4 + 3 files changed, 107 insertions(+) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 016de5e60..aba4d68de 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -991,6 +991,31 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition) _buffer->IncrementCircularBuffer(); proposedCursorPosition.Y--; rowsPushedOffTopOfBuffer++; + + // Update our selection too, so it doesn't move as the buffer is cycled + if (_selection) + { + // If the start of the selection is above 0, we can reduce both the start and end by 1 + if (_selection->start.Y > 0) + { + _selection->start.Y -= 1; + _selection->end.Y -= 1; + } + else + { + // The start of the selection is at 0, if the end is greater than 0, then only reduce the end + if (_selection->end.Y > 0) + { + _selection->start.X = 0; + _selection->end.Y -= 1; + } + else + { + // Both the start and end of the selection are at 0, clear the selection + _selection.reset(); + } + } + } } // manually erase our pattern intervals since the locations have changed now diff --git a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp index a94ed8778..dba09c9e4 100644 --- a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp @@ -36,6 +36,7 @@ namespace ControlUnitTests TEST_METHOD(TestDragSelectOutsideBounds); TEST_METHOD(PointerClickOutsideActiveRegion); + TEST_METHOD(IncrementCircularBufferWithSelection); TEST_CLASS_SETUP(ClassSetup) { @@ -719,4 +720,81 @@ namespace ControlUnitTests Log::Comment(L"Verify that there's still no selection"); VERIFY_IS_FALSE(core->HasSelection()); } + + void ControlInteractivityTests::IncrementCircularBufferWithSelection() + { + // This is a test for GH#10749 + WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{}; + + auto [settings, conn] = _createSettingsAndConnection(); + auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn); + _standardInit(core, interactivity); + + Log::Comment(L"Fill up the history buffer"); + // Output lines equal to history size + viewport height to make sure we're + // at the point where outputting more lines causes circular incrementing + for (int i = 0; i < settings->HistorySize() + core->ViewHeight(); ++i) + { + conn->WriteInput(L"Foo\r\n"); + } + + // For this test, don't use any modifiers + const auto modifiers = ControlKeyStates(); + const Control::MouseButtonState leftMouseDown{ Control::MouseButtonState::IsLeftButtonDown }; + const Control::MouseButtonState noMouseDown{}; + + const til::size fontSize{ 9, 21 }; + + Log::Comment(L"Click on the terminal"); + const til::point terminalPosition0{ 5, 5 }; + const til::point cursorPosition0{ terminalPosition0 * fontSize }; + interactivity->PointerPressed(leftMouseDown, + WM_LBUTTONDOWN, //pointerUpdateKind + 0, // timestamp + modifiers, + cursorPosition0); + + Log::Comment(L"Verify that there's not yet a selection"); + VERIFY_IS_FALSE(core->HasSelection()); + + VERIFY_IS_TRUE(interactivity->_singleClickTouchdownPos.has_value()); + VERIFY_ARE_EQUAL(cursorPosition0, interactivity->_singleClickTouchdownPos.value()); + + Log::Comment(L"Drag the mouse just a little"); + // move not quite a whole cell, but enough to start a selection + const til::point cursorPosition1{ cursorPosition0 + til::point{ 6, 0 } }; + interactivity->PointerMoved(leftMouseDown, + WM_LBUTTONDOWN, //pointerUpdateKind + modifiers, + true, // focused, + cursorPosition1, + true); + Log::Comment(L"Verify that there's one selection"); + VERIFY_IS_TRUE(core->HasSelection()); + VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size()); + + Log::Comment(L"Verify the location of the selection"); + // The viewport is on row (historySize + 5), so the selection will be on: + // {(5, (historySize+5))+(0, 21)} to {(5, (historySize+5))+(0, 21)} + COORD expectedAnchor{ 5, gsl::narrow_cast(settings->HistorySize()) + 5 }; + VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor()); + VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionEnd()); + + Log::Comment(L"Output a line of text"); + conn->WriteInput(L"Foo\r\n"); + + Log::Comment(L"Verify the location of the selection"); + // The selection should now be 1 row lower + expectedAnchor.Y -= 1; + VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionAnchor()); + VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionEnd()); + + // Output enough text for the selection to get pushed off the buffer + for (int i = 0; i < settings->HistorySize() + core->ViewHeight(); ++i) + { + conn->WriteInput(L"Foo\r\n"); + } + // Verify that the selection got reset + VERIFY_IS_FALSE(core->HasSelection()); + } } diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 863bcd24f..e5103bc3f 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -466,12 +466,16 @@ void Renderer::TriggerScroll(const COORD* const pcoordDelta) // - void Renderer::TriggerCircling() { + const auto rects = _GetSelectionRects(); + for (IRenderEngine* const pEngine : _rgpEngines) { bool fEngineRequestsRepaint = false; HRESULT hr = pEngine->InvalidateCircling(&fEngineRequestsRepaint); LOG_IF_FAILED(hr); + LOG_IF_FAILED(pEngine->InvalidateSelection(rects)); + if (SUCCEEDED(hr) && fEngineRequestsRepaint) { LOG_IF_FAILED(_PaintFrameForEngine(pEngine));