Repository: microsoft/terminal (language: cpp)

Issue:
Fix the location that selecting a mark uses
I think this subtly regressed in #16611. Jump to https://github.com/microsoft/terminal/commit/90b8bb7c2d8932fc601aed9c26f13c77d5a1321d#diff-f9112caf8cb75e7a48a7b84987724d754181227385fbfcc2cc09a879b1f97c12L171-L223

`Terminal::SelectNewRegion` is the only thing that uses the return value from `Terminal::_ScrollToPoints`. Before that PR, `_ScrollToPoints` was just a part of `SelectNewRegion`, and it moved the start & end coords by the `_VisibleStartIndex`, not the `_scrollOffset`.

Kinda weird there weren't any _other_ tests for `SelectNewRegion`?

I also caught a second bug while I was here - If you had a line with an exact wrap, and tried to select that like with selectOutput, we'd explode.

Closes #17131



Discussion / hints:

Fix patch:
diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp
index 86987e68b59..4ee701982eb 100644
--- a/src/buffer/out/textBuffer.cpp
+++ b/src/buffer/out/textBuffer.cpp
@@ -790,7 +790,7 @@ const til::CoordType TextBuffer::GetFirstRowIndex() const noexcept
 
 const Viewport TextBuffer::GetSize() const noexcept
 {
-    return Viewport::FromDimensions({ _width, _height });
+    return Viewport::FromDimensions({}, { _width, _height });
 }
 
 void TextBuffer::_SetFirstRowIndex(const til::CoordType FirstRowIndex) noexcept
@@ -1404,7 +1404,7 @@ til::point TextBuffer::_GetWordEndForSelection(const til::point target, const st
             }
         }
 
-        bufferSize.IncrementInBoundsCircular(result);
+        bufferSize.IncrementInBounds(result);
     }
 
     if (_GetDelimiterClassAt(result, wordDelimiters) != initialDelimiter)
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
index 0cda0c81475..6ff980d9c35 100644
--- a/src/cascadia/TerminalControl/ControlCore.cpp
+++ b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -2691,21 +2691,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // Select the region of text between [s.start, s.end), in buffer space
     void ControlCore::_selectSpan(til::point_span s)
     {
-        // s.end is an _exclusive_ point. We need an inclusive one. But
-        // decrement in bounds wants an inclusive one. If you pass an exclusive
-        // one, then it might assert at you for being out of bounds. So we also
-        // take care of the case that the end point is outside the viewport
-        // manually.
+        // s.end is an _exclusive_ point. We need an inclusive one.
         const auto bufferSize{ _terminal->GetTextBuffer().GetSize() };
         til::point inclusiveEnd = s.end;
-        if (s.end.x == bufferSize.Width())
-        {
-            inclusiveEnd = til::point{ std::max(0, s.end.x - 1), s.end.y };
-        }
-        else
-        {
-            bufferSize.DecrementInBounds(inclusiveEnd);
-        }
+        bufferSize.DecrementInBounds(inclusiveEnd);
 
         _terminal->SelectNewRegion(s.start, inclusiveEnd);
         _renderer->TriggerSelection();
diff --git a/src/cascadia/TerminalControl/HwndTerminal.cpp b/src/cascadia/TerminalControl/HwndTerminal.cpp
index 91cb3aaa4df..5086515e975 100644
--- a/src/cascadia/TerminalControl/HwndTerminal.cpp
+++ b/src/cascadia/TerminalControl/HwndTerminal.cpp
@@ -356,7 +356,7 @@ HRESULT HwndTerminal::Refresh(const til::size windowSize, _Out_ til::size* dimen
     _renderer->TriggerRedrawAll();
 
     // Convert our new dimensions to characters
-    const auto viewInPixels = Viewport::FromDimensions(windowSize);
+    const auto viewInPixels = Viewport::FromDimensions({}, windowSize);
     const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels);
 
     // Guard against resizing the window to 0 columns/rows, which the text buffer classes don't really support.
@@ -464,7 +464,7 @@ try
 
     Viewport viewInPixels;
     {
-        const auto viewInCharacters = Viewport::FromDimensions(dimensionsInCharacters);
+        const auto viewInCharacters = Viewport::FromDimensions({}, dimensionsInCharacters);
         const auto lock = publicTerminal->_terminal->LockForReading();
         viewInPixels = publicTerminal->_renderEngine->GetViewportInPixels(viewInCharacters);
     }
@@ -491,7 +491,7 @@ try
 {
     const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
 
-    const auto viewInPixels = Viewport::FromDimensions({ width, height });
+    const auto viewInPixels = Viewport::FromDimensions({}, { width, height });
     const auto lock = publicTerminal->_terminal->LockForReading();
     const auto viewInCharacters = publicTerminal->_renderEngine->GetViewportInCharacters(viewInPixels);
 
diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp
index 963ae2c19a8..c6301dd1eeb 100644
--- a/src/cascadia/TerminalCore/Terminal.cpp
+++ b/src/cascadia/TerminalCore/Terminal.cpp
@@ -972,7 +972,7 @@ Viewport Terminal::_GetMutableViewport() const noexcept
     // GH#3493: if we're in the alt buffer, then it's possible that the mutable
     // viewport's size hasn't been updated yet. In that case, use the
     // temporarily stashed _altBufferSize instead.
-    return _inAltBuffer() ? Viewport::FromDimensions(_altBufferSize) :
+    return _inAltBuffer() ? Viewport::FromDimensions({}, _altBufferSize) :
                             _mutableViewport;
 }
 
diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp
index 44c9051ed01..11f3ae72c0c 100644
--- a/src/cascadia/TerminalCore/TerminalSelection.cpp
+++ b/src/cascadia/TerminalCore/TerminalSelection.cpp
@@ -495,7 +495,7 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
             searchEnd = { bufferSize.RightInclusive(), searchStart.y - 1 };
             searchStart = { bufferSize.Left(), std::max(searchStart.y - viewportHeight, bufferSize.Top()) };
         }
