From 7925f107814e46deb0848d9e6016721ceebfb521 Mon Sep 17 00:00:00 2001
From: Christophe Oosterlynck <christophe.oosterlynck@dzine.be>
Date: Wed, 2 Nov 2011 10:00:12 +0100
Subject: Correction for effectiveBoundingRect() calculation for QGraphicsItem
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

QGraphicsItemPrivate::effectiveBoundingRect() should use ItemClipsChildrenToShape
flag from the parent while looping to check for clipping and not use
its own AncestorClipsChildren flag.
By using AncestorClipsChildren, you're checking if one of your ancestors
clips you, but you really want to know if your direct parent clips you.

Merge-request: 1419
Reviewed-by: Samuel Rødal <samuel.rodal@nokia.com>
---
 src/gui/graphicsview/qgraphicsitem.cpp             | 15 +++++++---
 tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp | 34 ++++++++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 73e8eed..ff21296 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -2845,16 +2845,23 @@ QRectF QGraphicsItemPrivate::effectiveBoundingRect(const QRectF &rect) const
     If the item has an effect, the effective rect can be larger than the item's
     bouding rect, depending on the effect.
 
+    \a topMostEffectItem is the top most parent of which a possible QGraphicsEffect
+    should be taken into account (\a topMostEffectItem is inclusive). Any effects
+    of any ancestors of \a topMostEffectItem are not taken into consideration.
+
     \sa boundingRect()
 */
 QRectF QGraphicsItemPrivate::effectiveBoundingRect(QGraphicsItem *topMostEffectItem) const
 {
 #ifndef QT_NO_GRAPHICSEFFECT
     Q_Q(const QGraphicsItem);
+    // Take into account the items own effect
     QRectF brect = effectiveBoundingRect(q_ptr->boundingRect());
-    if (ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren || topMostEffectItem == q)
+
+    if (topMostEffectItem == q)
         return brect;
 
+    // Take into account any effects applied to the parents
     const QGraphicsItem *effectParent = parent;
     while (effectParent) {
         QGraphicsEffect *effect = effectParent->d_ptr->graphicsEffect;
@@ -2863,10 +2870,10 @@ QRectF QGraphicsItemPrivate::effectiveBoundingRect(QGraphicsItem *topMostEffectI
             const QRectF effectRectInParentSpace = effectParent->d_ptr->effectiveBoundingRect(brectInParentSpace);
             brect = effectParent->mapRectToItem(q, effectRectInParentSpace);
         }
-        if (effectParent->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren
-            || topMostEffectItem == effectParent) {
+        if (effectParent && (effectParent->d_ptr->flags & QGraphicsItem::ItemClipsChildrenToShape))
+            return brect.intersected(q->mapRectFromItem(effectParent, effectParent->boundingRect()));
+        else if (topMostEffectItem == effectParent)
             return brect;
-        }
         effectParent = effectParent->d_ptr->parent;
     }
 
diff --git a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
index 9c189cb..3d45a07 100644
--- a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
+++ b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
@@ -68,6 +68,7 @@ private slots:
     void boundingRectFor();
     void boundingRect();
     void boundingRect2();
+    void boundingRect3();
     void draw();
     void opacity();
     void grayscale();
@@ -319,6 +320,39 @@ void tst_QGraphicsEffect::boundingRect2()
     QCOMPARE(effect->boundingRect(), effect->boundingRectFor(childRect | grandChildRect));
 }
 
+void tst_QGraphicsEffect::boundingRect3()
+{
+    QGraphicsScene scene;
+    QGraphicsRectItem *root = new QGraphicsRectItem;
+    scene.addItem(root);
+    QRectF rootRect(0, 0, 100, 100);
+    root->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+    root->setRect(rootRect);
+
+    QGraphicsRectItem *child = new QGraphicsRectItem;
+    child->setPos(5,5);
+    child->setParentItem(root);
+    CustomEffect *effect1 = new CustomEffect;
+    child->setGraphicsEffect(effect1);
+
+    QGraphicsRectItem *grandChild = new QGraphicsRectItem;
+    QRectF grandChildRect(0, 0, 50, 50);
+    grandChild->setRect(grandChildRect);
+    grandChild->setPos(10,10);
+    grandChild->setParentItem(child);
+    CustomEffect *effect2 = new CustomEffect;
+    grandChild->setGraphicsEffect(effect2);
+
+    QRectF effectiveBoundingRectGrandChild = effect2->boundingRectFor(grandChildRect);
+    QCOMPARE(effect2->boundingRect(), effectiveBoundingRectGrandChild);
+
+    QRectF effectiveBoundingRectChildAndGrandChild = effect1->boundingRectFor(child->mapRectFromItem(grandChild, effectiveBoundingRectGrandChild));
+    QCOMPARE(effect1->boundingRect(),effectiveBoundingRectChildAndGrandChild);
+
+    // The children bounding rect of the root should be the rectangle of the grandChild enlarged twice by the CustomEffect on the grandChild and the child, but clipped by the root item
+    QCOMPARE(root->childrenBoundingRect(), root->boundingRect().intersected(root->mapRectFromItem(child, effectiveBoundingRectChildAndGrandChild)));
+}
+
 void tst_QGraphicsEffect::draw()
 {
     QGraphicsScene scene;
-- 
cgit v0.12