COOKED_READ_DATA: Fix scrolling under ConPTY (#15930)

This commit fixes 3 bugs:
* `COOKED_READ_DATA` failed to initialize its `_distanceCursor` and
  `_distanceEnd` members. I took this as an opportunity to make them
  `ptrdiff_t`, to reduce the likelihood of overflows in the future.
* `COOKED_READ_DATA::_writeChars` added `scrollY` to the written
  distance, even though `WriteCharsLegacy` writes a negative value into
  that out parameter. This was fixed by changing `WriteCharsLegacy` to
  write positive values and by adding a debug assertion.
* `StreamScrollRegion` calls `IncrementCircularBuffer` which causes a
  synchronous (!) ConPTY flush to the output pipe (side note: this is
  the primary reason why newlines are so slow in ConPTY).
  Since cooked reads are supposed to behave like a pager and not write
  into the scrollback, we temporarily mark the buffer as inactive
  which prevents `TextBuffer` from snitching about it to VtEngine.

Even after this change, there's still some weird behavior left:
* You cannot move your cursor back beyond (0,0), because this isn't a
  real pager-like implementation. That might be a neat future extension.
* Writing a lot of text and pressing Ctrl+C doesn't properly place the
  cursor and scroll the buffer, unless the cursor is at the end.
  That might also be worth investigating in the future (minor issue).
* When the viewport is full, backspacing more than 1 line of text
  (using Ctrl+Backspace) doesn't erase all of the affected lines,
  because `COOKED_READ_DATA::_erase` uses the same `WriteCharsLegacy`
  function to write whitespace to erase that text. It's only gone
  after typing one more character.
  I've written the code to mostly fix this, but decided against it
  as I considered the problem to be too niche to warrant extra code.

Closes #15899

## Validation Steps Performed
* Generate some text to paste in PowerShell:
  ```pwsh
  "" + (0..512 | % { "word" + $_.ToString().PadLeft(4, "0") })
  ```
* Launch cmd.exe and paste that text
* No flickering ✅
* No writing into the scrollback ✅
* No weird behavior when backspacing ✅

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -3401,7 +3401,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottomLikeMSYS()
         }
         else if (writingMethod == PrintWithWriteCharsLegacy)
         {
-            WriteCharsLegacy(si, str, true, nullptr);
+            WriteCharsLegacy(si, str, false, nullptr);
         }
     };
 

src/host/_stream.cpp

@@ -70,13 +70,50 @@ static void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point
 
     if (coordCursor.y >= bufferSize.height)
     {
-        // At the end of the buffer. Scroll contents of screen buffer so new position is visible.
-        StreamScrollRegion(screenInfo);
+        const auto vtIo = ServiceLocator::LocateGlobals().getConsoleInformation().GetVtIo();
+        const auto renderer = ServiceLocator::LocateGlobals().pRender;
+        const auto needsConPTYWorkaround = interactive && vtIo->IsUsingVt();
+        auto& buffer = screenInfo.GetTextBuffer();
+        const auto isActiveBuffer = buffer.IsActiveBuffer();
+
+        // ConPTY translates scrolling into newlines. We don't want that during cooked reads (= "cmd.exe prompts")
+        // because the entire prompt is supposed to fit into the VT viewport, with no scrollback. If we didn't do that,
+        // any prompt larger than the viewport will cause >1 lines to be added to scrollback, even if typing backspaces.
+        // You can test this by removing this branch, launch Windows Terminal, and fill the entire viewport with text in cmd.exe.
+        if (needsConPTYWorkaround)
+        {
+            buffer.SetAsActiveBuffer(false);
+            buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes());
+            buffer.SetAsActiveBuffer(isActiveBuffer);
+
+            if (isActiveBuffer && renderer)
+            {
+                renderer->TriggerRedrawAll();
+            }
+        }
+        else
+        {
+            buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes());
+
+            if (isActiveBuffer)
+            {
+                if (const auto notifier = ServiceLocator::LocateAccessibilityNotifier())
+                {
+                    notifier->NotifyConsoleUpdateScrollEvent(0, -1);
+                }
+                if (renderer)
+                {
+                    static constexpr til::point delta{ 0, -1 };
+                    renderer->TriggerScroll(&delta);
+                }
+            }
+        }
 