-        searchArea = Viewport::FromDimensions(searchStart, searchEnd.x + 1, searchEnd.y + 1);
+        searchArea = Viewport::FromDimensions(searchStart, { searchEnd.x + 1, searchEnd.y + 1 });
 
         const til::point bufferStart{ bufferSize.Origin() };
         const til::point bufferEnd{ bufferSize.RightInclusive(), ViewEndIndex() };
@@ -516,7 +516,7 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
                     searchEnd.y -= 1;
                     searchStart.y = std::max(searchEnd.y - viewportHeight, bufferSize.Top());
                 }
-                searchArea = Viewport::FromDimensions(searchStart, searchEnd.x + 1, searchEnd.y + 1);
+                searchArea = Viewport::FromDimensions(searchStart, { searchEnd.x + 1, searchEnd.y + 1 });
             }
         }
 
diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
index 42099a24e56..af174f0497d 100644
--- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
+++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
@@ -4873,13 +4873,7 @@ void ConptyRoundtripTests::ReflowPromptRegions()
             til::point afterPos = originalPos;
             // walk that original pos dx times into the actual real place in the buffer.
             auto bufferViewport = tb.GetSize();
-            const auto walkDir = Viewport::WalkDir{ dx < 0 ? Viewport::XWalk::LeftToRight : Viewport::XWalk::RightToLeft,
-                                                    dx < 0 ? Viewport::YWalk::TopToBottom : Viewport::YWalk::BottomToTop };
-            for (auto i = 0; i < std::abs(dx); i++)
-            {
-                bufferViewport.WalkInBounds(afterPos,
-                                            walkDir);
-            }
+            bufferViewport.WalkInBounds(afterPos, -dx);
             const auto expectedOutputStart = !afterResize ?
                                                  originalPos : // printed exactly a row, so we're exactly below the prompt
                                                  afterPos;
diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp
index 3170d3a8185..cd2cead0f5a 100644
--- a/src/host/VtIo.cpp
+++ b/src/host/VtIo.cpp
@@ -173,9 +173,7 @@ VtIo::VtIo() :
 
         if (IsValidHandle(_hOutput.get()))
         {
-            auto initialViewport = Viewport::FromDimensions({ 0, 0 },
-                                                            gci.GetWindowSize().width,
-                                                            gci.GetWindowSize().height);
+            auto initialViewport = Viewport::FromDimensions({ 0, 0 }, gci.GetWindowSize());
             switch (_IoMode)
             {
             case VtIoMode::XTERM_256:
diff --git a/src/host/_output.cpp b/src/host/_output.cpp
index bc8d6a104ad..9db5ea4c8d6 100644
--- a/src/host/_output.cpp
+++ b/src/host/_output.cpp
@@ -225,7 +225,7 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region)
         {
             // Notify accessibility
             auto endingCoordinate = startingCoordinate;
-            bufferSize.MoveInBounds(cellsModifiedCoord, endingCoordinate);
+            bufferSize.WalkInBounds(endingCoordinate, cellsModifiedCoord);
             screenBuffer.NotifyAccessibilityEventing(startingCoordinate.x, startingCoordinate.y, endingCoordinate.x, endingCoordinate.y);
         }
     }
@@ -292,7 +292,7 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region)
         if (screenInfo.HasAccessibilityEventing())
         {
             auto endingCoordinate = startingCoordinate;
-            bufferSize.MoveInBounds(cellsModifiedCoord, endingCoordinate);
+            bufferSize.WalkInBounds(endingCoordinate, cellsModifiedCoord);
             screenInfo.NotifyAccessibilityEventing(startingCoordinate.x, startingCoordinate.y, endingCoordinate.x, endingCoordinate.y);
         }
 
diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp
index 6a45c44c019..59b81b40e1d 100644
--- a/src/host/outputStream.cpp
+++ b/src/host/outputStream.cpp
@@ -321,7 +321,7 @@ bool ConhostInternalGetSet::ResizeWindow(const til::CoordType sColumns, const ti
     api->GetConsoleScreenBufferInfoExImpl(screenInfo, csbiex);
 
     const auto oldViewport = screenInfo.GetVirtualViewport();
-    auto newViewport = Viewport::FromDimensions(oldViewport.Origin(), sColumns, sRows);
+    auto newViewport = Viewport::FromDimensions(oldViewport.Origin(), { sColumns, sRows });
     // Always resize the width of the console
     csbiex.dwSize.X = gsl::narrow_cast<short>(sColumns);
     // Only set the screen buffer's height if it's currently less than
diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp
index 3a8141e81e2..6a823b3b39f 100644
--- a/src/host/screenInfo.cpp
+++ b/src/host/screenInfo.cpp
@@ -152,7 +152,7 @@ Viewport SCREEN_INFORMATION::GetTerminalBufferSize() const
     auto v = _textBuffer->GetSize();
     if (gci.IsTerminalScrolling() && v.Height() > _virtualBottom)
     {
-        v = Viewport::FromDimensions({ 0, 0 }, v.Width(), _virtualBottom + 1);
+        v = Viewport::FromDimensions({}, { v.Width(), _virtualBottom + 1 });
     }
     return v;
 }
