Prevent unnecessary graphics item updates when graphics effect changes.

Don't invalidate a QGraphicsItem (neither its cache) when an update is
triggered because of a QGraphicsEffect attached to it.

Autotest for QGraphicsEffect extended with 2 cache invalidation tests

Merge-request: 2681
Reviewed-by: Samuel Rødal <samuel.rodal@nokia.com>

2 files changed, 115 insertions, 16 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 0c218fc..9092593 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -5378,11 +5378,9 @@ void QGraphicsItemPrivate::invalidateParentGraphicsEffectsRecursively()
 {
     QGraphicsItemPrivate *itemPrivate = this;
     do {
-        if (itemPrivate->graphicsEffect) {
+        if (itemPrivate->graphicsEffect && !itemPrivate->updateDueToGraphicsEffect) {
             itemPrivate->notifyInvalidated = 1;
-
-            if (!itemPrivate->updateDueToGraphicsEffect)
-                static_cast<QGraphicsItemEffectSourcePrivate *>(itemPrivate->graphicsEffect->d_func()->source->d_func())->invalidateCache();
+            static_cast<QGraphicsItemEffectSourcePrivate *>(itemPrivate->graphicsEffect->d_func()->source->d_func())->invalidateCache();
         }
     } while ((itemPrivate = itemPrivate->parent ? itemPrivate->parent->d_ptr.data() : 0));
 }
@@ -5690,21 +5688,27 @@ void QGraphicsItem::update(const QRectF &rect)
     d_ptr->invalidateParentGraphicsEffectsRecursively();
 #endif //QT_NO_GRAPHICSEFFECT
 
-    if (CacheMode(d_ptr->cacheMode) != NoCache) {
-        // Invalidate cache.
-        QGraphicsItemCache *cache = d_ptr->extraItemCache();
-        if (!cache->allExposed) {
-            if (rect.isNull()) {
-                cache->allExposed = true;
-                cache->exposed.clear();
-            } else {
-                cache->exposed.append(rect);
+#ifndef QT_NO_GRAPHICSEFFECT
+    if (!d_ptr->updateDueToGraphicsEffect) {
+#endif
+        if (CacheMode(d_ptr->cacheMode) != NoCache) {
+            // Invalidate cache.
+            QGraphicsItemCache *cache = d_ptr->extraItemCache();
+            if (!cache->allExposed) {
+                if (rect.isNull()) {
+                    cache->allExposed = true;
+                    cache->exposed.clear();
+                } else {
+                    cache->exposed.append(rect);
+                }
             }
+            // Only invalidate cache; item is already dirty.
+            if (d_ptr->fullUpdatePending)
+                return;
         }
-        // Only invalidate cache; item is already dirty.
-        if (d_ptr->fullUpdatePending)
-            return;
+#ifndef QT_NO_GRAPHICSEFFECT
     }
+#endif
 
     if (d_ptr->scene)
         d_ptr->scene->d_func()->markDirty(this, rect);
diff --git a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
index 1f5563c..9c189cb 100644
--- a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
+++ b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
@@ -78,6 +78,8 @@ private slots:
     void dropShadowClipping();
     void childrenVisibilityShouldInvalidateCache();
     void prepareGeometryChangeInvalidateCache();
+    void graphicsEffectUpdateShouldNotInvalidateGraphicsItemCache();
+    void graphicsEffectUpdateShouldInvalidateParentGraphicsEffect();
     void itemHasNoContents();
 };
 
@@ -732,6 +734,99 @@ void tst_QGraphicsEffect::prepareGeometryChangeInvalidateCache()
     QCOMPARE(item->nbPaint, 0);
 }
 
+class MyGraphicsEffect : public QGraphicsEffect
+{
+    public:
+        MyGraphicsEffect(QObject *parent = 0) :
+            QGraphicsEffect(parent), nbSourceInvalidations(0)
+            {}
+        int nbSourceInvalidations;
+    protected:
+        void draw(QPainter *painter)
+        {
+            drawSource(painter);
+        }
+
+        void sourceChanged(ChangeFlags flags)
+        {
+            if (flags == SourceInvalidated)
+                nbSourceInvalidations++;
+        }
+};
+
+void tst_QGraphicsEffect::graphicsEffectUpdateShouldNotInvalidateGraphicsItemCache()
+{
+    QGraphicsScene scene;
+    MyGraphicsItem parent;
+    parent.resize(200, 200);
+    parent.setCacheMode(QGraphicsItem::ItemCoordinateCache);
+    scene.addItem(&parent);
+
+    QGraphicsView view(&scene);
+    view.show();
+    QApplication::setActiveWindow(&view);
+    QTest::qWaitForWindowShown(&view);
+    QTRY_COMPARE(parent.nbPaint, 1);
+
+    //we set an effect on the parent
+    MyGraphicsEffect* opacityEffect = new MyGraphicsEffect(&parent);
+    opacityEffect->update();
+    parent.setGraphicsEffect(opacityEffect);
+    //flush the events
+    QApplication::processEvents();
+    //new effect applied->repaint
+    QCOMPARE(parent.nbPaint, 1);
+
+    opacityEffect->update();
+    //flush the events
+    QApplication::processEvents();
+    //A change to the effect shouldn't invalidate the graphicsitem's cache
+    // => it shouldn't trigger a paint
+    QCOMPARE(parent.nbPaint, 1);
+}
+
+void tst_QGraphicsEffect::graphicsEffectUpdateShouldInvalidateParentGraphicsEffect()
+{
+    QGraphicsScene scene;
+    // Add the parent
+    MyGraphicsItem parent;
+    parent.resize(200, 200);
+    scene.addItem(&parent);
+    // Add a child to the parent
+    MyGraphicsItem child(&parent);
+    child.resize(100, 100);
+
+    QGraphicsView view(&scene);
+    view.show();
+    QApplication::setActiveWindow(&view);
+    QTest::qWaitForWindowShown(&view);
+    //flush the events
+    QApplication::processEvents();
+    QTRY_COMPARE(parent.nbPaint, 1);
+    QTRY_COMPARE(child.nbPaint, 1);
+
+    //we set an effect on the parent and the child
+    MyGraphicsEffect* effectForParent = new MyGraphicsEffect(&parent);
+    parent.setGraphicsEffect(effectForParent);
+
+    MyGraphicsEffect* effectForChild = new MyGraphicsEffect(&child);
+    child.setGraphicsEffect(effectForChild);
+    //flush the events
+    QApplication::processEvents();
+    // Both effects should start with no source invalidations
+    QCOMPARE(effectForParent->nbSourceInvalidations, 0);
+    QCOMPARE(effectForChild->nbSourceInvalidations, 0);
+
+    // Trigger an update of the child graphics effect
+    effectForChild->update();
+    //flush the events
+    QApplication::processEvents();
+    // An update of the effect on the child shouldn't tell that effect that its source has been invalidated
+    QCOMPARE(effectForChild->nbSourceInvalidations, 0);
+    // The effect on the parent should however be notified of an invalidated source
+    QCOMPARE(effectForParent->nbSourceInvalidations, 1);
+}
+
 void tst_QGraphicsEffect::itemHasNoContents()
 {
     QGraphicsRectItem *parent = new QGraphicsRectItem;