Skip to content

Commit

Permalink
Reduce cost of cursor invalidation (#15500)
Browse files Browse the repository at this point in the history
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? ✅
  • Loading branch information
lhecker authored Apr 10, 2024
1 parent 4fd15c9 commit 20b0bed
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 126 deletions.
11 changes: 7 additions & 4 deletions src/buffer/out/LineRendition.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
6 changes: 1 addition & 5 deletions src/buffer/out/cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1265,19 +1265,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);
}
}

Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -2032,17 +2032,17 @@ 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
}
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
Expand Down
2 changes: 1 addition & 1 deletion src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,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);
Expand Down
6 changes: 2 additions & 4 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
Close();

if (_renderer)
{
_renderer->TriggerTeardown();
}
_renderer.reset();
_renderEngine.reset();
}

void ControlCore::Detach()
Expand Down
11 changes: 2 additions & 9 deletions src/cascadia/TerminalControl/HwndTerminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() })
{
Expand Down
2 changes: 1 addition & 1 deletion src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Loading

0 comments on commit 20b0bed

Please sign in to comment.