Fix several ScrollConsoleScreenBuffer ConPTY issues

.github/actions/spelling/expect/expect.txt

@@ -820,6 +820,7 @@ Ith
 IUI
 IWIC
 IXP
+izz
 jconcpp
 JOBOBJECT
 JOBOBJECTINFOCLASS

src/host/VtInputThread.cpp

@@ -194,8 +194,8 @@ void VtInputThread::CaptureNextCursorPositionReport() const noexcept
     engine.CaptureNextCursorPositionReport();
 }
 
-til::enumset<DeviceAttribute, uint64_t> VtInputThread::WaitUntilDA1(DWORD timeout) const noexcept
+void VtInputThread::WaitUntilDA1(DWORD timeout) const noexcept
 {
     auto& engine = static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
-    return engine.WaitUntilDA1(timeout);
+    engine.WaitUntilDA1(timeout);
 }

src/host/VtInputThread.hpp

@@ -18,19 +18,14 @@ Author(s):
 
 namespace Microsoft::Console
 {
-    namespace VirtualTerminal
-    {
-        enum class DeviceAttribute : uint64_t;
-    }
-
     class VtInputThread
     {
     public:
         explicit VtInputThread(_In_ wil::unique_hfile hPipe);
 
         [[nodiscard]] HRESULT Start();
         void CaptureNextCursorPositionReport() const noexcept;
-        til::enumset<VirtualTerminal::DeviceAttribute, uint64_t> WaitUntilDA1(DWORD timeout) const noexcept;
+        void WaitUntilDA1(DWORD timeout) const noexcept;
 
     private:
         static DWORD WINAPI StaticVtInputThreadProc(_In_ LPVOID lpParameter);

src/host/VtIo.cpp

@@ -213,7 +213,7 @@ bool VtIo::IsUsingVt() const
             // Allow the input thread to momentarily gain the console lock.
             auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
             const auto suspension = gci.SuspendLock();
-            _deviceAttributes = _pVtInputThread->WaitUntilDA1(1000);
+            _pVtInputThread->WaitUntilDA1(1000);
         }
     }
 

src/host/VtIo.hpp