diff --git a/src/host/selectionInput.cpp b/src/host/selectionInput.cpp
index 1fce41836f9..a64874c52a7 100644
--- a/src/host/selectionInput.cpp
+++ b/src/host/selectionInput.cpp
@@ -955,7 +955,7 @@ bool Selection::_HandleMarkModeSelectionNav(const INPUT_KEY_INFO* const pInputKe
             if (pcoordInputEnd != nullptr)
             {
                 // - 1 so the coordinate is on top of the last position of the text, not one past it.
-                gci.GetActiveOutputBuffer().GetBufferSize().MoveInBounds(-1, boundaries.end);
+                gci.GetActiveOutputBuffer().GetBufferSize().WalkInBounds(boundaries.end, -1);
                 *pcoordInputEnd = boundaries.end;
             }
             return true;
diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp
index 54b48a3e967..4016152a9af 100644
--- a/src/host/ut_host/ScreenBufferTests.cpp
+++ b/src/host/ut_host/ScreenBufferTests.cpp
@@ -79,7 +79,7 @@ class ScreenBufferTests
         VERIFY_SUCCEEDED(currentBuffer.SetViewportOrigin(true, { 0, 0 }, true));
         // Make sure the viewport always starts off at the default size.
         auto defaultSize = til::size{ CommonState::s_csWindowWidth, CommonState::s_csWindowHeight };
-        currentBuffer.SetViewport(Viewport::FromDimensions(defaultSize), true);
+        currentBuffer.SetViewport(Viewport::FromDimensions({}, defaultSize), true);
         VERIFY_ARE_EQUAL(til::point(0, 0), currentBuffer.GetTextBuffer().GetCursor().GetPosition());
         // Make sure the virtual bottom is correctly positioned.
         currentBuffer.UpdateBottom();
diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp
index d39be844de6..7b963fbd91a 100644
--- a/src/host/ut_host/TextBufferTests.cpp
+++ b/src/host/ut_host/TextBufferTests.cpp
@@ -2829,13 +2829,7 @@ void TextBufferTests::ReflowPromptRegions()
             til::point afterPos = originalPos;
             // walk that original pos dx times into the actual real place in the buffer.
             auto bufferViewport = tb.GetSize();
-            const auto walkDir = Viewport::WalkDir{ dx < 0 ? Viewport::XWalk::LeftToRight : Viewport::XWalk::RightToLeft,
-                                                    dx < 0 ? Viewport::YWalk::TopToBottom : Viewport::YWalk::BottomToTop };
-            for (auto i = 0; i < std::abs(dx); i++)
-            {
-                bufferViewport.WalkInBounds(afterPos,
-                                            walkDir);
-            }
+            bufferViewport.WalkInBounds(afterPos, -dx);
             const auto expectedOutputStart = !afterResize ?
                                                  originalPos : // printed exactly a row, so we're exactly below the prompt
                                                  afterPos;
diff --git a/src/host/ut_host/ViewportTests.cpp b/src/host/ut_host/ViewportTests.cpp
index edc9b70a2d6..04947a8debb 100644
--- a/src/host/ut_host/ViewportTests.cpp
+++ b/src/host/ut_host/ViewportTests.cpp
@@ -109,7 +109,7 @@ class ViewportTests
         dimensions.width = rect.right - rect.left + 1;
         dimensions.height = rect.bottom - rect.top + 1;
 
-        const auto v = Viewport::FromDimensions(origin, dimensions.width, dimensions.height);
+        const auto v = Viewport::FromDimensions(origin, dimensions);
 
         VERIFY_ARE_EQUAL(rect.left, v.Left());
         VERIFY_ARE_EQUAL(rect.right, v.RightInclusive());
@@ -169,7 +169,7 @@ class ViewportTests
         dimensions.width = rect.right - rect.left + 1;
         dimensions.height = rect.bottom - rect.top + 1;
 
-        const auto v = Viewport::FromDimensions(dimensions);
+        const auto v = Viewport::FromDimensions({}, dimensions);
 
         VERIFY_ARE_EQUAL(rect.left, v.Left());
         VERIFY_ARE_EQUAL(rect.right, v.RightInclusive());
@@ -183,28 +183,6 @@ class ViewportTests
         VERIFY_ARE_EQUAL(dimensions, v.Dimensions());
     }
 
-    TEST_METHOD(CreateFromCoord)
-    {
-        til::point origin;
-        origin.x = 12;
-        origin.y = 24;
-
-        const auto v = Viewport::FromCoord(origin);
-
-        VERIFY_ARE_EQUAL(origin.x, v.Left());
-        VERIFY_ARE_EQUAL(origin.x, v.RightInclusive());
-        VERIFY_ARE_EQUAL(origin.x + 1, v.RightExclusive());
-        VERIFY_ARE_EQUAL(origin.y, v.Top());
-        VERIFY_ARE_EQUAL(origin.y, v.BottomInclusive());
-        VERIFY_ARE_EQUAL(origin.y + 1, v.BottomExclusive());
-        VERIFY_ARE_EQUAL(1, v.Height());
-        VERIFY_ARE_EQUAL(1, v.Width());
-        VERIFY_ARE_EQUAL(origin, v.Origin());
-        // clang-format off
-        VERIFY_ARE_EQUAL(til::size(1, 1), v.Dimensions());
-        // clang-format on
-    }
-
     TEST_METHOD(IsInBoundsCoord)
     {
         til::inclusive_rect r;
@@ -503,51 +481,6 @@ class ViewportTests
         VERIFY_ARE_EQUAL(screen.y, edges.bottom);
     }
 
-    TEST_METHOD(IncrementInBoundsCircular)
-    {
-        auto success = false;
-
-        til::inclusive_rect edges;
-        edges.left = 10;
-        edges.right = 19;
-        edges.top = 20;
-        edges.bottom = 29;
-
-        const auto v = Viewport::FromInclusive(edges);
-        til::point original;
-        til::point screen;
-
-        // #1 coord inside region
-        original.x = screen.x = 15;
-        original.y = screen.y = 25;
-
-        success = v.IncrementInBoundsCircular(screen);
-
-        VERIFY_IS_TRUE(success);
-        VERIFY_ARE_EQUAL(screen.x, original.x + 1);
-        VERIFY_ARE_EQUAL(screen.y, original.y);
-
-        // #2 coord right edge, not bottom
-        original.x = screen.x = edges.right;
-        original.y = screen.y = 25;
-
-        success = v.IncrementInBoundsCircular(screen);
-
-        VERIFY_IS_TRUE(success);
-        VERIFY_ARE_EQUAL(screen.x, edges.left);
-        VERIFY_ARE_EQUAL(screen.y, original.y + 1);
-
-        // #3 coord right edge, bottom
-        original.x = screen.x = edges.right;
-        original.y = screen.y = edges.bottom;
-
-        success = v.IncrementInBoundsCircular(screen);
-
-        VERIFY_IS_FALSE(success);
-        VERIFY_ARE_EQUAL(screen.x, edges.left);
-        VERIFY_ARE_EQUAL(screen.y, edges.top);
-    }
-
     TEST_METHOD(DecrementInBounds)
     {
         auto success = false;
@@ -593,52 +526,7 @@ class ViewportTests
         VERIFY_ARE_EQUAL(screen.y, edges.top);
     }
 
