QGraphicsItem::childrenBoundingRect behavior breaks QGraphicsEffect::sourceBoundingRect().

Context:
QGraphicsEffect::sourceBoundingRect() returns:
item->boundingRect() | item->childrenBoundingRect();

Problem was that item->childrenBoundingRect() adjusted the children's
bounding rect with the children's ancestor effects (child -> root item),
which means the source bounding rect was bigger than needed. We should
only account for effects downwards in the hierarchy.

   root (has effect)
    |
   item (has effect)
    |
  child
    |
grandChild

Auto test included.

Task-number: QT-3633, QT-3828
Reviewed-by: samuel

3 files changed, 45 insertions, 12 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 364fdbb..b404692 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -1272,33 +1272,36 @@ void QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem *newParent, const Q
 
     Returns the bounding rect of this item's children (excluding itself).
 */
-void QGraphicsItemPrivate::childrenBoundingRectHelper(QTransform *x, QRectF *rect)
+void QGraphicsItemPrivate::childrenBoundingRectHelper(QTransform *x, QRectF *rect, QGraphicsItem *topMostEffectItem)
 {
     Q_Q(QGraphicsItem);
 
     QRectF childrenRect;
     QRectF *result = rect;
     rect = &childrenRect;
+    const bool setTopMostEffectItem = !topMostEffectItem;
 
     for (int i = 0; i < children.size(); ++i) {
         QGraphicsItem *child = children.at(i);
         QGraphicsItemPrivate *childd = child->d_ptr.data();
+        if (setTopMostEffectItem)
+            topMostEffectItem = child;
         bool hasPos = !childd->pos.isNull();
         if (hasPos || childd->transformData) {
             // COMBINE
             QTransform matrix = childd->transformToParent();
             if (x)
                 matrix *= *x;
-            *rect |= matrix.mapRect(child->d_ptr->effectiveBoundingRect());
+            *rect |= matrix.mapRect(child->d_ptr->effectiveBoundingRect(topMostEffectItem));
             if (!childd->children.isEmpty())
-                childd->childrenBoundingRectHelper(&matrix, rect);
+                childd->childrenBoundingRectHelper(&matrix, rect, topMostEffectItem);
         } else {
             if (x)
-                *rect |= x->mapRect(child->d_ptr->effectiveBoundingRect());
+                *rect |= x->mapRect(child->d_ptr->effectiveBoundingRect(topMostEffectItem));
             else
-                *rect |= child->d_ptr->effectiveBoundingRect();
+                *rect |= child->d_ptr->effectiveBoundingRect(topMostEffectItem);
             if (!childd->children.isEmpty())
-                childd->childrenBoundingRectHelper(x, rect);
+                childd->childrenBoundingRectHelper(x, rect, topMostEffectItem);
         }
     }
 
@@ -2804,6 +2807,8 @@ QRectF QGraphicsItemPrivate::effectiveBoundingRect(const QRectF &rect) const
     Q_Q(const QGraphicsItem);
     QGraphicsEffect *effect = graphicsEffect;
     if (scene && effect && effect->isEnabled()) {
+        if (scene->d_func()->views.isEmpty())
+            return effect->boundingRectFor(rect);
         QRectF sceneRect = q->mapRectToScene(rect);
         QRectF sceneEffectRect;
         foreach (QGraphicsView *view, scene->views()) {
@@ -2827,12 +2832,12 @@ QRectF QGraphicsItemPrivate::effectiveBoundingRect(const QRectF &rect) const
 
     \sa boundingRect()
 */
-QRectF QGraphicsItemPrivate::effectiveBoundingRect() const
+QRectF QGraphicsItemPrivate::effectiveBoundingRect(QGraphicsItem *topMostEffectItem) const
 {
 #ifndef QT_NO_GRAPHICSEFFECT
     Q_Q(const QGraphicsItem);
     QRectF brect = effectiveBoundingRect(q_ptr->boundingRect());
-    if (ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren)
+    if (ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren || topMostEffectItem == q)
         return brect;
 
     const QGraphicsItem *effectParent = parent;
@@ -2843,8 +2848,10 @@ QRectF QGraphicsItemPrivate::effectiveBoundingRect() const
             const QRectF effectRectInParentSpace = effectParent->d_ptr->effectiveBoundingRect(brectInParentSpace);
             brect = effectParent->mapRectToItem(q, effectRectInParentSpace);
         }
-        if (effectParent->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren)
+        if (effectParent->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren
+            || topMostEffectItem == effectParent) {
             return brect;
+        }
         effectParent = effectParent->d_ptr->parent;
     }
 
@@ -4719,7 +4726,7 @@ QRectF QGraphicsItem::childrenBoundingRect() const
         return d_ptr->childrenBoundingRect;
 
     d_ptr->childrenBoundingRect = QRectF();
-    d_ptr->childrenBoundingRectHelper(0, &d_ptr->childrenBoundingRect);
+    d_ptr->childrenBoundingRectHelper(0, &d_ptr->childrenBoundingRect, 0);
     d_ptr->dirtyChildrenBoundingRect = 0;
     return d_ptr->childrenBoundingRect;
 }
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index bc5e5ad..77e4054 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -300,10 +300,10 @@ public:
     QDeclarativeListProperty<QGraphicsObject> childrenList();
     void setParentItemHelper(QGraphicsItem *parent, const QVariant *newParentVariant,
                              const QVariant *thisPointerVariant);
-    void childrenBoundingRectHelper(QTransform *x, QRectF *rect);
+    void childrenBoundingRectHelper(QTransform *x, QRectF *rect, QGraphicsItem *topMostEffectItem);
     void initStyleOption(QStyleOptionGraphicsItem *option, const QTransform &worldTransform,
                          const QRegion &exposedRegion, bool allItems = false) const;
-    QRectF effectiveBoundingRect() const;
+    QRectF effectiveBoundingRect(QGraphicsItem *topMostEffectItem = 0) const;
     QRectF sceneEffectiveBoundingRect() const;
 
     QRectF effectiveBoundingRect(const QRectF &rect) const;
diff --git a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
index 985d466..07fa630 100644
--- a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
+++ b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
@@ -288,6 +288,32 @@ void tst_QGraphicsEffect::boundingRect2()
     // Disable ItemClipsChildrenToShape; effect's bounding rect is no longer clipped.
     child->setFlag(QGraphicsItem::ItemClipsChildrenToShape, false);
     QCOMPARE(effect->boundingRect(), effect->boundingRectFor(childRect | grandChildRect));
+
+    // Add root item to a scene, do the same tests as above. Results should be the same.
+    QGraphicsScene scene;
+    scene.addItem(root);
+
+    child->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+    QCOMPARE(effect->boundingRect(), effect->boundingRectFor(childRect));
+
+    child->setFlag(QGraphicsItem::ItemClipsChildrenToShape, false);
+    QCOMPARE(effect->boundingRect(), effect->boundingRectFor(childRect | grandChildRect));
+
+    // Now add the scene to a view, results should be the same.
+    QGraphicsView view(&scene);
+
+    child->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+    QCOMPARE(effect->boundingRect(), effect->boundingRectFor(childRect));
+
+    child->setFlag(QGraphicsItem::ItemClipsChildrenToShape, false);
+    QCOMPARE(effect->boundingRect(), effect->boundingRectFor(childRect | grandChildRect));
+
+    CustomEffect *childEffect = new CustomEffect;
+    child->setGraphicsEffect(childEffect);
+    QCOMPARE(effect->boundingRect(), effect->boundingRectFor(childEffect->boundingRectFor(childRect | grandChildRect)));
+
+    child->setGraphicsEffect(0);
+    QCOMPARE(effect->boundingRect(), effect->boundingRectFor(childRect | grandChildRect));
 }
 
 void tst_QGraphicsEffect::draw()