From e44843bfc8fc8bfd5c987fde08816ec3d81f0cae Mon Sep 17 00:00:00 2001 From: Michael Brasser Date: Tue, 7 Jul 2015 16:51:55 -0500 Subject: Fix potential use of incorrect bounds in delegate tracking. Using GridView.SnapToRow and GridView.ApplyRange with a top margin could lead to the view jumping (rather than smoothly transitioning) when changing the currentIndex. Change-Id: I6936b378220f59e8d416f7531cf8b6906c723cb2 Task-number: QTBUG-45640 Reviewed-by: Martin Jones --- src/quick/items/qquickitemview.cpp | 45 +++++++++++++++------- src/quick/items/qquickitemview_p_p.h | 2 + .../auto/quick/qquickgridview/data/qtbug45640.qml | 24 ++++++++++++ .../quick/qquickgridview/tst_qquickgridview.cpp | 21 ++++++++++ 4 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 tests/auto/quick/qquickgridview/data/qtbug45640.qml diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 7a55bd6e0a..d4c8c3f8ee 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -910,11 +910,7 @@ void QQuickItemViewPrivate::positionViewAtIndex(int index, int mode) qreal pos = isContentFlowReversed() ? -position() - size() : position(); FxViewItem *item = visibleItem(idx); - qreal maxExtent; - if (layoutOrientation() == Qt::Vertical) - maxExtent = isContentFlowReversed() ? q->minYExtent()-size(): -q->maxYExtent(); - else - maxExtent = isContentFlowReversed() ? q->minXExtent()-size(): -q->maxXExtent(); + qreal maxExtent = calculatedMaxExtent(); if (!item) { qreal itemPos = positionAt(idx); changedVisibleIndex(idx); @@ -960,11 +956,7 @@ void QQuickItemViewPrivate::positionViewAtIndex(int index, int mode) break; } pos = qMin(pos, maxExtent); - qreal minExtent; - if (layoutOrientation() == Qt::Vertical) - minExtent = isContentFlowReversed() ? q->maxYExtent()-size(): -q->minYExtent(); - else - minExtent = isContentFlowReversed() ? q->maxXExtent()-size(): -q->minXExtent(); + qreal minExtent = calculatedMinExtent(); pos = qMax(pos, minExtent); moveReason = QQuickItemViewPrivate::Other; q->cancelFlick(); @@ -1135,6 +1127,29 @@ qreal QQuickItemViewPrivate::maxExtentForAxis(const AxisData &axisData, bool for return extent; } +qreal QQuickItemViewPrivate::calculatedMinExtent() const +{ + Q_Q(const QQuickItemView); + qreal minExtent; + if (layoutOrientation() == Qt::Vertical) + minExtent = isContentFlowReversed() ? q->maxYExtent() - size(): -q->minYExtent(); + else + minExtent = isContentFlowReversed() ? q->maxXExtent() - size(): -q->minXExtent(); + return minExtent; + +} + +qreal QQuickItemViewPrivate::calculatedMaxExtent() const +{ + Q_Q(const QQuickItemView); + qreal maxExtent; + if (layoutOrientation() == Qt::Vertical) + maxExtent = isContentFlowReversed() ? q->minYExtent() - size(): -q->maxYExtent(); + else + maxExtent = isContentFlowReversed() ? q->minXExtent() - size(): -q->maxXExtent(); + return maxExtent; +} + // for debugging only void QQuickItemViewPrivate::checkVisible() const { @@ -1282,10 +1297,12 @@ void QQuickItemView::trackedPositionChanged() if (trackedPos < pos + d->highlightRangeStart) pos = trackedPos - d->highlightRangeStart; if (d->highlightRange != StrictlyEnforceRange) { - if (pos > d->endPosition() - d->size()) - pos = d->endPosition() - d->size(); - if (pos < d->startPosition()) - pos = d->startPosition(); + qreal maxExtent = d->calculatedMaxExtent(); + if (pos > maxExtent) + pos = maxExtent; + qreal minExtent = d->calculatedMinExtent(); + if (pos < minExtent) + pos = minExtent; } } else { if (d->trackedItem != d->currentItem) { diff --git a/src/quick/items/qquickitemview_p_p.h b/src/quick/items/qquickitemview_p_p.h index 67ef413d31..f535963fdb 100644 --- a/src/quick/items/qquickitemview_p_p.h +++ b/src/quick/items/qquickitemview_p_p.h @@ -200,6 +200,8 @@ public: qreal minExtentForAxis(const AxisData &axisData, bool forXAxis) const; qreal maxExtentForAxis(const AxisData &axisData, bool forXAxis) const; + qreal calculatedMinExtent() const; + qreal calculatedMaxExtent() const; void applyPendingChanges(); bool applyModelChanges(ChangeResult *insertionResult, ChangeResult *removalResult); diff --git a/tests/auto/quick/qquickgridview/data/qtbug45640.qml b/tests/auto/quick/qquickgridview/data/qtbug45640.qml new file mode 100644 index 0000000000..6973773432 --- /dev/null +++ b/tests/auto/quick/qquickgridview/data/qtbug45640.qml @@ -0,0 +1,24 @@ +import QtQuick 2.0 + +GridView { + id: gridView + width: 400; height: 400 + cellHeight: 100 + cellWidth: 100 + model: 32 + + topMargin: 50 + snapMode: GridView.SnapToRow + preferredHighlightBegin: 50 + preferredHighlightEnd: 50 + highlightRangeMode: GridView.ApplyRange + + delegate: Rectangle { + width: 100 + height: 100 + color: index % 2 == 0 ? "#FFFF00" : "#0000FF" + } + + highlight: Rectangle { color: "red"; z: 2 } + highlightMoveDuration: 1000 +} diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp index d53ee00b45..b8ad1948ed 100644 --- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp +++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp @@ -209,6 +209,8 @@ private slots: void contentHeightWithDelayRemove_data(); void contentHeightWithDelayRemove(); + void QTBUG_45640(); + private: QList toIntList(const QVariantList &list); void matchIndexLists(const QVariantList &indexLists, const QList &expectedIndexes); @@ -6542,6 +6544,25 @@ void tst_QQuickGridView::contentHeightWithDelayRemove() delete window; } +void tst_QQuickGridView::QTBUG_45640() +{ + QQuickView *window = createView(); + window->setSource(testFileUrl("qtbug45640.qml")); + window->show(); + QVERIFY(QTest::qWaitForWindowExposed(window)); + + QQuickGridView *gridview = qobject_cast(window->rootObject()); + QVERIFY(gridview != 0); + + QCOMPARE(gridview->contentY(), qreal(-50.0)); + + gridview->moveCurrentIndexDown(); + + QTRY_VERIFY(gridview->contentY() > qreal(-50.0) && gridview->contentY() < qreal(0.0)); + + delete window; +} + QTEST_MAIN(tst_QQuickGridView) #include "tst_qquickgridview.moc" -- cgit v1.2.3