@@ -10,6 +10,8 @@ class ConsoleArguments;
 
 namespace Microsoft::Console::VirtualTerminal
 {
+    enum class DeviceAttribute : uint64_t;
+
     class VtIo
     {
     public:

src/host/directio.cpp

@@ -439,7 +439,10 @@ CATCH_RETURN();
         // clippedRectangle.Top/Left() cannot be negative due to the previous Clamp() call.
         auto totalOffset = offsetY * bufferStride + offsetX;
 
-        if (bufferStride <= 0 || targetBuffer.size() < gsl::narrow_cast<size_t>(clippedRectangle.Height() * bufferStride))
+        // NOTE: totalOffset points to the first cell (i.e. .x is possibly >0). The requiredSize is not simply
+        // `offset + height * stride` then, since that may extend beyond the end of a valid, fully allocated buffer.
+        const auto requiredSize = totalOffset + (clippedRectangle.Height() - 1) * bufferStride + width;
+        if (bufferStride <= 0 || targetBuffer.size() < requiredSize)
         {
             return E_INVALIDARG;
         }
@@ -530,9 +533,12 @@ CATCH_RETURN();
         // clippedRectangle.Top/Left() cannot be negative due to the previous Clamp() call.
         const auto offsetY = clippedRectangle.Top() - requestRectangle.Top();
         const auto offsetX = clippedRectangle.Left() - requestRectangle.Left();
-        auto totalOffset = offsetY * bufferStride + offsetX;
+        auto totalOffset = gsl::narrow_cast<size_t>(offsetY) * bufferStride + offsetX;
 
-        if (bufferStride <= 0 || buffer.size() < gsl::narrow_cast<size_t>(clippedRectangle.Height() * bufferStride))
+        // NOTE: totalOffset points to the first cell (i.e. .x is possibly >0). The requiredSize is not simply
+        // `offset + height * stride` then, since that may extend beyond the end of a valid, fully allocated buffer.
+        const auto requiredSize = totalOffset + gsl::narrow_cast<size_t>(clippedRectangle.Height() - 1) * bufferStride + width;
+        if (bufferStride < requestRectangle.Width() || buffer.size() < requiredSize)
         {
             return E_INVALIDARG;
         }

src/host/getset.cpp

@@ -969,6 +969,31 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
         auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
         auto& buffer = context.GetActiveBuffer();
         const auto bufferSize = buffer.GetBufferSize();
+        const auto clipViewport = clip ? Viewport::FromInclusive(*clip).Clamp(bufferSize) : bufferSize;
+        const auto sourceViewport = Viewport::FromInclusive(source);
+        const auto fillViewport = sourceViewport.Clamp(clipViewport);
+        const til::point targetSourceDistance{ target - sourceViewport.Origin() };
+        const til::point sourceTargetDistance{ -targetSourceDistance.x, -targetSourceDistance.y };
+
+        // To figure out what part of "source" we can copy to "target" without
+        // * reading outside the bufferSize
+        // * writing outside the clipViewport
+        // we move the clipViewport into a coordinate system relative to the source rectangle (= clipAtSource).
+        // Then we can intersect the source rectangle with both the valid bufferSize and clipAtSource at once.
+        const auto clipAtSource = Viewport::Offset(clipViewport, sourceTargetDistance);
+        auto copySourceViewport = sourceViewport.Clamp(bufferSize).Clamp(clipAtSource);
+        if (!copySourceViewport.IsValid())
+        {
+            copySourceViewport = Viewport::Empty();
+        }
+
+        // Afterward we can undo the translation of clipAtSource to get the target rectangle.
+        const auto copyTargetViewport = Viewport::Offset(copySourceViewport, targetSourceDistance);
+        if (!fillViewport.IsValid() && !copyTargetViewport.IsValid())
+        {
+            return S_OK; // nothing to copy
+        }
+
         auto writer = gci.GetVtWriterForBuffer(&context);
 
         // Applications like to pass 0/0 for the fill char/attribute.
@@ -996,33 +1021,33 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
         }
         else if (writer)
         {
-            const auto clipViewport = clip ? Viewport::FromInclusive(*clip).Clamp(bufferSize) : bufferSize;
-            const auto sourceViewport = Viewport::FromInclusive(source);
-            const auto fillViewport = sourceViewport.Clamp(clipViewport);
+            if (sourceViewport == bufferSize && clipViewport == bufferSize &&
+                targetSourceDistance.x == 0 && fillCharacter == UNICODE_SPACE &&
+                WI_AreAllFlagsClear(fillAttribute, ~(FG_ATTRS | BG_ATTRS)))
+            {
+                // Several applications scroll the entire screen vertically with a whitespace fill, to essentially replicate
+                // what DL/IL can do. Our VT application is faster and better, so let's use that whenever applicable.
+                // NOTE: SU/SD is technically not portable (terminals implement it in different ways), so it's best avoided here.
 
-            writer.BackupCursor();
+                std::wstring buf;
+                buf.reserve(32);
+                buf.append(L"\x1b\x37"); // DECSC: DEC Save Cursor (+ attributes)
 
-            if (gci.GetVtIo()->GetDeviceAttributes().test(Microsoft::Console::VirtualTerminal::DeviceAttribute::RectangularAreaOperations))
-            {
-                const til::point targetSourceDistance{ target - sourceViewport.Origin() };
-                const til::point sourceTargetDistance{ -targetSourceDistance.x, -targetSourceDistance.y };
-
-                // To figure out what part of "source" we can copy to "target" without
-                // * reading outside the bufferSize
-                // * writing outside the clipViewport
-                // we move the clipViewport into a coordinate system relative to the source rectangle (= clipAtSource).
-                // Then we can intersect the source rectangle with both the valid bufferSize and clipAtSource at once.
-                const auto clipAtSource = Viewport::Offset(clipViewport, sourceTargetDistance);
-                auto copySourceViewport = sourceViewport.Clamp(bufferSize).Clamp(clipAtSource);
-                if (!copySourceViewport.IsValid())
-                {
-                    copySourceViewport = Viewport::Empty();
-                }
+                Microsoft::Console::VirtualTerminal::VtIo::FormatAttributes(buf, TextAttribute{ fillAttribute });
+
+                const auto distance = std::min(std::abs(targetSourceDistance.y), bufferSize.Height());
+                fmt::format_to(std::back_inserter(buf), FMT_COMPILE(L"\x1b[H\x1b[{}{}"), distance, targetSourceDistance.y < 0 ? L'M' : L'L');
 
-                // Afterward we can undo the translation of clipAtSource to get the target rectangle.
-                const auto copyTargetViewport = Viewport::Offset(copySourceViewport, targetSourceDistance);
+                buf.append(L"\x1b\x38"); // DECRC: DEC Restore Cursor (+ attributes)
+                WriteCharsVT(context, buf);
+            }
+            else if (gci.GetVtIo()->GetDeviceAttributes().test(Microsoft::Console::VirtualTerminal::DeviceAttribute::RectangularAreaOperations))
+            {
                 const auto fills = Viewport::Subtract(fillViewport, copyTargetViewport);
+
                 std::wstring buf;
+                buf.reserve(128);
+                buf.append(L"\x1b\x37"); // DECSC: DEC Save Cursor (+ attributes)
 
                 if (!fills.empty())
                 {
@@ -1056,30 +1081,32 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
                         fill.RightExclusive());
                 }
 
+                buf.append(L"\x1b\x38"); // DECRC: DEC Restore Cursor (+ attributes)
                 WriteCharsVT(context, buf);
             }
             else
             {
-                const auto w = std::max(0, sourceViewport.Width());
-                const auto h = std::max(0, sourceViewport.Height());
-                const auto a = static_cast<size_t>(w * h);
-                if (a == 0)
-                {
-                    return S_OK;
-                }
-
                 til::small_vector<CHAR_INFO, 1024> backup;
                 til::small_vector<CHAR_INFO, 1024> fill;
-
-                backup.resize(a, CHAR_INFO{ fillCharacter, fillAttribute });
-                fill.resize(a, CHAR_INFO{ fillCharacter, fillAttribute });
-
                 Viewport readViewport;
                 Viewport writtenViewport;
 
-                RETURN_IF_FAILED(ReadConsoleOutputWImplHelper(context, backup, sourceViewport, readViewport));
-                RETURN_IF_FAILED(WriteConsoleOutputWImplHelper(context, fill, w, fillViewport, writtenViewport));
-                RETURN_IF_FAILED(WriteConsoleOutputWImplHelper(context, backup, w, Viewport::FromDimensions(target, readViewport.Dimensions()).Clamp(clipViewport), writtenViewport));
+                writer.BackupCursor();
+
+                if (copySourceViewport.IsValid())
+                {
+                    backup.resize(static_cast<size_t>(copySourceViewport.Width()) * copySourceViewport.Height());
+                    RETURN_IF_FAILED(ReadConsoleOutputWImplHelper(context, backup, copySourceViewport, readViewport));
+                }
+                if (fillViewport.IsValid())
+                {
+                    fill.resize(static_cast<size_t>(fillViewport.Width()) * fillViewport.Height(), CHAR_INFO{ fillCharacter, fillAttribute });
+                    RETURN_IF_FAILED(WriteConsoleOutputWImplHelper(context, fill, fillViewport.Width(), fillViewport, writtenViewport));
+                }
+                if (copyTargetViewport.IsValid())
+                {
+                    RETURN_IF_FAILED(WriteConsoleOutputWImplHelper(context, backup, copySourceViewport.Width(), copyTargetViewport, writtenViewport));
+                }
             }
         }
         else

src/host/tracing.hpp

@@ -25,13 +25,13 @@ Author(s):
         TraceLoggingPackedMetadata(TlgInINT16, "X"), \
         TraceLoggingPackedMetadata(TlgInINT16, "Y")
 
-#define TraceLoggingConsoleSmallRect(value, name)       \
-    TraceLoggingPackedData(&value, sizeof(SMALL_RECT)), \
-        TraceLoggingPackedStruct(4, name),              \
-        TraceLoggingInt16(TlgInINT16, "Left"),          \
-        TraceLoggingInt16(TlgInINT16, "Top"),           \
-        TraceLoggingInt16(TlgInINT16, "Right"),         \
-        TraceLoggingInt16(TlgInINT16, "Bottom")
+#define TraceLoggingConsoleSmallRect(value, name)        \
+    TraceLoggingPackedData(&value, sizeof(SMALL_RECT)),  \
+        TraceLoggingPackedStruct(4, name),               \
+        TraceLoggingPackedMetadata(TlgInINT16, "Left"),  \
+        TraceLoggingPackedMetadata(TlgInINT16, "Top"),   \
+        TraceLoggingPackedMetadata(TlgInINT16, "Right"), \
+        TraceLoggingPackedMetadata(TlgInINT16, "Bottom")
 
 // We intentionally don't differentiate between A and W versions of CHAR_INFO, because some particularly nasty
 // applications smuggle data in the upper bytes of the UnicodeChar field while using the A APIs and then they