First three interactivity fixes

src/cascadia/TerminalControl/ControlInteractivity.cpp

@@ -35,6 +35,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _selectionNeedsToBeCopied{ false }
     {
         _core = winrt::make_self<ControlCore>(settings, connection);
+
+        _core->ScrollPositionChanged({ this, &ControlInteractivity::_coreScrollPositionChanged });
     }
 
     void ControlInteractivity::UpdateSettings()
@@ -197,7 +199,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             if (multiClickMapper == 1)
             {
                 _singleClickTouchdownPos = pixelPosition;
-                _singleClickTouchdownTerminalPos = terminalPosition;
 
                 if (!_core->HasSelection())
                 {
@@ -266,10 +267,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 const auto fontSizeInDips{ _core->FontSizeInDips() };
                 if (distance >= (std::min(fontSizeInDips.width(), fontSizeInDips.height()) / 4.f))
                 {
-                    _core->SetSelectionAnchor(terminalPosition);
+                    // GH#9955.c: Make sure to use the temrinal location of the
+                    // _touchdown_ point here. We want to start the selection
+                    // from where the user initially clicked, not where they are
+                    // now.
+                    _core->SetSelectionAnchor(_getTerminalPosition(touchdownPoint));
+
                     // stop tracking the touchdown point
                     _singleClickTouchdownPos = std::nullopt;
-                    _singleClickTouchdownTerminalPos = std::nullopt;
                 }
             }
 
@@ -303,12 +308,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 // panning down)
                 const float numRows = -1.0f * (dy / fontSizeInDips.height<float>());
 
-                const auto currentOffset = ::base::ClampedNumeric<double>(_core->ScrollOffset());
-                const auto newValue = numRows + currentOffset;
+                const double currentOffset = ::base::ClampedNumeric<double>(_core->ScrollOffset());
+                const double newValue = numRows + currentOffset;
 
                 // Update the Core's viewport position, and raise a
                 // ScrollPositionChanged event to update the scrollbar
-                _updateScrollbar(newValue);
+                UpdateScrollbar(newValue);
 
                 // Use this point as our new scroll anchor.
                 _touchAnchor = newTouchPoint;
@@ -341,7 +346,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
 
         _singleClickTouchdownPos = std::nullopt;
-        _singleClickTouchdownTerminalPos = std::nullopt;
     }
 
     void ControlInteractivity::TouchReleased()
@@ -396,7 +400,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         else
         {
-            _mouseScrollHandler(delta, terminalPosition, state.isLeftButtonDown);
+            _mouseScrollHandler(delta, pixelPosition, state.isLeftButtonDown);
         }
         return false;
     }
@@ -428,35 +432,40 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - Scroll the visible viewport in response to a mouse wheel event.
     // Arguments:
     // - mouseDelta: the mouse wheel delta that triggered this event.
-    // - point: the location of the mouse during this event
+    // - pixelPosition: the location of the mouse during this event
     // - isLeftButtonPressed: true iff the left mouse button was pressed during this event.
     void ControlInteractivity::_mouseScrollHandler(const double mouseDelta,
-                                                   const til::point terminalPosition,
+                                                   const til::point pixelPosition,
                                                    const bool isLeftButtonPressed)
     {
-        const auto currentOffset = _core->ScrollOffset();
+        // GH#9955.b: Start scrolling from our internal scrollbar position. This
+        // lets us accumulate fractional numbers of rows to scroll with each
+        // event. Especially for precision trackpads, we might be getting scroll
+        // deltas smaller than a single row, but we still want lots of those to
+        // accumulate.
+        const double currentOffset = _internalScrollbarPosition;
 
         // negative = down, positive = up
         // However, for us, the signs are flipped.
         // With one of the precision mice, one click is always a multiple of 120 (WHEEL_DELTA),
         // but the "smooth scrolling" mode results in non-int values
-        const auto rowDelta = mouseDelta / (-1.0 * WHEEL_DELTA);
+        const double rowDelta = mouseDelta / (-1.0 * 120.0);
 
         // WHEEL_PAGESCROLL is a Win32 constant that represents the "scroll one page
         // at a time" setting. If we ignore it, we will scroll a truly absurd number
         // of rows.
-        const auto rowsToScroll{ _rowsToScroll == WHEEL_PAGESCROLL ? _core->ViewHeight() : _rowsToScroll };
+        const double rowsToScroll{ _rowsToScroll == WHEEL_PAGESCROLL ? ::base::saturated_cast<double>(_core->ViewHeight()) : _rowsToScroll };
         double newValue = (rowsToScroll * rowDelta) + (currentOffset);
 
         // Update the Core's viewport position, and raise a
         // ScrollPositionChanged event to update the scrollbar
-        _updateScrollbar(::base::saturated_cast<int>(newValue));
+        UpdateScrollbar(newValue);
 
         if (isLeftButtonPressed)
         {
             // If user is mouse selecting and scrolls, they then point at new
             // character. Make sure selection reflects that immediately.
-            SetEndSelectionPoint(terminalPosition);
+            SetEndSelectionPoint(pixelPosition);
         }
     }
 
