From 7aa3731c561e30cad3612fe7903a199ab5d8c6de Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 2 Jun 2023 02:51:40 +0200 Subject: [PATCH 1/5] Reduce cost of cursor invalidation --- src/buffer/out/LineRendition.hpp | 14 ++ src/buffer/out/cursor.cpp | 6 +- src/buffer/out/textBuffer.cpp | 8 +- src/buffer/out/textBuffer.hpp | 2 +- src/cascadia/TerminalControl/ControlCore.cpp | 5 - src/cascadia/TerminalControl/HwndTerminal.cpp | 11 +- src/host/getset.cpp | 2 +- src/renderer/base/renderer.cpp | 190 +++++++++--------- src/renderer/base/renderer.hpp | 3 +- 9 files changed, 124 insertions(+), 117 deletions(-) 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 5c71dd1d398..78365c9d9ef 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1240,19 +1240,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 419e01471e5..86dade36e42 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -203,8 +203,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/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 68702b27500..ff321eabb72 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -217,11 +217,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation ControlCore::~ControlCore() { Close(); - - if (_renderer) - { - _renderer->TriggerTeardown(); - } } void ControlCore::Detach() diff --git a/src/cascadia/TerminalControl/HwndTerminal.cpp b/src/cascadia/TerminalControl/HwndTerminal.cpp index b5a930f701a..67fc117fd87 100644 --- a/src/cascadia/TerminalControl/HwndTerminal.cpp +++ b/src/cascadia/TerminalControl/HwndTerminal.cpp @@ -244,15 +244,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 3d371d4b24f..0119346da31 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 c7c5c491815..9953db53ee6 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()); } } @@ -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.has_value()) { - LOG_IF_FAILED(pEngine->PaintCursor(cursorInfo.value())); + LOG_IF_FAILED(pEngine->PaintCursor(_currentCursorOptions.value())); } } @@ -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 1cd61799a8f..dc027acea20 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; From 0600f08f707360fa0d991cd66017149479718bea Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 26 Mar 2024 19:46:41 +0100 Subject: [PATCH 2/5] Address feedback --- src/cascadia/TerminalControl/ControlCore.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index b77ab758b45..9a8ac9c8965 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -216,6 +216,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation ControlCore::~ControlCore() { Close(); + + _renderer.reset(); + _renderEngine.reset(); } void ControlCore::Detach() From ec7ceb87c998eab89a5c14979cfbb6fffc0a3913 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Sat, 30 Mar 2024 02:21:41 +0100 Subject: [PATCH 3/5] Address first half of feedback --- src/buffer/out/LineRendition.hpp | 9 ++------- src/buffer/out/textBuffer.cpp | 12 ++++++------ 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/buffer/out/LineRendition.hpp b/src/buffer/out/LineRendition.hpp index e74e4dac0fc..6a7baf08298 100644 --- a/src/buffer/out/LineRendition.hpp +++ b/src/buffer/out/LineRendition.hpp @@ -23,35 +23,30 @@ enum class LineRendition : uint8_t 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. 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) { - // 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 }; + 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/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 78365c9d9ef..d83cd7751ce 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1894,8 +1894,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 }; @@ -2006,8 +2006,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 @@ -2015,8 +2015,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 From 7cae3c55425646a73ee3524cd75323c552525925 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Sat, 30 Mar 2024 03:02:10 +0100 Subject: [PATCH 4/5] Address remaining feedback --- src/buffer/out/LineRendition.hpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/buffer/out/LineRendition.hpp b/src/buffer/out/LineRendition.hpp index 6a7baf08298..564f39f03f0 100644 --- a/src/buffer/out/LineRendition.hpp +++ b/src/buffer/out/LineRendition.hpp @@ -21,12 +21,6 @@ enum class LineRendition : uint8_t DoubleHeightBottom }; -constexpr til::rect ScreenToBufferLine(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 ScreenToBufferLine(const til::inclusive_rect& line, const LineRendition lineRendition) { const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1; From 47ef6e730534081e0f7f7d93c83a916af08a6939 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 2 Apr 2024 20:38:47 +0200 Subject: [PATCH 5/5] Address feedback --- src/renderer/base/renderer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 9953db53ee6..4137d140d36 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -1109,9 +1109,9 @@ bool Renderer::_isInHoveredInterval(const til::point coordTarget) const noexcept // - void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine) { - if (_currentCursorOptions.has_value()) + if (_currentCursorOptions) { - LOG_IF_FAILED(pEngine->PaintCursor(_currentCursorOptions.value())); + LOG_IF_FAILED(pEngine->PaintCursor(*_currentCursorOptions)); } }