Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce cost of cursor invalidation #15500

Merged
merged 6 commits into from
Apr 10, 2024
+125 −126
Merged

Reduce cost of cursor invalidation #15500

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7aa3731
Reduce cost of cursor invalidation
lhecker Jun 2, 2023
4da71ff
Merge remote-tracking branch 'origin/main' into dev/lhecker/vt-perf4
lhecker Mar 26, 2024
0600f08
Address feedback
lhecker Mar 26, 2024
ec7ceb8
Address first half of feedback
lhecker Mar 30, 2024
7cae3c5
Address remaining feedback
lhecker Mar 30, 2024
47ef6e7
Address feedback
lhecker Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/buffer/out/LineRendition.hpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -1894,8 +1894,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 @@ -2006,17 +2006,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions src/cascadia/TerminalControl/ControlCore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
Close();

if (_renderer)
{
_renderer->TriggerTeardown();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer need to _pThread->WaitForPaintCompletionAndDisable(INFINITE);?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort of. The destructor blocks until the renderer is fully shut down, which is similar to WaitForPaintCompletionAndDisable but simpler and faster.

I'll re-add the explicit destructor calls here just to be sure nothing regresses. We use a lot of plain/unsafe pointers after all.

}
_renderer.reset();
_renderEngine.reset();
}

void ControlCore::Detach()
Expand Down
11 changes: 2 additions & 9 deletions src/cascadia/TerminalControl/HwndTerminal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,15 +249,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();
lhecker marked this conversation as resolved.
Show resolved Hide resolved
// 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
View file
Open in desktop
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
Loading