diff --git a/src/buffer/out/LineRendition.hpp b/src/buffer/out/LineRendition.hpp index 954bd38509..564f39f03f 100644 --- a/src/buffer/out/LineRendition.hpp +++ b/src/buffer/out/LineRendition.hpp @@ -23,21 +23,24 @@ enum class LineRendition : uint8_t 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. const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1; return { line.left >> scale, line.top, line.right >> scale, line.bottom }; } -constexpr til::point ScreenToBufferLine(const til::point& line, const LineRendition lineRendition) +constexpr til::point ScreenToBufferLineInclusive(const til::point& 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.x >> scale, line.y }; } +constexpr til::rect BufferToScreenLine(const til::rect& line, const LineRendition lineRendition) +{ + const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1; + return { line.left << scale, line.top, line.right << 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. const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1; return { line.left << scale, line.top, (line.right << scale) + scale, line.bottom }; } diff --git a/src/buffer/out/cursor.cpp b/src/buffer/out/cursor.cpp index 9084a60003..273c05f8ea 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 bb2be49fd1..1b5b28c621 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1265,6 +1265,14 @@ Microsoft::Console::Render::Renderer& TextBuffer::GetRenderer() noexcept return _renderer; } +void TextBuffer::NotifyPaintFrame() noexcept +{ + if (_isActiveBuffer) + { + _renderer.NotifyPaintFrame(); + } +} + void TextBuffer::TriggerRedraw(const Viewport& viewport) { if (_isActiveBuffer) @@ -1273,14 +1281,6 @@ void TextBuffer::TriggerRedraw(const Viewport& viewport) } } -void TextBuffer::TriggerRedrawCursor(const til::point position) -{ - if (_isActiveBuffer) - { - _renderer.TriggerRedrawCursor(&position); - } -} - void TextBuffer::TriggerRedrawAll() { if (_isActiveBuffer) @@ -1920,8 +1920,8 @@ std::vector TextBuffer::GetTextSpans(til::point start, til::poi // equivalent buffer offsets, taking line rendition into account. if (!bufferCoordinates) { - higherCoord = ScreenToBufferLine(higherCoord, GetLineRendition(higherCoord.y)); - lowerCoord = ScreenToBufferLine(lowerCoord, GetLineRendition(lowerCoord.y)); + higherCoord = ScreenToBufferLineInclusive(higherCoord, GetLineRendition(higherCoord.y)); + lowerCoord = ScreenToBufferLineInclusive(lowerCoord, GetLineRendition(lowerCoord.y)); } til::inclusive_rect asRect = { higherCoord.x, higherCoord.y, lowerCoord.x, lowerCoord.y }; @@ -2032,8 +2032,8 @@ std::tuple TextBuffer::_RowCopyHelper(cons if (req.blockSelection) { const auto lineRendition = row.GetLineRendition(); - const auto minX = req.bufferCoordinates ? req.minX : ScreenToBufferLine(til::point{ req.minX, iRow }, lineRendition).x; - const auto maxX = req.bufferCoordinates ? req.maxX : ScreenToBufferLine(til::point{ req.maxX, iRow }, lineRendition).x; + const auto minX = req.bufferCoordinates ? req.minX : ScreenToBufferLineInclusive(til::point{ req.minX, iRow }, lineRendition).x; + const auto maxX = req.bufferCoordinates ? req.maxX : ScreenToBufferLineInclusive(til::point{ req.maxX, iRow }, lineRendition).x; rowBeg = minX; rowEnd = maxX + 1; // +1 to get an exclusive end @@ -2041,8 +2041,8 @@ std::tuple TextBuffer::_RowCopyHelper(cons else { const auto lineRendition = row.GetLineRendition(); - const auto beg = req.bufferCoordinates ? req.beg : ScreenToBufferLine(req.beg, lineRendition); - const auto end = req.bufferCoordinates ? req.end : ScreenToBufferLine(req.end, lineRendition); + const auto beg = req.bufferCoordinates ? req.beg : ScreenToBufferLineInclusive(req.beg, lineRendition); + const auto end = req.bufferCoordinates ? req.end : ScreenToBufferLineInclusive(req.end, lineRendition); rowBeg = iRow != beg.y ? 0 : beg.x; rowEnd = iRow != end.y ? row.GetReadableColumnCount() : end.x + 1; // +1 to get an exclusive end diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 2e1d053221..c7b68926ef 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -169,8 +169,8 @@ public: 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/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index fbe2eaafb7..8a71034217 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -218,10 +218,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation { Close(); - if (_renderer) - { - _renderer->TriggerTeardown(); - } + _renderer.reset(); + _renderEngine.reset(); } void ControlCore::Detach() diff --git a/src/cascadia/TerminalControl/HwndTerminal.cpp b/src/cascadia/TerminalControl/HwndTerminal.cpp index bd05b7ca8a..598028a88e 100644 --- a/src/cascadia/TerminalControl/HwndTerminal.cpp +++ b/src/cascadia/TerminalControl/HwndTerminal.cpp @@ -248,15 +248,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/host/getset.cpp b/src/host/getset.cpp index 495771fdad..304b4fd65c 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -730,7 +730,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 c7c5c49181..4137d140d3 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -64,42 +64,88 @@ Renderer::~Renderer() // - HRESULT S_OK, GDI error, Safe Math error, or state/argument errors. [[nodiscard]] HRESULT Renderer::PaintFrame() { + auto tries = maxRetriesForRenderEngine; + while (tries > 0) + { + if (_destructing) + { + 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); + + if (--tries == 0) + { + // Stop trying. + _pThread->DisablePainting(); + if (_pfnRendererEnteredErrorState) + { + _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)); + } + + return S_OK; +} + +[[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)) + { + FOREACH_ENGINE(pEngine) + { + LOG_IF_FAILED(pEngine->InvalidateCursor(&cursorRect)); + } + } + } + + _currentCursorOptions = _GetCursorInfo(); + + FOREACH_ENGINE(pEngine) + { + RETURN_IF_FAILED(_PaintFrameForEngine(pEngine)); + } + } + FOREACH_ENGINE(pEngine) { - auto tries = maxRetriesForRenderEngine; - while (tries > 0) - { - if (_destructing) - { - return S_FALSE; - } - - const auto hr = _PaintFrameForEngine(pEngine); - if (SUCCEEDED(hr)) - { - break; - } - - LOG_HR_IF(hr, hr != E_PENDING); - - if (--tries == 0) - { - // Stop trying. - _pThread->DisablePainting(); - if (_pfnRendererEnteredErrorState) - { - _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)); - } + 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()); } } @@ -468,6 +466,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) { @@ -477,10 +476,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()); } } @@ -642,7 +646,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) @@ -1106,10 +1109,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) { - LOG_IF_FAILED(pEngine->PaintCursor(cursorInfo.value())); + LOG_IF_FAILED(pEngine->PaintCursor(*_currentCursorOptions)); } } @@ -1127,7 +1129,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); } @@ -1340,6 +1342,11 @@ void Renderer::_ScrollPreviousSelection(const til::point delta) { rc += delta; } + + if (_currentCursorOptions) + { + _currentCursorOptions->coordCursor += delta; + } } } @@ -1361,6 +1368,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 1cd61799a8..dc027acea2 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); @@ -126,6 +126,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::vector _previousSearchSelection;