diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index 323bd9aaac5..8ced4e65146 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -313,7 +313,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // 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; @@ -459,7 +459,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Update the Core's viewport position, and raise a // ScrollPositionChanged event to update the scrollbar - _updateScrollbar(newValue); + UpdateScrollbar(newValue); if (isLeftButtonPressed) { @@ -485,12 +485,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - newValue: The new top of the viewport // Return Value: // - - void ControlInteractivity::_updateScrollbar(const double newValue) + void ControlInteractivity::UpdateScrollbar(const double 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 = newValue; + _internalScrollbarPosition = std::clamp(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 diff --git a/src/cascadia/TerminalControl/ControlInteractivity.h b/src/cascadia/TerminalControl/ControlInteractivity.h index 87114add6ce..f1b1b22aab1 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.h +++ b/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, @@ -124,7 +127,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool _canSendVTMouseInput(const ::Microsoft::Terminal::Core::ControlKeyStates modifiers); void _sendPastedTextToConnection(std::wstring_view wstr); - void _updateScrollbar(const double newValue); til::point _getTerminalPosition(const til::point& pixelPosition); void _coreScrollPositionChanged(const IInspectable& /*sender*/, const Control::ScrollPositionChangedArgs& args); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 5346d0507c9..3c9108da3b9 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1302,8 +1302,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation return; } - const auto newValue = static_cast(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. diff --git a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp index 65c78d8edb8..84c2f905ae0 100644 --- a/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp @@ -30,6 +30,7 @@ namespace ControlUnitTests TEST_METHOD(CreateSubsequentSelectionWithDragging); TEST_METHOD(ScrollWithSelection); + TEST_METHOD(TestScrollWithTrackpad); TEST_METHOD(TestQuickDragOnSelect); TEST_CLASS_SETUP(ClassSetup) @@ -228,11 +229,13 @@ namespace ControlUnitTests Log::Comment(L"Scroll down 21 more times, to the bottom"); for (int i = 0; i < 21; ++i) { + Log::Comment(NoThrowString().Format(L"---scroll down #%d---", i)); expectedTop++; interactivity->MouseWheel(modifiers, -WHEEL_DELTA, til::point{ 0, 0 }, { false, false, false }); + Log::Comment(NoThrowString().Format(L"internal scrollbar pos:%f", interactivity->_internalScrollbarPosition)); } Log::Comment(L"Scrolling up more should do nothing"); expectedTop = 21; @@ -410,6 +413,113 @@ namespace ControlUnitTests VERIFY_ARE_EQUAL(expectedAnchor, core->_terminal->GetSelectionEnd()); } + void ControlInteractivityTests::TestScrollWithTrackpad() + { + WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{}; + + auto [settings, conn] = _createSettingsAndConnection(); + auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn); + _standardInit(core, interactivity); + // For the sake of this test, scroll one line at a time + interactivity->_rowsToScroll = 1; + + // int expectedTop = 0; + // int expectedViewHeight = 20; + // int expectedBufferHeight = 20; + + // auto scrollChangedHandler = [&](auto&&, const Control::ScrollPositionChangedArgs& args) mutable { + // VERIFY_ARE_EQUAL(expectedTop, args.ViewTop()); + // VERIFY_ARE_EQUAL(expectedViewHeight, args.ViewHeight()); + // VERIFY_ARE_EQUAL(expectedBufferHeight, args.BufferSize()); + // }; + // core->ScrollPositionChanged(scrollChangedHandler); + // interactivity->ScrollPositionChanged(scrollChangedHandler); + + for (int i = 0; i < 40; ++i) + { + // Log::Comment(NoThrowString().Format(L"Writing line #%d", i)); + // The \r\n in the 19th loop will cause the view to start moving + // if (i >= 19) + // { + // expectedTop++; + // expectedBufferHeight++; + // } + + conn->WriteInput(L"Foo\r\n"); + } + // We printed that 40 times, but the final \r\n bumped the view down one MORE row. + VERIFY_ARE_EQUAL(20, core->_terminal->GetViewport().Height()); + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + VERIFY_ARE_EQUAL(20, core->ViewHeight()); + VERIFY_ARE_EQUAL(41, core->BufferHeight()); + + Log::Comment(L"Scroll up a line"); + const auto modifiers = ControlKeyStates(); + // expectedBufferHeight = 41; + // expectedTop = 20; + + // Deltas that I saw while scrolling with the surface laptop trackpad + // were on the range [-22, 7], though I'm sure they could be greater in + // magnitude. + // + // WHEEL_DELTA is 120, so we'll use 24 for now as the delta, just so the tests don't take forever. + + const int delta = WHEEL_DELTA / 5; + const til::point mousePos{ 0, 0 }; + TerminalInput::MouseButtonState state{ false, false, false }; + + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 1/5 + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + + Log::Comment(L"Scroll up 4 more times. Once we're at 3/5 scrolls, " + L"we'll round the internal scrollbar position to scrolling to the next row."); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 2/5 + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 3/5 + VERIFY_ARE_EQUAL(20, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 4/5 + VERIFY_ARE_EQUAL(20, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 5/5 + VERIFY_ARE_EQUAL(20, core->ScrollOffset()); + + Log::Comment(L"Jump to line 5, so we can scroll down from there."); + interactivity->UpdateScrollbar(5); + VERIFY_ARE_EQUAL(5, core->ScrollOffset()); + Log::Comment(L"Scroll down 5 times, at which point we should accumulate a whole row of delta."); + interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 1/5 + VERIFY_ARE_EQUAL(5, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 2/5 + VERIFY_ARE_EQUAL(5, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 3/5 + VERIFY_ARE_EQUAL(6, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 4/5 + VERIFY_ARE_EQUAL(6, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, -delta, mousePos, state); // 5/5 + VERIFY_ARE_EQUAL(6, core->ScrollOffset()); + + Log::Comment(L"Jump to the bottom."); + interactivity->UpdateScrollbar(21); + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + Log::Comment(L"Scroll a bit, then emit a line of text. We should reset our internal scroll position."); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 1/5 + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 2/5 + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + + conn->WriteInput(L"Foo\r\n"); + VERIFY_ARE_EQUAL(22, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 1/5 + VERIFY_ARE_EQUAL(22, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 2/5 + VERIFY_ARE_EQUAL(22, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 3/5 + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 4/5 + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + interactivity->MouseWheel(modifiers, delta, mousePos, state); // 5/5 + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); + } + void ControlInteractivityTests::TestQuickDragOnSelect() { // This is a test for GH#9955.c