@@ -476,15 +485,42 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - newValue: The new top of the viewport
     // Return Value:
     // - <none>
-    void ControlInteractivity::_updateScrollbar(const int newValue)
+    void ControlInteractivity::UpdateScrollbar(const double newValue)
     {
-        _core->UserScrollViewport(newValue);
+        // Set this as the new value of our internal scrollbar representation.
+        // We're doing this so we can accumulate fractional amounts of a row to
+        // scroll each time the mouse scrolls.
+        _internalScrollbarPosition = std::clamp<double>(newValue, 0.0, _core->BufferHeight());
+
+        // If the new scrollbar position, rounded to an int, is at a different
+        // row, then actually update the scroll position in the core, and raise
+        // a ScrollPositionChanged to inform the control.
+        int viewTop = ::base::saturated_cast<int>(::std::round(_internalScrollbarPosition));
+        if (viewTop != _core->ScrollOffset())
+        {
+            _core->UserScrollViewport(viewTop);
 
-        // _core->ScrollOffset() is now set to newValue
-        _ScrollPositionChangedHandlers(*this,
-                                       winrt::make<ScrollPositionChangedArgs>(_core->ScrollOffset(),
-                                                                              _core->ViewHeight(),
-                                                                              _core->BufferHeight()));
+            // _core->ScrollOffset() is now set to newValue
+            _ScrollPositionChangedHandlers(*this,
+                                           winrt::make<ScrollPositionChangedArgs>(_core->ScrollOffset(),
+                                                                                  _core->ViewHeight(),
+                                                                                  _core->BufferHeight()));
+        }
+    }
+
+    // Method Description:
+    // - Event handler for the core's ScrollPositionChanged event. This is
+    //   called when the core changes its viewport position, due to more text
+    //   being output. We'll use this event to update our own internal scrollbar
+    //   tracker to the position the viewport is at now.
+    // Arguments:
+    // - args: args containing infor about the position of the viewport in the buffer.
+    // Return Value:
+    // - <none>
+    void ControlInteractivity::_coreScrollPositionChanged(const IInspectable& /*sender*/,
+                                                          const Control::ScrollPositionChangedArgs& args)
+    {
+        _internalScrollbarPosition = args.ViewTop();
     }
 
     void ControlInteractivity::_hyperlinkHandler(const std::wstring_view uri)

src/cascadia/TerminalControl/ControlInteractivity.h

@@ -73,6 +73,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                         const int32_t delta,
                         const til::point pixelPosition,
                         const ::Microsoft::Console::VirtualTerminal::TerminalInput::MouseButtonState state);
+
+        void UpdateScrollbar(const double newValue);
+
 #pragma endregion
 
         bool CopySelectionToClipboard(bool singleLine,
@@ -83,6 +86,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     private:
         winrt::com_ptr<ControlCore> _core{ nullptr };
         unsigned int _rowsToScroll;
+        double _internalScrollbarPosition{ 0.0 };
 
         // If this is set, then we assume we are in the middle of panning the
         //      viewport via touch input.
@@ -97,7 +101,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         Timestamp _lastMouseClickTimestamp;
         std::optional<til::point> _lastMouseClickPos;
         std::optional<til::point> _singleClickTouchdownPos;
-        std::optional<til::point> _singleClickTouchdownTerminalPos;
         std::optional<til::point> _lastMouseClickPosNoSelection;
         // This field tracks whether the selection has changed meaningfully
         // since it was last copied. It's generally used to prevent copyOnSelect
@@ -124,9 +127,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         bool _canSendVTMouseInput(const ::Microsoft::Terminal::Core::ControlKeyStates modifiers);
 
         void _sendPastedTextToConnection(std::wstring_view wstr);
-        void _updateScrollbar(const int newValue);
         til::point _getTerminalPosition(const til::point& pixelPosition);
 
+        void _coreScrollPositionChanged(const IInspectable& /*sender*/, const Control::ScrollPositionChangedArgs& args);
+
         TYPED_EVENT(OpenHyperlink, IInspectable, Control::OpenHyperlinkEventArgs);
         TYPED_EVENT(PasteFromClipboard, IInspectable, Control::PasteFromClipboardEventArgs);
         TYPED_EVENT(ScrollPositionChanged, IInspectable, Control::ScrollPositionChangedArgs);

src/cascadia/TerminalControl/TermControl.cpp

@@ -1302,8 +1302,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             return;
         }
 
-        const auto newValue = static_cast<int>(args.NewValue());
-        _core->UserScrollViewport(newValue);
+        const auto newValue = args.NewValue();
+        _interactivity->UpdateScrollbar(newValue);
 
         // User input takes priority over terminal events so cancel
         // any pending scroll bar update if the user scrolls.