QAIV - allow users to control single step in ScrollPerPixel mode

The documentation says that we scroll one pixel, but changing the
behavior in 934f06220391eb0e0ebf66a2eb037f48adb4c43c to do that
was not well received.

People were relying on the undocumented behavior - and the new behavior
was considered to be a regression. (Nobody called setSingleStep since
Qt in many cases provide a reasonable singleStep - which implied that their
programs scrolled with 1 pixel which was quite slow). Furthermore getting
the old behavior (auto set of single step) was nearly impossible.

However the revert (done in 0e69230d02813f0b7a050645fb7e443bd504ab6a)
gets us back to QScrollbar::setSingleStep not working in pixel scroll mode
(even without it being documented - but we should also have a working API
rather than documenting that it is not working)

The previous approach was directly prevented Qt from changing
single step (on e.g resize) at all. This patch only prevents Qt from
changing when a user explicitly has called the function
QScrollBar::setSingleStep (in pixel scroll mode).

That is we expect that calls to setSingleStep means that the user actually
wants to set the singleStep and doesn't want Qt to control that value.

Furthermore it is possible to switch back to the automatically adjusted
singlestep with QScrollBar::setSingleStep(-1).

[ChangeLog][QtWidgets][QAbstractItemView] QTBUG-7232 - In ItemViews
when scrollMode is set to scrollPerPixel, it is now possible to
change the single step. Qt will automatically adjust the single step
until setSingleStep is called. When setSingleStep is called it will
however respect the set and stop doing automatic changes of the value.
Calling setSingleStep(-1) will switch mode back to automatic adjust.

Task-number: QTBUG-7232
Change-Id: Ibfe0caa9751d3bcc11bfc6e0654a3d1ac35ac8ae
Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>

11 files changed, 174 insertions, 7 deletions

diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp
index 639f18c4bc..fe98fae17a 100644
--- a/src/widgets/itemviews/qabstractitemview.cpp
+++ b/src/widgets/itemviews/qabstractitemview.cpp
@@ -54,6 +54,7 @@
 #include <private/qabstractitemview_p.h>
 #include <private/qabstractitemmodel_p.h>
 #include <private/qguiapplication_p.h>
+#include <private/qscrollbar_p.h>
 #ifndef QT_NO_ACCESSIBILITY
 #include <qaccessible.h>
 #endif
@@ -427,6 +428,11 @@ void QAbstractItemViewPrivate::_q_scrollerStateChanged()
     \since 4.2
     \enum QAbstractItemView::ScrollMode
 
+    Describes how the scrollbar should behave. When setting the scroll mode
+    to ScrollPerPixel the single step size will adjust automatically unless
+    it was set explicitly using \l{QAbstractSlider::}{setSingleStep()}.
+    The automatic adjustment can be restored by setting the single step size to -1.
+
     \value ScrollPerItem    The view will scroll the contents one item at a time.
     \value ScrollPerPixel   The view will scroll the contents one pixel at a time.
 */
@@ -1235,6 +1241,10 @@ void QAbstractItemView::setVerticalScrollMode(ScrollMode mode)
         return;
     QModelIndex topLeft = indexAt(QPoint(0, 0));
     d->verticalScrollMode = mode;
+    if (mode == ScrollPerItem)
+        verticalScrollBar()->d_func()->itemviewChangeSingleStep(1); // setSingleStep(-1) => step with 1
+    else
+        verticalScrollBar()->setSingleStep(-1); // Ensure that the view can update single step
     updateGeometries(); // update the scroll bars
     scrollTo(topLeft, QAbstractItemView::PositionAtTop);
 }
@@ -1257,7 +1267,13 @@ QAbstractItemView::ScrollMode QAbstractItemView::verticalScrollMode() const
 void QAbstractItemView::setHorizontalScrollMode(ScrollMode mode)
 {
     Q_D(QAbstractItemView);
+    if (mode == d->horizontalScrollMode)
+        return;
     d->horizontalScrollMode = mode;
+    if (mode == ScrollPerItem)
+        horizontalScrollBar()->d_func()->itemviewChangeSingleStep(1); // setSingleStep(-1) => step with 1
+    else
+        horizontalScrollBar()->setSingleStep(-1); // Ensure that the view can update single step
     updateGeometries(); // update the scroll bars
 }
 