-    TEST_METHOD(DecrementInBoundsCircular)
-    {
-        auto success = false;
-
-        til::inclusive_rect edges;
-        edges.left = 10;
-        edges.right = 19;
-        edges.top = 20;
-        edges.bottom = 29;
-
-        const auto v = Viewport::FromInclusive(edges);
-        til::point original;
-        til::point screen;
-
-        // #1 coord inside region
-        original.x = screen.x = 15;
-        original.y = screen.y = 25;
-
-        success = v.DecrementInBoundsCircular(screen);
-
-        VERIFY_IS_TRUE(success);
-        VERIFY_ARE_EQUAL(screen.x, original.x - 1);
-        VERIFY_ARE_EQUAL(screen.y, original.y);
-
-        // #2 coord left edge, not top
-        original.x = screen.x = edges.left;
-        original.y = screen.y = 25;
-
-        success = v.DecrementInBoundsCircular(screen);
-
-        VERIFY_IS_TRUE(success);
-        VERIFY_ARE_EQUAL(screen.x, edges.right);
-        VERIFY_ARE_EQUAL(screen.y, original.y - 1);
-
-        // #3 coord left edge, top
-        original.x = screen.x = edges.left;
-        original.y = screen.y = edges.top;
-
-        success = v.DecrementInBoundsCircular(screen);
-
-        VERIFY_IS_FALSE(success);
-        VERIFY_ARE_EQUAL(screen.x, edges.right);
-        VERIFY_ARE_EQUAL(screen.y, edges.bottom);
-    }
-
-    til::CoordType RandomCoord()
+    static til::CoordType RandomCoord()
     {
         til::CoordType s;
 
@@ -673,29 +561,16 @@ class ViewportTests
 
             auto sAddAmount = RandomCoord() % (sRowWidth * sRowWidth);
 
-            til::point coordFinal;
-            coordFinal.x = (coordPos.x + sAddAmount) % sRowWidth;
-            coordFinal.y = coordPos.y + ((coordPos.x + sAddAmount) / sRowWidth);
-
-            Log::Comment(String().Format(L"Add To Position: (%d, %d)  Amount to add: %d", coordPos.y, coordPos.x, sAddAmount));
-
-            // Movement result is expected to be true, unless there's an error.
-            auto fExpectedResult = true;
-
-            // if we've calculated past the final row, then the function will reset to the original position and the output will be false.
-            if (coordFinal.y >= sRowWidth)
-            {
-                coordFinal = coordPos;
-                fExpectedResult = false;
-            }
-
-            const bool fActualResult = v.MoveInBounds(sAddAmount, coordPos);
+            const til::point coord{
+                (coordPos.x + sAddAmount) % sRowWidth,
+                coordPos.y + ((coordPos.x + sAddAmount) / sRowWidth),
+            };
+            const auto coordClamped = std::clamp(coord, v.Origin(), v.BottomRightInclusive());
+            const auto fExpectedResult = coord == coordClamped;
+            const bool fActualResult = v.WalkInBounds(coordPos, sAddAmount);
 
             VERIFY_ARE_EQUAL(fExpectedResult, fActualResult);
-            VERIFY_ARE_EQUAL(coordPos.x, coordFinal.x);
-            VERIFY_ARE_EQUAL(coordPos.y, coordFinal.y);
-
-            Log::Comment(String().Format(L"Actual: (%d, %d) Expected: (%d, %d)", coordPos.y, coordPos.x, coordFinal.y, coordFinal.x));
+            VERIFY_ARE_EQUAL(coordPos, coordClamped);
         }
     }
 
diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp
index b3b431726d5..07cda4d9ee6 100644
--- a/src/renderer/base/renderer.cpp
+++ b/src/renderer/base/renderer.cpp
@@ -274,7 +274,7 @@ void Renderer::TriggerRedraw(const Viewport& region)
 // - <none>
 void Renderer::TriggerRedraw(const til::point* const pcoord)
 {
-    TriggerRedraw(Viewport::FromCoord(*pcoord)); // this will notify to paint if we need it.
+    TriggerRedraw(Viewport::FromDimensions(*pcoord, { 1, 1 })); // this will notify to paint if we need it.
 }
 
 // Routine Description:
diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp
index 9b35e29f458..f5b0ac777bc 100644
--- a/src/terminal/adapter/adaptDispatch.cpp
+++ b/src/terminal/adapter/adaptDispatch.cpp
@@ -616,8 +616,8 @@ void AdaptDispatch::_ScrollRectVertically(const Page& page, const til::rect& scr
             // requested buffer range one cell at a time.
             const auto srcOrigin = til::point{ scrollRect.left, top };
             const auto dstOrigin = til::point{ scrollRect.left, top + actualDelta };
-            const auto srcView = Viewport::FromDimensions(srcOrigin, width, height);
-            const auto dstView = Viewport::FromDimensions(dstOrigin, width, height);
+            const auto srcView = Viewport::FromDimensions(srcOrigin, { width, height });
+            const auto dstView = Viewport::FromDimensions(dstOrigin, { width, height });
             const auto walkDirection = Viewport::DetermineWalkDirection(srcView, dstView);
             auto srcPos = srcView.GetWalkOrigin(walkDirection);
             auto dstPos = dstView.GetWalkOrigin(walkDirection);
@@ -663,7 +663,7 @@ void AdaptDispatch::_ScrollRectHorizontally(const Page& page, const til::rect& s
         const auto height = scrollRect.height();
         const auto actualDelta = delta > 0 ? absoluteDelta : -absoluteDelta;
 
-        const auto source = Viewport::FromDimensions({ left, top }, width, height);
+        const auto source = Viewport::FromDimensions({ left, top }, { width, height });
         const auto target = Viewport::Offset(source, { actualDelta, 0 });
         const auto walkDirection = Viewport::DetermineWalkDirection(source, target);
         auto sourcePos = source.GetWalkOrigin(walkDirection);
@@ -901,7 +901,7 @@ void AdaptDispatch::_SelectiveEraseRect(const Page& page, const til::rect& erase
                     rowBuffer.ClearCell(col);
                     // Any image content also needs to be erased.
                     ImageSlice::EraseCells(rowBuffer, col, col + 1);
-                    page.Buffer().TriggerRedraw(Viewport::FromCoord({ col, row }));
+                    page.Buffer().TriggerRedraw(Viewport::FromDimensions({ col, row }, { 1, 1 }));
                 }
             }
         }
diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp
index d2aa4aa27f2..8e83ef47aa0 100644
--- a/src/types/UiaTextRangeBase.cpp
+++ b/src/types/UiaTextRangeBase.cpp
@@ -561,7 +561,7 @@ try
         const auto originY{ std::min(_start.y, inclusiveEnd.y) };
         const auto width{ std::abs(inclusiveEnd.x - _start.x + 1) };
         const auto height{ std::abs(inclusiveEnd.y - _start.y + 1) };
-        viewportRange = Viewport::FromDimensions({ originX, originY }, width, height);
+        viewportRange = Viewport::FromDimensions({ originX, originY }, { width, height });
     }
     auto iter{ buffer.GetCellDataAt(searchStart, viewportRange) };
     const auto iterStep{ searchBackwards ? -1 : 1 };
@@ -827,7 +827,7 @@ try
         const auto originY{ std::min(_start.y, inclusiveEnd.y) };
         const auto width{ std::abs(inclusiveEnd.x - _start.x + 1) };
         const auto height{ std::abs(inclusiveEnd.y - _start.y + 1) };
-        viewportRange = Viewport::FromDimensions({ originX, originY }, width, height);
+        viewportRange = Viewport::FromDimensions({ originX, originY }, { width, height });
     }
     auto iter{ buffer.GetCellDataAt(_start, viewportRange) };
     for (; iter && iter.Pos() != inclusiveEnd; ++iter)
@@ -1337,7 +1337,7 @@ Viewport UiaTextRangeBase::_getOptimizedBufferSize() const noexcept
     const auto width = textBufferEnd.x + 1;
     const auto height = textBufferEnd.y + 1;
 
-    return Viewport::FromDimensions({ 0, 0 }, width, height);
+    return Viewport::FromDimensions({ 0, 0 }, { width, height });
 }
 
 // We consider the "document end" to be the line beneath the cursor or
diff --git a/src/types/inc/viewport.hpp b/src/types/inc/viewport.hpp
index b8b2029504e..fd0d5b302f8 100644
--- a/src/types/inc/viewport.hpp
+++ b/src/types/inc/viewport.hpp
@@ -28,17 +28,7 @@ namespace Microsoft::Console::Types
 
         static Viewport FromInclusive(const til::inclusive_rect& sr) noexcept;
         static Viewport FromExclusive(const til::rect& sr) noexcept;
-
-        static Viewport FromDimensions(const til::point origin,
-                                       const til::CoordType width,
-                                       const til::CoordType height) noexcept;
-
-        static Viewport FromDimensions(const til::point origin,
-                                       const til::size dimensions) noexcept;
-
-        static Viewport FromDimensions(const til::size dimensions) noexcept;
-
-        static Viewport FromCoord(const til::point origin) noexcept;
+        static Viewport FromDimensions(const til::point origin, const til::size dimensions) noexcept;
 
         til::CoordType Left() const noexcept;
         til::CoordType RightInclusive() const noexcept;
@@ -60,35 +50,13 @@ namespace Microsoft::Console::Types
         void Clamp(til::point& pos) const;
         Viewport Clamp(const Viewport& other) const noexcept;
 
-        bool MoveInBounds(const til::CoordType move, til::point& pos) const noexcept;
         bool IncrementInBounds(til::point& pos, bool allowEndExclusive = false) const noexcept;
-        bool IncrementInBoundsCircular(til::point& pos) const noexcept;
         bool DecrementInBounds(til::point& pos, bool allowEndExclusive = false) const noexcept;
-        bool DecrementInBoundsCircular(til::point& pos) const noexcept;
         int CompareInBounds(const til::point first, const til::point second, bool allowEndExclusive = false) const noexcept;
 
-        enum class XWalk
-        {
-            LeftToRight,
-            RightToLeft
-        };
-
-        enum class YWalk
-        {
-            TopToBottom,
-            BottomToTop
-        };
-
-        struct WalkDir final
-        {
-            const XWalk x;
-            const YWalk y;
-        };
-
-        bool WalkInBounds(til::point& pos, const WalkDir dir, bool allowEndExclusive = false) const noexcept;
-        bool WalkInBoundsCircular(til::point& pos, const WalkDir dir, bool allowEndExclusive = false) const noexcept;
-        til::point GetWalkOrigin(const WalkDir dir) const noexcept;
-        static WalkDir DetermineWalkDirection(const Viewport& source, const Viewport& target) noexcept;
+        bool WalkInBounds(til::point& pos, const til::CoordType delta, bool allowEndExclusive = false) const noexcept;
+        til::point GetWalkOrigin(const til::CoordType delta) const noexcept;
+        static til::CoordType DetermineWalkDirection(const Viewport& source, const Viewport& target) noexcept;
 
         bool TrimToViewport(_Inout_ til::rect* psr) const noexcept;
         void ConvertToOrigin(_Inout_ til::rect* psr) const noexcept;
diff --git a/src/types/viewport.cpp b/src/types/viewport.cpp
index 2459e19ab61..6a64ecf7f62 100644
--- a/src/types/viewport.cpp
+++ b/src/types/viewport.cpp
@@ -26,26 +26,6 @@ Viewport Viewport::FromExclusive(const til::rect& sr) noexcept
     return FromInclusive(sr.to_inclusive_rect());
 }
 
-// Function Description:
-// - Creates a new Viewport at the given origin, with the given dimensions.
-// Arguments:
-// - origin: The origin of the new Viewport. Becomes the Viewport's Left, Top
-// - width: The width of the new viewport
-// - height: The height of the new viewport
-// Return Value:
-// - a new Viewport at the given origin, with the given dimensions.
-Viewport Viewport::FromDimensions(const til::point origin,
-                                  const til::CoordType width,
-                                  const til::CoordType height) noexcept
-{
-    return Viewport(til::inclusive_rect{
-        origin.x,
-        origin.y,
-        origin.x + width - 1,
-        origin.y + height - 1,
-    });
-}
-
 // Function Description:
 // - Creates a new Viewport at the given origin, with the given dimensions.
 // Arguments:
