Don't create a separate section header for currentItem

The currentItem FxViewItem contained it's own section item,
which when created would cause the current item delegate
to be repositioned.  This change associates the section item
with the delegate item, via the attached object.

Change-Id: Ie675d545539b56d0f1cf5a9b4ea26668978a5e72
Reviewed-by: Bea Lam <bea.lam@nokia.com>

diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp
index 533e1f6..f62fa94 100644 (file)
--- a/src/quick/items/qquickitemview.cpp
+++ b/src/quick/items/qquickitemview.cpp
@@ -46,11 +46,12 @@ QT_BEGIN_NAMESPACE
 
 
 FxViewItem::FxViewItem(QQuickItem *i, bool own)
-    : item(i), ownItem(own), index(-1), releaseAfterTransition(false)
-    , transition(0)
-    , nextTransitionType(FxViewItemTransitionManager::NoTransition)
+    : item(i), ownItem(own), releaseAfterTransition(false)
     , isTransitionTarget(false)
     , nextTransitionToSet(false)
+    , index(-1)
+    , transition(0)
+    , nextTransitionType(FxViewItemTransitionManager::NoTransition)
 {
 }
 
@@ -2768,21 +2769,23 @@ void QQuickItemView::destroyingItem(QQuickItem *item)
     d->unrequestedItems.remove(item);
 }
 
-void QQuickItemViewPrivate::releaseItem(FxViewItem *item)
+bool QQuickItemViewPrivate::releaseItem(FxViewItem *item)
 {
     Q_Q(QQuickItemView);
     if (!item || !model)
-        return;
+        return true;
     if (trackedItem == item)
         trackedItem = 0;
     QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item->item);
     itemPrivate->removeItemChangeListener(this, QQuickItemPrivate::Geometry);
-    if (model->release(item->item) == 0) {
+    QQuickVisualModel::ReleaseFlags flags = model->release(item->item);
+    if (flags == 0) {
         // item was not destroyed, and we no longer reference it.
         item->item->setVisible(false);
         unrequestedItems.insert(item->item, model->indexOf(item->item, q));
     }
     delete item;
+    return flags != QQuickVisualModel::Referenced;
 }
 
 QQuickItem *QQuickItemViewPrivate::createHighlightItem()

diff --git a/src/quick/items/qquickitemview_p_p.h b/src/quick/items/qquickitemview_p_p.h
index 05927c0..fce6e4e 100644 (file)
--- a/src/quick/items/qquickitemview_p_p.h
+++ b/src/quick/items/qquickitemview_p_p.h
@@ -117,15 +117,15 @@ public:
 
     QQuickItem *item;
     bool ownItem;
-    int index;
     bool releaseAfterTransition;
+    bool isTransitionTarget;
+    bool nextTransitionToSet;
+    int index;
     QQuickItemViewAttached *attached;
 
     FxViewItemTransitionManager *transition;
     QPointF nextTransitionTo;
     FxViewItemTransitionManager::TransitionType nextTransitionType;
-    bool isTransitionTarget;
-    bool nextTransitionToSet;
 
 protected:
     void moveTo(const QPointF &pos);
@@ -225,7 +225,7 @@ public:
     void mirrorChange();
 
     FxViewItem *createItem(int modelIndex, bool asynchronous = false);
-    virtual void releaseItem(FxViewItem *item);
+    virtual bool releaseItem(FxViewItem *item);
 
     QQuickItem *createHighlightItem();
     QQuickItem *createComponentItem(QDeclarativeComponent *component, bool receiveItemGeometryChanges, bool createDefault = false);

diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp
index 6324c7d..0e643a1 100644 (file)
--- a/src/quick/items/qquicklistview.cpp
+++ b/src/quick/items/qquicklistview.cpp
@@ -93,7 +93,7 @@ public:
 
     virtual FxViewItem *newViewItem(int index, QQuickItem *item);
     virtual void initializeViewItem(FxViewItem *item);