diff --git a/src/widgets/itemviews/qabstractitemview.h b/src/widgets/itemviews/qabstractitemview.h
index ff1848b149..e32c5dad2f 100644
--- a/src/widgets/itemviews/qabstractitemview.h
+++ b/src/widgets/itemviews/qabstractitemview.h
@@ -362,6 +362,7 @@ private:
     friend class QTreeViewPrivate; // needed to compile with MSVC
     friend class QListModeViewBase;
     friend class QListViewPrivate;
+    friend class QAbstractSlider;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QAbstractItemView::EditTriggers)
diff --git a/src/widgets/itemviews/qlistview.cpp b/src/widgets/itemviews/qlistview.cpp
index 7745004774..2dbbd211c0 100644
--- a/src/widgets/itemviews/qlistview.cpp
+++ b/src/widgets/itemviews/qlistview.cpp
@@ -46,6 +46,7 @@
 #include <qscrollbar.h>
 #include <qrubberband.h>
 #include <private/qlistview_p.h>
+#include <private/qscrollbar_p.h>
 #include <qdebug.h>
 #ifndef QT_NO_ACCESSIBILITY
 #include <qaccessible.h>
@@ -1868,7 +1869,7 @@ void QCommonListViewBase::paintDragDrop(QPainter *painter)
 
 void QCommonListViewBase::updateHorizontalScrollBar(const QSize &step)
 {
-    horizontalScrollBar()->setSingleStep(step.width() + spacing());
+    horizontalScrollBar()->d_func()->itemviewChangeSingleStep(step.width() + spacing());
     horizontalScrollBar()->setPageStep(viewport()->width());
 
     // If both scroll bars are set to auto, we might end up in a situation with enough space
@@ -1898,7 +1899,7 @@ void QCommonListViewBase::updateHorizontalScrollBar(const QSize &step)
 
 void QCommonListViewBase::updateVerticalScrollBar(const QSize &step)
 {
-    verticalScrollBar()->setSingleStep(step.height() + spacing());
+    verticalScrollBar()->d_func()->itemviewChangeSingleStep(step.height() + spacing());
     verticalScrollBar()->setPageStep(viewport()->height());
 
     // If both scroll bars are set to auto, we might end up in a situation with enough space
diff --git a/src/widgets/itemviews/qtableview.cpp b/src/widgets/itemviews/qtableview.cpp
index 0e38485f5a..2e88310b2f 100644
--- a/src/widgets/itemviews/qtableview.cpp
+++ b/src/widgets/itemviews/qtableview.cpp
@@ -46,6 +46,7 @@
 #include <qabstractbutton.h>
 #include <private/qtableview_p.h>
 #include <private/qheaderview_p.h>
+#include <private/qscrollbar_p.h>
 #ifndef QT_NO_ACCESSIBILITY
 #include <qaccessible.h>
 #endif
@@ -2166,7 +2167,7 @@ void QTableView::updateGeometries()
     } else { // ScrollPerPixel
         horizontalScrollBar()->setPageStep(vsize.width());
         horizontalScrollBar()->setRange(0, horizontalLength - vsize.width());
-        horizontalScrollBar()->setSingleStep(qMax(vsize.width() / (columnsInViewport + 1), 2));
+        horizontalScrollBar()->d_func()->itemviewChangeSingleStep(qMax(vsize.width() / (columnsInViewport + 1), 2));
     }
 
     // vertical scroll bar
@@ -2194,7 +2195,7 @@ void QTableView::updateGeometries()
     } else { // ScrollPerPixel
         verticalScrollBar()->setPageStep(vsize.height());
         verticalScrollBar()->setRange(0, verticalLength - vsize.height());
-        verticalScrollBar()->setSingleStep(qMax(vsize.height() / (rowsInViewport + 1), 2));
+        verticalScrollBar()->d_func()->itemviewChangeSingleStep(qMax(vsize.height() / (rowsInViewport + 1), 2));
     }
 
     d->geometryRecursionBlock = false;
diff --git a/src/widgets/itemviews/qtreeview.cpp b/src/widgets/itemviews/qtreeview.cpp
index 57092a7cdc..41f1ca7502 100644
--- a/src/widgets/itemviews/qtreeview.cpp
+++ b/src/widgets/itemviews/qtreeview.cpp
@@ -45,6 +45,7 @@
 #include <qpen.h>
 #include <qdebug.h>
 #include <QMetaMethod>
+#include <private/qscrollbar_p.h>
 #ifndef QT_NO_ACCESSIBILITY
 #include <qaccessible.h>
 #endif
