From 53080d304103e6d40af33938e1277d13e08e2dc9 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Mon, 23 Mar 2020 23:06:53 -0700 Subject: [PATCH] Rework and simplify selection in TermControl This commit rewrites selection handling at the TermControl layer. Previously, we were keeping track of a number of redundant variables that were easy to get out of sync. The new selection model is as follows: * A single left click will always begin a _pending_ selection operation * A single left click will always clear a selection (#4477) * A double left click will always begin a word selection * A triple left click will always begin a line selection * A selection will only truly start when the cursor moves a quarter of the smallest dimension of a cell (usually its width) in any direction _This eliminates the selection of a single cell on one click._ (#4282, #5082) * We now keep track of whether the selection has been "copied", or "updated" since it was last copied. If an endpoint moves, it is updated. For copy-on-select, it is only copied if it's updated. (#4740) Because of this, we can stop tracking the position of the focus-raising click, and whether it was part of click-drag operation. All clicks can _become_ part of a click-drag operation if the user drags. We can also eliminate the special handling of single cell selection at the TerminalCore layer: since TermControl determines when to begin a selection, TerminalCore no longer needs to know whether copy on select is enabled _or_ whether the user has started and then backtracked over a single cell. This is now implicit in TermControl. Fixes #4082; Fixes #4477 --- src/cascadia/TerminalControl/TermControl.cpp | 60 ++++++++++--------- src/cascadia/TerminalControl/TermControl.h | 7 +-- src/cascadia/TerminalCore/Terminal.cpp | 6 +- src/cascadia/TerminalCore/Terminal.hpp | 4 -- .../TerminalCore/TerminalSelection.cpp | 34 ----------- 5 files changed, 35 insertions(+), 76 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 47d93ca8591..ea370d19889 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -69,9 +69,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _cursorTimer{}, _lastMouseClickTimestamp{}, _lastMouseClickPos{}, - _searchBox{ nullptr }, - _focusRaisedClickPos{ std::nullopt }, - _clickDrag{ false } + _selectionUpdatedThisCycle{ false }, + _searchBox{ nullptr } { _EnsureStaticInitialization(); InitializeComponent(); @@ -911,9 +910,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (!_focused) { Focus(FocusState::Pointer); - // Save the click position that brought this control into focus - // in case the user wants to perform a click-drag selection. - _focusRaisedClickPos = point.Position(); } if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse || ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Pen) @@ -933,13 +929,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (point.Properties().IsLeftButtonPressed()) { - // A single left click from out of focus should only focus. - if (_focusRaisedClickPos) - { - args.Handled(true); - return; - } - const auto cursorPosition = point.Position(); const auto terminalPosition = _GetTerminalPosition(cursorPosition); @@ -956,20 +945,26 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (multiClickMapper == 3) { _terminal->MultiClickSelection(terminalPosition, ::Terminal::SelectionExpansionMode::Line); + _selectionUpdatedThisCycle = true; } else if (multiClickMapper == 2) { _terminal->MultiClickSelection(terminalPosition, ::Terminal::SelectionExpansionMode::Word); + _selectionUpdatedThisCycle = true; } else { if (shiftEnabled && _terminal->IsSelectionActive()) { _terminal->SetSelectionEnd(terminalPosition, ::Terminal::SelectionExpansionMode::Cell); + _selectionUpdatedThisCycle = true; } else { - _terminal->SetSelectionAnchor(terminalPosition); + // A single click down resets the selection and begins a new one. + _terminal->ClearSelection(); + _singleClickTouchdownPos = cursorPosition; + _selectionUpdatedThisCycle = false; // there's no selection, so there's nothing to update } _lastMouseClickTimestamp = point.Timestamp(); @@ -980,7 +975,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation else if (point.Properties().IsRightButtonPressed()) { // CopyOnSelect right click always pastes - if (_terminal->IsCopyOnSelectActive() || !_terminal->IsSelectionActive()) + if (!_selectionUpdatedThisCycle || !_terminal->IsSelectionActive()) { PasteTextFromClipboard(); } @@ -1028,16 +1023,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (point.Properties().IsLeftButtonPressed()) { - _clickDrag = true; + const auto cursorPosition = point.Position(); - // PointerPressedHandler doesn't set the SelectionAnchor when the click was - // from out of focus, so PointerMoved has to set it. - if (_focusRaisedClickPos) + if (_singleClickTouchdownPos) { - _terminal->SetSelectionAnchor(_GetTerminalPosition(*_focusRaisedClickPos)); + // Figure out if the user's moved a quarter of a cell's smaller axis away from the clickdown point + auto& touchdownPoint{ *_singleClickTouchdownPos }; + auto distance{ std::sqrtf(std::powf(cursorPosition.X - touchdownPoint.X, 2) + std::powf(cursorPosition.Y - touchdownPoint.Y, 2)) }; + const auto fontSize{ _actualFont.GetSize() }; + if (distance >= (std::min(fontSize.X, fontSize.Y) / 4.f)) + { + _terminal->SetSelectionAnchor(_GetTerminalPosition(touchdownPoint)); + // stop tracking the touchdown point + _singleClickTouchdownPos = std::nullopt; + } } - const auto cursorPosition = point.Position(); _SetEndSelectionPointAtCursor(cursorPosition); const double cursorBelowBottomDist = cursorPosition.Y - SwapChainPanel().Margin().Top - SwapChainPanel().ActualHeight(); @@ -1121,17 +1122,14 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Only a left click release when copy on select is active should perform a copy. // Right clicks and middle clicks should not need to do anything when released. - if (_terminal->IsCopyOnSelectActive() && point.Properties().PointerUpdateKind() == Windows::UI::Input::PointerUpdateKind::LeftButtonReleased) + if (_settings.CopyOnSelect() && point.Properties().PointerUpdateKind() == Windows::UI::Input::PointerUpdateKind::LeftButtonReleased) { const auto modifiers = static_cast(args.KeyModifiers()); // static_cast to a uint32_t because we can't use the WI_IsFlagSet // macro directly with a VirtualKeyModifiers const auto shiftEnabled = WI_IsFlagSet(modifiers, static_cast(VirtualKeyModifiers::Shift)); - // In a Copy on Select scenario, - // All left click drags should copy, - // All left clicks on a focused control should copy if a selection is active. - if (_clickDrag || !_focusRaisedClickPos) + if (_selectionUpdatedThisCycle) { CopySelectionToClipboard(shiftEnabled); } @@ -1142,8 +1140,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _touchAnchor = std::nullopt; } - _focusRaisedClickPos = std::nullopt; - _clickDrag = false; + _singleClickTouchdownPos = std::nullopt; _TryStopAutoScroll(ptr.PointerId()); @@ -1649,6 +1646,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // save location (for rendering) + render _terminal->SetSelectionEnd(terminalPosition); _renderer->TriggerSelection(); + _selectionUpdatedThisCycle = true; } // Method Description: @@ -1799,6 +1797,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { return false; } + + // Mark the current selection as copied + _selectionUpdatedThisCycle = false; + // extract text from buffer const auto bufferData = _terminal->RetrieveSelectedTextFromBuffer(collapseText); @@ -1822,7 +1824,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _actualFont.GetFaceName(), _settings.DefaultBackground()); - if (!_terminal->IsCopyOnSelectActive()) + if (!_settings.CopyOnSelect()) { _terminal->ClearSelection(); _renderer->TriggerSelection(); diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 57606506bee..39c900d1026 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -155,12 +155,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // imported from WinUser // Used for PointerPoint.Timestamp Property (https://docs.microsoft.com/en-us/uwp/api/windows.ui.input.pointerpoint.timestamp#Windows_UI_Input_PointerPoint_Timestamp) Timestamp _multiClickTimer; - Timestamp _lastMouseClickTimestamp; unsigned int _multiClickCounter; + Timestamp _lastMouseClickTimestamp; std::optional _lastMouseClickPos; - - std::optional _focusRaisedClickPos; - bool _clickDrag; + std::optional _singleClickTouchdownPos; + bool _selectionUpdatedThisCycle; winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker; diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 36daae54fce..265d1f3d372 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -45,9 +45,7 @@ Terminal::Terminal() : _scrollOffset{ 0 }, _snapOnInput{ true }, _blockSelection{ false }, - _selection{ std::nullopt }, - _allowSingleCharSelection{ true }, - _copyOnSelect{ false } + _selection{ std::nullopt } { auto dispatch = std::make_unique(*this); auto engine = std::make_unique(std::move(dispatch)); @@ -145,8 +143,6 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting _wordDelimiters = settings.WordDelimiters(); - _copyOnSelect = settings.CopyOnSelect(); - _suppressApplicationTitle = settings.SuppressApplicationTitle(); _startingTitle = settings.StartingTitle(); diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 8498efa1aff..c50e3a91143 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -181,7 +181,6 @@ class Microsoft::Terminal::Core::Terminal final : Word, Line }; - const bool IsCopyOnSelectActive() const noexcept; void MultiClickSelection(const COORD viewportPos, SelectionExpansionMode expansionMode); void SetSelectionAnchor(const COORD position); void SetSelectionEnd(const COORD position, std::optional newExpansionMode = std::nullopt); @@ -222,8 +221,6 @@ class Microsoft::Terminal::Core::Terminal final : }; std::optional _selection; bool _blockSelection; - bool _allowSingleCharSelection; - bool _copyOnSelect; std::wstring _wordDelimiters; SelectionExpansionMode _multiClickSelectionMode; #pragma endregion @@ -275,7 +272,6 @@ class Microsoft::Terminal::Core::Terminal final : std::pair _PivotSelection(const COORD targetPos) const; std::pair _ExpandSelectionAnchors(std::pair anchors) const; COORD _ConvertToBufferCell(const COORD viewportPos) const; - const bool _IsSingleCellSelection() const noexcept; #pragma endregion #ifdef UNIT_TESTING diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 3d096e2ec5e..f8a13a56151 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -82,27 +82,12 @@ const COORD Terminal::GetSelectionEnd() const noexcept return _selection->end; } -// Method Description: -// - Checks if selection is on a single cell -// Return Value: -// - bool representing if selection is only a single cell. Used for copyOnSelect -const bool Terminal::_IsSingleCellSelection() const noexcept -{ - return (_selection->start == _selection->end); -} - // Method Description: // - Checks if selection is active // Return Value: // - bool representing if selection is active. Used to decide copy/paste on right click const bool Terminal::IsSelectionActive() const noexcept { - // A single cell selection is not considered an active selection, - // if it's not allowed - if (!_allowSingleCharSelection && _IsSingleCellSelection()) - { - return false; - } return _selection.has_value(); } @@ -111,15 +96,6 @@ const bool Terminal::IsBlockSelection() const noexcept return _blockSelection; } -// Method Description: -// - Checks if the CopyOnSelect setting is active -// Return Value: -// - true if feature is active, false otherwise. -const bool Terminal::IsCopyOnSelectActive() const noexcept -{ - return _copyOnSelect; -} - // Method Description: // - Perform a multi-click selection at viewportPos expanding according to the expansionMode // Arguments: @@ -148,8 +124,6 @@ void Terminal::SetSelectionAnchor(const COORD viewportPos) _selection = SelectionAnchors{}; _selection->pivot = _ConvertToBufferCell(viewportPos); - _allowSingleCharSelection = (_copyOnSelect) ? false : true; - _multiClickSelectionMode = SelectionExpansionMode::Cell; SetSelectionEnd(viewportPos); @@ -172,13 +146,6 @@ void Terminal::SetSelectionEnd(const COORD viewportPos, std::optionalstart, _selection->end) = _ExpandSelectionAnchors(anchors); - - // moving the endpoint of what used to be a single cell selection - // allows the user to drag back and select just one cell - if (_copyOnSelect && !_IsSingleCellSelection()) - { - _allowSingleCharSelection = true; - } } // Method Description: @@ -248,7 +215,6 @@ void Terminal::SetBlockSelection(const bool isEnabled) noexcept #pragma warning(disable : 26440) // changing this to noexcept would require a change to ConHost's selection model void Terminal::ClearSelection() { - _allowSingleCharSelection = false; _selection = std::nullopt; }