-        if (nullptr != psScrollY)
+        if (psScrollY)
         {
-            *psScrollY += bufferSize.height - coordCursor.y - 1;
+            *psScrollY += 1;
         }
+
         coordCursor.y = bufferSize.height - 1;
     }
 

src/host/output.cpp

@@ -293,38 +293,6 @@ static void _ScrollScreen(SCREEN_INFORMATION& screenInfo, const Viewport& source
     textBuffer.TriggerRedraw(fill);
 }
 
-// Routine Description:
-// - This routine is a special-purpose scroll for use by AdjustCursorPosition.
-// Arguments:
-// - screenInfo - reference to screen buffer info.
-// Return Value:
-// - true if we succeeded in scrolling the buffer, otherwise false (if we're out of memory)
-void StreamScrollRegion(SCREEN_INFORMATION& screenInfo)
-{
-    // Rotate the circular buffer around and wipe out the previous final line.
-    auto& buffer = screenInfo.GetTextBuffer();
-    buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes());
-
-    // Trigger a graphical update if we're active.
-    if (screenInfo.IsActiveScreenBuffer())
-    {
-        til::point coordDelta;
-        coordDelta.y = -1;
-
-        auto pNotifier = ServiceLocator::LocateAccessibilityNotifier();
-        if (pNotifier)
-        {
-            // Notify accessibility that a scroll has occurred.
-            pNotifier->NotifyConsoleUpdateScrollEvent(coordDelta.x, coordDelta.y);
-        }
-
-        if (ServiceLocator::LocateGlobals().pRender != nullptr)
-        {
-            ServiceLocator::LocateGlobals().pRender->TriggerScroll(&coordDelta);
-        }
-    }
-}
-
 // Routine Description:
 // - This routine copies ScrollRectangle to DestinationOrigin then fills in ScrollRectangle with Fill.
 // - The scroll region is copied to a third buffer, the scroll region is filled, then the original contents of the scroll region are copied to the destination.

src/host/output.h

@@ -47,7 +47,5 @@ void ScrollRegion(SCREEN_INFORMATION& screenInfo,
 
 VOID SetConsoleWindowOwner(const HWND hwnd, _Inout_opt_ ConsoleProcessHandle* pProcessData);
 
-void StreamScrollRegion(SCREEN_INFORMATION& screenInfo);
-
 // For handling process handle state, not the window state itself.
 void CloseConsoleProcessState();

src/host/readDataCooked.cpp

@@ -752,7 +752,7 @@ void COOKED_READ_DATA::_flushBuffer()
         const auto distanceBeforeCursor = _writeChars(view.substr(0, _bufferCursor));
         const auto distanceAfterCursor = _writeChars(view.substr(_bufferCursor));
         const auto distanceEnd = distanceBeforeCursor + distanceAfterCursor;
-        const auto eraseDistance = std::max(0, _distanceEnd - distanceEnd);
+        const auto eraseDistance = std::max<ptrdiff_t>(0, _distanceEnd - distanceEnd);
 
         // If the contents of _buffer became shorter we'll have to erase the previously printed contents.
         _erase(eraseDistance);
@@ -766,7 +766,7 @@ void COOKED_READ_DATA::_flushBuffer()
 }
 
 // This is just a small helper to fill the next N cells starting at the current cursor position with whitespace.