@@ -3698,7 +3699,7 @@ void QTreeViewPrivate::updateScrollBars()
         }
         vbar->setRange(0, contentsHeight - viewportSize.height());
         vbar->setPageStep(viewportSize.height());
-        vbar->setSingleStep(qMax(viewportSize.height() / (itemsInViewport + 1), 2));
+        vbar->d_func()->itemviewChangeSingleStep(qMax(viewportSize.height() / (itemsInViewport + 1), 2));
     }
 
     const int columnCount = header->count();
@@ -3724,7 +3725,7 @@ void QTreeViewPrivate::updateScrollBars()
             viewportSize = maxSize;
         hbar->setPageStep(viewportSize.width());
         hbar->setRange(0, qMax(horizontalLength - viewportSize.width(), 0));
-        hbar->setSingleStep(qMax(viewportSize.width() / (columnsInViewport + 1), 2));
+        hbar->d_func()->itemviewChangeSingleStep(qMax(viewportSize.width() / (columnsInViewport + 1), 2));
     }
 }
 
diff --git a/src/widgets/widgets/qabstractslider.cpp b/src/widgets/widgets/qabstractslider.cpp
index 8d8c3aa4bc..c2d4d3278f 100644
--- a/src/widgets/widgets/qabstractslider.cpp
+++ b/src/widgets/widgets/qabstractslider.cpp
@@ -205,7 +205,7 @@ QT_BEGIN_NAMESPACE
 
 QAbstractSliderPrivate::QAbstractSliderPrivate()
     : minimum(0), maximum(99), pageStep(10), value(0), position(0), pressValue(-1),
-      singleStep(1), offset_accumulated(0), tracking(true),
+      singleStep(1), singleStepFromItemView(-1), viewMayChangeSingleStep(true), offset_accumulated(0), tracking(true),
       blocktracking(false), pressed(false),
       invertedAppearance(false), invertedControls(false),
       orientation(Qt::Horizontal), repeatAction(QAbstractSlider::SliderNoAction)
@@ -378,6 +378,11 @@ int QAbstractSlider::maximum() const
 void QAbstractSlider::setSingleStep(int step)
 {
     Q_D(QAbstractSlider);
+
+    d->viewMayChangeSingleStep = (step < 0);
+    if (step < 0 && d->singleStepFromItemView > 0)
+        step = d->singleStepFromItemView;
+
     if (step != d->singleStep)
         d->setSteps(step, d->pageStep);
 }
@@ -936,4 +941,16 @@ bool QAbstractSlider::event(QEvent *e)
     return QWidget::event(e);
 }
 
+// This function is called from itemviews when doing scroll per pixel (on updateGeometries())
+// It will not have any effect if there has been a call to setSingleStep with
+// a 'reasonable' value (since viewMayChangeSingleStep will be set to false).
+// (If setSingleStep is called with -1 it will however allow the views to change singleStep.)
+
+void QAbstractSliderPrivate::itemviewChangeSingleStep(int step)
+{
+    singleStepFromItemView = step;
+    if (viewMayChangeSingleStep && singleStep != step)
+        setSteps(step, pageStep);
+}
+
 QT_END_NAMESPACE
diff --git a/src/widgets/widgets/qabstractslider_p.h b/src/widgets/widgets/qabstractslider_p.h
index 3df73cf172..32ae55795b 100644
--- a/src/widgets/widgets/qabstractslider_p.h
+++ b/src/widgets/widgets/qabstractslider_p.h
@@ -67,6 +67,8 @@ public:
      * Call effectiveSingleStep() when changing the slider value.
      */
     int singleStep;
+    int singleStepFromItemView; // If we have itemViews we track the views preferred singleStep value.
+    bool viewMayChangeSingleStep;
 
     float offset_accumulated;
     uint tracking : 1;
@@ -108,6 +110,7 @@ public:
 #endif
         ;
     }
+    void itemviewChangeSingleStep(int step);
 
     virtual int bound(int val) const { return qMax(minimum, qMin(maximum, val)); }
     inline int overflowSafeAdd(int add) const
diff --git a/src/widgets/widgets/qscrollbar.h b/src/widgets/widgets/qscrollbar.h
index 4af5fb4a55..d044ec0519 100644
--- a/src/widgets/widgets/qscrollbar.h
+++ b/src/widgets/widgets/qscrollbar.h
@@ -78,6 +78,13 @@ private:
 
     Q_DISABLE_COPY(QScrollBar)
     Q_DECLARE_PRIVATE(QScrollBar)
