Reduce cost of cursor invalidation (#15500)

Performance of printing enwik8.txt at the following block sizes:
4KiB (printf): 53MB/s -> 58MB/s
128KiB (cat): 170MB/s -> 235MB/s

This commit is imperfect. Support for more than one rendering
engine was "hacked" into `Renderer` and is not quite correct.
As such, this commit cannot fix cursor invalidation correctly either,
and while some bugs are fixed (engines may see highly inconsistent
TextBuffer and Cursor states), it introduces others (an error in the
first engine may result in the second engine not executing).
Neither of those are good and the underlying issue remains to be fixed.

## Validation Steps Performed
* Seems ok? 
This commit is contained in:
Leonard Hecker 2024-04-10 20:51:02 +02:00 committed by GitHub
parent 4fd15c9937
commit 20b0bed46d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 140 additions and 141 deletions

View File

@ -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 };
}

View File

@ -188,11 +188,7 @@ void Cursor::_RedrawCursor() noexcept
// - <none>
void Cursor::_RedrawCursorAlways() noexcept
{
try
{
_parentBuffer.TriggerRedrawCursor(_cPosition);
}
CATCH_LOG();
_parentBuffer.NotifyPaintFrame();
}
void Cursor::SetPosition(const til::point cPosition) noexcept

View File

@ -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<til::point_span> 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<til::CoordType, til::CoordType, bool> 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<til::CoordType, til::CoordType, bool> 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

View File

@ -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);

View File

@ -218,10 +218,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
Close();
if (_renderer)
{
_renderer->TriggerTeardown();
}
_renderer.reset();
_renderEngine.reset();
}
void ControlCore::Detach()

View File

@ -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() })
{

View File

@ -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)
{

View File

@ -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:
// - <none>
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
// - <none>
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;
}
}

View File

@ -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_tree::IntervalTree<til::point, size_t>::interval> _hoveredInterval;
Microsoft::Console::Types::Viewport _viewport;
std::optional<CursorOptions> _currentCursorOptions;
std::vector<Cluster> _clusterBuffer;
std::vector<til::rect> _previousSelection;
std::vector<til::rect> _previousSearchSelection;