-void COOKED_READ_DATA::_erase(const til::CoordType distance) const
+void COOKED_READ_DATA::_erase(ptrdiff_t distance) const
 {
     if (distance <= 0)
     {
@@ -793,7 +793,7 @@ void COOKED_READ_DATA::_erase(const til::CoordType distance) const
 // A helper to write text and calculate the number of cells we've written.
 // _unwindCursorPosition then allows us to go that many cells back. Tracking cells instead of explicit
 // buffer positions allows us to pay no further mind to whether the buffer scrolled up or not.
-til::CoordType COOKED_READ_DATA::_writeChars(const std::wstring_view& text) const
+ptrdiff_t COOKED_READ_DATA::_writeChars(const std::wstring_view& text) const
 {
     if (text.empty())
     {
@@ -802,18 +802,20 @@ til::CoordType COOKED_READ_DATA::_writeChars(const std::wstring_view& text) cons
 
     const auto& textBuffer = _screenInfo.GetTextBuffer();
     const auto& cursor = textBuffer.GetCursor();
-    const auto width = textBuffer.GetSize().Width();
+    const auto width = static_cast<ptrdiff_t>(textBuffer.GetSize().Width());
     const auto initialCursorPos = cursor.GetPosition();
     til::CoordType scrollY = 0;
 
     WriteCharsLegacy(_screenInfo, text, true, &scrollY);
 
     const auto finalCursorPos = cursor.GetPosition();
-    return (finalCursorPos.y - initialCursorPos.y + scrollY) * width + finalCursorPos.x - initialCursorPos.x;
+    const auto distance = (finalCursorPos.y - initialCursorPos.y + scrollY) * width + finalCursorPos.x - initialCursorPos.x;
+    assert(distance >= 0);
+    return distance;
 }
 
 // Moves the given point by the given distance inside the text buffer, as if moving a cursor with the left/right arrow keys.
-til::point COOKED_READ_DATA::_offsetPosition(til::point pos, til::CoordType distance) const
+til::point COOKED_READ_DATA::_offsetPosition(til::point pos, ptrdiff_t distance) const
 {
     const auto size = _screenInfo.GetTextBuffer().GetSize().Dimensions();
     const auto w = static_cast<ptrdiff_t>(size.width);
@@ -832,7 +834,7 @@ til::point COOKED_READ_DATA::_offsetPosition(til::point pos, til::CoordType dist
 
 // This moves the cursor `distance`-many cells back up in the buffer.
 // It's intended to be used in combination with _writeChars.
-void COOKED_READ_DATA::_unwindCursorPosition(til::CoordType distance) const
+void COOKED_READ_DATA::_unwindCursorPosition(ptrdiff_t distance) const
 {
     if (distance <= 0)
     {

src/host/readDataCooked.hpp

@@ -113,10 +113,10 @@ class COOKED_READ_DATA final : public ReadData
     void _handlePostCharInputLoop(bool isUnicode, size_t& numBytes, ULONG& controlKeyState);
     void _markAsDirty();
     void _flushBuffer();
-    void _erase(til::CoordType distance) const;
-    til::CoordType _writeChars(const std::wstring_view& text) const;
-    til::point _offsetPosition(til::point pos, til::CoordType distance) const;
-    void _unwindCursorPosition(til::CoordType distance) const;
+    void _erase(ptrdiff_t distance) const;
+    ptrdiff_t _writeChars(const std::wstring_view& text) const;
+    til::point _offsetPosition(til::point pos, ptrdiff_t distance) const;
+    void _unwindCursorPosition(ptrdiff_t distance) const;
     void _replaceBuffer(const std::wstring_view& str);
 
     void _popupPush(PopupKind kind);
@@ -140,8 +140,8 @@ class COOKED_READ_DATA final : public ReadData
 
     std::wstring _buffer;
     size_t _bufferCursor = 0;
-    til::CoordType _distanceCursor;
-    til::CoordType _distanceEnd;
+    ptrdiff_t _distanceCursor = 0;
+    ptrdiff_t _distanceEnd = 0;
     bool _bufferDirty = false;
     bool _insertMode = false;