+#ifndef QT_NO_ITEMVIEWS
+    friend class QTableView;
+    friend class QTreeViewPrivate;
+    friend class QCommonListViewBase;
+    friend class QListModeViewBase;
+    friend class QAbstractItemView;
+#endif
 };
 
 #endif // QT_NO_SCROLLBAR
diff --git a/tests/auto/widgets/itemviews/qlistview/tst_qlistview.cpp b/tests/auto/widgets/itemviews/qlistview/tst_qlistview.cpp
index 13b953c48b..6540c544cc 100644
--- a/tests/auto/widgets/itemviews/qlistview/tst_qlistview.cpp
+++ b/tests/auto/widgets/itemviews/qlistview/tst_qlistview.cpp
@@ -161,6 +161,7 @@ private slots:
     void taskQTBUG_39902_mutualScrollBars_data();
     void taskQTBUG_39902_mutualScrollBars();
     void horizontalScrollingByVerticalWheelEvents();
+    void taskQTBUG_7232_AllowUserToControlSingleStep();
 };
 
 // Testing get/set functions
@@ -2469,5 +2470,43 @@ void tst_QListView::horizontalScrollingByVerticalWheelEvents()
     QVERIFY(lv.verticalScrollBar()->value() > vValue);
 }
 
+void tst_QListView::taskQTBUG_7232_AllowUserToControlSingleStep()
+{
+    // When we set the scrollMode to ScrollPerPixel it will adjust the scrollbars singleStep automatically
+    // Setting a singlestep on a scrollbar should however imply that the user takes control.
+    // Setting a singlestep to -1 return to an automatic control of the singleStep.
+    QListView lv;
+    lv.setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
+    lv.setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
+
+    QStandardItemModel model(1000, 100);
+    QString str = QString::fromLatin1("This is a long string made to ensure that we get some horizontal scroll (and we want scroll)");
+    model.setData(model.index(0, 0), str);
+    lv.setModel(&model);
+    lv.setGeometry(150, 150, 150, 150);
+    lv.show();
+    lv.setVerticalScrollMode(QAbstractItemView::ScrollPerPixel);
+    lv.setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel);
+    QVERIFY(QTest::qWaitForWindowExposed(&lv));
+
+    int vStep1 = lv.verticalScrollBar()->singleStep();
+    int hStep1 = lv.horizontalScrollBar()->singleStep();
+    QVERIFY(lv.verticalScrollBar()->singleStep() > 1);
+    QVERIFY(lv.horizontalScrollBar()->singleStep() > 1);
+
+    lv.verticalScrollBar()->setSingleStep(1);
+    lv.setGeometry(200, 200, 200, 200);
+    QCOMPARE(lv.verticalScrollBar()->singleStep(), 1);
+
+    lv.horizontalScrollBar()->setSingleStep(1);
+    lv.setGeometry(150, 150, 150, 150);
+    QCOMPARE(lv.horizontalScrollBar()->singleStep(), 1);
+
+    lv.verticalScrollBar()->setSingleStep(-1);
+    lv.horizontalScrollBar()->setSingleStep(-1);
+    QCOMPARE(vStep1, lv.verticalScrollBar()->singleStep());
+    QCOMPARE(hStep1, lv.horizontalScrollBar()->singleStep());
+}
+
 QTEST_MAIN(tst_QListView)
 #include "tst_qlistview.moc"
diff --git a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp
index 45dab3f2c1..432c03a2df 100644
--- a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp
+++ b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp
@@ -214,6 +214,8 @@ private slots:
 
     void changeHeaderData();
     void viewOptions();
+
+    void taskQTBUG_7232_AllowUserToControlSingleStep();
 };
 
 // Testing get/set functions
@@ -4483,5 +4485,41 @@ void tst_QTableView::taskQTBUG_30653_doItemsLayout()
     QCOMPARE(scrollToBottomOffset, doItemsLayoutOffset);
 }
 
