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

Proposed by Albert Astals Cid
Status: Merged
Approved by: MichaƂ Sawicz
Approved revision: 1402
Merged at revision: 1506
Proposed branch: lp:~aacid/unity8/moreAsyncDash
Merge into: lp:unity8
Prerequisite: lp:~aacid/unity8/photoscopeimprovements
Diff against target: 413 lines (+102/-35)
16 files modified
plugins/Dash/abstractdashview.cpp (+24/-5)
plugins/Dash/abstractdashview.h (+6/-0)
plugins/Dash/listviewwithpageheader.cpp (+7/-2)
plugins/Dash/listviewwithpageheader.h (+4/-4)
qml/Components/ResponsiveVerticalJournal.qml (+1/-0)
qml/Dash/CardGrid.qml (+1/-2)
qml/Dash/CardVerticalJournal.qml (+1/-0)
qml/Dash/DashContent.qml (+2/-0)
qml/Dash/DashRenderer.qml (+2/-0)
qml/Dash/GenericScopeView.qml (+45/-19)
qml/Dash/ScopeListView.qml (+4/-2)
tests/plugins/Dash/horizontaljournaltest.qml (+1/-0)
tests/plugins/Dash/listviewwithpageheadertest.cpp (+1/-1)
tests/plugins/Dash/organicgridtest.qml (+1/-0)
tests/plugins/Dash/verticaljournaltest.qml (+1/-0)
tests/qmltests/Dash/tst_GenericScopeView.qml (+1/-0)
To merge this branch: bzr merge lp:~aacid/unity8/moreAsyncDash
Reviewer Review Type Date Requested Status
Andrea Cimitan (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+241524@code.launchpad.net

Commit message

Rework how we set the ranges so we get some more asynchronousity from item views

Description of the change

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

 * Did you perform an exploratory manual test run of your code change and any related functionality?
A few, still have to do more

 * 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

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/moreAsyncDash updated
1392. By Albert Astals Cid

Merge lp:~aacid/unity8/photoscopeimprovements

1393. By Albert Astals Cid

Remove unused static

1394. By Albert Astals Cid

Set cache buffer to the value it had when the test was written

1395. By Albert Astals Cid

Repair the cachebuffer here too

1396. By Albert Astals Cid

Fix more tests

1397. By Albert Astals Cid

Make test pass

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/moreAsyncDash updated
1398. By Albert Astals Cid

Fix infinite loop in creation/deletion of delegates

There's two fixes actually:
 * One fix the calculation of visibleRange
 * Two make sure the displayMargins don't cross the wrong way (too much) otherwise GridView gets all confused and starts the infinite creation/destruction

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~aacid/unity8/moreAsyncDash updated
1399. By Albert Astals Cid

Merge photoscopeimprovements

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Andrea Cimitan (cimi) :
Revision history for this message
Albert Astals Cid (aacid) :
lp:~aacid/unity8/moreAsyncDash updated
1400. By Albert Astals Cid

Make cacheBuffer int for consistency with Qt own's classes

1401. By Albert Astals Cid

Merge

1402. By Albert Astals Cid

Small fix after previous real -> int change

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Andrea Cimitan (cimi) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes
 * Did CI run pass? If not, please explain why.
Let's see if it kicks again soon
 * Did you make sure that the branch does not contain spurious tags?
yes

waiting ci again to top approve

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Dash/abstractdashview.cpp'
2--- plugins/Dash/abstractdashview.cpp 2014-05-22 13:37:05 +0000
3+++ plugins/Dash/abstractdashview.cpp 2014-12-03 11:06:35 +0000
4@@ -16,13 +16,12 @@
5
6 #include "abstractdashview.h"
7
8-static const qreal bufferRatio = 0.5;
9-
10 AbstractDashView::AbstractDashView()
11 : m_delegateModel(nullptr)
12 , m_asyncRequestedIndex(-1)
13 , m_columnSpacing(0)
14 , m_rowSpacing(0)
15+ , m_buffer(320) // Same value used in qquickitemview.cpp in Qt 5.4
16 , m_displayMarginBeginning(0)
17 , m_displayMarginEnd(0)
18 , m_needsRelayout(false)
19@@ -112,6 +111,27 @@
20 }
21 }
22
23+int AbstractDashView::cacheBuffer() const
24+{
25+ return m_buffer;
26+}
27+
28+void AbstractDashView::setCacheBuffer(int buffer)
29+{
30+ if (buffer < 0) {
31+ qmlInfo(this) << "Cannot set a negative cache buffer";
32+ return;
33+ }
34+
35+ if (m_buffer != buffer) {
36+ m_buffer = buffer;
37+ if (isComponentComplete()) {
38+ polish();
39+ }
40+ emit cacheBufferChanged();
41+ }
42+}
43+
44 qreal AbstractDashView::displayMarginBeginning() const
45 {
46 return m_displayMarginBeginning;
47@@ -160,9 +180,8 @@
48
49 const qreal from = -m_displayMarginBeginning;
50 const qreal to = height() + m_displayMarginEnd;
51- const qreal buffer = (to - from) * bufferRatio;
52- const qreal bufferFrom = from - buffer;
53- const qreal bufferTo = to + buffer;
54+ const qreal bufferFrom = from - m_buffer;
55+ const qreal bufferTo = to + m_buffer;
56
57 bool added = addVisibleItems(from, to, false);
58 bool removed = removeNonVisibleItems(bufferFrom, bufferTo);
59
60=== modified file 'plugins/Dash/abstractdashview.h'
61--- plugins/Dash/abstractdashview.h 2014-11-26 10:09:06 +0000
62+++ plugins/Dash/abstractdashview.h 2014-12-03 11:06:35 +0000
63@@ -35,6 +35,7 @@
64 Q_PROPERTY(QQmlComponent *delegate READ delegate WRITE setDelegate NOTIFY delegateChanged)
65 Q_PROPERTY(qreal columnSpacing READ columnSpacing WRITE setColumnSpacing NOTIFY columnSpacingChanged)
66 Q_PROPERTY(qreal rowSpacing READ rowSpacing WRITE setRowSpacing NOTIFY rowSpacingChanged)
67+ Q_PROPERTY(int cacheBuffer READ cacheBuffer WRITE setCacheBuffer NOTIFY cacheBufferChanged)
68 Q_PROPERTY(qreal displayMarginBeginning READ displayMarginBeginning
69 WRITE setDisplayMarginBeginning
70 NOTIFY displayMarginBeginningChanged)
71@@ -61,6 +62,9 @@
72 qreal rowSpacing() const;
73 void setRowSpacing(qreal rowSpacing);
74
75+ int cacheBuffer() const;
76+ void setCacheBuffer(int);
77+
78 qreal displayMarginBeginning() const;
79 void setDisplayMarginBeginning(qreal);
80
81@@ -72,6 +76,7 @@
82 void delegateChanged();
83 void columnSpacingChanged();
84 void rowSpacingChanged();
85+ void cacheBufferChanged();
86 void displayMarginBeginningChanged();
87 void displayMarginEndChanged();
88
89@@ -117,6 +122,7 @@
90
91 int m_columnSpacing;
92 int m_rowSpacing;
93+ int m_buffer;
94 qreal m_displayMarginBeginning;
95 qreal m_displayMarginEnd;
96 bool m_needsRelayout;
97
98=== modified file 'plugins/Dash/listviewwithpageheader.cpp'
99--- plugins/Dash/listviewwithpageheader.cpp 2014-11-26 10:09:06 +0000
100+++ plugins/Dash/listviewwithpageheader.cpp 2014-12-03 11:06:35 +0000
101@@ -334,13 +334,18 @@
102 return m_headerItemShownHeight;
103 }
104
105-qreal ListViewWithPageHeader::cacheBuffer() const
106+int ListViewWithPageHeader::cacheBuffer() const
107 {
108 return m_cacheBuffer;
109 }
110
111-void ListViewWithPageHeader::setCacheBuffer(qreal cacheBuffer)
112+void ListViewWithPageHeader::setCacheBuffer(int cacheBuffer)
113 {
114+ if (cacheBuffer < 0) {
115+ qmlInfo(this) << "Cannot set a negative cache buffer";
116+ return;
117+ }
118+
119 if (cacheBuffer != m_cacheBuffer) {
120 m_cacheBuffer = cacheBuffer;
121 Q_EMIT cacheBufferChanged();
122
123=== modified file 'plugins/Dash/listviewwithpageheader.h'
124--- plugins/Dash/listviewwithpageheader.h 2014-10-10 11:13:26 +0000
125+++ plugins/Dash/listviewwithpageheader.h 2014-12-03 11:06:35 +0000
126@@ -54,7 +54,7 @@
127 Q_PROPERTY(bool forceNoClip READ forceNoClip WRITE setForceNoClip NOTIFY forceNoClipChanged)
128 Q_PROPERTY(int stickyHeaderHeight READ stickyHeaderHeight NOTIFY stickyHeaderHeightChanged)
129 Q_PROPERTY(qreal headerItemShownHeight READ headerItemShownHeight NOTIFY headerItemShownHeightChanged)
130- Q_PROPERTY(qreal cacheBuffer READ cacheBuffer WRITE setCacheBuffer NOTIFY cacheBufferChanged)
131+ Q_PROPERTY(int cacheBuffer READ cacheBuffer WRITE setCacheBuffer NOTIFY cacheBufferChanged)
132
133 friend class ListViewWithPageHeaderTest;
134 friend class ListViewWithPageHeaderTestSection;
135@@ -85,8 +85,8 @@
136 int stickyHeaderHeight() const;
137 qreal headerItemShownHeight() const;
138
139- qreal cacheBuffer() const;
140- void setCacheBuffer(qreal cacheBuffer);
141+ int cacheBuffer() const;
142+ void setCacheBuffer(int cacheBuffer);
143
144 Q_INVOKABLE void positionAtBeginning();
145 Q_INVOKABLE void showHeader();
146@@ -204,7 +204,7 @@
147 bool m_forceNoClip;
148 bool m_inLayout;
149 bool m_inContentHeightKeepHeaderShown;
150- qreal m_cacheBuffer;
151+ int m_cacheBuffer;
152
153 // Qt 5.0 doesn't like releasing the items just after itemCreated
154 // so we delay the releasing until the next updatePolish
155
156=== modified file 'qml/Components/ResponsiveVerticalJournal.qml'
157--- qml/Components/ResponsiveVerticalJournal.qml 2014-10-02 11:08:26 +0000
158+++ qml/Components/ResponsiveVerticalJournal.qml 2014-12-03 11:06:35 +0000
159@@ -45,6 +45,7 @@
160 property alias rowSpacing: verticalJournalView.rowSpacing
161 property alias model: verticalJournalView.model
162 property alias delegate: verticalJournalView.delegate
163+ property alias cacheBuffer: verticalJournalView.cacheBuffer
164 property alias displayMarginBeginning: verticalJournalView.displayMarginBeginning
165 property alias displayMarginEnd: verticalJournalView.displayMarginEnd
166
167
168=== modified file 'qml/Dash/CardGrid.qml'
169--- qml/Dash/CardGrid.qml 2014-10-30 09:13:09 +0000
170+++ qml/Dash/CardGrid.qml 2014-12-03 11:06:35 +0000
171@@ -50,7 +50,7 @@
172 model: root.model
173 displayMarginBeginning: root.displayMarginBeginning
174 displayMarginEnd: root.displayMarginEnd
175- cacheBuffer: 0
176+ cacheBuffer: root.cacheBuffer
177 interactive: false
178 delegate: Item {
179 width: grid.cellWidth
180@@ -59,7 +59,6 @@
181 id: loader
182 sourceComponent: cardTool.cardComponent
183 anchors.horizontalCenter: parent.horizontalCenter
184- asynchronous: true
185 onLoaded: {
186 item.objectName = "delegate" + index;
187 item.width = Qt.binding(function() { return cardTool.cardWidth; });
188
189=== modified file 'qml/Dash/CardVerticalJournal.qml'
190--- qml/Dash/CardVerticalJournal.qml 2014-11-05 15:06:42 +0000
191+++ qml/Dash/CardVerticalJournal.qml 2014-12-03 11:06:35 +0000
192@@ -59,6 +59,7 @@
193 rowSpacing: minimumColumnSpacing
194 columnWidth: cardTool.cardWidth
195
196+ cacheBuffer: root.cacheBuffer
197 displayMarginBeginning: root.displayMarginBeginning
198 displayMarginEnd: root.displayMarginEnd
199
200
201=== modified file 'qml/Dash/DashContent.qml'
202--- qml/Dash/DashContent.qml 2014-10-30 21:43:18 +0000
203+++ qml/Dash/DashContent.qml 2014-12-03 11:06:35 +0000
204@@ -153,6 +153,7 @@
205
206 delegate:
207 Loader {
208+ id: loader
209 width: ListView.view.width
210 height: ListView.view.height
211 opacity: { // hide delegate if offscreen
212@@ -181,6 +182,7 @@
213 dashContent.scopeLoaded(item.scope.id)
214 item.paginationCount = Qt.binding(function() { return dashContentList.count } )
215 item.paginationIndex = Qt.binding(function() { return dashContentList.currentIndex } )
216+ item.visibleToParent = Qt.binding(function() { return loader.opacity != 0 });
217 item.holdingList = dashContentList;
218 item.forceNonInteractive = Qt.binding(function() { return dashContent.forceNonInteractive } )
219 }
220
221=== modified file 'qml/Dash/DashRenderer.qml'
222--- qml/Dash/DashRenderer.qml 2014-10-28 09:32:48 +0000
223+++ qml/Dash/DashRenderer.qml 2014-12-03 11:06:35 +0000
224@@ -23,6 +23,8 @@
225
226 property int collapsedItemCount: -1
227
228+ property int cacheBuffer: 0
229+
230 property int displayMarginBeginning: 0
231
232 property int displayMarginEnd: 0
233
234=== modified file 'qml/Dash/GenericScopeView.qml'
235--- qml/Dash/GenericScopeView.qml 2014-11-06 15:25:51 +0000
236+++ qml/Dash/GenericScopeView.qml 2014-12-03 11:06:35 +0000
237@@ -38,6 +38,7 @@
238 readonly property alias subPageShown: subPageLoader.subPageShown
239 property int paginationCount: 0
240 property int paginationIndex: 0
241+ property bool visibleToParent: false
242 property alias pageHeaderTotallyVisible: categoryView.pageHeaderTotallyVisible
243 property var holdingList: null
244
245@@ -357,6 +358,7 @@
246 Connections {
247 target: scopeView
248 onIsCurrentChanged: rendererLoader.updateRanges();
249+ onVisibleToParentChanged: rendererLoader.updateRanges();
250 }
251 Connections {
252 target: holdingList
253@@ -364,7 +366,9 @@
254 }
255
256 function updateRanges() {
257- if (holdingList && holdingList.moving) {
258+ // Don't want to create stress by requesting more items during scope
259+ // changes so unless you're not part of the visible scopes just return
260+ if (holdingList && holdingList.moving && !scopeView.visibleToParent) {
261 return;
262 }
263
264@@ -380,28 +384,50 @@
265 }
266
267 if (item && item.hasOwnProperty("displayMarginBeginning")) {
268- // A item view is considered visible from
269+ // A item view creates its delegates synchronously from
270 // -displayMarginBeginning
271 // to
272- // height + item.displayMarginEnd
273+ // height + displayMarginEnd
274+ // Around that area it adds the cacheBuffer area where delegates are created async
275+ //
276 // We adjust displayMarginBeginning and displayMarginEnd so
277- // * In non visible scopes only the viewport is considered visible
278- // that way when you switch to it the visible items are there
279- // * For visible scopes we increase the visible range by categoryView.height * 1.5
280- // in both directions to make scrolling nicer by mantaining a higher number of
281+ // * In non visible scopes nothing is considered visible and we set cacheBuffer
282+ // so that creates the items that would be in the viewport asynchronously
283+ // * For the current scope set the visible range to the viewport and then
284+ // use cacheBuffer to create extra items for categoryView.height * 1.5
285+ // to make scrolling nicer by mantaining a higher number of
286 // cached items
287- // TODO Improvements
288- // - For non visible scopes we should always have a visible range of 0 and
289- // make sure the items in the viewport are created with the cache buffer feature
290- // - For visible scopes we should always the have a visible range be exactly the
291- // viewport and make sure the rest of items are created with the cache buffer feature
292- // To be able to implement that feature VerticalJournal/AbstractDashView needs to
293- // make the cache buffer value setable externally
294- var extraMargins = scopeView.isCurrent ? categoryView.height * 1.5 : 0;
295-
296- item.displayMarginBeginning = Math.round(-Math.max(-baseItem.y - extraMargins, 0));
297- item.displayMarginEnd = -Math.round(Math.max(baseItem.height - extraMargins - seeAll.height -
298- categoryView.height + baseItem.y, 0));
299+ // * For non current but visible scopes (i.e. when the user changes from one scope
300+ // to the next, we set the visible range to the viewport so
301+ // items are not culled (invisible) but still use no cacheBuffer
302+ // (it will be set once the scope is the current one)
303+ var displayMarginBeginning = baseItem.y;
304+ displayMarginBeginning = -Math.max(-displayMarginBeginning, 0);
305+ displayMarginBeginning = -Math.min(-displayMarginBeginning, baseItem.height);
306+ displayMarginBeginning = Math.round(displayMarginBeginning);
307+ var displayMarginEnd = -baseItem.height + seeAll.height + categoryView.height - baseItem.y;
308+ displayMarginEnd = -Math.max(-displayMarginEnd, 0);
309+ displayMarginEnd = -Math.min(-displayMarginEnd, baseItem.height);
310+ displayMarginEnd = Math.round(displayMarginEnd);
311+ if (scopeView.isCurrent || scopeView.visibleToParent) {
312+ item.displayMarginBeginning = displayMarginBeginning;
313+ item.displayMarginEnd = displayMarginEnd;
314+ item.cacheBuffer = scopeView.isCurrent ? categoryView.height * 1.5 : 0;
315+ } else {
316+ var visibleRange = baseItem.height + displayMarginEnd + displayMarginBeginning;
317+ if (visibleRange < 0) {
318+ item.displayMarginBeginning = displayMarginBeginning;
319+ item.displayMarginEnd = displayMarginEnd;
320+ item.cacheBuffer = 0;
321+ } else {
322+ // This should be visibleRange/2 in each of the properties
323+ // but some item views still (like GridView) like creating sync delegates even if
324+ // the visible range is 0 so let's make sure the visible range is negative
325+ item.displayMarginBeginning = displayMarginBeginning - visibleRange;
326+ item.displayMarginEnd = displayMarginEnd - visibleRange;
327+ item.cacheBuffer = visibleRange;
328+ }
329+ }
330 }
331 }
332 }
333
334=== modified file 'qml/Dash/ScopeListView.qml'
335--- qml/Dash/ScopeListView.qml 2014-10-23 11:59:22 +0000
336+++ qml/Dash/ScopeListView.qml 2014-12-03 11:06:35 +0000
337@@ -1,5 +1,5 @@
338 /*
339- * Copyright (C) 2013 Canonical, Ltd.
340+ * Copyright (C) 2014 Canonical, Ltd.
341 *
342 * This program is free software; you can redistribute it and/or modify
343 * it under the terms of the GNU General Public License as published by
344@@ -20,5 +20,7 @@
345 ListViewWithPageHeader {
346 maximumFlickVelocity: height * 10
347 flickDeceleration: height * 2
348- cacheBuffer: Number.MAX_VALUE
349+ // 1073741823 is s^30 -1. A quite big number so that you have "infinite" cache, but not so
350+ // big so that if you add if with itself you're outside the 2^31 int range
351+ cacheBuffer: 1073741823
352 }
353
354=== modified file 'tests/plugins/Dash/horizontaljournaltest.qml'
355--- tests/plugins/Dash/horizontaljournaltest.qml 2014-04-30 10:06:33 +0000
356+++ tests/plugins/Dash/horizontaljournaltest.qml 2014-12-03 11:06:35 +0000
357@@ -25,6 +25,7 @@
358 rowHeight: 150
359 columnSpacing: 10
360 rowSpacing: 10
361+ cacheBuffer: Math.max(0, (height + displayMarginEnd + displayMarginBeginning) / 2)
362
363 delegate: Rectangle {
364 property real randomValue: Math.random()
365
366=== modified file 'tests/plugins/Dash/listviewwithpageheadertest.cpp'
367--- tests/plugins/Dash/listviewwithpageheadertest.cpp 2014-08-26 08:41:02 +0000
368+++ tests/plugins/Dash/listviewwithpageheadertest.cpp 2014-12-03 11:06:35 +0000
369@@ -1920,7 +1920,7 @@
370
371 void testAllCacheBuffer()
372 {
373- lvwph->setCacheBuffer(std::numeric_limits<qreal>::max());
374+ lvwph->setCacheBuffer(std::numeric_limits<int>::max());
375 QTRY_COMPARE(lvwph->m_visibleItems.count(), 6);
376 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
377 verifyItem(0, 50., 150., false);
378
379=== modified file 'tests/plugins/Dash/organicgridtest.qml'
380--- tests/plugins/Dash/organicgridtest.qml 2014-04-30 10:06:33 +0000
381+++ tests/plugins/Dash/organicgridtest.qml 2014-12-03 11:06:35 +0000
382@@ -27,6 +27,7 @@
383 rowSpacing: 10
384 smallDelegateSize: Qt.size(90, 90)
385 bigDelegateSize: Qt.size(180, 180)
386+ cacheBuffer: Math.max(0, (height + displayMarginEnd + displayMarginBeginning) / 2)
387
388 delegate: Rectangle {
389 property real randomValue: Math.random()
390
391=== modified file 'tests/plugins/Dash/verticaljournaltest.qml'
392--- tests/plugins/Dash/verticaljournaltest.qml 2014-04-30 10:06:33 +0000
393+++ tests/plugins/Dash/verticaljournaltest.qml 2014-12-03 11:06:35 +0000
394@@ -26,6 +26,7 @@
395 columnWidth: 150
396 columnSpacing: 10
397 rowSpacing: 10
398+ cacheBuffer: Math.max(0, (height + displayMarginEnd + displayMarginBeginning) / 2)
399
400 delegate: Rectangle {
401 property real randomValue: Math.random()
402
403=== modified file 'tests/qmltests/Dash/tst_GenericScopeView.qml'
404--- tests/qmltests/Dash/tst_GenericScopeView.qml 2014-11-26 08:27:45 +0000
405+++ tests/qmltests/Dash/tst_GenericScopeView.qml 2014-12-03 11:06:35 +0000
406@@ -59,6 +59,7 @@
407 GenericScopeView {
408 id: genericScopeView
409 anchors.fill: parent
410+ visibleToParent: true
411
412 UT.UnityTestCase {
413 id: testCase

Subscribers

People subscribed via source and target branches