-    virtual void releaseItem(FxViewItem *item);
+    virtual bool releaseItem(FxViewItem *item);
     virtual void repositionItemAt(FxViewItem *item, int index, qreal sizeBuffer);
     virtual void repositionPackageItemAt(QQuickItem *item, int index);
     virtual void resetFirstItemPosition(qreal pos = 0.0);
@@ -236,7 +236,7 @@ void QQuickViewSection::setLabelPositioning(int l)
 class FxListItemSG : public FxViewItem
 {
 public:
-    FxListItemSG(QQuickItem *i, QQuickListView *v, bool own) : FxViewItem(i, own), section(0), view(v) {
+    FxListItemSG(QQuickItem *i, QQuickListView *v, bool own) : FxViewItem(i, own), view(v) {
         attached = static_cast<QQuickListViewAttached*>(qmlAttachedPropertiesObject<QQuickListView>(item));
         if (attached)
             static_cast<QQuickListViewAttached*>(attached)->setView(view);
@@ -244,12 +244,21 @@ public:
 
     ~FxListItemSG() {}
 
+    inline QQuickItem *section() const {
+        return attached ? static_cast<QQuickListViewAttached*>(attached)->m_sectionItem : 0;
+    }
+    void setSection(QQuickItem *s) {
+        if (!attached)
+            attached = static_cast<QQuickListViewAttached*>(qmlAttachedPropertiesObject<QQuickListView>(item));
+        static_cast<QQuickListViewAttached*>(attached)->m_sectionItem = s;
+    }
+
     qreal position() const {
-        if (section) {
+        if (section()) {
             if (view->orientation() == QQuickListView::Vertical)
-                return section->y();
+                return section()->y();
             else
-                return (view->effectiveLayoutDirection() == Qt::RightToLeft ? -section->width()-section->x() : section->x());
+                return (view->effectiveLayoutDirection() == Qt::RightToLeft ? -section()->width()-section()->x() : section()->x());
         } else {
             return itemPosition();
         }
@@ -261,8 +270,8 @@ public:
             return (view->effectiveLayoutDirection() == Qt::RightToLeft ? -item->width()-itemX() : itemX());
     }
     qreal size() const {
-        if (section)
-            return (view->orientation() == QQuickListView::Vertical ? item->height()+section->height() : item->width()+section->width());
+        if (section())
+            return (view->orientation() == QQuickListView::Vertical ? item->height()+section()->height() : item->width()+section()->width());
         else
             return (view->orientation() == QQuickListView::Vertical ? item->height() : item->width());
     }
@@ -270,8 +279,8 @@ public:
         return (view->orientation() == QQuickListView::Vertical ? item->height() : item->width());
     }
     qreal sectionSize() const {
-        if (section)
-            return (view->orientation() == QQuickListView::Vertical ? section->height() : section->width());
+        if (section())
+            return (view->orientation() == QQuickListView::Vertical ? section()->height() : section()->width());
         return 0.0;
     }
     qreal endPosition() const {
@@ -285,14 +294,14 @@ public:
     }
     void setPosition(qreal pos) {
         // position the section immediately even if there is a transition
-        if (section) {
+        if (section()) {
             if (view->orientation() == QQuickListView::Vertical) {
-                section->setY(pos);
+                section()->setY(pos);
             } else {
                 if (view->effectiveLayoutDirection() == Qt::RightToLeft)
-                    section->setX(-section->width()-pos);
+                    section()->setX(-section()->width()-pos);
                 else
-                    section->setX(pos);
+                    section()->setX(pos);
             }
         }
         moveTo(pointForPosition(pos));
@@ -311,23 +320,22 @@ public:
         return view;
     }
 
-    QQuickItem *section;
     QQuickListView *view;
 
 private:
     QPointF pointForPosition(qreal pos) const {
         if (view->orientation() == QQuickListView::Vertical) {
-            if (section)
-                pos += section->height();
+            if (section())
+                pos += section()->height();
             return QPointF(itemX(), pos);
         } else {
             if (view->effectiveLayoutDirection() == Qt::RightToLeft) {
-                if (section)
-                    pos += section->width();
+                if (section())
+                    pos += section()->width();
                 return QPointF(-item->width() - pos, itemY());
             } else {
-                if (section)
-                    pos += section->width();
+                if (section())
+                    pos += section()->width();
                 return QPointF(pos, itemY());
             }
         }
@@ -566,25 +574,31 @@ void QQuickListViewPrivate::initializeViewItem(FxViewItem *item)
     }
 }
 
-void QQuickListViewPrivate::releaseItem(FxViewItem *item)
+bool QQuickListViewPrivate::releaseItem(FxViewItem *item)
 {
-    if (item) {
-        FxListItemSG* listItem = static_cast<FxListItemSG*>(item);
-        if (listItem->section) {
-            int i = 0;
-            do {
-                if (!sectionCache[i]) {
-                    sectionCache[i] = listItem->section;
-                    sectionCache[i]->setVisible(false);
-                    listItem->section = 0;
-                    break;
-                }
-                ++i;
-            } while (i < sectionCacheSize);
-            delete listItem->section;
-        }
+    if (!item || !model)
+        return true;
+
+    QQuickListViewAttached *att = static_cast<QQuickListViewAttached*>(item->attached);
+
+    bool released = QQuickItemViewPrivate::releaseItem(item);
+    if (released && att && att->m_sectionItem) {
+        // We hold no more references to this item
+        int i = 0;
+        do {
+            if (!sectionCache[i]) {
+                sectionCache[i] = att->m_sectionItem;
+                sectionCache[i]->setVisible(false);
+                att->m_sectionItem = 0;
+                break;
+            }
+            ++i;
+        } while (i < sectionCacheSize);
+        delete att->m_sectionItem;
+        att->m_sectionItem = 0;
     }
-    QQuickItemViewPrivate::releaseItem(item);
+
+    return released;
 }
 
 bool QQuickListViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, bool doBuffer)
@@ -944,18 +958,18 @@ void QQuickListViewPrivate::updateInlineSection(FxListItemSG *listItem)
     if (listItem->attached->m_prevSection != listItem->attached->m_section
             && (sectionCriteria->labelPositioning() & QQuickViewSection::InlineLabels
                 || (listItem->index == 0 && sectionCriteria->labelPositioning() & QQuickViewSection::CurrentLabelAtStart))) {
-        if (!listItem->section) {
+        if (!listItem->section()) {
             qreal pos = listItem->position();
-            listItem->section = getSectionItem(listItem->attached->m_section);
+            listItem->setSection(getSectionItem(listItem->attached->m_section));
             listItem->setPosition(pos);
         } else {
-            QDeclarativeContext *context = QDeclarativeEngine::contextForObject(listItem->section)->parentContext();
+            QDeclarativeContext *context = QDeclarativeEngine::contextForObject(listItem->section())->parentContext();
             context->setContextProperty(QLatin1String("section"), listItem->attached->m_section);
         }
-    } else if (listItem->section) {
+    } else if (listItem->section()) {
         qreal pos = listItem->position();
-        releaseSectionItem(listItem->section);
-        listItem->section = 0;
+        releaseSectionItem(listItem->section());
+        listItem->setSection(0);
         listItem->setPosition(pos);
     }
 }
@@ -972,7 +986,7 @@ void QQuickListViewPrivate::updateStickySections()
     QQuickItem *lastSectionItem = 0;
     int index = 0;
     while (index < visibleItems.count()) {
-        if (QQuickItem *section = static_cast<FxListItemSG *>(visibleItems.at(index))->section) {
+        if (QQuickItem *section = static_cast<FxListItemSG *>(visibleItems.at(index))->section()) {
             // Find the current section header and last visible section header
             // and hide them if they will overlap a static section header.
             qreal sectionPos = orient == QQuickListView::Vertical ? section->y() : section->x();
@@ -1173,9 +1187,9 @@ void QQuickListViewPrivate::initializeCurrentItem()
     if (currentItem) {
         FxListItemSG *listItem = static_cast<FxListItemSG *>(currentItem);
 
-        // don't reposition the item if it's about to be transitioned to another position
+        // don't reposition the item if it is already in the visibleItems list
         FxViewItem *actualItem = visibleItem(currentIndex);
-        if ((!actualItem || !actualItem->transitionScheduledOrRunning())) {
+        if (!actualItem) {
             if (currentIndex == visibleIndex - 1 && visibleItems.count()) {
                 // We can calculate exact postion in this case
                 listItem->setPosition(visibleItems.first()->position() - currentItem->size() - spacing);
@@ -1186,12 +1200,6 @@ void QQuickListViewPrivate::initializeCurrentItem()
             }
         }
 
-        // Avoid showing section delegate twice.  We still need the section heading so that
-        // currentItem positioning works correctly.
-        // This is slightly sub-optimal, but section heading caching minimizes the impact.
-        if (listItem->section)
-            listItem->section->setVisible(false);
-
         if (visibleItems.isEmpty())
             averageSize = listItem->size();
     }

diff --git a/src/quick/items/qquicklistview_p.h b/src/quick/items/qquicklistview_p.h
index bffd935..058c50f 100644 (file)
--- a/src/quick/items/qquicklistview_p.h
+++ b/src/quick/items/qquicklistview_p.h
@@ -181,7 +181,7 @@ class QQuickListViewAttached : public QQuickItemViewAttached
 
 public:
     QQuickListViewAttached(QObject *parent)
-        : QQuickItemViewAttached(parent), m_view(0) {}
+        : QQuickItemViewAttached(parent), m_view(0), m_sectionItem(0) {}
     ~QQuickListViewAttached() {}
 
     Q_PROPERTY(QQuickListView *view READ view NOTIFY viewChanged)
@@ -198,6 +198,7 @@ Q_SIGNALS:
 
 public:
     QDeclarativeGuard<QQuickListView> m_view;
+    QQuickItem *m_sectionItem;
 };
 
 

diff --git a/tests/auto/qtquick2/qquicklistview/data/sectionpropertychange.qml b/tests/auto/qtquick2/qquicklistview/data/sectionpropertychange.qml
new file mode 100644 (file)
index 0000000..f4679b5
--- /dev/null
+++ b/tests/auto/qtquick2/qquicklistview/data/sectionpropertychange.qml
@@ -0,0 +1,92 @@
+import QtQuick 2.0
+
+Rectangle {
+    width: 320; height: 480
+
+    Rectangle {
+        id: groupButtons
+        width: 300; height: 30
+        color: "yellow"
+        border.width: 1
+        Text {
+            anchors.centerIn: parent
+            text: "swap"
+        }
+        anchors {
+            top: parent.top
+            horizontalCenter: parent.horizontalCenter
+        }
+        MouseArea {
+            anchors.fill: parent
+            onClicked: switchGroups()
+        }
+    }
+
+    function switchGroups() {
+        if ("title" === myListView.groupBy)
+            myListView.groupBy = "genre"
+        else
+            myListView.groupBy = "title"
+    }
+
+    Component.onCompleted: {
+        myListView.model = generateModel(myListView)
+    }
+
+    ListView {
+        id: myListView
+        objectName: "list"
+
+        clip: true
+        property string groupBy: "title"
+
+        anchors {
+            top: groupButtons.bottom
+            left: parent.left
+            right: parent.right
+            bottom: parent.bottom
+        }
+
+        delegate: Item {
+            objectName: "wrapper"
+            height: 50
+            width: 320
+            Text { id: t; text: model.title }
+            Text { text: model.author; font.pixelSize: 10; anchors.top: t.bottom }
+            Text { text: parent.y; anchors.right: parent.right }
+        }
+
+        onGroupByChanged: {
+            model.move(0,1,1)
+            section.property = groupBy
+        }
+
+        section {
+            criteria: ViewSection.FullString
+            delegate: Rectangle { width: 320; height: 25; color: "lightblue"
+                objectName: "sect"
+                Text {text: section }
+                Text { text: parent.y; anchors.right: parent.right }
+            }
+            property: "title"
+        }
+    }
+
+    function generateModel(theParent)
+    {
+        var books = [
+                    { "author": "Billy Bob", "genre": "Anarchism", "title": "Frogs and Love" },
+                    { "author": "Lefty Smith", "genre": "Horror", "title": "Chainsaws for Noobs" }
+                ];
+
+        var model = Qt.createQmlObject("import QtQuick 2.0; ListModel {}", theParent);
+
+        for (var i = 0; i < books.length; ++i) {
+            var book = books[i];
+            model.append(book);
+        }
+        return model;
+    }
+
+}
+

diff --git a/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp b/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp
index 959bb2b..253df32 100644 (file)
--- a/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/qtquick2/qquicklistview/tst_qquicklistview.cpp
@@ -125,6 +125,7 @@ private slots:
     void qAbstractItemModel_sections();
     void sectionsPositioning();
     void sectionsDelegate();
+    void sectionPropertyChange();
     void cacheBuffer();
     void positionViewAtIndex();
     void resetModel();
@@ -2095,9 +2096,9 @@ void tst_QQuickListView::sectionsPositioning()
     QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false);
     model.removeItem(5);
     QTRY_COMPARE(listview->count(), model.count());
-    for (int i = 0; i < 3; ++i) {
-        QQuickItem *item = findItem<QQuickItem>(contentItem,
-                "sect_" + (i == 0 ? QString("aaa") : QString::number(i)));
+    for (int i = 1; i < 3; ++i) {
+        QQuickItem *item = findVisibleChild(contentItem,
+                "sect_" + QString::number(i));
         QVERIFY(item);
         QTRY_COMPARE(item->y(), qreal(i*20*6));
     }
@@ -2135,6 +2136,52 @@ void tst_QQuickListView::sectionsPositioning()
     delete canvas;
 }
 
+void tst_QQuickListView::sectionPropertyChange()
+{
+    QQuickView *canvas = createView();
+
+    canvas->setSource(testFileUrl("sectionpropertychange.qml"));
+    canvas->show();
+    qApp->processEvents();
+
+    QQuickListView *listview = findItem<QQuickListView>(canvas->rootObject(), "list");
+    QTRY_VERIFY(listview != 0);
+
+    QQuickItem *contentItem = listview->contentItem();
+    QTRY_VERIFY(contentItem != 0);
+
+    QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false);
+
+    // Confirm items positioned correctly
+    for (int i = 0; i < 2; ++i) {
+        QQuickItem *item = findItem<QQuickItem>(contentItem, "wrapper", i);
+        QTRY_VERIFY(item);
+        QTRY_COMPARE(item->y(), qreal(25. + i*75.));
+    }
+
+    QMetaObject::invokeMethod(canvas->rootObject(), "switchGroups");
+    QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false);
+
+    // Confirm items positioned correctly
+    for (int i = 0; i < 2; ++i) {
+        QQuickItem *item = findItem<QQuickItem>(contentItem, "wrapper", i);
+        QTRY_VERIFY(item);
+        QTRY_COMPARE(item->y(), qreal(25. + i*75.));
+    }
+
+    QMetaObject::invokeMethod(canvas->rootObject(), "switchGroups");
+    QTRY_COMPARE(QQuickItemPrivate::get(listview)->polishScheduled, false);
+
+    // Confirm items positioned correctly
+    for (int i = 0; i < 2; ++i) {
+        QQuickItem *item = findItem<QQuickItem>(contentItem, "wrapper", i);
+        QTRY_VERIFY(item);
+        QTRY_COMPARE(item->y(), qreal(25. + i*75.));
+    }
+
+    delete canvas;
+}
+
 void tst_QQuickListView::currentIndex_delayedItemCreation()
 {
     QFETCH(bool, setCurrentToZero);