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

Jump to bottom

Introduce the concept of "selection spans" instead of "rects" #17638

Merged
merged 4 commits into from
Aug 15, 2024
Merged

Introduce the concept of "selection spans" instead of "rects" #17638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9cc9d90
conhost: port Selection to Spans instead of Rects
DHowett Jul 26, 2024
c51114e
terminal: port TerminalSelection to GetSelectionSpans
DHowett Jul 26, 2024
96a4510
render: Move all empty InvalidateSelection to RenderEngineBase
DHowett Jul 26, 2024
05c9435
render: Make InvalidateSelection take a span of viewport rects
DHowett Jul 26, 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
6 changes: 4 additions & 2 deletions src/cascadia/TerminalCore/Terminal.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class Microsoft::Terminal::Core::Terminal final :
const std::vector<size_t> GetPatternId(const til::point location) const override;

std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
std::vector<Microsoft::Console::Types::Viewport> GetSelectionRects() noexcept override;
std::span<const til::point_span> GetSelectionSpans() const noexcept override;
std::span<const til::point_span> GetSearchHighlights() const noexcept override;
const til::point_span* GetSearchHighlightFocused() const noexcept override;
const bool IsSelectionActive() const noexcept override;
Expand Down Expand Up @@ -356,6 +356,9 @@ class Microsoft::Terminal::Core::Terminal final :
std::vector<til::point_span> _searchHighlights;
size_t _searchHighlightFocused = 0;

mutable std::vector<til::point_span> _lastSelectionSpans;
mutable til::generation_t _lastSelectionGeneration{};
DHowett marked this conversation as resolved.
Show resolved Hide resolved

CursorType _defaultCursorShape = CursorType::Legacy;

til::enumset<Mode> _systemMode{ Mode::AutoWrap };
Expand Down Expand Up @@ -466,7 +469,6 @@ class Microsoft::Terminal::Core::Terminal final :

#pragma region TextSelection
// These methods are defined in TerminalSelection.cpp
std::vector<til::inclusive_rect> _GetSelectionRects() const noexcept;
std::vector<til::point_span> _GetSelectionSpans() const noexcept;
std::pair<til::point, til::point> _PivotSelection(const til::point targetPos, bool& targetStart) const noexcept;
std::pair<til::point, til::point> _ExpandSelectionAnchors(std::pair<til::point, til::point> anchors) const;
Expand Down
21 changes: 0 additions & 21 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,27 +42,6 @@ DEFINE_ENUM_FLAG_OPERATORS(Terminal::SelectionEndpoint);
* The pivot never moves until a new selection is created. It ensures that that cell will always be selected.
*/

// Method Description:
// - Helper to determine the selected region of the buffer. Used for rendering.
// Return Value:
// - A vector of rectangles representing the regions to select, line by line. They are absolute coordinates relative to the buffer origin.
std::vector<til::inclusive_rect> Terminal::_GetSelectionRects() const noexcept
{
std::vector<til::inclusive_rect> result;

if (!IsSelectionActive())
{
return result;
}

try
{
return _activeBuffer().GetTextRects(_selection->start, _selection->end, _selection->blockSelection, false);
}
CATCH_LOG();
return result;
}

// Method Description:
// - Identical to GetTextRects if it's a block selection, else returns a single span for the whole selection.
// Return Value:
Expand Down
11 changes: 5 additions & 6 deletions src/cascadia/TerminalCore/terminalrenderdata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,16 @@ std::pair<COLORREF, COLORREF> Terminal::GetAttributeColors(const TextAttribute&
return GetRenderSettings().GetAttributeColors(attr);
}

std::vector<Microsoft::Console::Types::Viewport> Terminal::GetSelectionRects() noexcept
std::span<const til::point_span> Terminal::GetSelectionSpans() const noexcept
try
{
std::vector<Viewport> result;

for (const auto& lineRect : _GetSelectionRects())
if (_selection.generation() != _lastSelectionGeneration)
{
result.emplace_back(Viewport::FromInclusive(lineRect));
_lastSelectionSpans = _GetSelectionSpans();
_lastSelectionGeneration = _selection.generation();
}

return result;
return _lastSelectionSpans;
}
catch (...)
{
Expand Down
11 changes: 0 additions & 11 deletions src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,6 @@ namespace ControlUnitTests
true);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());