+void tst_QTableView::taskQTBUG_7232_AllowUserToControlSingleStep()
+{
+    // When we set the scrollMode to ScrollPerPixel it will adjust the scrollbars singleStep automatically
+    // Setting a singlestep on a scrollbar should however imply that the user takes control (and it is not changed by geometry updates).
+    // Setting a singlestep to -1 return to an automatic control of the singleStep.
+    QTableView t;
+    t.setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
+    t.setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
+    QStandardItemModel model(200, 200);
+    t.setModel(&model);
+    t.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&t));
+    t.setVerticalScrollMode(QAbstractItemView::ScrollPerPixel);
+    t.setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel);
+
+    t.setGeometry(200, 200, 200, 200);
+    int vStep1 = t.verticalScrollBar()->singleStep();
+    int hStep1 = t.horizontalScrollBar()->singleStep();
+    QVERIFY(vStep1 > 1);
+    QVERIFY(hStep1 > 1);
+
+    t.verticalScrollBar()->setSingleStep(1);
+    t.setGeometry(300, 300, 300, 300);
+    QCOMPARE(t.verticalScrollBar()->singleStep(), 1);
+
+    t.horizontalScrollBar()->setSingleStep(1);
+    t.setGeometry(400, 400, 400, 400);
+    QCOMPARE(t.horizontalScrollBar()->singleStep(), 1);
+
+    t.setGeometry(200, 200, 200, 200);
+    t.verticalScrollBar()->setSingleStep(-1);
+    t.horizontalScrollBar()->setSingleStep(-1);
+    QCOMPARE(vStep1, t.verticalScrollBar()->singleStep());
+    QCOMPARE(hStep1, t.horizontalScrollBar()->singleStep());
+}
+
 QTEST_MAIN(tst_QTableView)
 #include "tst_qtableview.moc"
diff --git a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
index dec6a66a7f..07413f5040 100644
--- a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
+++ b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
@@ -255,6 +255,7 @@ private slots:
     void taskQTBUG_8176_emitOnExpandAll();
     void taskQTBUG_37813_crash();
     void taskQTBUG_45697_crash();
+    void taskQTBUG_7232_AllowUserToControlSingleStep();
     void testInitialFocus();
 };
 
@@ -4447,5 +4448,47 @@ void tst_QTreeView::taskQTBUG_45697_crash()
     QTRY_VERIFY(testWidget.timerTick() >= 2);
 }
 
+void tst_QTreeView::taskQTBUG_7232_AllowUserToControlSingleStep()
+{
+    // When we set the scrollMode to ScrollPerPixel it will adjust the scrollbars singleStep automatically
+    // Setting a singlestep on a scrollbar should however imply that the user takes control.
+    // Setting a singlestep to -1 return to an automatic control of the singleStep.
+    QTreeWidget t;
+    t.setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
+    t.setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
+    t.setColumnCount(2);
+    QTreeWidgetItem *mainItem = new QTreeWidgetItem(&t, QStringList() << "Root");
+    for (int i = 0; i < 200; ++i) {
+        QTreeWidgetItem *item = new QTreeWidgetItem(mainItem, QStringList(QString("Item")));
+        new QTreeWidgetItem(item, QStringList() << "Child" << "1");
+        new QTreeWidgetItem(item, QStringList() << "Child" << "2");
+        new QTreeWidgetItem(item, QStringList() << "Child" << "3");
+    }
+    t.expandAll();
+
+    t.setVerticalScrollMode(QAbstractItemView::ScrollPerPixel);
+    t.setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel);
+
+    t.setGeometry(200, 200, 200, 200);
+    int vStep1 = t.verticalScrollBar()->singleStep();
+    int hStep1 = t.horizontalScrollBar()->singleStep();
+    QVERIFY(vStep1 > 1);
+    QVERIFY(hStep1 > 1);
+
+    t.verticalScrollBar()->setSingleStep(1);
+    t.setGeometry(300, 300, 300, 300);
+    QCOMPARE(t.verticalScrollBar()->singleStep(), 1);
+
+    t.horizontalScrollBar()->setSingleStep(1);
+    t.setGeometry(400, 400, 400, 400);
+    QCOMPARE(t.horizontalScrollBar()->singleStep(), 1);
+
+    t.setGeometry(200, 200, 200, 200);
+    t.verticalScrollBar()->setSingleStep(-1);
+    t.horizontalScrollBar()->setSingleStep(-1);
+    QCOMPARE(vStep1, t.verticalScrollBar()->singleStep());
+    QCOMPARE(hStep1, t.horizontalScrollBar()->singleStep());
+}
+
 QTEST_MAIN(tst_QTreeView)
 #include "tst_qtreeview.moc"