From 143e89dbf85b13e1acccb76877d774a7fca1b016 Mon Sep 17 00:00:00 2001 From: Marc Mondesir Date: Wed, 4 Dec 2024 16:57:49 -0800 Subject: [PATCH] tdf#119745: Fix touchpad scrolling for Slides and Pages panes. Use fractional lines scrolled to calculate distance to scroll view, instead of jumping forward or backward a full page every call based on sign. Also fixes existing bugs: horizontal pane can scroll too far to right; and snapping pane from vertical to horizontal can break scrolling if view partially scrolled. Change-Id: I0ead4710a296aac26f1cf1a0fc48e6ea403271a6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177836 Tested-by: Jenkins Reviewed-by: Patrick Luby --- .../controller/SlideSorterController.cxx | 7 +- .../controller/SlsScrollBarManager.cxx | 78 ++++++++++--------- .../inc/controller/SlsScrollBarManager.hxx | 2 +- .../ui/slidesorter/inc/view/SlsLayouter.hxx | 6 ++ sd/source/ui/slidesorter/view/SlsLayouter.cxx | 10 ++- 5 files changed, 61 insertions(+), 42 deletions(-) diff --git a/sd/source/ui/slidesorter/controller/SlideSorterController.cxx b/sd/source/ui/slidesorter/controller/SlideSorterController.cxx index ad1d84bc1564..1589488dd6f8 100644 --- a/sd/source/ui/slidesorter/controller/SlideSorterController.cxx +++ b/sd/source/ui/slidesorter/controller/SlideSorterController.cxx @@ -382,6 +382,9 @@ bool SlideSorterController::Command ( // We do not support zooming with control+mouse wheel. return false; } + // tdf#119745: ScrollLines gives accurate distance scrolled on touchpad. NotchDelta sign + // gives direction. Default is 3 lines at a time, so factor that out. + double scrollDistance = -pData->GetScrollLines() * pData->GetNotchDelta() / 3.0; // Determine whether to scroll horizontally or vertically. This // depends on the orientation of the scroll bar and the // IsHoriz() flag of the event. @@ -390,13 +393,13 @@ bool SlideSorterController::Command ( { GetScrollBarManager().Scroll( ScrollBarManager::Orientation_Vertical, - -pData->GetNotchDelta()); + scrollDistance); } else { GetScrollBarManager().Scroll( ScrollBarManager::Orientation_Horizontal, - -pData->GetNotchDelta()); + scrollDistance); } mrSlideSorter.GetView().UpdatePageUnderMouse(rEvent.GetMousePosPixel()); diff --git a/sd/source/ui/slidesorter/controller/SlsScrollBarManager.cxx b/sd/source/ui/slidesorter/controller/SlsScrollBarManager.cxx index 6ee20c8dd93c..6a0244f41655 100644 --- a/sd/source/ui/slidesorter/controller/SlsScrollBarManager.cxx +++ b/sd/source/ui/slidesorter/controller/SlsScrollBarManager.cxx @@ -516,7 +516,7 @@ IMPL_LINK_NOARG(ScrollBarManager, AutoScrollTimeoutHandler, Timer *, void) void ScrollBarManager::Scroll( const Orientation eOrientation, - const sal_Int32 nDistance) + const double nDistance) { bool bIsVertical (false); switch (eOrientation) @@ -528,51 +528,53 @@ void ScrollBarManager::Scroll( return; } - Point aNewTopLeft ( - mpHorizontalScrollBar ? mpHorizontalScrollBar->GetThumbPos() : 0, - mpVerticalScrollBar ? mpVerticalScrollBar->GetThumbPos() : 0); + // Get current scrolling view position + // Fix bug where 1) scrolling view is partially scrolled, 2) view window is snapped from vertical + // to horizontal or vice-versa, then 3) mouse wheel scrolled. Scrolling broke because we got an + // invalid starting position from the scrollbar that's no longer displayed. Check scrollbars + // visible before getting position from each. + bool bHorizontalScrollShown = mpHorizontalScrollBar && mpHorizontalScrollBar->IsVisible(); + bool bVerticalScrollShown = mpVerticalScrollBar && mpVerticalScrollBar->IsVisible(); + Point aNewTopLeft( + bHorizontalScrollShown ? mpHorizontalScrollBar->GetThumbPos() : 0, + bVerticalScrollShown ? mpVerticalScrollBar->GetThumbPos() : 0); view::Layouter& rLayouter (mrSlideSorter.GetView().GetLayouter()); - // Calculate estimate of new location. + // Calculate new location + Size objectSize( rLayouter.GetPageObjectSize() ); // Size of a page in pane + Size objectAndGapSize = view::Layouter::AddGap(objectSize); // Add gap between pages to size if (bIsVertical) - aNewTopLeft.AdjustY(nDistance * rLayouter.GetPageObjectSize().Height() ); + aNewTopLeft.AdjustY(nDistance * objectAndGapSize.Height() ); // New position else - aNewTopLeft.AdjustX(nDistance * rLayouter.GetPageObjectSize().Width() ); + aNewTopLeft.AdjustX(nDistance * objectAndGapSize.Width() ); // New position - // Adapt location to show whole slides. + // Get displayable area for pages + ::tools::Rectangle scrollArea = rLayouter.GetTotalBoundingBox(); + + // Prevent scrolling out of bounds by limiting scroll destination point. if (bIsVertical) - if (nDistance > 0) - { - const sal_Int32 nIndex (rLayouter.GetIndexAtPoint( - Point(aNewTopLeft.X(), aNewTopLeft.Y()+mpVerticalScrollBar->GetVisibleSize()), - true)); - aNewTopLeft.setY( rLayouter.GetPageObjectBox(nIndex,true).Bottom() - - mpVerticalScrollBar->GetVisibleSize() ); - } - else - { - const sal_Int32 nIndex (rLayouter.GetIndexAtPoint( - Point(aNewTopLeft.X(), aNewTopLeft.Y()), - true)); - aNewTopLeft.setY( rLayouter.GetPageObjectBox(nIndex,true).Top() ); - } + { + // Subtract size of view itself to get scrollable area. + ::tools::Long scrollbarSize = mpVerticalScrollBar->GetVisibleSize(); + // Prevent (-) height. std::max needs type specified explicitly so params match. + ::tools::Long scrollHeight = std::max(static_cast<::tools::Long>(0), scrollArea.GetHeight() - scrollbarSize); + scrollArea.SetHeight(scrollHeight); + // Constrain scroll point to valid area + ::tools::Long constrainedY = std::clamp(aNewTopLeft.getY(), scrollArea.Top(), scrollArea.Bottom()); + aNewTopLeft.setY(constrainedY); + } else - if (nDistance > 0) - { - const sal_Int32 nIndex (rLayouter.GetIndexAtPoint( - Point(aNewTopLeft.X()+mpVerticalScrollBar->GetVisibleSize(), aNewTopLeft.Y()), - true)); - aNewTopLeft.setX( rLayouter.GetPageObjectBox(nIndex,true).Right() - - mpVerticalScrollBar->GetVisibleSize() ); - } - else - { - const sal_Int32 nIndex (rLayouter.GetIndexAtPoint( - Point(aNewTopLeft.X(), aNewTopLeft.Y()), - true)); - aNewTopLeft.setX( rLayouter.GetPageObjectBox(nIndex,true).Left() ); - } + { + // Subtract size of view itself to get scrollable area. + ::tools::Long scrollbarSize = mpHorizontalScrollBar->GetVisibleSize(); + // Prevent (-) width. std::max needs type specified explicitly so params match. + ::tools::Long scrollWidth = std::max(static_cast<::tools::Long>(0), scrollArea.GetWidth() - scrollbarSize); + scrollArea.SetWidth(scrollWidth); + // Constrain scroll point to valid area + ::tools::Long constrainedX = std::clamp(aNewTopLeft.getX(), scrollArea.Left(), scrollArea.Right()); + aNewTopLeft.setX(constrainedX); + } mrSlideSorter.GetController().GetVisibleAreaManager().DeactivateCurrentSlideTracking(); SetTopLeft(aNewTopLeft); diff --git a/sd/source/ui/slidesorter/inc/controller/SlsScrollBarManager.hxx b/sd/source/ui/slidesorter/inc/controller/SlsScrollBarManager.hxx index df04a3b5952f..1ed5b9fead87 100644 --- a/sd/source/ui/slidesorter/inc/controller/SlsScrollBarManager.hxx +++ b/sd/source/ui/slidesorter/inc/controller/SlsScrollBarManager.hxx @@ -155,7 +155,7 @@ public: */ void Scroll( const Orientation eOrientation, - const sal_Int32 nDistance); + const double nDistance); private: SlideSorter& mrSlideSorter; diff --git a/sd/source/ui/slidesorter/inc/view/SlsLayouter.hxx b/sd/source/ui/slidesorter/inc/view/SlsLayouter.hxx index b91ae83c544c..c54dee655503 100644 --- a/sd/source/ui/slidesorter/inc/view/SlsLayouter.hxx +++ b/sd/source/ui/slidesorter/inc/view/SlsLayouter.hxx @@ -111,6 +111,12 @@ public: Size const & GetPageObjectSize() const; + /** Returns the passed Size plus the gap between pages. + @param rObjectSize + Object size to start from. + */ + static Size AddGap(const Size & rObjectSize); + /** Return the bounding box in window coordinates of the nIndex-th page object. */ diff --git a/sd/source/ui/slidesorter/view/SlsLayouter.cxx b/sd/source/ui/slidesorter/view/SlsLayouter.cxx index 8bc0daf3f5e9..371e410d3fee 100644 --- a/sd/source/ui/slidesorter/view/SlsLayouter.cxx +++ b/sd/source/ui/slidesorter/view/SlsLayouter.cxx @@ -183,7 +183,7 @@ public: const sal_Int32 nColumn, const bool bClampToValidRange) const; - ::tools::Rectangle GetPageObjectBox ( + ::tools::Rectangle GetPageObjectBox ( const sal_Int32 nIndex, const bool bIncludeBorderAndGap = false) const; @@ -349,6 +349,14 @@ Size const & Layouter::GetPageObjectSize() const return mpImplementation->maPageObjectSize; } +Size Layouter::AddGap(const Size & rObjectSize) +{ + Size newSize = rObjectSize; + newSize.AdjustWidth(Implementation::gnHorizontalGap); + newSize.AdjustHeight(Implementation::gnVerticalGap); + return newSize; +} + ::tools::Rectangle Layouter::GetPageObjectBox ( const sal_Int32 nIndex, const bool bIncludeBorderAndGap) const