Port selection in conhost and Terminal to use til::generational #17676
Conversation
|
Ugh, I apparently need to split up my commit that fixes the tests for SelectionSpans to move some of the generational changes down here |
f6f45e5 to
f9884e8
Compare
We will avoid regenerating the selection rects every time we draw the screen
f9884e8 to
49a0277
Compare
| selection->start = buffer.GetWordStart(selection->start, _wordDelimiters); | ||
| selection->pivot = selection->start; | ||
| selection->end = buffer.GetWordEnd(selection->end, _wordDelimiters); |
There was a problem hiding this comment.
Doesn't this have essentially the same issue as the wide <> narrow cell issue you mention in the PR message? So, in a way it's actually perhaps more than fine to ignore the issue for now.
(I wonder if we should have a "SelectionMode" enum or something in the future that can be set to word-wise and then it extends by words in the renderer and clipboard code? Possible over-engineering though hah.)
There was a problem hiding this comment.
(I wonder if we should have a "SelectionMode" enum or something in the future that can be set to word-wise and then it extends by words in the renderer and clipboard code? Possible over-engineering though hah.)
I think wordwise and linewise selection actually impacts the literal selection, rather than only the display of the selection. I think we should not mix those things yet.
| auto selection{ m_pSelection->_d.write() }; | ||
| // #2: right to left selection | ||
| selection->coordSelectionAnchor.x = selection->srSelectionRect.right; | ||
| VERIFY_IS_TRUE(selection->srSelectionRect.bottom == selection->srSelectionRect.top); |
There was a problem hiding this comment.
what even is this check? i mean, i guess it's defending the tests against a bad test author. but we set bottom ourselves.
In #17638, I am moving selection to an earlier phase of rendering (so
that further phases can take it into account). Since I am drafting off
the design of search highlights, one of the required changes is moving
to passing
spans ofpoint_spans around to make selection effectivelyzero-copy.
We can't easily have zero-copy selection propagation without caching,
and we can't have caching without mandatory cache invalidation.
This pull request moves both conhost and Terminal to use
til::generationalfor all selection members that impact the rangesthat would be produced from
GetSelectionRects.This required a move from
std::optional<>to a boolean to determinewhether a selection was active in Terminal.
We will no longer regenerate the selection rects from the selection
anchors plus the text buffer every single frame.
Apart from being annoying to read, there is one downside.
If you begin a selection on a narrow character, and that narrow
character later turns into a wide character, we will show it as
half-selected.
This should be a rare-enough case that we can accept it as a regression.