Artifacts when moving a child when the parent has a graphics effect.

We have to use the effectiveBoundingRect() when finding out which items
to repaint within a specific area. However, we don't want items to be
clickable on the shadow, so we shouldn't use effectiveBoundingRect for
normal item-lookup. Solution to this is to only use
effectiveBoundingRect() in the BSP (used by estimateTopLevels) and in
drawSubtreeRecursive.

Auto-test included.

3 files changed, 113 insertions, 8 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index b1c19d8..78a5410 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -2227,8 +2227,11 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect)
         // Unset current effect.
         QGraphicsEffectPrivate *oldEffectPrivate = d_ptr->graphicsEffect->d_func();
         d_ptr->graphicsEffect = 0;
-        if (oldEffectPrivate)
+        if (oldEffectPrivate) {
             oldEffectPrivate->setGraphicsEffectSource(0); // deletes the current source.
+            if (d_ptr->scene) // Update the views directly.
+                d_ptr->scene->d_func()->markDirty(this, QRectF(), false, false, false, false, true);
+        }
     } else {
         // Set new effect.
         QGraphicsEffectSourcePrivate *sourced = new QGraphicsItemEffectSourcePrivate(this);
@@ -2237,8 +2240,7 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect)
         effect->d_func()->setGraphicsEffectSource(source);
     }
 
