117 lines
5.5 KiB
Diff
117 lines
5.5 KiB
Diff
From 1b0e366092bcfae0392592c3b7891f0e47af1018 Mon Sep 17 00:00:00 2001
|
|
From: Oliver Eftevaag <oliver.eftevaag@qt.io>
|
|
Date: Fri, 9 Dec 2022 18:40:54 +0100
|
|
Subject: [PATCH 30/30] Flickable: prevent fixup() from being called while
|
|
dragging
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
A previous patch 5647527a8cde84b51fff66fc482f02435770b3dd causes
|
|
a regression. The purpose of the patch, that caused this regression,
|
|
was to update the pressPos variables, in cases where the contentItem's
|
|
geometry was modified externally, while a user were dragging the
|
|
contentItem around.
|
|
|
|
The mistake that was made, was how width and height changes were
|
|
handled. We had previously added logic in setContentWidth() and
|
|
setContentHeight() that would call fixup() (with immediate fixupMode)
|
|
to ensure that the contentItem would immediately be repositioned
|
|
inside the flickable's viewport, if the contentItem was being dragged.
|
|
|
|
It turns out that setContentWidth() and setContentHeight() are being
|
|
called from QQuickItemViewPrivate::updateViewport(), which happens
|
|
quite often, while dragging. This would make fixup() and dragging
|
|
constantly interfere with each other, since they'd not always agree on
|
|
a specific position for the contentItem.
|
|
|
|
This patch reverts the changes made to setContentWidth() and
|
|
setContentHeight(), since it turns out that those changes weren't
|
|
necessary after all. QQuickFlickablePrivate::itemGeometryChanged() only
|
|
calls viewportMoved() on x and y changes anyways.
|
|
|
|
Done-with: Jan Arve Sæther <jan-arve.saether@qt.io>
|
|
Done-with: Santhosh Kumar Selvaraj <santhosh.kumar.selvaraj@qt.io>
|
|
Fixes: QTBUG-109140
|
|
Pick-to: 5.15 6.2 6.3 6.4 6.5
|
|
Change-Id: I0bddf8685d3afc1ae04b2c092212d3c1bd742c3b
|
|
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
|
|
(cherry picked from commit b307bf3c4f63c6e04874a972c747f18e18ddc199)
|
|
---
|
|
src/quick/items/qquickflickable.cpp | 8 ++------
|
|
src/quick/items/qquickflickable_p_p.h | 1 +
|
|
tests/auto/quick/qquickflickable/tst_qquickflickable.cpp | 8 +++++++-
|
|
3 files changed, 10 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/src/quick/items/qquickflickable.cpp b/src/quick/items/qquickflickable.cpp
|
|
index ea357d819d..2634b68248 100644
|
|
--- a/src/quick/items/qquickflickable.cpp
|
|
+++ b/src/quick/items/qquickflickable.cpp
|
|
@@ -2120,11 +2120,9 @@ void QQuickFlickable::setContentWidth(qreal w)
|
|
d->contentItem->setWidth(w);
|
|
d->hData.markExtentsDirty();
|
|
// Make sure that we're entirely in view.
|
|
- if ((!d->pressed && !d->hData.moving && !d->vData.moving) || d->hData.dragging) {
|
|
- d->hData.contentPositionChangedExternallyDuringDrag = d->hData.dragging;
|
|
+ if (!d->pressed && !d->hData.moving && !d->vData.moving) {
|
|
d->fixupMode = QQuickFlickablePrivate::Immediate;
|
|
d->fixupX();
|
|
- d->hData.contentPositionChangedExternallyDuringDrag = false;
|
|
} else if (!d->pressed && d->hData.fixingUp) {
|
|
d->fixupMode = QQuickFlickablePrivate::ExtentChanged;
|
|
d->fixupX();
|
|
@@ -2151,11 +2149,9 @@ void QQuickFlickable::setContentHeight(qreal h)
|
|
d->contentItem->setHeight(h);
|
|
d->vData.markExtentsDirty();
|
|
// Make sure that we're entirely in view.
|
|
- if ((!d->pressed && !d->hData.moving && !d->vData.moving) || d->vData.dragging) {
|
|
- d->vData.contentPositionChangedExternallyDuringDrag = d->vData.dragging;
|
|
+ if (!d->pressed && !d->hData.moving && !d->vData.moving) {
|
|
d->fixupMode = QQuickFlickablePrivate::Immediate;
|
|
d->fixupY();
|
|
- d->vData.contentPositionChangedExternallyDuringDrag = false;
|
|
} else if (!d->pressed && d->vData.fixingUp) {
|
|
d->fixupMode = QQuickFlickablePrivate::ExtentChanged;
|
|
d->fixupY();
|
|
diff --git a/src/quick/items/qquickflickable_p_p.h b/src/quick/items/qquickflickable_p_p.h
|
|
index d5d838eaea..aef15e150a 100644
|
|
--- a/src/quick/items/qquickflickable_p_p.h
|
|
+++ b/src/quick/items/qquickflickable_p_p.h
|
|
@@ -120,6 +120,7 @@ public:
|
|
dragStartOffset = 0;
|
|
fixingUp = false;
|
|
inOvershoot = false;
|
|
+ contentPositionChangedExternallyDuringDrag = false;
|
|
}
|
|
|
|
void markExtentsDirty() {
|
|
diff --git a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
|
|
index d092cd0170..62f7c67dd4 100644
|
|
--- a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
|
|
+++ b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
|
|
@@ -2642,7 +2642,12 @@ void tst_qquickflickable::setContentPositionWhileDragging() // QTBUG-104966
|
|
} else if (newExtent >= 0) {
|
|
// ...or reduce the content size be be less than current (contentX, contentY) position
|
|
// This forces the content item to move.
|
|
- expectedContentPos = moveDelta;
|
|
+ // contentY: 150
|
|
+ // 320 - 150 = 170 pixels down to bottom
|
|
+ // Now reduce contentHeight to 200
|
|
+ // since we are at the bottom, and the flickable is 100 pixels tall, contentY must land
|
|
+ // at newExtent - 100.
|
|
+
|
|
if (isHorizontal) {
|
|
flickable->setContentWidth(newExtent);
|
|
} else {
|
|
@@ -2652,6 +2657,7 @@ void tst_qquickflickable::setContentPositionWhileDragging() // QTBUG-104966
|
|
// We therefore cannot scroll/flick it further down. Drag it up towards the top instead
|
|
// (by moving mouse down).
|
|
pos += moveDelta;
|
|
+ expectedContentPos = unitDelta * (newExtent - (isHorizontal ? flickable->width() : flickable->height()));
|
|
}
|
|
|
|
QTest::mouseMove(window.data(), pos);
|
|
--
|
|
2.41.0
|
|
|