Prevent the VT engine painting unnecessarily

src/renderer/vt/XtermEngine.cpp

@@ -37,7 +37,16 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
 //      the pipe.
 [[nodiscard]] HRESULT XtermEngine::StartPaint() noexcept
 {
-    RETURN_IF_FAILED(VtEngine::StartPaint());
+    const auto hr = VtEngine::StartPaint();
+    if (hr != S_OK)
+    {
+        // If _noFlushOnEnd was set, and we didn't return early, it would usually
+        // have been reset in the EndPaint call. But since that's not going to
+        // happen now, we need to reset it here, otherwise we may mistakenly skip
+        // the flush on the next frame.
+        _noFlushOnEnd = false;
+        return hr;
+    }
 
     _trace.TraceLastText(_lastText);
 
@@ -83,15 +92,6 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
         }
     }
 
-    if (!_quickReturn)
-    {
-        if (_WillWriteSingleChar())
-        {
-            // Don't re-enable the cursor.
-            _quickReturn = true;
-        }
-    }
-
     return S_OK;
 }
 

src/renderer/vt/invalidate.cpp

@@ -75,7 +75,7 @@ CATCH_RETURN();
     }
     _skipCursor = false;
 
-    _cursorMoved = true;
+    _cursorMoved = psrRegion->origin() != _lastText;
     return S_OK;
 }
 

src/renderer/vt/math.cpp

@@ -52,38 +52,3 @@ void VtEngine::_OrRect(_Inout_ til::inclusive_rect* const pRectExisting, const t
     pRectExisting->right = std::max(pRectExisting->right, pRectToOr->right);
     pRectExisting->bottom = std::max(pRectExisting->bottom, pRectToOr->bottom);
 }
-
-// Method Description:
-// - Returns true if the invalidated region indicates that we only need to
-//      simply print text from the current cursor position. This will prevent us
-//      from sending extra VT set-up/tear down sequences (?12h/l) when all we
-//      need to do is print more text at the current cursor position.
-// Arguments:
-// - <none>
-// Return Value:
-// - true iff only the next character is invalid
-bool VtEngine::_WillWriteSingleChar() const
-{
-    // If there is no scroll delta, return false.
-    if (til::point{ 0, 0 } != _scrollDelta)
-    {
-        return false;
-    }
-
-    // If there is more than one invalid char, return false.
-    if (!_invalidMap.one())
-    {
-        return false;
-    }
-
-    // Get the single point at which things are invalid.
-    const auto invalidPoint = _invalidMap.runs().front().origin();
-
-    // Either the next character to the right or the immediately previous
-    //      character should follow this code path
-    //      (The immediate previous character would suggest a backspace)
-    auto invalidIsNext = invalidPoint == _lastText;
-    auto invalidIsLast = invalidPoint == til::point{ _lastText.x - 1, _lastText.y };
-
-    return invalidIsNext || invalidIsLast;
-}

src/renderer/vt/paint.cpp

@@ -20,30 +20,30 @@ using namespace Microsoft::Console::Types;
 //      HRESULT error code if painting didn't start successfully.
 [[nodiscard]] HRESULT VtEngine::StartPaint() noexcept
 {
+    // When unit testing, there may be no pipe, but we still need to paint.
     if (!_hFile)
     {
-        return S_FALSE;
+        return S_OK;
     }
 
     // If we're using line renditions, and this is a full screen paint, we can
     // potentially stop using them at the end of this frame.
     _stopUsingLineRenditions = _usingLineRenditions && _AllIsInvalid();
 
-    // If there's nothing to do, quick return
+    // If there's nothing to do, we won't need to paint.
     auto somethingToDo = _invalidMap.any() ||
                          _scrollDelta != til::point{ 0, 0 } ||
                          _cursorMoved ||
                          _titleChanged;
 
-    _quickReturn = !somethingToDo;
-    _trace.TraceStartPaint(_quickReturn,
+    _trace.TraceStartPaint(!somethingToDo,
                            _invalidMap,
                            _lastViewport.ToExclusive(),
                            _scrollDelta,
                            _cursorMoved,
                            _wrappedRow);
 
-    return _quickReturn ? S_FALSE : S_OK;
+    return somethingToDo ? S_OK : S_FALSE;
 }
 
 // Routine Description:
@@ -142,9 +142,9 @@ using namespace Microsoft::Console::Types;
         _usingLineRenditions = true;
     }
     // One simple optimization is that we can skip sending the line attributes
-    // when _quickReturn is true. That indicates that we're writing out a single
-    // character, which should preclude there being a rendition switch.
-    if (_usingLineRenditions && !_quickReturn)
+    // when we're writing out a single character, which should preclude there
+    // being a rendition switch.
+    if (_usingLineRenditions && !_invalidMap.one())
     {
         RETURN_IF_FAILED(_MoveCursor({ _lastText.x, targetRow }));
         switch (lineRendition)

src/renderer/vt/state.cpp

@@ -37,7 +37,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
     _pool(til::pmr::get_default_resource()),
     _invalidMap(initialViewport.Dimensions(), false, &_pool),
     _scrollDelta(0, 0),
-    _quickReturn(false),
     _clearedAllThisFrame(false),
     _cursorMoved(false),
     _resized(false),

src/renderer/vt/vtrenderer.hpp

@@ -113,7 +113,6 @@ namespace Microsoft::Console::Render
         til::point _lastText;
         til::point _scrollDelta;
 
-        bool _quickReturn;
         bool _clearedAllThisFrame;
         bool _cursorMoved;
         bool _resized;
@@ -214,8 +213,6 @@ namespace Microsoft::Console::Render
         [[nodiscard]] HRESULT _RgbUpdateDrawingBrushes(const TextAttribute& textAttributes) noexcept;
         [[nodiscard]] HRESULT _16ColorUpdateDrawingBrushes(const TextAttribute& textAttributes) noexcept;
 
-        bool _WillWriteSingleChar() const;
-
         // buffer space for these two functions to build their lines
         // so they don't have to alloc/free in a tight loop
         std::wstring _bufferLine;