diff --git a/src/buffer/out/LineRendition.hpp b/src/buffer/out/LineRendition.hpp index 954bd38509f..e74e4dac0fc 100644 --- a/src/buffer/out/LineRendition.hpp +++ b/src/buffer/out/LineRendition.hpp @@ -21,6 +21,13 @@ enum class LineRendition : uint8_t DoubleHeightBottom }; +constexpr til::rect ScreenToBufferLine(const til::rect& line, const LineRendition lineRendition) +{ + // Use shift right to quickly divide the Left and Right by 2 for double width lines. + const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1; + return { line.left >> scale, line.top, line.right >> scale, line.bottom }; +} + constexpr til::inclusive_rect ScreenToBufferLine(const til::inclusive_rect& line, const LineRendition lineRendition) { // Use shift right to quickly divide the Left and Right by 2 for double width lines. @@ -35,6 +42,13 @@ constexpr til::point ScreenToBufferLine(const til::point& line, const LineRendit return { line.x >> scale, line.y }; } +constexpr til::rect BufferToScreenLine(const til::rect& line, const LineRendition lineRendition) +{ + // Use shift left to quickly multiply the Left and Right by 2 for double width lines. + const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1; + return { line.left << scale, line.top, (line.right << scale) + scale, line.bottom }; +} + constexpr til::inclusive_rect BufferToScreenLine(const til::inclusive_rect& line, const LineRendition lineRendition) { // Use shift left to quickly multiply the Left and Right by 2 for double width lines. diff --git a/src/buffer/out/cursor.cpp b/src/buffer/out/cursor.cpp index 9084a60003d..273c05f8ea1 100644 --- a/src/buffer/out/cursor.cpp +++ b/src/buffer/out/cursor.cpp @@ -188,11 +188,7 @@ void Cursor::_RedrawCursor() noexcept // - void Cursor::_RedrawCursorAlways() noexcept { - try - { - _parentBuffer.TriggerRedrawCursor(_cPosition); - } - CATCH_LOG(); + _parentBuffer.NotifyPaintFrame(); } void Cursor::SetPosition(const til::point cPosition) noexcept diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index b3c2e590e37..4eb6d5e0689 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1020,19 +1020,19 @@ Microsoft::Console::Render::Renderer& TextBuffer::GetRenderer() noexcept return _renderer; } -void TextBuffer::TriggerRedraw(const Viewport& viewport) +void TextBuffer::NotifyPaintFrame() noexcept { if (_isActiveBuffer) { - _renderer.TriggerRedraw(viewport); + _renderer.NotifyPaintFrame(); } } -void TextBuffer::TriggerRedrawCursor(const til::point position) +void TextBuffer::TriggerRedraw(const Viewport& viewport) { if (_isActiveBuffer) { - _renderer.TriggerRedrawCursor(&position); + _renderer.TriggerRedraw(viewport); } } diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 555531a25aa..0826ec8d7e9 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -147,8 +147,8 @@ class TextBuffer final Microsoft::Console::Render::Renderer& GetRenderer() noexcept; + void NotifyPaintFrame() noexcept; void TriggerRedraw(const Microsoft::Console::Types::Viewport& viewport); - void TriggerRedrawCursor(const til::point position); void TriggerRedrawAll(); void TriggerScroll(); void TriggerScroll(const til::point delta); diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp index ad4e753c9a6..8da5e59a989 100644 --- a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp +++ b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp @@ -238,15 +238,8 @@ try // This ensures that teardown is reentrant. // Shut down the renderer (and therefore the thread) before we implode - if (auto localRenderEngine{ std::exchange(_renderEngine, nullptr) }) - { - if (auto localRenderer{ std::exchange(_renderer, nullptr) }) - { - localRenderer->TriggerTeardown(); - // renderer is destroyed - } - // renderEngine is destroyed - } + _renderer.reset(); + _renderEngine.reset(); if (auto localHwnd{ _hwnd.release() }) { diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 4aabcfa1414..bab5101afc8 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -210,11 +210,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation ControlCore::~ControlCore() { Close(); - - if (_renderer) - { - _renderer->TriggerTeardown(); - } } void ControlCore::Detach() diff --git a/src/host/getset.cpp b/src/host/getset.cpp index f2cd485842f..fda44943aaf 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -743,7 +743,7 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont // When evaluating the X offset, we must convert the buffer position to // equivalent screen coordinates, taking line rendition into account. const auto lineRendition = buffer.GetTextBuffer().GetLineRendition(position.y); - const auto screenPosition = BufferToScreenLine({ position.x, position.y, position.x, position.y }, lineRendition); + const auto screenPosition = BufferToScreenLine(til::inclusive_rect{ position.x, position.y, position.x, position.y }, lineRendition); if (currentViewport.left > screenPosition.left) { diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index c23bd85bbfa..06fd5f6ab7b 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -64,44 +64,90 @@ Renderer::~Renderer() // - HRESULT S_OK, GDI error, Safe Math error, or state/argument errors. [[nodiscard]] HRESULT Renderer::PaintFrame() { - FOREACH_ENGINE(pEngine) + auto tries = maxRetriesForRenderEngine; + while (tries > 0) { - auto tries = maxRetriesForRenderEngine; - while (tries > 0) + if (_destructing) { - if (_destructing) - { - return S_FALSE; - } + return S_FALSE; + } + + // BODGY: Optimally we would want to retry per engine, but that causes different + // problems (intermittent inconsistent states between text renderer and UIA output, + // not being able to lock the cursor location, etc.). + const auto hr = _PaintFrame(); + if (SUCCEEDED(hr)) + { + break; + } + + LOG_HR_IF(hr, hr != E_PENDING); - const auto hr = _PaintFrameForEngine(pEngine); - if (SUCCEEDED(hr)) + if (--tries == 0) + { + // Stop trying. + _pThread->DisablePainting(); + if (_pfnRendererEnteredErrorState) { - break; + _pfnRendererEnteredErrorState(); } + // If there's no callback, we still don't want to FAIL_FAST: the renderer going black + // isn't near as bad as the entire application aborting. We're a component. We shouldn't + // abort applications that host us. + return S_FALSE; + } + + // Add a bit of backoff. + // Sleep 150ms, 300ms, 450ms before failing out and disabling the renderer. + Sleep(renderBackoffBaseTimeMilliseconds * (maxRetriesForRenderEngine - tries)); + } - LOG_HR_IF(hr, hr != E_PENDING); + return S_OK; +} - if (--tries == 0) +[[nodiscard]] HRESULT Renderer::_PaintFrame() noexcept +{ + { + _pData->LockConsole(); + auto unlock = wil::scope_exit([&]() { + _pData->UnlockConsole(); + }); + + // Last chance check if anything scrolled without an explicit invalidate notification since the last frame. + _CheckViewportAndScroll(); + + if (_currentCursorOptions) + { + const auto coord = _currentCursorOptions->coordCursor; + const auto& buffer = _pData->GetTextBuffer(); + const auto lineRendition = buffer.GetLineRendition(coord.y); + const auto cursorWidth = _pData->IsCursorDoubleWidth() ? 2 : 1; + + til::rect cursorRect{ coord.x, coord.y, coord.x + cursorWidth, coord.y + 1 }; + cursorRect = BufferToScreenLine(cursorRect, lineRendition); + + if (buffer.GetSize().TrimToViewport(&cursorRect)) { - // Stop trying. - _pThread->DisablePainting(); - if (_pfnRendererEnteredErrorState) + FOREACH_ENGINE(pEngine) { - _pfnRendererEnteredErrorState(); + LOG_IF_FAILED(pEngine->InvalidateCursor(&cursorRect)); } - // If there's no callback, we still don't want to FAIL_FAST: the renderer going black - // isn't near as bad as the entire application aborting. We're a component. We shouldn't - // abort applications that host us. - return S_FALSE; } + } - // Add a bit of backoff. - // Sleep 150ms, 300ms, 450ms before failing out and disabling the renderer. - Sleep(renderBackoffBaseTimeMilliseconds * (maxRetriesForRenderEngine - tries)); + _currentCursorOptions = _GetCursorInfo(); + + FOREACH_ENGINE(pEngine) + { + RETURN_IF_FAILED(_PaintFrameForEngine(pEngine)); } } + FOREACH_ENGINE(pEngine) + { + RETURN_IF_FAILED(pEngine->Present()); + } + return S_OK; } @@ -110,14 +156,6 @@ try { FAIL_FAST_IF_NULL(pEngine); // This is a programming error. Fail fast. - _pData->LockConsole(); - auto unlock = wil::scope_exit([&]() { - _pData->UnlockConsole(); - }); - - // Last chance check if anything scrolled without an explicit invalidate notification since the last frame. - _CheckViewportAndScroll(); - // Try to start painting a frame const auto hr = pEngine->StartPaint(); RETURN_IF_FAILED(hr); @@ -172,12 +210,6 @@ try // Force scope exit end paint to finish up collecting information and possibly painting endPaint.reset(); - // Force scope exit unlock to let go of global lock so other threads can run - unlock.reset(); - - // Trigger out-of-lock presentation for renderers that can support it - RETURN_IF_FAILED(pEngine->Present()); - // As we leave the scope, EndPaint will be called (declared above) return S_OK; } @@ -255,47 +287,6 @@ void Renderer::TriggerRedraw(const til::point* const pcoord) TriggerRedraw(Viewport::FromCoord(*pcoord)); // this will notify to paint if we need it. } -// Routine Description: -// - Called when the cursor has moved in the buffer. Allows for RenderEngines to -// differentiate between cursor movements and other invalidates. -// Visual Renderers (ex GDI) should invalidate the position, while the VT -// engine ignores this. See MSFT:14711161. -// Arguments: -// - pcoord: The buffer-space position of the cursor. -// Return Value: -// - -void Renderer::TriggerRedrawCursor(const til::point* const pcoord) -{ - // We first need to make sure the cursor position is within the buffer, - // otherwise testing for a double width character can throw an exception. - const auto& buffer = _pData->GetTextBuffer(); - if (buffer.GetSize().IsInBounds(*pcoord)) - { - // We then calculate the region covered by the cursor. This requires - // converting the buffer coordinates to an equivalent range of screen - // cells for the cursor, taking line rendition into account. - const auto lineRendition = buffer.GetLineRendition(pcoord->y); - const auto cursorWidth = _pData->IsCursorDoubleWidth() ? 2 : 1; - til::inclusive_rect cursorRect = { pcoord->x, pcoord->y, pcoord->x + cursorWidth - 1, pcoord->y }; - cursorRect = BufferToScreenLine(cursorRect, lineRendition); - - // That region is then clipped within the viewport boundaries and we - // only trigger a redraw if the resulting region is not empty. - auto view = _pData->GetViewport(); - auto updateRect = til::rect{ cursorRect }; - if (view.TrimToViewport(&updateRect)) - { - view.ConvertToOrigin(&updateRect); - FOREACH_ENGINE(pEngine) - { - LOG_IF_FAILED(pEngine->InvalidateCursor(&updateRect)); - } - - NotifyPaintFrame(); - } - } -} - // Routine Description: // - Called when something that changes the output state has occurred and the entire frame is now potentially invalid. // - NOTE: Use sparingly. Try to reduce the refresh region where possible. Only use when a global state change has occurred. @@ -336,6 +327,8 @@ void Renderer::TriggerTeardown() noexcept // We need to shut down the paint thread on teardown. _pThread->WaitForPaintCompletionAndDisable(INFINITE); + auto repaint = false; + // Then walk through and do one final paint on the caller's thread. FOREACH_ENGINE(pEngine) { @@ -343,10 +336,15 @@ void Renderer::TriggerTeardown() noexcept auto hr = pEngine->PrepareForTeardown(&fEngineRequestsRepaint); LOG_IF_FAILED(hr); - if (SUCCEEDED(hr) && fEngineRequestsRepaint) - { - LOG_IF_FAILED(_PaintFrameForEngine(pEngine)); - } + repaint |= SUCCEEDED(hr) && fEngineRequestsRepaint; + } + + // BODGY: The only time repaint is true is when VtEngine is used. + // Coincidentally VtEngine always runs alone, so if repaint is true, there's only a single engine + // to repaint anyways and there's no danger is accidentally repainting an engine that didn't want to. + if (repaint) + { + LOG_IF_FAILED(_PaintFrame()); } } @@ -464,6 +462,7 @@ void Renderer::TriggerScroll(const til::point* const pcoordDelta) void Renderer::TriggerFlush(const bool circling) { const auto rects = _GetSelectionRects(); + auto repaint = false; FOREACH_ENGINE(pEngine) { @@ -473,10 +472,15 @@ void Renderer::TriggerFlush(const bool circling) LOG_IF_FAILED(pEngine->InvalidateSelection(rects)); - if (SUCCEEDED(hr) && fEngineRequestsRepaint) - { - LOG_IF_FAILED(_PaintFrameForEngine(pEngine)); - } + repaint |= SUCCEEDED(hr) && fEngineRequestsRepaint; + } + + // BODGY: The only time repaint is true is when VtEngine is used. + // Coincidentally VtEngine always runs alone, so if repaint is true, there's only a single engine + // to repaint anyways and there's no danger is accidentally repainting an engine that didn't want to. + if (repaint) + { + LOG_IF_FAILED(_PaintFrame()); } } @@ -638,7 +642,6 @@ void Renderer::EnablePainting() // When the renderer is constructed, the initial viewport won't be available yet, // but once EnablePainting is called it should be safe to retrieve. _viewport = _pData->GetViewport(); - _forceUpdateViewport = true; // When running the unit tests, we may be using a render without a render thread. if (_pThread) @@ -1086,10 +1089,9 @@ bool Renderer::_isInHoveredInterval(const til::point coordTarget) const noexcept // - void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine) { - const auto cursorInfo = _GetCursorInfo(); - if (cursorInfo.has_value()) + if (_currentCursorOptions.has_value()) { - LOG_IF_FAILED(pEngine->PaintCursor(cursorInfo.value())); + LOG_IF_FAILED(pEngine->PaintCursor(_currentCursorOptions.value())); } } @@ -1107,7 +1109,7 @@ void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine) [[nodiscard]] HRESULT Renderer::_PrepareRenderInfo(_In_ IRenderEngine* const pEngine) { RenderFrameInfo info; - info.cursorInfo = _GetCursorInfo(); + info.cursorInfo = _currentCursorOptions; return pEngine->PrepareRenderInfo(info); } @@ -1282,6 +1284,11 @@ void Renderer::_ScrollPreviousSelection(const til::point delta) { rc += delta; } + + if (_currentCursorOptions) + { + _currentCursorOptions->coordCursor += delta; + } } } @@ -1303,6 +1310,7 @@ void Renderer::AddRenderEngine(_In_ IRenderEngine* const pEngine) if (!p) { p = pEngine; + _forceUpdateViewport = true; return; } } diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp index ae3ed2cfda9..559403267e0 100644 --- a/src/renderer/base/renderer.hpp +++ b/src/renderer/base/renderer.hpp @@ -50,7 +50,6 @@ namespace Microsoft::Console::Render void TriggerSystemRedraw(const til::rect* const prcDirtyClient); void TriggerRedraw(const Microsoft::Console::Types::Viewport& region); void TriggerRedraw(const til::point* const pcoord); - void TriggerRedrawCursor(const til::point* const pcoord); void TriggerRedrawAll(const bool backgroundChanged = false, const bool frameChanged = false); void TriggerTeardown() noexcept; @@ -96,6 +95,7 @@ namespace Microsoft::Console::Render static GridLineSet s_GetGridlines(const TextAttribute& textAttribute) noexcept; static bool s_IsSoftFontChar(const std::wstring_view& v, const size_t firstSoftFontChar, const size_t lastSoftFontChar); + [[nodiscard]] HRESULT _PaintFrame() noexcept; [[nodiscard]] HRESULT _PaintFrameForEngine(_In_ IRenderEngine* const pEngine) noexcept; bool _CheckViewportAndScroll(); [[nodiscard]] HRESULT _PaintBackground(_In_ IRenderEngine* const pEngine); @@ -125,6 +125,7 @@ namespace Microsoft::Console::Render uint16_t _hyperlinkHoveredId = 0; std::optional::interval> _hoveredInterval; Microsoft::Console::Types::Viewport _viewport; + std::optional _currentCursorOptions; std::vector _clusterBuffer; std::vector _previousSelection; std::function _pfnBackgroundColorChanged;