aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/quickcontrols2/qquicktumblerview.cpp113
-rw-r--r--src/quickcontrols2/qquicktumblerview_p.h2
-rw-r--r--src/quicktemplates2/qquicktumbler.cpp13
-rw-r--r--src/quicktemplates2/qquicktumbler_p_p.h1
-rw-r--r--tests/auto/controls/data/TumblerDatePicker.qml1
-rw-r--r--tests/auto/controls/data/tst_tumbler.qml39
6 files changed, 123 insertions, 46 deletions
diff --git a/src/quickcontrols2/qquicktumblerview.cpp b/src/quickcontrols2/qquicktumblerview.cpp
index 817ec370..5e6d9f5a 100644
--- a/src/quickcontrols2/qquicktumblerview.cpp
+++ b/src/quickcontrols2/qquicktumblerview.cpp
@@ -36,7 +36,6 @@
#include "qquicktumblerview_p.h"
-#include <QtQml/private/qqmldelegatemodel_p.h>
#include <QtQuick/private/qquickitem_p.h>
#include <QtQuick/private/qquicklistview_p.h>
#include <QtQuick/private/qquickpathview_p.h>
@@ -126,7 +125,17 @@ void QQuickTumblerView::createView()
// the count yet, because we rely on the view to tell us the count.
if (m_tumbler->wrap()) {
if (m_listView) {
- delete m_listView;
+ // It's necessary to call deleteLater() rather than delete,
+ // as this code is most likely being run in rensponse to a signal
+ // emission somewhere in the list view's internals, so we need to
+ // wait until that has finished.
+ m_listView->deleteLater();
+ QQml_setParent_noEvent(m_listView, nullptr);
+ // The auto tests pass with unparenting the list view alone, but
+ // just to be sure, we unset some other things as well.
+ m_listView->setParentItem(nullptr);
+ m_listView->setVisible(false);
+ m_listView->setModel(QVariant());
m_listView = nullptr;
}
@@ -143,12 +152,16 @@ void QQuickTumblerView::createView()
// Give the view a size.
updateView();
- // Ensure that the model is set eventually.
- polish();
+ // Set the model.
+ updateModel();
}
} else {
if (m_pathView) {
- delete m_pathView;
+ m_pathView->deleteLater();
+ QQml_setParent_noEvent(m_pathView, nullptr);
+ m_pathView->setParentItem(nullptr);
+ m_pathView->setVisible(false);
+ m_pathView->setModel(QVariant());
m_pathView = nullptr;
}
@@ -164,8 +177,8 @@ void QQuickTumblerView::createView()
// Give the view a size.
updateView();
- // Ensure that the model is set eventually.
- polish();
+ // Set the model.
+ updateModel();
}
}
}
@@ -193,6 +206,56 @@ void QQuickTumblerView::updateView()
}
}
+void QQuickTumblerView::updateModel()
+{
+ if (m_pathView && !m_pathView->model().isValid() && m_model.isValid()) {
+ // QQuickPathView::setPathItemCount() resets the offset animation,
+ // so we just skip the animation while constructing the view.
+ const int oldHighlightMoveDuration = m_pathView->highlightMoveDuration();
+ m_pathView->setHighlightMoveDuration(0);
+
+ // Setting model can change the count, which can affect the wrap, which can cause
+ // the current view to be deleted before setModel() is finished, which causes a crash.
+ // Since QQuickTumbler can't know about QQuickTumblerView, we use its private API to
+ // inform it that it should delay setting wrap.
+ QQuickTumblerPrivate *tumblerPrivate = QQuickTumblerPrivate::get(m_tumbler);
+ tumblerPrivate->lockWrap();
+ m_pathView->setModel(m_model);
+ tumblerPrivate->unlockWrap();
+
+ // The count-depends-on-wrap behavior could cause wrap to change after
+ // the call above, so we must check that we're still using a PathView.
+ if (m_pathView)
+ m_pathView->setHighlightMoveDuration(oldHighlightMoveDuration);
+ } else if (m_listView && !m_listView->model().isValid() && m_model.isValid()) {
+ const int currentIndex = m_tumbler->currentIndex();
+ QQuickTumblerPrivate *tumblerPrivate = QQuickTumblerPrivate::get(m_tumbler);
+
+ // setModel() causes QQuickTumblerPrivate::_q_onViewCountChanged() to
+ // be called called, which calls QQuickTumbler::setCurrentIndex(),
+ // which results in QQuickItemViewPrivate::createHighlightItem() being
+ // called. When the highlight item is created,
+ // QQuickTumblerPrivate::itemChildAdded() is notified and
+ // QQuickTumblerPrivate::_q_updateItemHeights() is called, which causes
+ // a geometry change in the item and createHighlight() is called again.
+ // However, since the highlight item hadn't been assigned yet in the
+ // previous call frame, the "if (highlight) { delete highlight; }"
+ // check doesn't succeed, so the item is never deleted.
+ //
+ // To avoid this, we tell QQuickTumblerPrivate to ignore signals while
+ // setting the model, and manually call _q_onViewCountChanged() to
+ // ensure the correct sequence of calls happens (_q_onViewCountChanged()
+ // has to be within the ignoreSignals scope, because it also generates
+ // recursion otherwise).
+ tumblerPrivate->ignoreSignals = true;
+ m_listView->setModel(m_model);
+ m_listView->setCurrentIndex(currentIndex);
+
+ tumblerPrivate->_q_onViewCountChanged();
+ tumblerPrivate->ignoreSignals = false;
+ }
+}
+
void QQuickTumblerView::geometryChanged(const QRectF &newGeometry, const QRectF &oldGeometry)
{
QQuickItem::geometryChanged(newGeometry, oldGeometry);
@@ -223,42 +286,6 @@ void QQuickTumblerView::itemChange(QQuickItem::ItemChange change, const QQuickIt
}
}
-void QQuickTumblerView::updatePolish()
-{
- // There are certain cases where model count changes can potentially cause problems.
- // An example of this is a ListModel that appends items in a for loop in Component.onCompleted.
- // If we didn't delay assignment of the model, the PathView/ListView would be deleted in
- // response to it emitting countChanged(), causing a crash. To avoid this issue,
- // and to avoid the overhead of count affecting the wrap property, which in turn may
- // unnecessarily create delegates that are never seen, we delay setting the model. This ensures that
- // Component.onCompleted would have been finished, for example.
- if (m_pathView && !m_pathView->model().isValid() && m_model.isValid()) {
- // QQuickPathView::setPathItemCount() resets the offset animation,
- // so we just skip the animation while constructing the view.
- const int oldHighlightMoveDuration = m_pathView->highlightMoveDuration();
- m_pathView->setHighlightMoveDuration(0);
-
- // Setting model can change the count, which can affect the wrap, which can cause
- // the current view to be deleted before setModel() is finished, which causes a crash.
- // Since QQuickTumbler can't know about QQuickTumblerView, we use its private API to
- // inform it that it should delay setting wrap.
- QQuickTumblerPrivate *tumblerPrivate = QQuickTumblerPrivate::get(m_tumbler);
- tumblerPrivate->lockWrap();
- m_pathView->setModel(m_model);
- tumblerPrivate->unlockWrap();
-
- // The count-depends-on-wrap behavior could cause wrap to change after
- // the call above, so we must check that we're still using a PathView.
- if (m_pathView)
- m_pathView->setHighlightMoveDuration(oldHighlightMoveDuration);
- } else if (m_listView && !m_listView->model().isValid() && m_model.isValid()) {
- // Usually we'd do this in QQuickTumbler::setWrap(), but that will be too early for polishes.
- const int currentIndex = m_tumbler->currentIndex();
- m_listView->setModel(m_model);
- m_listView->setCurrentIndex(currentIndex);
- }
-}
-
QQuickItem *QQuickTumblerView::view()
{
if (!m_tumbler)
diff --git a/src/quickcontrols2/qquicktumblerview_p.h b/src/quickcontrols2/qquicktumblerview_p.h
index e83a8bd2..0ab0c3a9 100644
--- a/src/quickcontrols2/qquicktumblerview_p.h
+++ b/src/quickcontrols2/qquicktumblerview_p.h
@@ -87,12 +87,12 @@ protected:
void geometryChanged(const QRectF &newGeometry, const QRectF &oldGeometry) override;
void componentComplete() override;
void itemChange(ItemChange change, const ItemChangeData &data) override;
- void updatePolish() override;
private:
QQuickItem *view();
void createView();
void updateView();
+ void updateModel();
void wrapChange();
diff --git a/src/quicktemplates2/qquicktumbler.cpp b/src/quicktemplates2/qquicktumbler.cpp
index 5cdd140d..80ab71ea 100644
--- a/src/quicktemplates2/qquicktumbler.cpp
+++ b/src/quicktemplates2/qquicktumbler.cpp
@@ -97,7 +97,8 @@ QQuickTumblerPrivate::QQuickTumblerPrivate()
currentIndex(-1),
pendingCurrentIndex(-1),
ignoreCurrentIndexChanges(false),
- count(0)
+ count(0),
+ ignoreSignals(false)
{
}
@@ -163,6 +164,9 @@ QQuickTumblerPrivate *QQuickTumblerPrivate::get(QQuickTumbler *tumbler)
void QQuickTumblerPrivate::_q_updateItemHeights()
{
+ if (ignoreSignals)
+ return;
+
// Can't use our own private padding members here, as the padding property might be set,
// which doesn't affect them, only their getters.
Q_Q(const QQuickTumbler);
@@ -174,6 +178,9 @@ void QQuickTumblerPrivate::_q_updateItemHeights()
void QQuickTumblerPrivate::_q_updateItemWidths()
{
+ if (ignoreSignals)
+ return;
+
Q_Q(const QQuickTumbler);
const qreal availableWidth = q->availableWidth();
const auto items = viewContentItemChildItems();
@@ -195,6 +202,8 @@ void QQuickTumblerPrivate::_q_onViewCurrentIndexChanged()
void QQuickTumblerPrivate::_q_onViewCountChanged()
{
Q_Q(QQuickTumbler);
+ if (ignoreSignals)
+ return;
setCount(view->property("count").toInt());
@@ -336,7 +345,9 @@ void QQuickTumbler::setCurrentIndex(int currentIndex)
couldSet = true;
} else {
d->ignoreCurrentIndexChanges = true;
+ d->ignoreSignals = true;
d->view->setProperty("currentIndex", currentIndex);
+ d->ignoreSignals = false;
d->ignoreCurrentIndexChanges = false;
couldSet = d->view->property("currentIndex").toInt() == currentIndex;
diff --git a/src/quicktemplates2/qquicktumbler_p_p.h b/src/quicktemplates2/qquicktumbler_p_p.h
index 301b8402..0dcae762 100644
--- a/src/quicktemplates2/qquicktumbler_p_p.h
+++ b/src/quicktemplates2/qquicktumbler_p_p.h
@@ -88,6 +88,7 @@ public:
int pendingCurrentIndex;
bool ignoreCurrentIndexChanges;
int count;
+ bool ignoreSignals;
void _q_updateItemHeights();
void _q_updateItemWidths();
diff --git a/tests/auto/controls/data/TumblerDatePicker.qml b/tests/auto/controls/data/TumblerDatePicker.qml
index ac04284a..72e57bed 100644
--- a/tests/auto/controls/data/TumblerDatePicker.qml
+++ b/tests/auto/controls/data/TumblerDatePicker.qml
@@ -87,6 +87,7 @@ Row {
id: yearTumbler
objectName: "yearTumbler"
model: ListModel {
+ objectName: "yearTumblerListModel"
Component.onCompleted: {
for (var i = 2000; i < 2100; ++i) {
append({value: i.toString()});
diff --git a/tests/auto/controls/data/tst_tumbler.qml b/tests/auto/controls/data/tst_tumbler.qml
index 00d13e23..aaf888c2 100644
--- a/tests/auto/controls/data/tst_tumbler.qml
+++ b/tests/auto/controls/data/tst_tumbler.qml
@@ -389,7 +389,7 @@ TestCase {
compare(tumbler.monthTumbler.currentIndex, 0);
compare(tumbler.monthTumbler.count, 12);
compare(tumbler.yearTumbler.currentIndex, 0);
- compare(tumbler.yearTumbler.count, 100);
+ tryCompare(tumbler.yearTumbler, "count", 100);
verify(findView(tumbler.dayTumbler).children.length >= tumbler.dayTumbler.visibleItemCount);
verify(findView(tumbler.monthTumbler).children.length >= tumbler.monthTumbler.visibleItemCount);
@@ -1057,4 +1057,41 @@ TestCase {
compare(tumbler.moving, true)
tryCompare(tumbler, "moving", false)
}
+
+ Component {
+ id: qtbug61374Component
+
+ Row {
+ property alias tumbler: tumbler
+ property alias label: label
+
+ Component.onCompleted: {
+ tumbler.currentIndex = 2
+ }
+
+ Tumbler {
+ id: tumbler
+ model: 5
+ // ...
+ }
+
+ Label {
+ id: label
+ text: tumbler.currentItem.text
+ }
+ }
+ }
+
+ function test_qtbug61374() {
+ var row = createTemporaryObject(qtbug61374Component, testCase);
+ verify(row);
+
+ var tumbler = row.tumbler;
+ tryCompare(tumbler, "currentIndex", 2);
+
+ tumblerView = findView(tumbler);
+
+ var label = row.label;
+ compare(label.text, "2");
+ }
}
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-22 11:08:04 +0000