From 43f983350a548b1b663ea07a0e87e4cc58834214 Mon Sep 17 00:00:00 2001
From: J-P Nurmi <jpnurmi@digia.com>
Date: Wed, 19 Feb 2014 16:35:15 +0100
Subject: [PATCH] Fix item polishing

After "Dont call updatePolish if an item is not visible" (439f31f) and
"Fix polishItems bug" (01e609e), desktop style animations for QtQuick
Controls were no longer running. This was because QQuickItem::polish()
no longer polished a _visible_ item when there were other _hidden_
items in the queue to be polished.

This change restores the old logic that QQuickWindow only keeps track
of visible items to be polished, whilst the idea of hidden items not
being polished still remains valid. QQuickItem is made responsible
for polishing itself if necessary when it becomes visible.

Task-number: QTBUG-36934
Change-Id: I4d48d3a3e2c841d337cd52ec4fd27092f84a8626
Reviewed-by: Gunnar Sletta <gunnar.sletta@jollamobile.com>
---
 src/quick/items/qquickitem.cpp                |  4 +-
 src/quick/items/qquickwindow.cpp              | 17 ++----
 .../auto/quick/qquickitem/tst_qquickitem.cpp  | 61 +++++++++++++++++++
 3 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index 3b63028a2e..62ed5e0c85 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -3857,7 +3857,7 @@ void QQuickItem::polish()
     Q_D(QQuickItem);
     if (!d->polishScheduled) {
         d->polishScheduled = true;
-        if (d->window) {
+        if (d->window && (isVisible() || (d->extra.isAllocated() && d->extra->effectRefCount>0))) {
             QQuickWindowPrivate *p = QQuickWindowPrivate::get(d->window);
             bool maybeupdate = p->itemsToPolish.isEmpty();
             p->itemsToPolish.insert(this);
@@ -5241,6 +5241,8 @@ bool QQuickItemPrivate::setEffectiveVisibleRecur(bool newEffectiveVisible)
         QQuickWindowPrivate *windowPriv = QQuickWindowPrivate::get(window);
         if (windowPriv->mouseGrabberItem == q)
             q->ungrabMouse();
+        if (polishScheduled)
+            windowPriv->itemsToPolish.insert(q);
     }
 
     bool childVisibilityChanged = false;
diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp
index bc8558e1d8..7f62267859 100644
--- a/src/quick/items/qquickwindow.cpp
+++ b/src/quick/items/qquickwindow.cpp
@@ -258,23 +258,16 @@ void QQuickWindowPrivate::polishItems()
 {
     int maxPolishCycles = 100000;
 
-    int removedItems;
-    do {
-        removedItems = 0;
+    while (!itemsToPolish.isEmpty() && --maxPolishCycles > 0) {
         QSet<QQuickItem *> itms = itemsToPolish;
+        itemsToPolish.clear();
 
         for (QSet<QQuickItem *>::iterator it = itms.begin(); it != itms.end(); ++it) {
             QQuickItem *item = *it;
-            QQuickItemPrivate *itemPrivate = QQuickItemPrivate::get(item);
-
-            if (item->isVisible() || (itemPrivate->extra.isAllocated() && itemPrivate->extra->effectRefCount>0)) {
-                itemPrivate->polishScheduled = false;
-                itemsToPolish.remove(item);
-                item->updatePolish();
-                ++removedItems;
-            }
+            QQuickItemPrivate::get(item)->polishScheduled = false;
+            item->updatePolish();
         }
-    } while (removedItems > 0 && --maxPolishCycles > 0);
+    }
 
     if (maxPolishCycles == 0)
         qWarning("QQuickWindow: possible QQuickItem::polish() loop");
diff --git a/tests/auto/quick/qquickitem/tst_qquickitem.cpp b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
index f4f2374183..efe9266d0a 100644
--- a/tests/auto/quick/qquickitem/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
@@ -40,11 +40,13 @@
 ****************************************************************************/
 
 #include <qtest.h>
+#include <qsignalspy.h>
 
 #include <QtQuick/qquickitem.h>
 #include <QtQuick/qquickwindow.h>
 #include <QtQuick/qquickview.h>
 #include "private/qquickfocusscope_p.h"
+#include "private/qquickwindow_p.h"
 #include "private/qquickitem_p.h"
 #include <qpa/qwindowsysteminterface.h>
 #include <QDebug>
@@ -156,6 +158,8 @@ private slots:
     void touchEventAcceptIgnore();
     void polishOutsideAnimation();
     void polishOnCompleted();
+    void polishLaterWhenVisible();
+    void polishWhenOtherHidden();
 
     void wheelEvent_data();
     void wheelEvent();
@@ -1396,6 +1400,63 @@ void tst_qquickitem::polishOnCompleted()
     QTRY_VERIFY(item->wasPolished);
 }
 
+void tst_qquickitem::polishLaterWhenVisible()
+{
+    QQuickWindow window;
+    QQuickWindowPrivate *wp = QQuickWindowPrivate::get(&window);
+    window.resize(200, 200);
+    window.show();
+    QTest::qWaitForWindowExposed(&window);
+
+    TestPolishItem *item = new TestPolishItem(window.contentItem());
+    item->setSize(QSizeF(200, 100));
+    item->setVisible(false);
+    item->polish();
+
+    QVERIFY(!wp->itemsToPolish.contains(item));
+    window.grabWindow(); // trigger QQuickWindowPrivate::polishItems()
+    QVERIFY(!item->wasPolished);
+
+    item->setVisible(true);
+    QVERIFY(wp->itemsToPolish.contains(item));
+    window.grabWindow(); // trigger QQuickWindowPrivate::polishItems()
+    QVERIFY(item->wasPolished);
+    QVERIFY(!wp->itemsToPolish.contains(item));
+}
+
+void tst_qquickitem::polishWhenOtherHidden()
+{
+    QQuickWindow window;
+    QQuickWindowPrivate *wp = QQuickWindowPrivate::get(&window);
+    window.resize(200, 200);
+    window.show();
+    QTest::qWaitForWindowExposed(&window);
+
+    // a hidden item pending for polish...
+    TestPolishItem *hiddenItem = new TestPolishItem(window.contentItem());
+    hiddenItem->setSize(QSizeF(200, 100));
+    hiddenItem->setVisible(false);
+    hiddenItem->polish();
+
+    QVERIFY(!wp->itemsToPolish.contains(hiddenItem));
+    window.grabWindow(); // trigger QQuickWindowPrivate::polishItems()
+    QVERIFY(!hiddenItem->wasPolished);
+
+    // ...should not block a visible item from being polished
+    TestPolishItem *visibleItem = new TestPolishItem(window.contentItem());
+    visibleItem->setSize(QSizeF(200, 100));
+    visibleItem->setVisible(true);
+    visibleItem->polish();
+
+    QVERIFY(wp->itemsToPolish.contains(visibleItem));
+    QVERIFY(!wp->itemsToPolish.contains(hiddenItem));
+    window.grabWindow(); // trigger QQuickWindowPrivate::polishItems()
+    QVERIFY(visibleItem->wasPolished);
+    QVERIFY(!hiddenItem->wasPolished);
+    QVERIFY(!wp->itemsToPolish.contains(visibleItem));
+    QVERIFY(!wp->itemsToPolish.contains(hiddenItem));
+}
+
 void tst_qquickitem::wheelEvent_data()
 {
     QTest::addColumn<bool>("visible");
-- 
GitLab