Unity 8

Merge lp:~aacid/unity8/optimize_LVWPH_layout into lp:unity8

optimize_LVWPH_layout
Merge into trunk

Proposed by Albert Astals Cid on 2016-03-24

Status:	Merged
Approved by:	Josh Arenson on 2016-03-30
Approved revision:	2281
Merged at revision:	2416
Proposed branch:	lp:~aacid/unity8/optimize_LVWPH_layout
Merge into:	lp:unity8
Prerequisite:	lp:~aacid/unity8/sectionDelegateResizes
Diff against target:	246 lines (+31/-38) 8 files modified plugins/Dash/listviewwithpageheader.cpp (+13/-16) qml/Dash/DashCategoryBase.qml (+3/-11) qml/Dash/GenericScopeView.qml (+2/-2) tests/plugins/Dash/listviewwithpageheadersectionexternalmodeltest.cpp (+2/-2) tests/plugins/Dash/listviewwithpageheadersectiontest.cpp (+2/-2) tests/plugins/Dash/listviewwithpageheadertestsection.qml (+3/-2) tests/plugins/Dash/listviewwithpageheadertestsectionexternalmodel.qml (+2/-1) tests/plugins/Dash/tst_ListViewWithPageHeaderQML.qml (+4/-2)
To merge this branch:	bzr merge lp:~aacid/unity8/optimize_LVWPH_layout
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot	continuous-integration	2016-03-24	Needs Fixing on 2016-04-07
Andrea Cimitan		2016-03-24	Pending
Josh Arenson		2016-03-24	Pending
Review via email: mp+290021@code.launchpad.net

This proposal supersedes a proposal from 2016-03-17.

Commit Message

Don't use context properties but properties of the delegates

This increases a bit the requirements for delegates of the LVWPH but since we control them it's not really a problem.

In ::layout the setContextProperty calls accounted for around 45% according to callgrind, now it's around 10% only.

Description of the Change

* Are there any related MPs required for this MP to build/function as expected?
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-17: Posted in a previous version of this proposal

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/764/rebuild

review: Needs Fixing (continuous-integration)

Josh Arenson (josharenson) wrote on 2016-03-18: Posted in a previous version of this proposal

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

* Did CI run pass?
No, the usual

* Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-23: Posted in a previous version of this proposal

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/816/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-24: Posted in a previous version of this proposal

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/822/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-24:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/862/rebuild

review: Needs Fixing (continuous-integration)

lp:~aacid/unity8/optimize_LVWPH_layout updated on 2016-04-07

2282. By Albert Astals Cid on 2016-04-07: Merge

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-07:

FAILED: Continuous integration, rev:2281
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/945/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/945/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-07:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/948/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Albert Astals Cid

Mohammad Rakibul Hasan

Unity Team