Log::Comment(L"Drag the mouse down a whole row");
const til::point terminalPosition2{ 1, 1 };
Expand All @@ -349,7 +348,6 @@ namespace ControlUnitTests
true);
Log::Comment(L"Verify that there's now two selections (one on each row)");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(2u, core->_terminal->GetSelectionRects().size());

Log::Comment(L"Release the mouse");
interactivity->PointerReleased(noMouseDown,
Expand All @@ -358,7 +356,6 @@ namespace ControlUnitTests
cursorPosition2.to_core_point());
Log::Comment(L"Verify that there's still two selections");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(2u, core->_terminal->GetSelectionRects().size());

Log::Comment(L"click outside the current selection");
const til::point terminalPosition3{ 2, 2 };
Expand All @@ -370,7 +367,6 @@ namespace ControlUnitTests
cursorPosition3.to_core_point());
Log::Comment(L"Verify that there's now no selection");
VERIFY_IS_FALSE(core->HasSelection());
VERIFY_ARE_EQUAL(0u, core->_terminal->GetSelectionRects().size());

Log::Comment(L"Drag the mouse");
const til::point terminalPosition4{ 3, 2 };
Expand All @@ -383,7 +379,6 @@ namespace ControlUnitTests
true);
Log::Comment(L"Verify that there's now one selection");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
}

void ControlInteractivityTests::ScrollWithSelection()
Expand Down Expand Up @@ -438,7 +433,6 @@ namespace ControlUnitTests
true);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());

Log::Comment(L"Verify the location of the selection");
// The viewport is on row 21, so the selection will be on:
Expand Down Expand Up @@ -586,7 +580,6 @@ namespace ControlUnitTests
true);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());

Log::Comment(L"Verify that it started on the first cell we clicked on, not the one we dragged to");
til::point expectedAnchor{ 0, 0 };
Expand Down Expand Up @@ -631,7 +624,6 @@ namespace ControlUnitTests
true);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());

Log::Comment(L"Verify that it started on the first cell we clicked on, not the one we dragged to");
til::point expectedAnchor{ 0, 0 };
Expand Down Expand Up @@ -801,7 +793,6 @@ namespace ControlUnitTests
true);
Log::Comment(L"Verify that there's one selection");
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());

Log::Comment(L"Verify the location of the selection");
// The viewport is on row (historySize + 5), so the selection will be on:
Expand Down Expand Up @@ -860,7 +851,6 @@ namespace ControlUnitTests
cursorPosition0.to_core_point(),
true);
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
{
const auto anchor{ core->_terminal->GetSelectionAnchor() };
const auto end{ core->_terminal->GetSelectionEnd() };
Expand All @@ -885,7 +875,6 @@ namespace ControlUnitTests
cursorPosition1.to_core_point(),
true);
VERIFY_IS_TRUE(core->HasSelection());
VERIFY_ARE_EQUAL(1u, core->_terminal->GetSelectionRects().size());
{
const auto anchor{ core->_terminal->GetSelectionAnchor() };
const auto end{ core->_terminal->GetSelectionEnd() };
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ namespace
HRESULT Invalidate(const til::rect* /*psrRegion*/) noexcept { return S_OK; }
HRESULT InvalidateCursor(const til::rect* /*psrRegion*/) noexcept { return S_OK; }
HRESULT InvalidateSystem(const til::rect* /*prcDirtyClient*/) noexcept { return S_OK; }
HRESULT InvalidateSelection(const std::vector<til::rect>& /*rectangles*/) noexcept { return S_OK; }
HRESULT InvalidateScroll(const til::point* pcoordDelta) noexcept
{
_triggerScrollDelta = *pcoordDelta;
Expand Down
Loading
Loading