@@ -65,29 +45,6 @@ Viewport Viewport::FromDimensions(const til::point origin,
     });
 }
 
-// Function Description:
-// - Creates a new Viewport at the origin, with the given dimensions.
-// Arguments:
-// - dimensions: A coordinate containing the width and height of the new viewport
-//      in the x and y coordinates respectively.
-// Return Value:
-// - a new Viewport at the origin, with the given dimensions.
-Viewport Viewport::FromDimensions(const til::size dimensions) noexcept
-{
-    return FromDimensions({}, dimensions);
-}
-
-// Method Description:
-// - Creates a Viewport equivalent to a 1x1 rectangle at the given coordinate.
-// Arguments:
-// - origin: origin of the rectangle to create.
-// Return Value:
-// - a 1x1 Viewport at the given coordinate
-Viewport Viewport::FromCoord(const til::point origin) noexcept
-{
-    return FromInclusive(til::inclusive_rect{ origin.x, origin.y, origin.x, origin.y });
-}
-
 til::CoordType Viewport::Left() const noexcept
 {
     return _sr.left;
@@ -251,51 +208,6 @@ Viewport Viewport::Clamp(const Viewport& other) const noexcept
     return Viewport::FromInclusive(clampMe);
 }
 
-// Method Description:
-// - Moves the coordinate given by the number of positions and
-//   in the direction given (repeated increment or decrement)
-// Arguments:
-// - move - Magnitude and direction of the move
-// - pos - The coordinate position to adjust
-// Return Value:
-// - True if we successfully moved the requested distance. False if we had to stop early.
-// - If False, we will restore the original position to the given coordinate.
-bool Viewport::MoveInBounds(const til::CoordType move, til::point& pos) const noexcept
-{
-    const auto backup = pos;
-    auto success = true; // If nothing happens, we're still successful (e.g. add = 0)
-
-    for (til::CoordType i = 0; i < move; i++)
-    {
-        success = IncrementInBounds(pos);
-
-        // If an operation fails, break.
-        if (!success)
-        {
-            break;
-        }
-    }
-
-    for (til::CoordType i = 0; i > move; i--)
-    {
-        success = DecrementInBounds(pos);
-
-        // If an operation fails, break.
-        if (!success)
-        {
-            break;
-        }
-    }
-
-    // If any operation failed, revert to backed up state.
-    if (!success)
-    {
-        pos = backup;
-    }
-
-    return success;
-}
-
 // Method Description:
 // - Increments the given coordinate within the bounds of this viewport.
 // Arguments:
@@ -307,20 +219,7 @@ bool Viewport::MoveInBounds(const til::CoordType move, til::point& pos) const no
 // - True if it could be incremented. False if it would move outside.
 bool Viewport::IncrementInBounds(til::point& pos, bool allowEndExclusive) const noexcept
 {
-    return WalkInBounds(pos, { XWalk::LeftToRight, YWalk::TopToBottom }, allowEndExclusive);
-}
-
-// Method Description:
-// - Increments the given coordinate within the bounds of this viewport
-//   rotating around to the top when reaching the bottom right corner.
-// Arguments:
-// - pos - Coordinate position that will be incremented.
-// Return Value:
-// - True if it could be incremented inside the viewport.
-// - False if it rolled over from the bottom right corner back to the top.
-bool Viewport::IncrementInBoundsCircular(til::point& pos) const noexcept
-{
-    return WalkInBoundsCircular(pos, { XWalk::LeftToRight, YWalk::TopToBottom });
+    return WalkInBounds(pos, 1, allowEndExclusive);
 }
 
 // Method Description:
@@ -334,20 +233,7 @@ bool Viewport::IncrementInBoundsCircular(til::point& pos) const noexcept
 // - True if it could be incremented. False if it would move outside.
 bool Viewport::DecrementInBounds(til::point& pos, bool allowEndExclusive) const noexcept
 {
-    return WalkInBounds(pos, { XWalk::RightToLeft, YWalk::BottomToTop }, allowEndExclusive);
-}
-
-// Method Description:
-// - Decrements the given coordinate within the bounds of this viewport
-//   rotating around to the bottom right when reaching the top left corner.
-// Arguments:
-// - pos - Coordinate position that will be decremented.
-// Return Value:
-// - True if it could be decremented inside the viewport.
-// - False if it rolled over from the top left corner back to the bottom right.
-bool Viewport::DecrementInBoundsCircular(til::point& pos) const noexcept
-{
-    return WalkInBoundsCircular(pos, { XWalk::RightToLeft, YWalk::BottomToTop });
+    return WalkInBounds(pos, -1, allowEndExclusive);
 }
 
 // Routine Description:
