From ef80f665d3b7b3fb0b1e39fe43393bd46905f256 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 27 Mar 2020 15:37:23 -0700 Subject: [PATCH] Correct scrolling invalidation region for tmux in pty w/ bitmap (#5122) Correct scrolling invalidation region for tmux in pty w/ bitmap Add tracing for circling and scrolling operations. Fix improper invalidation within AdjustCursorPosition routine in the subsection about scrolling down at the bottom with a set of margins enabled. ## References - Introduced with #5024 ## Detailed Description of the Pull Request / Additional comments - This occurs when there is a scroll region restriction applied and a newline operation is performed to attempt to spin the contents of just the scroll region. This is a frequent behavior of tmux. - Right now, the Terminal doesn't support any sort of "scroll content" operation, so what happens here generally speaking is that the PTY in the ConHost will repaint everything when this happens. - The PTY when doing `AdjustCursorPosition` with a scroll region restriction would do the following things: 1. Slide literally everything in the direction it needed to go to take advantage of rotating the circular buffer. (This would force a repaint in PTY as the PTY always forces repaint when the buffer circles.) 2. Copy the lines that weren't supposed to move back to where they were supposed to go. 3. Backfill the "revealed" region that encompasses what was supposed to be the newline. - The invalidations for the three operations above were: 1. Invalidate the number of rows of the delta at the top of the buffer (this part was wrong) 2. Invalidate the lines that got copied back into position (probably unnecessary, but OK) 3. Invalidate the revealed/filled-with-spaces line (this is good). - When we were using a simple single rectangle for invalidation, the union of the top row of the buffer from 1 and the bottom row of the buffer from 2 (and 3 was irrelevant as it was already unioned it) resulted in repainting the entire buffer and all was good. - When we switched to a bitmap, it dutifully only repainted the top line and the bottom two lines as the middle ones weren't a consequence of intersect. - The logic was wrong. We shouldn't be invalidating rows-from-the-top for the amount of the delta. The 1 part should be invalidating everything BUT the lines that were invalidated in parts 2 and 3. (Arguably part 2 shouldn't be happening at all, but I'm not optimizing for that right now.) - So this solves it by restoring an entire screen repaint for this sort of slide data operation by giving the correct number of invalidated lines to the bitmap. ## Validation Steps Performed - Manual validation with the steps described in #5104 - Automatic test `ConptyRoundtripTests::ScrollWithMargins`. Closes #5104 --- .../spell-check/whitelist/whitelist.txt | 4 + .../ConptyRoundtripTests.cpp | 247 ++++++++++++++++++ src/host/_stream.cpp | 28 +- src/host/ut_host/ConptyOutputTests.cpp | 3 + src/inc/til/point.h | 24 ++ src/renderer/vt/XtermEngine.cpp | 26 +- src/renderer/vt/invalidate.cpp | 2 + src/renderer/vt/math.cpp | 4 +- src/renderer/vt/paint.cpp | 4 +- src/renderer/vt/state.cpp | 2 +- src/renderer/vt/tracing.cpp | 17 ++ src/renderer/vt/tracing.hpp | 1 + src/renderer/vt/vtrenderer.hpp | 2 +- src/til/ut_til/PointTests.cpp | 161 ++++++++++++ 14 files changed, 505 insertions(+), 20 deletions(-) diff --git a/.github/actions/spell-check/whitelist/whitelist.txt b/.github/actions/spell-check/whitelist/whitelist.txt index e8ce38722b..b7ceec203a 100644 --- a/.github/actions/spell-check/whitelist/whitelist.txt +++ b/.github/actions/spell-check/whitelist/whitelist.txt @@ -238,6 +238,7 @@ CBoolean cbt cbuffer CCCBB +CCCC ccf cch CCHAR @@ -534,6 +535,7 @@ dcommon DCompile dcompiler ddb +DDDD dde DDESHARE DDevice @@ -710,6 +712,7 @@ edputil edu eeb eee +EEEE Efast EHsc EJO @@ -1928,6 +1931,7 @@ pythonw qi QJ qo +QQQQ qsort queryable QUESTIONMARK diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 2d5cb92136..abfcaad234 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -53,6 +53,9 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final static const SHORT TerminalViewWidth = 80; static const SHORT TerminalViewHeight = 32; + // This test class is for tests that are supposed to emit something in the PTY layer + // and then check that they've been staged for presentation correctly inside + // the Terminal application. Which sequences were used to get here don't matter. TEST_CLASS(ConptyRoundtripTests); TEST_CLASS_SETUP(ClassSetup) @@ -171,6 +174,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(TestResizeHeight); + TEST_METHOD(ScrollWithMargins); + private: bool _writeCallback(const char* const pch, size_t const cch); void _flushFirstFrame(); @@ -1055,3 +1060,245 @@ void ConptyRoundtripTests::PassthroughHardReset() TestUtils::VerifyExpectedString(termTb, std::wstring(TerminalViewWidth, L' '), { 0, y }); } } + +void ConptyRoundtripTests::ScrollWithMargins() +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& hostSm = si.GetStateMachine(); + + auto& hostTb = si.GetTextBuffer(); + auto& termTb = *term->_buffer; + const auto initialTermView = term->GetViewport(); + + Log::Comment(L"Flush first frame."); + _flushFirstFrame(); + + // Fill up the buffer with some text. + // We're going to write something like this: + // AAAA + // BBBB + // CCCC + // ........ + // QQQQ + // **************** + // The letters represent the data in the TMUX pane. + // The final *** line represents the mode line which we will + // attempt to hold in place and not scroll. + + Log::Comment(L"Fill host with text pattern by feeding it into VT parser."); + const auto rowsToWrite = initialTermView.Height() - 1; + + // For all lines but the last one, write out a few of a letter. + for (auto i = 0; i < rowsToWrite; ++i) + { + const wchar_t wch = static_cast(L'A' + i); + hostSm.ProcessCharacter(wch); + hostSm.ProcessCharacter(wch); + hostSm.ProcessCharacter(wch); + hostSm.ProcessCharacter(wch); + hostSm.ProcessCharacter('\n'); + } + + // For the last one, write out the asterisks for the mode line. + for (auto i = 0; i < initialTermView.Width(); ++i) + { + hostSm.ProcessCharacter('*'); + } + + // no newline in the bottom right corner or it will move unexpectedly. + + // Now set up the verification that the buffers are full of the pattern we expect. + // This function will verify the text backing buffers. + auto verifyBuffer = [&](const TextBuffer& tb) { + auto& cursor = tb.GetCursor(); + // Verify the cursor is waiting in the bottom right corner + VERIFY_ARE_EQUAL(initialTermView.Height() - 1, cursor.GetPosition().Y); + VERIFY_ARE_EQUAL(initialTermView.Width() - 1, cursor.GetPosition().X); + + // For all rows except the last one, verify that we have a run of four letters. + for (auto i = 0; i < rowsToWrite; ++i) + { + const std::wstring expectedString(4, static_cast(L'A' + i)); + const COORD expectedPos{ 0, gsl::narrow(i) }; + TestUtils::VerifyExpectedString(tb, expectedString, expectedPos); + } + + // For the last row, verify we have an entire row of asterisks for the mode line. + const std::wstring expectedModeLine(initialTermView.Width(), L'*'); + const COORD expectedPos{ 0, gsl::narrow(rowsToWrite) }; + TestUtils::VerifyExpectedString(tb, expectedModeLine, expectedPos); + }; + + // This will verify the text emitted from the PTY. + for (auto i = 0; i < rowsToWrite; ++i) + { + const std::string expectedString(4, static_cast('A' + i)); + expectedOutput.push_back(expectedString); + expectedOutput.push_back("\r\n"); + } + { + const std::string expectedString(initialTermView.Width(), '*'); + expectedOutput.push_back(expectedString); + + // Cursor gets reset into bottom right corner as we're writing all the way into that corner. + std::stringstream ss; + ss << "\x1b[" << initialTermView.Height() << ";" << initialTermView.Width() << "H"; + expectedOutput.push_back(ss.str()); + } + + Log::Comment(L"Verify host buffer contains pattern."); + // Verify the host side. + verifyBuffer(hostTb); + + Log::Comment(L"Emit PTY frame and validate it transmits the right data."); + // Paint the frame + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"Verify terminal buffer contains pattern."); + // Verify the terminal side. + verifyBuffer(termTb); + + Log::Comment(L"!!! OK. Set up the scroll region and let's get scrolling!"); + // This is a simulation of what TMUX does to scroll everything except the mode line. + // First build up our VT strings... + std::wstring reducedScrollRegion; + { + std::wstringstream wss; + // For 20 tall buffer... + // ESC[1;19r + // Set scroll region to lines 1-19. + wss << L"\x1b[1;" << initialTermView.Height() - 1 << L"r"; + reducedScrollRegion = wss.str(); + } + std::wstring completeScrollRegion; + { + std::wstringstream wss; + // For 20 tall buffer... + // ESC[1;20r + // Set scroll region to lines 1-20. (or the whole buffer) + wss << L"\x1b[1;" << initialTermView.Height() << L"r"; + completeScrollRegion = wss.str(); + } + std::wstring reducedCursorBottomRight; + { + std::wstringstream wss; + // For 20 tall and 100 wide buffer + // ESC[19;100H + // Put cursor on line 19 (1 before last) and the right most column 100. + // (Remember that in VT, we start counting from 1 not 0.) + wss << L"\x1b[" << initialTermView.Height() - 1 << L";" << initialTermView.Width() << "H"; + reducedCursorBottomRight = wss.str(); + } + std::wstring completeCursorAtPromptLine; + { + std::wstringstream wss; + // For 20 tall and 100 wide buffer + // ESC[19;1H + // Put cursor on line 19 (1 before last) and the left most column 1. + // (Remember that in VT, we start counting from 1 not 0.) + wss << L"\x1b[" << initialTermView.Height() - 1 << L";1H"; + completeCursorAtPromptLine = wss.str(); + } + + Log::Comment(L"Perform all the operations on the buffer."); + + // OK this is what TMUX does. + // 1. Mark off the scroll area as everything but the mode line. + hostSm.ProcessString(reducedScrollRegion); + // 2. Put the cursor in the bottom-right corner of the scroll region. + hostSm.ProcessString(reducedCursorBottomRight); + // 3. Send a single newline which should do the heavy lifting + // of pushing everything in the scroll region up by 1 line and + // leave everything outside the region alone. + + // This entire block is subject to change in the future with optimizations. + { + // Cursor gets redrawn in the bottom right of the scroll region with the repaint that is forced + // early while the screen is rotated. + std::stringstream ss; + ss << "\x1b[" << initialTermView.Height() - 1 << ";" << initialTermView.Width() << "H"; + expectedOutput.push_back(ss.str()); + + expectedOutput.push_back("\x1b[?25h"); // turn the cursor back on too. + } + + hostSm.ProcessString(L"\n"); + // 4. Remove the scroll area by setting it to the entire size of the viewport. + hostSm.ProcessString(completeScrollRegion); + // 5. Put the cursor back at the beginning of the new line that was just revealed. + hostSm.ProcessString(completeCursorAtPromptLine); + + // Set up the verifications like above. + auto verifyBufferAfter = [&](const TextBuffer& tb) { + auto& cursor = tb.GetCursor(); + // Verify the cursor is waiting on the freshly revealed line (1 above mode line) + // and in the left most column. + VERIFY_ARE_EQUAL(initialTermView.Height() - 2, cursor.GetPosition().Y); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().X); + + // For all rows except the last two, verify that we have a run of four letters. + for (auto i = 0; i < rowsToWrite - 1; ++i) + { + // Start with B this time because the A line got scrolled off the top. + const std::wstring expectedString(4, static_cast(L'B' + i)); + const COORD expectedPos{ 0, gsl::narrow(i) }; + TestUtils::VerifyExpectedString(tb, expectedString, expectedPos); + } + + // For the second to last row, verify that it is blank. + { + const std::wstring expectedBlankLine(initialTermView.Width(), L' '); + const COORD blankLinePos{ 0, gsl::narrow(rowsToWrite - 1) }; + TestUtils::VerifyExpectedString(tb, expectedBlankLine, blankLinePos); + } + + // For the last row, verify we have an entire row of asterisks for the mode line. + { + const std::wstring expectedModeLine(initialTermView.Width(), L'*'); + const COORD modeLinePos{ 0, gsl::narrow(rowsToWrite) }; + TestUtils::VerifyExpectedString(tb, expectedModeLine, modeLinePos); + } + }; + + // This will verify the text emitted from the PTY. + + expectedOutput.push_back("\x1b[H"); // cursor returns to top left corner. + for (auto i = 0; i < rowsToWrite - 1; ++i) + { + const std::string expectedString(4, static_cast('B' + i)); + expectedOutput.push_back(expectedString); + expectedOutput.push_back("\x1b[K"); // erase the rest of the line. + expectedOutput.push_back("\r\n"); + } + { + expectedOutput.push_back(""); // nothing for the empty line + expectedOutput.push_back("\x1b[K"); // erase the rest of the line. + expectedOutput.push_back("\r\n"); + } + { + const std::string expectedString(initialTermView.Width(), '*'); + expectedOutput.push_back(expectedString); + } + { + // Cursor gets reset into second line from bottom, left most column + std::stringstream ss; + ss << "\x1b[" << initialTermView.Height() - 1 << ";1H"; + expectedOutput.push_back(ss.str()); + } + expectedOutput.push_back("\x1b[?25h"); // turn the cursor back on too. + + Log::Comment(L"Verify host buffer contains pattern moved up one and mode line still in place."); + // Verify the host side. + verifyBufferAfter(hostTb); + + Log::Comment(L"Emit PTY frame and validate it transmits the right data."); + // Paint the frame + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"Verify terminal buffer contains pattern moved up one and mode line still in place."); + // Verify the terminal side. + verifyBufferAfter(termTb); +} diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index 38becbe501..6fe8be984c 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -171,7 +171,33 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100; // displays the correct text. if (newViewOrigin == viewport.Origin()) { - Viewport invalid = Viewport::FromDimensions(viewport.Origin(), { viewport.Width(), delta }); + // Inside this block, we're shifting down at the bottom. + // This means that we had something like this: + // AAAA + // BBBB + // CCCC + // DDDD + // EEEE + // + // Our margins were set for lines A-D, but not on line E. + // So we circled the whole buffer up by one: + // BBBB + // CCCC + // DDDD + // EEEE + // + // + // Then we scrolled the contents of everything OUTSIDE the margin frame down. + // BBBB + // CCCC + // DDDD + // + // EEEE + // + // And now we need to report that only the bottom line didn't "move" as we put the EEEE + // back where it started, but everything else moved. + // In this case, delta was 1. So the amount that moved is the entire viewport height minus the delta. + Viewport invalid = Viewport::FromDimensions(viewport.Origin(), { viewport.Width(), viewport.Height() - delta }); screenInfo.GetRenderTarget().TriggerRedraw(invalid); } diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp index 07493ab454..ecfd352af4 100644 --- a/src/host/ut_host/ConptyOutputTests.cpp +++ b/src/host/ut_host/ConptyOutputTests.cpp @@ -27,6 +27,9 @@ using namespace Microsoft::Console::Types; class ConptyOutputTests { + // This test class is to write some things into the PTY and then check that + // the rendering that is coming out of the VT-sequence generator is exactly + // as we expect it to be. BEGIN_TEST_CLASS(ConptyOutputTests) TEST_CLASS_PROPERTY(L"IsolationLevel", L"Class") END_TEST_CLASS() diff --git a/src/inc/til/point.h b/src/inc/til/point.h index c7643b0528..39a465f073 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -107,6 +107,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return point{ x, y }; } + point& operator+=(const point& other) + { + *this = *this + other; + return *this; + } + point operator-(const point& other) const { ptrdiff_t x; @@ -118,6 +124,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return point{ x, y }; } + point& operator-=(const point& other) + { + *this = *this - other; + return *this; + } + point operator*(const point& other) const { ptrdiff_t x; @@ -129,6 +141,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return point{ x, y }; } + point& operator*=(const point& other) + { + *this = *this * other; + return *this; + } + point operator/(const point& other) const { ptrdiff_t x; @@ -140,6 +158,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return point{ x, y }; } + point& operator/=(const point& other) + { + *this = *this / other; + return *this; + } + constexpr ptrdiff_t x() const noexcept { return _x; diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 8c0420e522..eaae0ad696 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -343,19 +343,20 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // Return Value: // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT XtermEngine::ScrollFrame() noexcept +try { - if (_scrollDelta.X != 0) + if (_scrollDelta.x() != 0) { // No easy way to shift left-right. Everything needs repainting. return InvalidateAll(); } - if (_scrollDelta.Y == 0) + if (_scrollDelta.y() == 0) { // There's nothing to do here. Do nothing. return S_OK; } - const short dy = _scrollDelta.Y; + const short dy = _scrollDelta.y(); const short absDy = static_cast(abs(dy)); HRESULT hr = S_OK; @@ -391,6 +392,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, return hr; } +CATCH_RETURN(); // Routine Description: // - Notifies us that the console is attempting to scroll the existing screen @@ -402,25 +404,23 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // Return Value: // - S_OK if we succeeded, else an appropriate HRESULT for safemath failure [[nodiscard]] HRESULT XtermEngine::InvalidateScroll(const COORD* const pcoordDelta) noexcept +try { - const short dx = pcoordDelta->X; - const short dy = pcoordDelta->Y; + const til::point delta{ *pcoordDelta }; - if (dx != 0 || dy != 0) + if (delta != til::point{ 0, 0 }) { + _trace.TraceInvalidateScroll(delta); + // Scroll the current offset and invalidate the revealed area - _invalidMap.translate(til::point(*pcoordDelta), true); + _invalidMap.translate(delta, true); - COORD invalidScrollNew; - RETURN_IF_FAILED(ShortAdd(_scrollDelta.X, dx, &invalidScrollNew.X)); - RETURN_IF_FAILED(ShortAdd(_scrollDelta.Y, dy, &invalidScrollNew.Y)); - - // Store if safemath succeeded - _scrollDelta = invalidScrollNew; + _scrollDelta += delta; } return S_OK; } +CATCH_RETURN(); // Routine Description: // - Draws one line of the buffer to the screen. Writes the characters to the diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp index 597f56b0f0..7e85a81cfd 100644 --- a/src/renderer/vt/invalidate.cpp +++ b/src/renderer/vt/invalidate.cpp @@ -121,6 +121,8 @@ CATCH_RETURN(); _circled = true; } + _trace.TraceTriggerCircling(*pForcePaint); + return S_OK; } diff --git a/src/renderer/vt/math.cpp b/src/renderer/vt/math.cpp index 1a131914e0..cab0c5ec7e 100644 --- a/src/renderer/vt/math.cpp +++ b/src/renderer/vt/math.cpp @@ -63,8 +63,8 @@ void VtEngine::_OrRect(_Inout_ SMALL_RECT* const pRectExisting, const SMALL_RECT // - true iff only the next character is invalid bool VtEngine::_WillWriteSingleChar() const { - // If there is scroll delta, return false. - if (til::point{ 0, 0 } != til::point{ _scrollDelta }) + // If there is no scroll delta, return false. + if (til::point{ 0, 0 } != _scrollDelta) { return false; } diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index babc6606ca..e643f51db5 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -27,7 +27,7 @@ using namespace Microsoft::Console::Types; // If there's nothing to do, quick return bool somethingToDo = _invalidMap.any() || - (_scrollDelta.X != 0 || _scrollDelta.Y != 0) || + _scrollDelta != til::point{ 0, 0 } || _cursorMoved || _titleChanged; @@ -52,7 +52,7 @@ using namespace Microsoft::Console::Types; _invalidMap.reset_all(); - _scrollDelta = { 0 }; + _scrollDelta = { 0, 0 }; _clearedAllThisFrame = false; _cursorMoved = false; _firstPaint = false; diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index a50179cd50..bcbaf4994f 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -38,7 +38,7 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, _invalidMap(initialViewport.Dimensions()), _lastRealCursor({ 0 }), _lastText({ 0 }), - _scrollDelta({ 0 }), + _scrollDelta({ 0, 0 }), _quickReturn(false), _clearedAllThisFrame(false), _cursorMoved(false), diff --git a/src/renderer/vt/tracing.cpp b/src/renderer/vt/tracing.cpp index 023a5453f8..304d15cc55 100644 --- a/src/renderer/vt/tracing.cpp +++ b/src/renderer/vt/tracing.cpp @@ -127,6 +127,23 @@ void RenderTracing::TraceTriggerCircling(const bool newFrame) const #endif UNIT_TESTING } +void RenderTracing::TraceInvalidateScroll(const til::point scroll) const +{ +#ifndef UNIT_TESTING + if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + const auto scrollDeltaStr = scroll.to_string(); + const auto scrollDelta = scrollDeltaStr.c_str(); + TraceLoggingWrite(g_hConsoleVtRendererTraceProvider, + "VtEngine_TraceInvalidateScroll", + TraceLoggingWideString(scrollDelta), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } +#else + UNREFERENCED_PARAMETER(scroll); +#endif +} + void RenderTracing::TraceStartPaint(const bool quickReturn, const til::bitmap invalidMap, const til::rectangle lastViewport, diff --git a/src/renderer/vt/tracing.hpp b/src/renderer/vt/tracing.hpp index f7c18c6f83..0c36b42f05 100644 --- a/src/renderer/vt/tracing.hpp +++ b/src/renderer/vt/tracing.hpp @@ -34,6 +34,7 @@ namespace Microsoft::Console::VirtualTerminal void TracePaintCursor(const til::point coordCursor) const; void TraceInvalidateAll(const til::rectangle view) const; void TraceTriggerCircling(const bool newFrame) const; + void TraceInvalidateScroll(const til::point scroll) const; void TraceStartPaint(const bool quickReturn, const til::bitmap invalidMap, const til::rectangle lastViewport, diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 7ebb7a71a8..2230410f4d 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -126,7 +126,7 @@ namespace Microsoft::Console::Render COORD _lastRealCursor; COORD _lastText; - COORD _scrollDelta; + til::point _scrollDelta; bool _quickReturn; bool _clearedAllThisFrame; diff --git a/src/til/ut_til/PointTests.cpp b/src/til/ut_til/PointTests.cpp index 724d37ad7e..bbeba7cd83 100644 --- a/src/til/ut_til/PointTests.cpp +++ b/src/til/ut_til/PointTests.cpp @@ -207,6 +207,50 @@ class PointTests } } + TEST_METHOD(AdditionInplace) + { + Log::Comment(L"0.) Addition of two things that should be in bounds."); + { + const til::point pt{ 5, 10 }; + const til::point pt2{ 23, 47 }; + + const til::point expected{ pt.x() + pt2.x(), pt.y() + pt2.y() }; + + auto actual = pt; + actual += pt2; + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"1.) Addition results in value that is too large (x)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ bigSize, static_cast(0) }; + const til::point pt2{ 1, 1 }; + + auto fn = [&]() { + auto actual = pt; + actual += pt2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Addition results in value that is too large (y)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ static_cast(0), bigSize }; + const til::point pt2{ 1, 1 }; + + auto fn = [&]() { + auto actual = pt; + actual += pt2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + TEST_METHOD(Subtraction) { Log::Comment(L"0.) Subtraction of two things that should be in bounds."); @@ -246,6 +290,50 @@ class PointTests } } + TEST_METHOD(SubtractionInplace) + { + Log::Comment(L"0.) Subtraction of two things that should be in bounds."); + { + const til::point pt{ 5, 10 }; + const til::point pt2{ 23, 47 }; + + const til::point expected{ pt.x() - pt2.x(), pt.y() - pt2.y() }; + + auto actual = pt; + actual -= pt2; + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"1.) Subtraction results in value that is too small (x)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ bigSize, static_cast(0) }; + const til::point pt2{ -2, -2 }; + + auto fn = [&]() { + auto actual = pt2; + actual -= pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Subtraction results in value that is too small (y)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ static_cast(0), bigSize }; + const til::point pt2{ -2, -2 }; + + auto fn = [&]() { + auto actual = pt2; + actual -= pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + TEST_METHOD(Multiplication) { Log::Comment(L"0.) Multiplication of two things that should be in bounds."); @@ -285,6 +373,50 @@ class PointTests } } + TEST_METHOD(MultiplicationInplace) + { + Log::Comment(L"0.) Multiplication of two things that should be in bounds."); + { + const til::point pt{ 5, 10 }; + const til::point pt2{ 23, 47 }; + + const til::point expected{ pt.x() * pt2.x(), pt.y() * pt2.y() }; + + auto actual = pt; + actual *= pt2; + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"1.) Multiplication results in value that is too large (x)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ bigSize, static_cast(0) }; + const til::point pt2{ 10, 10 }; + + auto fn = [&]() { + auto actual = pt; + actual *= pt2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Multiplication results in value that is too large (y)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ static_cast(0), bigSize }; + const til::point pt2{ 10, 10 }; + + auto fn = [&]() { + auto actual = pt; + actual *= pt2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + TEST_METHOD(Division) { Log::Comment(L"0.) Division of two things that should be in bounds."); @@ -311,6 +443,35 @@ class PointTests } } + TEST_METHOD(DivisionInplace) + { + Log::Comment(L"0.) Division of two things that should be in bounds."); + { + const til::point pt{ 555, 510 }; + const til::point pt2{ 23, 47 }; + + const til::point expected{ pt.x() / pt2.x(), pt.y() / pt2.y() }; + auto actual = pt; + actual /= pt2; + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"1.) Division by zero"); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ bigSize, static_cast(0) }; + const til::point pt2{ 1, 1 }; + + auto fn = [&]() { + auto actual = pt2; + actual /= pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + TEST_METHOD(X) { const til::point pt{ 5, 10 };