Wrong bounding rect returned by QGraphicsEffect::boundingRect().

Regression after: f5c5e20a
Problem was that the cached bounding rect was never invalidated. We can
also remove the cached bounding rect in QGraphicsEffectSource because
QGraphicsItem::childrenBoundingRect now clips by default. This basically
means partially reverting above commit and invalidate whenever
ItemClipsChildrenToShape flag changes.

Auto test included.

Task-number: Discovered while investigating QT-3633
Reviewed-by: samuel

2 files changed, 12 insertions, 20 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 2600d06..364fdbb 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -1272,7 +1272,7 @@ 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, bool doClip)
+void QGraphicsItemPrivate::childrenBoundingRectHelper(QTransform *x, QRectF *rect)
 {
     Q_Q(QGraphicsItem);
 
@@ -1302,7 +1302,7 @@ void QGraphicsItemPrivate::childrenBoundingRectHelper(QTransform *x, QRectF *rec
         }
     }
 
-    if (doClip && (flags & QGraphicsItem::ItemClipsChildrenToShape)){
+    if (flags & QGraphicsItem::ItemClipsChildrenToShape){
         if (x)
             *rect &= x->mapRect(q->boundingRect());
         else
@@ -1870,6 +1870,10 @@ void QGraphicsItem::setFlags(GraphicsItemFlags flags)
         // Item children clipping changes. Propagate the ancestor flag to
         // all children.
         d_ptr->updateAncestorFlag(ItemClipsChildrenToShape);
+        // The childrenBoundingRect is clipped to the boundingRect in case of ItemClipsChildrenToShape,
+        // which means we have to invalidate the cached childrenBoundingRect whenever this flag changes.
+        d_ptr->dirtyChildrenBoundingRect = 1;
+        d_ptr->markParentDirty(true);
     }
 
     if ((flags & ItemIgnoresTransformations) != (oldFlags & ItemIgnoresTransformations)) {
@@ -11168,14 +11172,8 @@ QRectF QGraphicsItemEffectSourcePrivate::boundingRect(Qt::CoordinateSystem syste
     }
 
     QRectF rect = item->boundingRect();
-    if (!item->d_ptr->children.isEmpty()) {
-        if (dirtyChildrenBoundingRect) {
-            childrenBoundingRect = QRectF();
-            item->d_ptr->childrenBoundingRectHelper(0, &childrenBoundingRect, true);
-            dirtyChildrenBoundingRect = false;
-        }
-        rect |= childrenBoundingRect;
-    }
+    if (!item->d_ptr->children.isEmpty())
+        rect |= item->childrenBoundingRect();
 
     if (deviceCoordinates) {
         Q_ASSERT(info->painter);
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index cce9439..bc5e5ad 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -300,7 +300,7 @@ public:
     QDeclarativeListProperty<QGraphicsObject> childrenList();
     void setParentItemHelper(QGraphicsItem *parent, const QVariant *newParentVariant,
                              const QVariant *thisPointerVariant);
-    void childrenBoundingRectHelper(QTransform *x, QRectF *rect, bool doClip = true);
+    void childrenBoundingRectHelper(QTransform *x, QRectF *rect);
     void initStyleOption(QStyleOptionGraphicsItem *option, const QTransform &worldTransform,
                          const QRegion &exposedRegion, bool allItems = false) const;
     QRectF effectiveBoundingRect() const;
@@ -661,7 +661,7 @@ class QGraphicsItemEffectSourcePrivate : public QGraphicsEffectSourcePrivate
 {
 public:
     QGraphicsItemEffectSourcePrivate(QGraphicsItem *i)
-        : QGraphicsEffectSourcePrivate(), dirtyChildrenBoundingRect(true), item(i), info(0)
+        : QGraphicsEffectSourcePrivate(), item(i), info(0)
     {}
 
     inline void detach()
@@ -712,9 +712,6 @@ public:
                    QGraphicsEffect::PixmapPadMode mode) const;
     QRect paddedEffectRect(Qt::CoordinateSystem system, QGraphicsEffect::PixmapPadMode mode, const QRectF &sourceRect, bool *unpadded = 0) const;
 
-    mutable bool dirtyChildrenBoundingRect;
-    mutable QRectF childrenBoundingRect;
-
     QGraphicsItem *item;
     QGraphicsItemPaintInfo *info;
     QTransform lastEffectTransform;
@@ -872,12 +869,9 @@ inline void QGraphicsItemPrivate::markParentDirty(bool updateBoundingRect)
 #ifndef QT_NO_GRAPHICSEFFECT
         if (parentp->graphicsEffect) {
             if (updateBoundingRect) {
-                QGraphicsItemEffectSourcePrivate *sourcep =
-                    static_cast<QGraphicsItemEffectSourcePrivate *>(parentp->graphicsEffect->d_func()
-                                                                    ->source->d_func());
-                parentp->dirtyChildrenBoundingRect = 1;
+                static_cast<QGraphicsItemEffectSourcePrivate *>(parentp->graphicsEffect->d_func()
+                                                                ->source->d_func())->invalidateCache();
                 parentp->notifyInvalidated = 1;
-                sourcep->invalidateCache();
             }
             if (parentp->scene && parentp->graphicsEffect->isEnabled()) {
                 parentp->dirty = 1;