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

[Albert Astals Cid]

Implement proper updateRanges for horizontal items (i.e. Carousel, Horizontal List)

Saves lots of unneeded card creation, e.g DashContent::test_noDelegateCreationDestructionOnMove number of buttons goes down from 1395 to 773 on my computer

 * 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?
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

[Albert Astals Cid (aacid)]

The diff in updateRanges is not great, basically what i did is add the

if (item.growsVertically) {
   // old code untouched here
} else {
   // new code
}

[lp:~aacid/unity8/properRangesHorizontalCategories]

1548. By Albert Astals Cid on 2015-01-14

Didn't want to commit this

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1547
http://jenkins.qa.ubuntu.com/job/unity8-ci/5145/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/797/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/309
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/310
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/310
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/694/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/795
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/795/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17065

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5145/rebuild

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

The failed qmluitest should go away with r1548, let's see CI re-run the tests

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1548
http://jenkins.qa.ubuntu.com/job/unity8-ci/5146/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/800
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/310
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/311
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/311
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/697
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/798
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/798/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17070

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5146/rebuild

review: Approve (continuous-integration)

[Andrea Cimitan (cimi)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?
y
 * Did CI run pass? If not, please explain why.
y
 * Did you make sure that the branch does not contain spurious tags?
y

review: Approve

=== modified file 'qml/Components/Carousel.qml'
--- qml/Components/Carousel.qml	2014-10-23 11:59:22 +0000
+++ qml/Components/Carousel.qml	2015-01-14 11:17:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Canonical, Ltd.
+ * Copyright (C) 2013-2015 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -14,7 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import QtQuick 2.0
+import QtQuick 2.3
 import Ubuntu.Components 0.1
 import "carousel.js" as CarouselJS
 
@@ -38,6 +38,8 @@
     property alias tileAspectRatio: listView.tileAspectRatio
     /// Used to cache some delegates for performance reasons. See the ListView documentation for details
     property alias cacheBuffer: listView.cacheBuffer
+    property alias displayMarginBeginning: listView.displayMarginBeginning
+    property alias displayMarginEnd: listView.displayMarginEnd
     /// Width of the "draw buffer" in pixel. The drawBuffer is an additional area at start/end where
     /// items drawn, even if it is not in the visible area.
     /// cacheBuffer controls only the to retain delegates outside the visible area (and is used on top of the drawBuffer)
@@ -51,6 +53,8 @@
     readonly property alias currentItem: listView.currentItem
     /// exposes the distance to the next row (only one row in carousel, so it's the topMargins)
     readonly property alias verticalSpacing: listView.verticalMargin
+    /// the width of the internal list
+    readonly property alias innerWidth: listView.width
 
     implicitHeight: listView.tileHeight * selectedItemScaleFactor
     opacity: listView.highlightIndex === -1 ? 1 : 0.6

=== modified file 'qml/Dash/CardCarousel.qml'
--- qml/Dash/CardCarousel.qml	2014-11-03 09:41:51 +0000
+++ qml/Dash/CardCarousel.qml	2015-01-14 11:17:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Canonical, Ltd.
+ * Copyright (C) 2013-2015 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -23,6 +23,8 @@
 
     expandedHeight: carousel.implicitHeight + units.gu(6)
     collapsedHeight: expandedHeight
+    growsVertically: false
+    innerWidth: carousel.innerWidth
 
     Carousel {
         id: carousel
@@ -32,7 +34,9 @@
         // and push others back.
         minimumTileWidth: cardTool.cardWidth / selectedItemScaleFactor
         selectedItemScaleFactor: cardTool.carouselSelectedItemScaleFactor
-        cacheBuffer: 1404 // 18px * 13gu * 6
+        cacheBuffer: cardCarousel.cacheBuffer
+        displayMarginBeginning: cardCarousel.displayMarginBeginning
+        displayMarginEnd: cardCarousel.displayMarginEnd
         model: cardCarousel.model
 
         property real fontScale: 1 / selectedItemScaleFactor

=== modified file 'qml/Dash/CardHorizontalList.qml'
--- qml/Dash/CardHorizontalList.qml	2014-11-05 08:37:55 +0000
+++ qml/Dash/CardHorizontalList.qml	2015-01-14 11:17:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Canonical, Ltd.
+ * Copyright (C) 2014-2015 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -14,7 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import QtQuick 2.2
+import QtQuick 2.3
 import Ubuntu.Components 1.1
 import "../Components"
 
@@ -23,6 +23,8 @@
 
     expandedHeight: cardTool.cardHeight + units.gu(2)
     collapsedHeight: expandedHeight
+    growsVertically: false
+    innerWidth: Math.max(0, listView.width)
     clip: true
 
     ListView {
@@ -36,6 +38,9 @@
         spacing: units.gu(1)
         model: root.model
         orientation: ListView.Horizontal
+        cacheBuffer: root.cacheBuffer
+        displayMarginBeginning: root.displayMarginBeginning
+        displayMarginEnd: root.displayMarginEnd
 
         delegate: Loader {
             id: loader

=== modified file 'qml/Dash/DashRenderer.qml'
--- qml/Dash/DashRenderer.qml	2014-11-07 16:16:12 +0000
+++ qml/Dash/DashRenderer.qml	2015-01-14 11:17:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Canonical, Ltd.
+ * Copyright (C) 2013-2015 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -31,6 +31,11 @@
 
     property real originY: 0
 
+    property bool growsVertically: true
+
+    // If growsVertically the width of the item inside the renderer
+    property real innerWidth: 0
+
     // The model to renderer
     property var model
 

=== modified file 'qml/Dash/GenericScopeView.qml'
--- qml/Dash/GenericScopeView.qml	2015-01-14 11:17:41 +0000
+++ qml/Dash/GenericScopeView.qml	2015-01-14 11:17:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2014 Canonical, Ltd.
+ * Copyright (C) 2013-2015 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -401,59 +401,102 @@
                     }
 
                     if (item && item.hasOwnProperty("displayMarginBeginning")) {
-                        // A item view creates its delegates synchronously from
-                        //     -displayMarginBeginning
-                        // to
-                        //     height + displayMarginEnd
-                        // Around that area it adds the cacheBuffer area where delegates are created async
-                        //
-                        // We adjust displayMarginBeginning and displayMarginEnd so
-                        //   * In non visible scopes nothing is considered visible and we set cacheBuffer
-                        //     so that creates the items that would be in the viewport asynchronously
-                        //   * For the current scope set the visible range to the viewport and then
-                        //     use cacheBuffer to create extra items for categoryView.height * 1.5
-                        //     to make scrolling nicer by mantaining a higher number of
-                        //     cached items
-                        //   * For non current but visible scopes (i.e. when the user changes from one scope
-                        //     to the next, we set the visible range to the viewport so
-                        //     items are not culled (invisible) but still use no cacheBuffer
-                        //     (it will be set once the scope is the current one)
-                        var displayMarginBeginning = baseItem.y;
-                        displayMarginBeginning = -Math.max(-displayMarginBeginning, 0);
-                        displayMarginBeginning = -Math.min(-displayMarginBeginning, baseItem.height);
-                        displayMarginBeginning = Math.round(displayMarginBeginning);
-                        var displayMarginEnd = -baseItem.height + seeAll.height + categoryView.height - baseItem.y;
-                        displayMarginEnd = -Math.max(-displayMarginEnd, 0);
-                        displayMarginEnd = -Math.min(-displayMarginEnd, baseItem.height);
-                        displayMarginEnd = Math.round(displayMarginEnd);
-                        if (scopeView.isCurrent || scopeView.visibleToParent) {
-                            item.displayMarginBeginning = displayMarginBeginning;
-                            item.displayMarginEnd = displayMarginEnd;
-                            if (holdingList && holdingList.moving) {
-                                // If we are moving we need to reset the cache buffer of the
-                                // view that was not visible (i.e. !wasCurrentOnMoveStart) to 0 since
-                                // otherwise the cache buffer we had set to preload the items of the
-                                // visible range will trigger some item creations and we want move to
-                                // be as smooth as possible meaning no need creations
-                                if (!wasCurrentOnMoveStart) {
+                        if (item.growsVertically) {
+                            // A item view creates its delegates synchronously from
+                            //     -displayMarginBeginning
+                            // to
+                            //     height + displayMarginEnd
+                            // Around that area it adds the cacheBuffer area where delegates are created async
+                            //
+                            // We adjust displayMarginBeginning and displayMarginEnd so
+                            //   * In non visible scopes nothing is considered visible and we set cacheBuffer
+                            //     so that creates the items that would be in the viewport asynchronously
+                            //   * For the current scope set the visible range to the viewport and then
+                            //     use cacheBuffer to create extra items for categoryView.height * 1.5
+                            //     to make scrolling nicer by mantaining a higher number of
+                            //     cached items
+                            //   * For non current but visible scopes (i.e. when the user changes from one scope
+                            //     to the next, we set the visible range to the viewport so
+                            //     items are not culled (invisible) but still use no cacheBuffer
+                            //     (it will be set once the scope is the current one)
+                            var displayMarginBeginning = baseItem.y;
+                            displayMarginBeginning = -Math.max(-displayMarginBeginning, 0);
+                            displayMarginBeginning = -Math.min(-displayMarginBeginning, baseItem.height);
+                            displayMarginBeginning = Math.round(displayMarginBeginning);
+                            var displayMarginEnd = -baseItem.height + seeAll.height + categoryView.height - baseItem.y;
+                            displayMarginEnd = -Math.max(-displayMarginEnd, 0);
+                            displayMarginEnd = -Math.min(-displayMarginEnd, baseItem.height);
+                            displayMarginEnd = Math.round(displayMarginEnd);
+
+                            if (scopeView.isCurrent || scopeView.visibleToParent) {
+                                item.displayMarginBeginning = displayMarginBeginning;
+                                item.displayMarginEnd = displayMarginEnd;
+                                if (holdingList && holdingList.moving) {
+                                    // If we are moving we need to reset the cache buffer of the
+                                    // view that was not visible (i.e. !wasCurrentOnMoveStart) to 0 since
+                                    // otherwise the cache buffer we had set to preload the items of the
+                                    // visible range will trigger some item creations and we want move to
+                                    // be as smooth as possible meaning no need creations
+                                    if (!wasCurrentOnMoveStart) {
+                                        item.cacheBuffer = 0;
+                                    }
+                                } else {
+                                    item.cacheBuffer = categoryView.height * 1.5;
+                                }
+                            } else {
+                                var visibleRange = baseItem.height + displayMarginEnd + displayMarginBeginning;
+                                if (visibleRange < 0) {
+                                    item.displayMarginBeginning = displayMarginBeginning;
+                                    item.displayMarginEnd = displayMarginEnd;
                                     item.cacheBuffer = 0;
+                                } else {
+                                    // This should be visibleRange/2 in each of the properties
+                                    // but some item views still (like GridView) like creating sync delegates even if
+                                    // the visible range is 0 so let's make sure the visible range is negative
+                                    item.displayMarginBeginning = displayMarginBeginning - visibleRange;
+                                    item.displayMarginEnd = displayMarginEnd - visibleRange;
+                                    item.cacheBuffer = visibleRange;
                                 }
-                            } else {
-                                item.cacheBuffer = categoryView.height * 1.5;
                             }
                         } else {
-                            var visibleRange = baseItem.height + displayMarginEnd + displayMarginBeginning;
-                            if (visibleRange < 0) {
-                                item.displayMarginBeginning = displayMarginBeginning;
-                                item.displayMarginEnd = displayMarginEnd;
+                            var buffer = wasCurrentOnMoveStart ? categoryView.height * 1.5 : 0;
+                            var onViewport = baseItem.y + baseItem.height > 0 &&
+                                             baseItem.y < categoryView.height;
+                            var onBufferViewport = baseItem.y + baseItem.height > -buffer &&
+                                                   baseItem.y < categoryView.height + buffer;
+                            if (!onBufferViewport) {
+                                // If not on the buffered viewport, don't load anything
+                                item.displayMarginBeginning = 0;
+                                item.displayMarginEnd = -item.innerWidth;
                                 item.cacheBuffer = 0;
                             } else {
-                                // This should be visibleRange/2 in each of the properties
-                                // but some item views still (like GridView) like creating sync delegates even if
-                                // the visible range is 0 so let's make sure the visible range is negative
-                                item.displayMarginBeginning = displayMarginBeginning - visibleRange;
-                                item.displayMarginEnd = displayMarginEnd - visibleRange;
-                                item.cacheBuffer = visibleRange;
+                                if (onViewport && (scopeView.isCurrent || scopeView.visibleToParent)) {
+                                    // If on the buffered viewport and the viewport and the on a visible scope
+                                    // Set displayMargin so that cards are rendered
+                                    // And if not moving the parent list also give it some extra asynchronously
+                                    // buffering
+                                    item.displayMarginBeginning = 0;
+                                    item.displayMarginEnd = 0;
+                                    if (holdingList && holdingList.moving) {
+                                        // If we are moving we need to reset the cache buffer of the
+                                        // view that was not visible (i.e. !wasCurrentOnMoveStart) to 0 since
+                                        // otherwise the cache buffer we had set to preload the items of the
+                                        // visible range will trigger some item creations and we want move to
+                                        // be as smooth as possible meaning no need creations
+                                        if (!wasCurrentOnMoveStart) {
+                                            item.cacheBuffer = 0;
+                                        }
+                                    } else {
+                                        item.cacheBuffer = baseItem.width * 1.5;
+                                    }
+                                } else {
+                                    // If on the buffered viewport but either not in the real viewport
+                                    // or in the viewport of the non current scope, use displayMargin + cacheBuffer
+                                    // to render asynchronously the width of cards
+                                    item.displayMarginBeginning = 0;
+                                    item.displayMarginEnd = -item.innerWidth;
+                                    item.cacheBuffer = item.innerWidth;
+                                }
                             }
                         }
                     }