From 8f5f8bb1e5e951fed9fcd8717292b572987dfa73 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 4 May 2024 20:30:24 +0100 Subject: [PATCH] Stop the VT engine painting unnecessarily. --- src/renderer/vt/XtermEngine.cpp | 20 +++++++++---------- src/renderer/vt/invalidate.cpp | 2 +- src/renderer/vt/math.cpp | 35 --------------------------------- src/renderer/vt/paint.cpp | 16 +++++++-------- src/renderer/vt/state.cpp | 1 - src/renderer/vt/vtrenderer.hpp | 3 --- 6 files changed, 19 insertions(+), 58 deletions(-) diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 75fc3510fa6..b6d0c58a862 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/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; } diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp index 489a6b50bc1..942ba58df26 100644 --- a/src/renderer/vt/invalidate.cpp +++ b/src/renderer/vt/invalidate.cpp @@ -75,7 +75,7 @@ CATCH_RETURN(); } _skipCursor = false; - _cursorMoved = true; + _cursorMoved = psrRegion->origin() != _lastText; return S_OK; } diff --git a/src/renderer/vt/math.cpp b/src/renderer/vt/math.cpp index d86e4644ca5..354ca0e9925 100644 --- a/src/renderer/vt/math.cpp +++ b/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: -// - -// 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; -} diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index af08cdd5b58..1fcb5335887 100644 --- a/src/renderer/vt/paint.cpp +++ b/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) diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index 881fb0c2cce..f192536e3e6 100644 --- a/src/renderer/vt/state.cpp +++ b/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), diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 0125fe17369..b5c487fc1c0 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/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;