@@ -402,110 +288,18 @@ int Viewport::CompareInBounds(const til::point first, const til::point second, b
 //                        includes the last til::point in a given viewport.
 // Return Value:
 // - True if it could be adjusted as specified and remain in bounds. False if it would move outside.
-bool Viewport::WalkInBounds(til::point& pos, const WalkDir dir, bool allowEndExclusive) const noexcept
-{
-    auto copy = pos;
-    if (WalkInBoundsCircular(copy, dir, allowEndExclusive))
-    {
-        pos = copy;
-        return true;
-    }
-    else
-    {
-        return false;
-    }
-}
-
-// Method Description:
-// - Walks the given coordinate within the bounds of this viewport
-//   rotating around to the opposite corner when reaching the final corner
-//   in the specified direction.
-// Arguments:
-// - pos - Coordinate position that will be adjusted.
-// - dir - Walking direction specifying which direction to go when reaching the end of a row/column
-// - allowEndExclusive - if true, allow the EndExclusive til::point as a valid position.
-//                        Used in accessibility to signify that the exclusive end
-//                        includes the last til::point in a given viewport.
-// Return Value:
-// - True if it could be adjusted inside the viewport.
-// - False if it rolled over from the final corner back to the initial corner
-//   for the specified walk direction.
-bool Viewport::WalkInBoundsCircular(til::point& pos, const WalkDir dir, bool allowEndExclusive) const noexcept
+bool Viewport::WalkInBounds(til::point& pos, const til::CoordType delta, bool allowEndExclusive) const noexcept
 {
-    // Assert that the position given fits inside this viewport.
-    assert(IsInBounds(pos, allowEndExclusive));
-
-    if (dir.x == XWalk::LeftToRight)
-    {
-        if (allowEndExclusive && pos.x == Left() && pos.y == BottomExclusive())
-        {
-            pos.y = Top();
-            return false;
-        }
-        else if (pos.x == RightInclusive())
-        {
-            pos.x = Left();
-
-            if (dir.y == YWalk::TopToBottom)
-            {
-                pos.y++;
-                if (allowEndExclusive && pos.y == BottomExclusive())
-                {
-                    return true;
-                }
-                else if (pos.y > BottomInclusive())
-                {
-                    pos.y = Top();
-                    return false;
-                }
-            }
-            else
-            {
-                pos.y--;
-                if (pos.y < Top())
-                {
-                    pos.y = BottomInclusive();
-                    return false;
-                }
-            }
-        }
-        else
-        {
-            pos.x++;
-        }
-    }
-    else
-    {
-        if (pos.x == Left())
-        {
-            pos.x = RightInclusive();
-
-            if (dir.y == YWalk::TopToBottom)
-            {
-                pos.y++;
-                if (pos.y > BottomInclusive())
-                {
-                    pos.y = Top();
-                    return false;
-                }
-            }
-            else
-            {
-                pos.y--;
-                if (pos.y < Top())
-                {
-                    pos.y = BottomInclusive();
-                    return false;
-                }
-            }
-        }
-        else
-        {
-            pos.x--;
-        }
-    }
-
-    return true;
+    const auto l = static_cast<ptrdiff_t>(_sr.left);
+    const auto t = static_cast<ptrdiff_t>(_sr.top);
+    const auto w = static_cast<ptrdiff_t>(std::max(0, _sr.right - _sr.left + 1));
+    const auto h = static_cast<ptrdiff_t>(std::max(0, _sr.bottom - _sr.top + 1));
+    const auto max = w * h - !allowEndExclusive;
+    const auto off = w * (pos.y - t) + (pos.x - l) + delta;
+    const auto offClamped = std::clamp(off, ptrdiff_t{ 0 }, max);
+    pos.x = gsl::narrow_cast<til::CoordType>(offClamped % w + l);
+    pos.y = gsl::narrow_cast<til::CoordType>(offClamped / w + t);
+    return off == offClamped;
 }
 
 // Routine Description:
@@ -518,172 +312,24 @@ bool Viewport::WalkInBoundsCircular(til::point& pos, const WalkDir dir, bool all
 // Return Value:
 // - The origin for the walk to reach every position without circling
 //   if using this same viewport with the `WalkInBounds` methods.
-til::point Viewport::GetWalkOrigin(const WalkDir dir) const noexcept
+til::point Viewport::GetWalkOrigin(const til::CoordType delta) const noexcept
 {
     til::point origin;
-    origin.x = dir.x == XWalk::LeftToRight ? Left() : RightInclusive();
-    origin.y = dir.y == YWalk::TopToBottom ? Top() : BottomInclusive();
+    origin.x = delta >= 0 ? Left() : RightInclusive();
+    origin.y = delta >= 0 ? Top() : BottomInclusive();
     return origin;
 }
 
-// Routine Description:
-// - Given two viewports that will be used for copying data from one to the other (source, target),
-//   determine which direction you will have to walk through them to ensure that an overlapped copy
-//   won't erase data in the source that hasn't yet been read and copied into the target at the same
-//   coordinate offset position from their respective origins.
-// - Note: See elaborate ASCII-art comment inside the body of this function for more details on how/why this works.
-// Arguments:
-// - source - The viewport representing the region that will be copied from
-// - target - The viewport representing the region that will be copied to
-// Return Value:
-// - The direction to walk through both viewports from the walk origins to touch every cell and not
-//   accidentally overwrite something that hasn't been read yet. (use with GetWalkOrigin and WalkInBounds)
-Viewport::WalkDir Viewport::DetermineWalkDirection(const Viewport& source, const Viewport& target) noexcept
+// A text buffer is basically a list (std::vector) of characters and we just tell it where the line breaks are.
+// This means that copying between overlapping ranges requires the same care as when using std::copy VS std::copy_backward:
+//   std::copy (aka forward copying) can only be used within an overlapping source/target range if target is before source.
+//   std::copy_backward, as the name implies, is to be used for the reverse situation: if target is after source.
+// This method returns 1 for the former and -1 for the latter. It's the delta you can pass to GetWalkOrigin and WalkInBounds.
+til::CoordType Viewport::DetermineWalkDirection(const Viewport& source, const Viewport& target) noexcept
 {
-    // We can determine which direction we need to walk based on solely the origins of the two rectangles.
-    // I'll use a few examples to prove the situation.
-    //
-    // For the cardinal directions, let's start with this sample:
-    //
-    // source        target
-    // origin 0,0    origin 4,0
-    // |             |
-    // v             V
-    // +--source-----+--target---------                  +--source-----+--target---------
-    // |  A  B  C  D | E | 1  2  3  4 |     becomes      |  A  B  C  D | A | B  C  D  E |
-    // |  F  G  H  I | J | 5  6  7  8 |    =========>    |  F  G  H  I | F | G  H  I  J |
-    // |  K  L  M  N | O | 9  $  %  @ |                  |  K  L  M  N | K | L  M  N  O |
-    // --------------------------------                  --------------------------------
-    //
-    // The source and target overlap in the 5th column (X=4).
-    // To ensure that we don't accidentally write over the source
-    // data before we copy it into the target, we want to start by
-    // reading that column (a.k.a. writing to the farthest away column
-    // of the target).
-    //
-    // This means we want to copy from right to left.
-    // Top to bottom and bottom to top don't really matter for this since it's
-    // a cardinal direction shift.
-    //
-    // If we do the right most column first as so...
-    //
-    // +--source-----+--target---------                  +--source-----+--target---------
-    // |  A  B  C  D | E | 1  2  3  4 |     step 1       |  A  B  C  D | E | 1  2  3  E |
-    // |  F  G  H  I | J | 5  6  7  8 |    =========>    |  F  G  H  I | J | 5  6  7  J |
-    // |  K  L  M  N | O | 9  $  %  @ |                  |  K  L  M  N | O | 9  $  %  O |
-    // --------------------------------                  --------------------------------
-    //
-    // ... then we can see that the EJO column is safely copied first out of the way and
-    // can be overwritten on subsequent steps without losing anything.
-    // The rest of the columns aren't overlapping, so they'll be fine.
-    //
-    // But we extrapolate this logic to follow for rectangles that overlap more columns, up
-    // to and including only leaving one column not overlapped...
-    //
-    // source   target
-    // origin   origin
-    // 0,0    / 1,0
-    // |     /
-    // v    v
-    // +----+------target-                  +----+------target-
-    // | A | B  C  D | E |     becomes      | A | A  B  C | D |
-    // | F | G  H  I | J |    =========>    | F | F  G  H | I |
-    // | K | L  M  N | O |                  | K | K  L  M | N |
-    // ---source----------                  ---source----------
-    //
-    // ... will still be OK following the same Right-To-Left rule as the first move.
-    //
-    // +----+------target-                  +----+------target-
-    // | A | B  C  D | E |     step 1       | A | B  C  D | D |
-    // | F | G  H  I | J |    =========>    | F | G  H  I | I |
-    // | K | L  M  N | O |                  | K | L  M  N | N |
-    // ---source----------                  ---source----------
-    //
-    // The DIN column from the source was moved to the target as the right most column
-    // of both rectangles. Now it is safe to iterate to the second column from the right
-    // and proceed with moving CHM on top of the source DIN as it was already moved.
-    //
-    // +----+------target-                  +----+------target-
-    // | A | B  C  D | E |     step 2       | A | B  C  C | D |
-    // | F | G  H  I | J |    =========>    | F | G  H  H | I |
-    // | K | L  M  N | O |                  | K | L  M  M | N |
-    // ---source----------                  ---source----------
-    //
-    // Continue walking right to left (an exercise left to the reader,) and we never lose
-    // any source data before it reaches the target with the Right To Left pattern.
-    //
-    // We notice that the target origin was Right of the source origin in this circumstance,
-    // (target origin X is > source origin X)
-    // so it is asserted that targets right of sources means that we should "walk" right to left.
-    //
-    // Reviewing the above, it doesn't appear to matter if we go Top to Bottom or Bottom to Top,
-    // so the conclusion is drawn that it doesn't matter as long as the source and target origin
-    // Y values are the same.
-    //
-    // Also, extrapolating this cardinal direction move to the other 3 cardinal directions,
-    // it should follow that they would follow the same rules.
-    // That is, a target left of a source, or a Westbound move, opposite of the above Eastbound move,
-    // should be "walked" left to right.
-    // (target origin X is < source origin X)
-    //
-    // We haven't given the sample yet that Northbound and Southbound moves are the same, but we
-    // could reason that the same logic applies and the conclusion would be a Northbound move
-    // would walk from the target toward the source again... a.k.a. Top to Bottom.
-    // (target origin Y is < source origin Y)
-    // Then the Southbound move would be the opposite, Bottom to Top.
-    // (target origin Y is > source origin Y)
-    //
-    // To confirm, let's try one more example but moving both at once in an ordinal direction Northeast.
-    //
-    //                 target
-    //                 origin 1, 0
-    //                 |
-    //                 v
-    //                 +----target--                         +----target--
-    //  source      A  |  B     C  |                      A  |  D     E  |
-    //  origin-->+------------     |     becomes       +------------     |
-    //   0, 1    |  D  |  E  |  F  |    =========>     |  D  |  G  |  H  |
-    //           |     -------------                   |     -------------
-    //           |  G     H  |  I                      |  G     H  |  I
-    //           --source-----                         --source-----
-    //
-    // Following our supposed rules from above, we have...
-    // Source Origin X = 0, Y = 1
-    // Target Origin X = 1, Y = 0
-    //
-    // Source Origin X < Target Origin X which means Right to Left
-    // Source Origin Y > Target Origin Y which means Top to Bottom
-    //
-    // So the first thing we should copy is the Top and Right most
-    // value from source to target.
-    //
-    //        +----target--                         +----target--
-    //     A  |  B     C  |                      A  |  B     E  |
-    //  +------------     |     step 1        +------------     |
-    //  |  D  |  E  |  F  |    =========>     |  D  |  E  |  F  |
-    //  |     -------------                   |     -------------
-    //  |  G     H  |  I                      |  G     H  |  I
-    //  --source-----                         --source-----
-    //
-    // And look. The E which was in the overlapping part of the source
-    // is the first thing copied out of the way and we're safe to copy the rest.
-    //
-    // We assume that this pattern then applies to all ordinal directions as well
-    // and it appears our rules hold.
-    //
-    // We've covered all cardinal and ordinal directions... all that is left is two
-    // rectangles of the same size and origin... and in that case, it doesn't matter
-    // as nothing is moving and therefore can't be covered up or lost.
-    //
-    // Therefore, we will codify our inequalities below as determining the walk direction
-    // for a given source and target viewport and use the helper `GetWalkOrigin`
-    // to return the place that we should start walking from when the copy commences.
-
     const auto sourceOrigin = source.Origin();
     const auto targetOrigin = target.Origin();
-
-    return Viewport::WalkDir{ targetOrigin.x < sourceOrigin.x ? Viewport::XWalk::LeftToRight : Viewport::XWalk::RightToLeft,
-                              targetOrigin.y < sourceOrigin.y ? Viewport::YWalk::TopToBottom : Viewport::YWalk::BottomToTop };
+    return targetOrigin < sourceOrigin ? 1 : -1;
 }
 
 // Method Description:


Test patch: