From 8e726ba8e5e2d8952adb9b5cf4ad5232fc385dde Mon Sep 17 00:00:00 2001
From: Leonard Hecker <lhecker@microsoft.com>
Date: Tue, 6 Aug 2024 00:35:05 +0200
Subject: [PATCH] Address feedback

---
 src/host/_stream.cpp    |  4 ++++
 src/host/screenInfo.cpp | 30 ++++++------------------------
 src/host/screenInfo.hpp | 10 +---------
 3 files changed, 11 insertions(+), 33 deletions(-)

diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp
index c9ca3986d8d5..461cce34664e 100644
--- a/src/host/_stream.cpp
+++ b/src/host/_stream.cpp
@@ -167,6 +167,8 @@ void WriteCharsLegacy(SCREEN_INFORMATION& screenInfo, const std::wstring_view& t
     auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
     auto writer = gci.GetVtWriterForBuffer(&screenInfo);
 
+    screenInfo.SnapOnOutput();
+
     // If we enter this if condition, then someone wrote text in VT mode and now switched to non-VT mode.
     // Since the Console APIs don't support delayed EOL wrapping, we need to first put the cursor back
     // to a position that the Console APIs expect (= not delayed).
@@ -343,6 +345,8 @@ void WriteCharsVT(SCREEN_INFORMATION& screenInfo, const std::wstring_view& str)
     // may change, so get the VtIo reference now, just in case.
     auto writer = gci.GetVtWriterForBuffer(&screenInfo);
 
+    screenInfo.SnapOnOutput();
+
     stateMachine.ProcessString(str);
 
     if (writer)
diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp
index b35f8b4172df..7843cc4574af 100644
--- a/src/host/screenInfo.cpp
+++ b/src/host/screenInfo.cpp
@@ -1687,23 +1687,23 @@ static constexpr bool IsInputKey(WORD vkey)
 
 void SCREEN_INFORMATION::MakeCursorVisible(til::point position)
 {
-    _makeLocationVisible(position, ViewportMovementMask::Vertical | ViewportMovementMask::Horizontal);
+    _makeLocationVisible(position, true, true);
 }
 
 void SCREEN_INFORMATION::SnapOnInput(const WORD vkey)
 {
     if (IsInputKey(vkey))
     {
-        _makeLocationVisible(_textBuffer->GetCursor().GetPosition(), ViewportMovementMask::Vertical);
+        _makeLocationVisible(_textBuffer->GetCursor().GetPosition(), true, false);
     }
 }
 
 void SCREEN_INFORMATION::SnapOnOutput()
 {
-    _makeLocationVisible(_textBuffer->GetCursor().GetPosition(), ViewportMovementMask::HorizontalCenter);
+    _makeLocationVisible(_textBuffer->GetCursor().GetPosition(), false, true);
 }
 
-void SCREEN_INFORMATION::_makeLocationVisible(til::point position, ViewportMovementMask movements)
+void SCREEN_INFORMATION::_makeLocationVisible(til::point position, bool vertical, bool horizontal)
 {
     const auto viewportOrigin = _viewport.Origin();
     const auto viewportSize = _viewport.Dimensions();
@@ -1714,36 +1714,18 @@ void SCREEN_INFORMATION::_makeLocationVisible(til::point position, ViewportMovem
     position.x = std::clamp(position.x, 0, bufferSize.width - 1);
     position.y = std::clamp(position.y, 0, bufferSize.height - 1);
 
-    if (WI_IsAnyFlagSet(movements, ViewportMovementMask::Vertical))
+    if (vertical)
     {
         origin.y = std::min(origin.y, position.y); // shift up if above
         origin.y = std::max(origin.y, position.y - (viewportSize.height - 1)); // shift down if below
     }
 
-    if (WI_IsAnyFlagSet(movements, ViewportMovementMask::Horizontal))
+    if (horizontal)
     {
         origin.x = std::min(origin.x, position.x); // shift left if left
         origin.x = std::max(origin.x, position.x - (viewportSize.width - 1)); // shift right if right
     }
 
-    if (WI_IsAnyFlagSet(movements, ViewportMovementMask::HorizontalCenter))
-    {
-        // If the position is horizontally outside the viewport, snap the
-        // viewport to the nearest multiple of half the viewport width.
-        if (position.x < origin.x || position.x >= (origin.x + viewportSize.width))
-        {
-            const auto div = viewportSize.width / 2;
-            // We want our viewport to be centered around the position (= "half-width" offset).
-            // Since the origin is the left edge, we must subtract a half-width from the position.
-            origin.x = position.x - div;
-            // Round down to the nearest multiple of the viewport width.
-            // This also works if origin.x is negative, because the "modulo operator"
-            // is not a modulo operator, it's a remainder operator. The remainder of a
-            // negative number is negative and so origin.x cannot end up being less than 0.
-            origin.x -= origin.x % div;
-        }
-    }
-
     if (origin != viewportOrigin)
     {
         std::ignore = SetViewportOrigin(true, origin, false);
diff --git a/src/host/screenInfo.hpp b/src/host/screenInfo.hpp
index 44eb9fe8f0ae..e1d9e645eedb 100644
--- a/src/host/screenInfo.hpp
+++ b/src/host/screenInfo.hpp
@@ -47,14 +47,6 @@ Revision History:
 #include "../types/inc/Viewport.hpp"
 class ConversionAreaInfo; // forward decl window. circular reference
 
-enum class ViewportMovementMask
-{
-    Vertical = 0x1,
-    Horizontal = 0x2,
-    HorizontalCenter = 0x4,
-};
-DEFINE_ENUM_FLAG_OPERATORS(ViewportMovementMask);
-
 class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console::IIoProvider
 {
 public:
@@ -244,7 +236,7 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
     void _CalculateViewportSize(const til::rect* const prcClientArea, _Out_ til::size* const pcoordSize);
     void _AdjustViewportSize(const til::rect* const prcClientNew, const til::rect* const prcClientOld, const til::size* const pcoordSize);
     void _InternalSetViewportSize(const til::size* pcoordSize, const bool fResizeFromTop, const bool fResizeFromLeft);
-    void _makeLocationVisible(til::point position, ViewportMovementMask movements);
+    void _makeLocationVisible(til::point position, bool vertical, bool horizontal);
 
     static void s_CalculateScrollbarVisibility(const til::rect* const prcClientArea,
                                                const til::size* const pcoordBufferSize,