1	=== modified file 'plugins/Dash/listviewwithpageheader.cpp'
2	--- plugins/Dash/listviewwithpageheader.cpp 2016-04-07 07:42:25 +0000
3	+++ plugins/Dash/listviewwithpageheader.cpp 2016-04-07 07:42:25 +0000
4	@@ -97,6 +97,7 @@
5	#include <qqmlengine.h>
6	#pragma GCC diagnostic push
7	#pragma GCC diagnostic ignored "-pedantic"
8	+#include <private/qqmlcontext_p.h>
9	#include <private/qqmldelegatemodel_p.h>
10	#include <private/qqmlglobal_p.h>
11	#include <private/qquickitem_p.h>
12	@@ -752,8 +753,6 @@
13
14	QQmlContext *creationContext = m_sectionDelegate->creationContext();
15	QQmlContext *context = new QQmlContext(creationContext ? creationContext : qmlContext(this));
16	- context->setContextProperty(QStringLiteral("section"), sectionText);
17	- context->setContextProperty(QStringLiteral("delegateIndex"), -1);
18	QObject *nobj = m_sectionDelegate->beginCreate(context);
19	if (nobj) {
20	QQml_setParent_noEvent(context, nobj);
21	@@ -761,6 +760,8 @@
22	if (!sectionItem) {
23	delete nobj;
24	} else {
25	+ sectionItem->setProperty("text", sectionText);
26	+ sectionItem->setProperty("delegateIndex", -1);
27	sectionItem->setZ(2);
28	QQml_setParent_noEvent(sectionItem, m_clipItem);
29	sectionItem->setParentItem(m_clipItem);
30	@@ -796,8 +797,7 @@
31	if (!item->sectionItem()) {
32	item->setSectionItem(getSectionItem(sectionText));
33	} else {
34	- QQmlContext *context = QQmlEngine::contextForObject(item->sectionItem())->parentContext();
35	- context->setContextProperty(QStringLiteral("section"), sectionText);
36	+ item->sectionItem()->setProperty("text", sectionText);
37	}
38	} else {
39	if (item->sectionItem()) {
40	@@ -918,8 +918,7 @@
41	polish();
42	}
43	if (listItem->sectionItem()) {
44	- QQmlContext *context = QQmlEngine::contextForObject(listItem->sectionItem())->parentContext();
45	- context->setContextProperty(QStringLiteral("delegateIndex"), modelIndex);
46	+ listItem->sectionItem()->setProperty("delegateIndex", modelIndex);
47	}
48	adjustMinYExtent();
49	m_contentHeightDirty = true;
50	@@ -943,9 +942,10 @@
51	return;
52
53	item->setParentItem(m_clipItem);
54	+ // FIXME Why do we need the refreshExpressions call?
55	QQmlContext *context = QQmlEngine::contextForObject(item)->parentContext();
56	- context->setContextProperty(QStringLiteral("ListViewWithPageHeader"), this);
57	- context->setContextProperty(QStringLiteral("heightToClip"), QVariant::fromValue<int>(0));
58	+ QQmlContextPrivate::get(context)->data->refreshExpressions();
59	+ item->setProperty("heightToClip", QVariant::fromValue<int>(0));
60	if (modelIndex == m_asyncRequestedIndex) {
61	createItem(modelIndex, false);
62	refill();
63	@@ -1111,8 +1111,7 @@
64	for (int i = 0; i < m_visibleItems.count(); ++i) {
65	ListItem *item = m_visibleItems[i];
66	if (item->sectionItem()) {
67	- QQmlContext *context = QQmlEngine::contextForObject(item->sectionItem())->parentContext();
68	- context->setContextProperty(QStringLiteral("delegateIndex"), m_firstVisibleIndex + i);
69	+ item->sectionItem()->setProperty("delegateIndex", m_firstVisibleIndex + i);
70	}
71	}
72
73	@@ -1282,8 +1281,7 @@
74	} else {
75	// Update the top sticky section header
76	const QString section = m_delegateModel->stringValue(modelIndex, m_sectionProperty);
77	- QQmlContext *context = QQmlEngine::contextForObject(m_topSectionItem)->parentContext();
78	- context->setContextProperty(QStringLiteral("section"), section);
79	+ m_topSectionItem->setProperty("text", section);
80
81	QQuickItemPrivate::get(m_topSectionItem)->setCulled(false);
82	m_topSectionItem->setY(topSectionStickPos);
83	@@ -1295,19 +1293,18 @@
84	break;
85	delegateIndex--;
86	}
87	- context->setContextProperty(QStringLiteral("delegateIndex"), delegateIndex);
88	+ m_topSectionItem->setProperty("delegateIndex", delegateIndex);
89	if (item->sectionItem()) {
90	QQuickItemPrivate::get(item->sectionItem())->setCulled(true);
91	}
92	}
93	}
94	}
95	- QQmlContext *context = QQmlEngine::contextForObject(item->m_item)->parentContext();
96	const qreal clipFrom = visibleFrom + (!item->sectionItem() && m_topSectionItem && !QQuickItemPrivate::get(m_topSectionItem)->culled ? m_topSectionItem->height() : 0);
97	if (!cull && pos < clipFrom) {
98	- context->setContextProperty(QStringLiteral("heightToClip"), clipFrom - pos);
99	+ item->m_item->setProperty("heightToClip", clipFrom - pos);
100	} else {
101	- context->setContextProperty(QStringLiteral("heightToClip"), QVariant::fromValue<int>(0));
102	+ item->m_item->setProperty("heightToClip", QVariant::fromValue<int>(0));
103	}
104	// qDebug() << "ListViewWithPageHeader::layout" << item->m_item;
105	pos += item->height();
106
107	=== modified file 'qml/Dash/DashCategoryBase.qml'
108	--- qml/Dash/DashCategoryBase.qml 2015-07-15 15:07:19 +0000
109	+++ qml/Dash/DashCategoryBase.qml 2016-04-07 07:42:25 +0000
110	@@ -24,17 +24,9 @@
111	/* Relevant really only for ListViewWithPageHeader case: specify how many pixels we can overlap with the section header */
112	readonly property int allowedOverlap: units.dp(1)
113
114	- property real __heightToClip: {
115	- // Check this is in position where clipping is needed
116	- if (typeof ListViewWithPageHeader !== 'undefined') {
117	- if (typeof heightToClip !== 'undefined') {
118	- if (heightToClip >= allowedOverlap) {
119	- return heightToClip - allowedOverlap;
120	- }
121	- }
122	- }
123	- return 0;
124	- }
125	+ property real heightToClip: 0
126	+ readonly property real __heightToClip: heightToClip >= allowedOverlap ? heightToClip - allowedOverlap : 0
127	+
128
129	/*!
130	\internal
131
132	=== modified file 'qml/Dash/GenericScopeView.qml'
133	--- qml/Dash/GenericScopeView.qml 2016-03-14 08:53:26 +0000
134	+++ qml/Dash/GenericScopeView.qml 2016-04-07 07:42:25 +0000
135	@@ -607,10 +607,10 @@
136	sectionProperty: "name"
137	sectionDelegate: ListItems.Header {
138	objectName: "dashSectionHeader" + (delegate ? delegate.category : "")
139	+ property int delegateIndex: -1
140	readonly property var delegate: categoryView.item(delegateIndex)
141	width: categoryView.width
142	- height: section != "" ? units.gu(5) : 0
143	- text: section
144	+ height: text != "" ? units.gu(5) : 0
145	color: scopeStyle ? scopeStyle.foreground : theme.palette.normal.baseText
146	iconName: delegate && delegate.headerLink ? "go-next" : ""
147	onClicked: {
148
149	=== modified file 'tests/plugins/Dash/listviewwithpageheadersectionexternalmodeltest.cpp'
150	--- tests/plugins/Dash/listviewwithpageheadersectionexternalmodeltest.cpp 2015-05-21 15:52:48 +0000
151	+++ tests/plugins/Dash/listviewwithpageheadersectionexternalmodeltest.cpp 2016-04-07 07:42:25 +0000
152	@@ -151,12 +151,12 @@
153
154	QString section(QQuickItem *item)
155	{
156	- return item ? QQmlEngine::contextForObject(item)->parentContext()->contextProperty(QLatin1String("section")).toString() : QString();
157	+ return item ? item->property("text").toString() : QString();
158	}
159
160	int sectionDelegateIndex(QQuickItem *item)
161	{
162	- return item ? QQmlEngine::contextForObject(item)->parentContext()->contextProperty(QLatin1String("delegateIndex")).toInt() : -1;
163	+ return item ? item->property("delegateIndex").toInt() : -1;
164	}
165
166	private Q_SLOTS:
167
168	=== modified file 'tests/plugins/Dash/listviewwithpageheadersectiontest.cpp'
169	--- tests/plugins/Dash/listviewwithpageheadersectiontest.cpp 2016-04-07 07:42:25 +0000
170	+++ tests/plugins/Dash/listviewwithpageheadersectiontest.cpp 2016-04-07 07:42:25 +0000
171	@@ -116,12 +116,12 @@
172
173	QString section(QQuickItem *item) const
174	{
175	- return item ? QQmlEngine::contextForObject(item)->parentContext()->contextProperty(QLatin1String("section")).toString() : QString();
176	+ return item ? item->property("text").toString() : QString();
177	}
178
179	int sectionDelegateIndex(QQuickItem *item) const
180	{
181	- return item ? QQmlEngine::contextForObject(item)->parentContext()->contextProperty(QLatin1String("delegateIndex")).toInt() : -1;
182	+ return item ? item->property("delegateIndex").toInt() : -1;
183	}
184
185	private Q_SLOTS:
186
187	=== modified file 'tests/plugins/Dash/listviewwithpageheadertestsection.qml'
188	--- tests/plugins/Dash/listviewwithpageheadertestsection.qml 2016-04-07 07:42:25 +0000
189	+++ tests/plugins/Dash/listviewwithpageheadertestsection.qml 2016-04-07 07:42:25 +0000
190	@@ -97,9 +97,10 @@
191	sectionProperty: "type"
192	sectionDelegate: Component {
193	Rectangle {
194	+ property alias text: theText.text
195	color: "green"
196	- height: section === "" ? 0 : section != "halfheight" ? 40 : 20;
197	- Text { text: section; font.pixelSize: 34 }
198	+ height: text === "" ? 0 : text != "halfheight" ? 40 : 20;
199	+ Text { id: theText; font.pixelSize: 34 }
200	anchors { left: parent.left; right: parent.right }
201	}
202	}
203
204	=== modified file 'tests/plugins/Dash/listviewwithpageheadertestsectionexternalmodel.qml'
205	--- tests/plugins/Dash/listviewwithpageheadertestsectionexternalmodel.qml 2015-07-15 15:07:19 +0000
206	+++ tests/plugins/Dash/listviewwithpageheadertestsectionexternalmodel.qml 2016-04-07 07:42:25 +0000
207	@@ -65,9 +65,10 @@
208	sectionProperty: "type"
209	sectionDelegate: Component {
210	Rectangle {
211	+ property alias text: theText.text
212	color: "green"
213	height: 40
214	- Text { text: section; font.pixelSize: 34 }
215	+ Text { id: theText; font.pixelSize: 34 }
216	anchors { left: parent.left; right: parent.right }
217	}
218	}
219
220	=== modified file 'tests/plugins/Dash/tst_ListViewWithPageHeaderQML.qml'
221	--- tests/plugins/Dash/tst_ListViewWithPageHeaderQML.qml 2015-07-15 15:07:19 +0000
222	+++ tests/plugins/Dash/tst_ListViewWithPageHeaderQML.qml 2016-04-07 07:42:25 +0000
223	@@ -81,9 +81,10 @@
224	sectionDelegate: Component {
225	id: sectionHeaderComponent
226	Rectangle {
227	+ property alias text: theText.text
228	color: "green"
229	height: listView.sectionHeaderHeight
230	- Text { text: section; font.pixelSize: 34 }
231	+ Text { id: theText; font.pixelSize: 34 }
232	anchors { left: parent.left; right: parent.right }
233	}
234	}
235	@@ -92,9 +93,10 @@
236	Component {
237	id: otherSectionHeaderComponent
238	Rectangle {
239	+ property alias text: theText.text
240	color: "green"
241	height: 50
242	- Text { text: section; font.pixelSize: 34 }
243	+ Text { id: theText; font.pixelSize: 34 }
244	anchors { left: parent.left; right: parent.right }
245	}
246	}