-    if (d_ptr->scene)
-        d_ptr->scene->d_func()->markDirty(this, QRectF(), false, false, false, false, !effect);
+    prepareGeometryChange();
 }
 
 /*!
@@ -2252,9 +2254,22 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect)
 */
 QRectF QGraphicsItem::effectiveBoundingRect() const
 {
-    if (d_ptr->graphicsEffect && d_ptr->graphicsEffect->isEnabled())
-        return d_ptr->graphicsEffect->boundingRect();
-    return boundingRect();
+    QGraphicsEffect *effect = d_ptr->graphicsEffect;
+    QRectF brect = effect && effect->isEnabled() ? effect->boundingRect() : boundingRect();
+    if (d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren)
+        return brect;
+
+    const QGraphicsItem *effectParent = d_ptr->parent;
+    while (effectParent) {
+        effect = effectParent->d_ptr->graphicsEffect;
+        if (effect && effect->isEnabled())
+            brect = effect->boundingRectFor(brect);
+        if (effectParent->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren)
+            return brect;
+        effectParent = effectParent->d_ptr->parent;
+    }
+
+    return brect;
 }
 
 /*!
diff --git a/src/gui/graphicsview/qgraphicsscenebsptreeindex.cpp b/src/gui/graphicsview/qgraphicsscenebsptreeindex.cpp
index 3cb33d1..78a77aa 100644
--- a/src/gui/graphicsview/qgraphicsscenebsptreeindex.cpp
+++ b/src/gui/graphicsview/qgraphicsscenebsptreeindex.cpp
@@ -173,7 +173,7 @@ void QGraphicsSceneBspTreeIndexPrivate::_q_updateIndex()
             if (item->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren)
                 continue;
 
-            bsp.insertItem(item, item->sceneBoundingRect());
+            bsp.insertItem(item, item->sceneEffectiveBoundingRect());
         }
     }
     unindexedItems.clear();
@@ -353,7 +353,7 @@ void QGraphicsSceneBspTreeIndexPrivate::removeItem(QGraphicsItem *item, bool rec
             purgePending = true;
             removedItems << item;
         } else if (!(item->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren)) {
-            bsp.removeItem(item, item->sceneBoundingRect());
+            bsp.removeItem(item, item->sceneEffectiveBoundingRect());
         }
     } else {
         unindexedItems.removeOne(item);
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index 011e480..ceb033e 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -57,6 +57,7 @@
 #include <QPainter>
 #include <QScrollBar>
 #include <QVBoxLayout>
+#include <QGraphicsEffect>
 
 //TESTED_CLASS=
 //TESTED_FILES=
@@ -274,6 +275,7 @@ private slots:
     void sorting();
     void itemHasNoContents();
     void hitTestUntransformableItem();
+    void hitTestGraphicsEffectItem();
     void focusProxy();
 
     // task specific tests below me
@@ -7385,6 +7387,94 @@ void tst_QGraphicsItem::hitTestUntransformableItem()
     QCOMPARE(items.at(0), static_cast<QGraphicsItem*>(item3));
 }
 
+void tst_QGraphicsItem::hitTestGraphicsEffectItem()
+{
+    QGraphicsScene scene;
+    scene.setSceneRect(-100, -100, 200, 200);
+
+    QGraphicsView view(&scene);
+    view.show();
+#ifdef Q_WS_X11
+    qt_x11_wait_for_window_manager(&view);
+#endif
+    QTest::qWait(100);
+
+    // Confuse the BSP with dummy items.
+    QGraphicsRectItem *dummy = new QGraphicsRectItem(0, 0, 20, 20);
+    dummy->setPos(-100, -100);
+    scene.addItem(dummy);
+    for (int i = 0; i < 100; ++i) {
+        QGraphicsItem *parent = dummy;
+        dummy = new QGraphicsRectItem(0, 0, 20, 20);
+        dummy->setPos(-100 + i, -100 + i);
+        dummy->setParentItem(parent);
+    }
+
+    const QRectF itemBoundingRect(0, 0, 20, 20);
+    EventTester *item1 = new EventTester;
+    item1->br = itemBoundingRect;
+    item1->setPos(-200, -200);
+
+    EventTester *item2 = new EventTester;
+    item2->br = itemBoundingRect;
+    item2->setFlag(QGraphicsItem::ItemIgnoresTransformations);
+    item2->setParentItem(item1);
+    item2->setPos(200, 200);
+
+    EventTester *item3 = new EventTester;
+    item3->br = itemBoundingRect;
+    item3->setParentItem(item2);
+    item3->setPos(80, 80);
+
+    scene.addItem(item1);
+    QTest::qWait(100);
+
+    item1->repaints = 0;
+    item2->repaints = 0;
+    item3->repaints = 0;
+
+    // Apply shadow effect to the entire sub-tree.
+    QGraphicsShadowEffect *shadow = new QGraphicsShadowEffect;
+    shadow->setShadowOffset(-20, -20);
+    item1->setGraphicsEffect(shadow);
+    QTest::qWait(50);
+
+    // Make sure all items are repainted.
+    QCOMPARE(item1->repaints, 1);
+    QCOMPARE(item2->repaints, 1);
+    QCOMPARE(item3->repaints, 1);
+
+    // Make sure an item doesn't respond to a click on its shadow.
+    QList<QGraphicsItem *> items = scene.items(QPointF(75, 75));
+    QVERIFY(items.isEmpty());
+    items = scene.items(QPointF(80, 80));
+    QCOMPARE(items.size(), 1);
+    QCOMPARE(items.at(0), static_cast<EventTester *>(item3));
+
+    item1->repaints = 0;
+    item2->repaints = 0;
+    item3->repaints = 0;
+
+    view.viewport()->update(75, 75, 20, 20);
+    QTest::qWait(50);
+
+    // item1 is the effect source and must therefore be repainted.
+    // item2 intersects with the exposed region
+    // item3 is just another child outside the exposed region
+    QCOMPARE(item1->repaints, 1);
+    QCOMPARE(item2->repaints, 1);
+    QCOMPARE(item3->repaints, 0);
+
+    scene.setItemIndexMethod(QGraphicsScene::NoIndex);
+    QTest::qWait(100);
+
+    items = scene.items(QPointF(75, 75));
+    QVERIFY(items.isEmpty());
+    items = scene.items(QPointF(80, 80));
+    QCOMPARE(items.size(), 1);
+    QCOMPARE(items.at(0), static_cast<EventTester *>(item3));
+}
+
 void tst_QGraphicsItem::focusProxy()
 {
     QGraphicsScene scene;