Merge lp:~mzanetti/kubuntu-packaging/qtdeclarative-opensource-src into lp:~kubuntu-packagers/kubuntu-packaging/qtdeclarative-opensource-src

Proposed by Robert Bruce Park
Status: Merged
Merged at revision: 159
Proposed branch: lp:~mzanetti/kubuntu-packaging/qtdeclarative-opensource-src
Merge into: lp:~kubuntu-packagers/kubuntu-packaging/qtdeclarative-opensource-src
Diff against target: 208 lines (+169/-1)
4 files modified
debian/changelog (+8/-0)
debian/libqt5qml5.symbols (+2/-1)
debian/patches/Implement-proper-support-for-layoutChange-in-QQmlDel.patch (+158/-0)
debian/patches/series (+1/-0)
To merge this branch: bzr merge lp:~mzanetti/kubuntu-packaging/qtdeclarative-opensource-src
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Kubuntu Packagers Pending
Review via email: mp+224517@code.launchpad.net

Commit message

Fix a performance hit when editing sorted lists (LP: #1303746)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
159. By Michael Zanetti

* debian/patches/Implement-proper-support-for-layoutChange-in-QQmlDel.patch:
  - Fix a performance hit when editing sorted lists (LP: #1303746)
  - update symbols

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2014-06-19 19:14:47 +0000
+++ debian/changelog 2014-06-26 09:47:31 +0000
@@ -1,3 +1,11 @@
1qtdeclarative-opensource-src (5.3.0-3ubuntu5) utopic; urgency=medium
2
3 * debian/patches/Implement-proper-support-for-layoutChange-in-QQmlDel.patch:
4 - Fix a performance hit when editing sorted lists (LP: #1303746)
5 - update symbols
6
7 -- Michael Zanetti <michael.zanetti@canonical.com> Wed, 25 Jun 2014 19:27:13 +0200
8
1qtdeclarative-opensource-src (5.3.0-3ubuntu4) utopic; urgency=medium9qtdeclarative-opensource-src (5.3.0-3ubuntu4) utopic; urgency=medium
210
3 [ Ricardo Salveti de Araujo ]11 [ Ricardo Salveti de Araujo ]
412
=== modified file 'debian/libqt5qml5.symbols'
--- debian/libqt5qml5.symbols 2014-05-21 15:17:21 +0000
+++ debian/libqt5qml5.symbols 2014-06-26 09:47:31 +0000
@@ -580,7 +580,7 @@
580 _ZN17QQmlDelegateModel15_q_rowsInsertedERK11QModelIndexii@Base 5.1.0 1580 _ZN17QQmlDelegateModel15_q_rowsInsertedERK11QModelIndexii@Base 5.1.0 1
581 _ZN17QQmlDelegateModel15setWatchedRolesE5QListI10QByteArrayE@Base 5.1.0 1581 _ZN17QQmlDelegateModel15setWatchedRolesE5QListI10QByteArrayE@Base 5.1.0 1
582 _ZN17QQmlDelegateModel16_q_itemsInsertedEii@Base 5.1.0 1582 _ZN17QQmlDelegateModel16_q_itemsInsertedEii@Base 5.1.0 1
583 _ZN17QQmlDelegateModel16_q_layoutChangedEv@Base 5.3.0 1583 _ZN17QQmlDelegateModel16_q_layoutChangedERK5QListI21QPersistentModelIndexEN18QAbstractItemModel16LayoutChangeHintE@Base 5.3.0 1
584 _ZN17QQmlDelegateModel16resetFilterGroupEv@Base 5.1.0 1584 _ZN17QQmlDelegateModel16resetFilterGroupEv@Base 5.1.0 1
585 _ZN17QQmlDelegateModel16rootIndexChangedEv@Base 5.1.0 1585 _ZN17QQmlDelegateModel16rootIndexChangedEv@Base 5.1.0 1
586 _ZN17QQmlDelegateModel16staticMetaObjectE@Base 5.1.0 1586 _ZN17QQmlDelegateModel16staticMetaObjectE@Base 5.1.0 1
@@ -589,6 +589,7 @@
589 _ZN17QQmlDelegateModel20defaultGroupsChangedEv@Base 5.1.0 1589 _ZN17QQmlDelegateModel20defaultGroupsChangedEv@Base 5.1.0 1
590 _ZN17QQmlDelegateModel21qmlAttachedPropertiesEP7QObject@Base 5.1.0 1590 _ZN17QQmlDelegateModel21qmlAttachedPropertiesEP7QObject@Base 5.1.0 1
591 _ZN17QQmlDelegateModel23_q_rowsAboutToBeRemovedERK11QModelIndexii@Base 5.1.0 1591 _ZN17QQmlDelegateModel23_q_rowsAboutToBeRemovedERK11QModelIndexii@Base 5.1.0 1
592 _ZN17QQmlDelegateModel25_q_layoutAboutToBeChangedERK5QListI21QPersistentModelIndexEN18QAbstractItemModel16LayoutChangeHintE@Base 5.3.0 1
592 _ZN17QQmlDelegateModel5eventEP6QEvent@Base 5.1.0 1593 _ZN17QQmlDelegateModel5eventEP6QEvent@Base 5.1.0 1
593 _ZN17QQmlDelegateModel5itemsEv@Base 5.1.0 1594 _ZN17QQmlDelegateModel5itemsEv@Base 5.1.0 1
594 _ZN17QQmlDelegateModel5partsEv@Base 5.1.0 1595 _ZN17QQmlDelegateModel5partsEv@Base 5.1.0 1
595596
=== added file 'debian/patches/Implement-proper-support-for-layoutChange-in-QQmlDel.patch'
--- debian/patches/Implement-proper-support-for-layoutChange-in-QQmlDel.patch 1970-01-01 00:00:00 +0000
+++ debian/patches/Implement-proper-support-for-layoutChange-in-QQmlDel.patch 2014-06-26 09:47:31 +0000
@@ -0,0 +1,158 @@
1From 4d4bd2ac531f0456135a1077f5fceea3517cd3cd Mon Sep 17 00:00:00 2001
2From: =?UTF-8?q?Daniel=20Vr=C3=A1til?= <dan@progdan.cz>
3Date: Wed, 16 Apr 2014 18:33:24 +0200
4Subject: [PATCH] Implement proper support for layoutChange in
5 QQmlDelegateModel
6
7Current implementation is treating model layoutChange the same way as modelReset,
8which causes problems when using ListView on top of a QSortFilterProxyModel. The
9model emits layoutChanged whenever a data within sorting column change. Treating
10it as modelReset leads to poor performance on large models and caused UI issues,
11because the scrolling position is reset every time.
12
13This patch implements proper handling for layoutChanged signals by first handling
14all items moves and then simulating dataChange for all items.
15
16This fixes regression from Qt 5.1 introduced by Change I16b859d9
17
18Task-number: QTBUG-37983
19Task-number: QTBUG-34391
20Change-Id: I6d3873b7b87e7f0e8fc0c1ed5dc80c6f8fdf6c22
21---
22 src/qml/types/qqmldelegatemodel.cpp | 59 +++++++++++++++++++++++++++++++++--
23 src/qml/types/qqmldelegatemodel_p.h | 3 +-
24 src/qml/types/qqmldelegatemodel_p_p.h | 2 ++
25 src/qml/util/qqmladaptormodel.cpp | 12 ++++---
26 4 files changed, 69 insertions(+), 7 deletions(-)
27
28diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp
29index be479ee..4591d42 100644
30--- a/src/qml/types/qqmldelegatemodel.cpp
31+++ b/src/qml/types/qqmldelegatemodel.cpp
32@@ -1521,9 +1521,64 @@ void QQmlDelegateModel::_q_dataChanged(const QModelIndex &begin, const QModelInd
33 _q_itemsChanged(begin.row(), end.row() - begin.row() + 1, roles);
34 }
35
36-void QQmlDelegateModel::_q_layoutChanged()
37+void QQmlDelegateModel::_q_layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint)
38 {
39- _q_modelReset();
40+ Q_D(QQmlDelegateModel);
41+ if (!d->m_complete)
42+ return;
43+
44+ if (hint == QAbstractItemModel::VerticalSortHint) {
45+ d->m_storedPersistentIndexes.clear();
46+ if (!parents.contains(d->m_adaptorModel.rootIndex))
47+ return;
48+
49+ for (int i = 0; i < d->m_count; ++i) {
50+ const QModelIndex index = d->m_adaptorModel.aim()->index(i, 0, d->m_adaptorModel.rootIndex);
51+ d->m_storedPersistentIndexes.append(index);
52+ }
53+ } else if (hint == QAbstractItemModel::HorizontalSortHint) {
54+ // Ignored
55+ } else {
56+ // Triggers model reset, no preparations for that are needed
57+ }
58+}
59+
60+void QQmlDelegateModel::_q_layoutChanged(const QList<QPersistentModelIndex> &parents, QAbstractItemModel::LayoutChangeHint hint)
61+{
62+ Q_D(QQmlDelegateModel);
63+ if (!d->m_complete)
64+ return;
65+
66+ if (hint == QAbstractItemModel::VerticalSortHint) {
67+ if (!parents.contains(d->m_adaptorModel.rootIndex))
68+ return;
69+
70+ for (int i = 0, c = d->m_storedPersistentIndexes.count(); i < c; ++i) {
71+ const QPersistentModelIndex &index = d->m_storedPersistentIndexes.at(i);
72+ if (i == index.row())
73+ continue;
74+
75+ QVector<Compositor::Insert> inserts;
76+ QVector<Compositor::Remove> removes;
77+ d->m_compositor.listItemsMoved(&d->m_adaptorModel, i, index.row(), 1, &removes, &inserts);
78+ if (!removes.isEmpty() || !inserts.isEmpty()) {
79+ d->itemsMoved(removes, inserts);
80+ }
81+ }
82+
83+ d->m_storedPersistentIndexes.clear();
84+
85+ // layoutUpdate does not necessarily have any move changes, but it can
86+ // also mean data changes. We can't detect what exactly has changed, so
87+ // just emit it for all items
88+ _q_itemsChanged(0, d->m_count, QVector<int>());
89+
90+ } else if (hint == QAbstractItemModel::HorizontalSortHint) {
91+ // Ignored
92+ } else {
93+ // We don't know what's going on, so reset the model
94+ _q_modelReset();
95+ }
96 }
97
98 QQmlDelegateModelAttached *QQmlDelegateModel::qmlAttachedProperties(QObject *obj)
99diff --git a/src/qml/types/qqmldelegatemodel_p.h b/src/qml/types/qqmldelegatemodel_p.h
100index 51f846e..0b67179 100644
101--- a/src/qml/types/qqmldelegatemodel_p.h
102+++ b/src/qml/types/qqmldelegatemodel_p.h
103@@ -139,7 +139,8 @@ private Q_SLOTS:
104 void _q_rowsRemoved(const QModelIndex &,int,int);
105 void _q_rowsMoved(const QModelIndex &, int, int, const QModelIndex &, int);
106 void _q_dataChanged(const QModelIndex&,const QModelIndex&,const QVector<int> &);
107- void _q_layoutChanged();
108+ void _q_layoutAboutToBeChanged(const QList<QPersistentModelIndex>&, QAbstractItemModel::LayoutChangeHint);
109+ void _q_layoutChanged(const QList<QPersistentModelIndex>&, QAbstractItemModel::LayoutChangeHint);
110
111 private:
112 Q_DISABLE_COPY(QQmlDelegateModel)
113diff --git a/src/qml/types/qqmldelegatemodel_p_p.h b/src/qml/types/qqmldelegatemodel_p_p.h
114index 32b1154..67deb4f 100644
115--- a/src/qml/types/qqmldelegatemodel_p_p.h
116+++ b/src/qml/types/qqmldelegatemodel_p_p.h
117@@ -331,6 +331,8 @@ public:
118 };
119 QQmlDelegateModelGroup *m_groups[Compositor::MaximumGroupCount];
120 };
121+
122+ QList<QPersistentModelIndex> m_storedPersistentIndexes;
123 };
124
125 class QQmlPartsModel : public QQmlInstanceModel, public QQmlDelegateModelGroupEmitter
126diff --git a/src/qml/util/qqmladaptormodel.cpp b/src/qml/util/qqmladaptormodel.cpp
127index d38e5ac..c1c8bfa 100644
128--- a/src/qml/util/qqmladaptormodel.cpp
129+++ b/src/qml/util/qqmladaptormodel.cpp
130@@ -467,8 +467,10 @@ public:
131 vdm, SLOT(_q_rowsMoved(QModelIndex,int,int,QModelIndex,int)));
132 QObject::disconnect(aim, SIGNAL(modelReset()),
133 vdm, SLOT(_q_modelReset()));
134- QObject::disconnect(aim, SIGNAL(layoutChanged()),
135- vdm, SLOT(_q_layoutChanged()));
136+ QObject::disconnect(aim, SIGNAL(layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
137+ vdm, SLOT(_q_layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
138+ QObject::disconnect(aim, SIGNAL(layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
139+ vdm, SLOT(_q_layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
140 }
141
142 const_cast<VDMAbstractItemModelDataType *>(this)->release();
143@@ -920,8 +922,10 @@ void QQmlAdaptorModel::setModel(const QVariant &variant, QQmlDelegateModel *vdm,
144 vdm, QQmlDelegateModel, SLOT(_q_rowsMoved(QModelIndex,int,int,QModelIndex,int)));
145 qmlobject_connect(model, QAbstractItemModel, SIGNAL(modelReset()),
146 vdm, QQmlDelegateModel, SLOT(_q_modelReset()));
147- qmlobject_connect(model, QAbstractItemModel, SIGNAL(layoutChanged()),
148- vdm, QQmlDelegateModel, SLOT(_q_layoutChanged()));
149+ qmlobject_connect(model, QAbstractItemModel, SIGNAL(layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
150+ vdm, QQmlDelegateModel, SLOT(_q_layoutAboutToBeChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
151+ qmlobject_connect(model, QAbstractItemModel, SIGNAL(layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)),
152+ vdm, QQmlDelegateModel, SLOT(_q_layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)));
153 } else {
154 accessors = new VDMObjectDelegateDataType;
155 }
156--
1571.9.1
158
0159
=== modified file 'debian/patches/series'
--- debian/patches/series 2014-06-18 08:15:27 +0000
+++ debian/patches/series 2014-06-26 09:47:31 +0000
@@ -5,3 +5,4 @@
5Make-ItemViews-displayMargin-work-correctly-when-set.patch5Make-ItemViews-displayMargin-work-correctly-when-set.patch
6v4_yarr_jit_push_pop_addressTempRegister.patch6v4_yarr_jit_push_pop_addressTempRegister.patch
7fix_qqmlobjectcreator.patch7fix_qqmlobjectcreator.patch
8Implement-proper-support-for-layoutChange-in-QQmlDel.patch

Subscribers

People subscribed via source and target branches