Fixes visibility update missing when doing setParentItem on graphicsitem

Calling setParentItem is causing the previous opacity/visible updates
to be discarded because the dirty flags were not propagated to the new parent.

Also removed some unnecessary 'markDirty' and 'update' calls.

Task-number: QTBUG-6738
Reviewed-by: bnilsen

3 files changed, 41 insertions, 36 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index f81cb23..2089206 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -1115,11 +1115,9 @@ void QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem *newParent, const Q
     }
 
     if ((parent = newParent)) {
-        bool implicitUpdate = false;
         if (parent->d_func()->scene && parent->d_func()->scene != scene) {
             // Move this item to its new parent's scene
             parent->d_func()->scene->addItem(q);
-            implicitUpdate = true;
         } else if (!parent->d_func()->scene && scene) {
             // Remove this item from its former scene
             scene->removeItem(q);
@@ -1129,25 +1127,25 @@ void QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem *newParent, const Q
         if (thisPointerVariant)
             parent->itemChange(QGraphicsItem::ItemChildAddedChange, *thisPointerVariant);
         if (scene) {
-            if (!implicitUpdate)
-                scene->d_func()->markDirty(q_ptr);
-
             // Re-enable scene pos notifications for new ancestors
             if (scenePosDescendants || (flags & QGraphicsItem::ItemSendsScenePositionChanges))
                 scene->d_func()->setScenePosItemEnabled(q, true);
         }
 
+        // Propagate dirty flags to the new parent
+        markParentDirty(/*updateBoundingRect=*/true);
+
         // Inherit ancestor flags from the new parent.
         updateAncestorFlags();
 
         // Update item visible / enabled.
         if (parent->d_ptr->visible != visible) {
             if (!parent->d_ptr->visible || !explicitlyHidden)
-                setVisibleHelper(parent->d_ptr->visible, /* explicit = */ false, /* update = */ !implicitUpdate);
+                setVisibleHelper(parent->d_ptr->visible, /* explicit = */ false, /* update = */ false);
         }
         if (parent->isEnabled() != enabled) {
             if (!parent->d_ptr->enabled || !explicitlyDisabled)
-                setEnabledHelper(parent->d_ptr->enabled, /* explicit = */ false, /* update = */ !implicitUpdate);
+                setEnabledHelper(parent->d_ptr->enabled, /* explicit = */ false, /* update = */ false);
         }
 
         // Auto-activate if visible and the parent is active.
@@ -1163,10 +1161,6 @@ void QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem *newParent, const Q
                 setVisibleHelper(true, /* explicit = */ false);
             if (!enabled && !explicitlyDisabled)
                 setEnabledHelper(true, /* explicit = */ false);
-
-            // If the item is being deleted, the whole scene will be updated.
-            if (scene)
-                scene->d_func()->markDirty(q_ptr);
         }
     }
 
@@ -7265,19 +7259,7 @@ void QGraphicsItem::prepareGeometryChange()
         }
     }
 
-    QGraphicsItem *parent = this;
-    while ((parent = parent->d_ptr->parent)) {
-        QGraphicsItemPrivate *parentp = parent->d_ptr.data();
-        parentp->dirtyChildrenBoundingRect = 1;
-        // ### Only do this if the parent's effect applies to the entire subtree.
-        parentp->notifyBoundingRectChanged = 1;
-#ifndef QT_NO_GRAPHICSEFFECT
-        if (parentp->scene && parentp->graphicsEffect) {
-            parentp->notifyInvalidated = 1;
-            static_cast<QGraphicsItemEffectSourcePrivate *>(parentp->graphicsEffect->d_func()->source->d_func())->invalidateCache();
-        }
-#endif
-    }
+    d_ptr->markParentDirty(/*updateBoundingRect=*/true);
 }
 
 /*!
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index 986a977..5ad6cd5 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -406,6 +406,8 @@ public:
         return !visible || (childrenCombineOpacity() && isFullyTransparent());
     }
 
+    inline void markParentDirty(bool updateBoundingRect = false);
+
     void setFocusHelper(Qt::FocusReason focusReason, bool climb);
     void setSubFocus(QGraphicsItem *rootItem = 0);
     void clearSubFocus(QGraphicsItem *rootItem = 0);
@@ -754,6 +756,37 @@ inline bool QGraphicsItemPrivate::insertionOrder(QGraphicsItem *a, QGraphicsItem
     return a->d_ptr->siblingIndex < b->d_ptr->siblingIndex;
 }
 
+/*!
+    \internal
+*/
+inline void QGraphicsItemPrivate::markParentDirty(bool updateBoundingRect)
+{
+    QGraphicsItemPrivate *parentp = this;
+    while (parentp->parent) {
+        parentp = parentp->parent->d_ptr.data();
+        parentp->dirtyChildren = 1;
+
+        if (updateBoundingRect) {
+            parentp->dirtyChildrenBoundingRect = 1;
+            // ### Only do this if the parent's effect applies to the entire subtree.
+            parentp->notifyBoundingRectChanged = 1;
+        }
+#ifndef QT_NO_GRAPHICSEFFECT
+        if (parentp->graphicsEffect) {
+            if (updateBoundingRect) {
+                parentp->notifyInvalidated = 1;
+                static_cast<QGraphicsItemEffectSourcePrivate *>(parentp->graphicsEffect->d_func()
+                                                                ->source->d_func())->invalidateCache();
+            }
+            if (parentp->graphicsEffect->isEnabled()) {
+                parentp->dirty = 1;
+                parentp->fullUpdatePending = 1;
+            }
+        }
+#endif
+    }
+}
+
 QT_END_NAMESPACE
 
 #endif // QT_NO_GRAPHICSVIEW
diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index ae48bee..9219773 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -1955,7 +1955,7 @@ QList<QGraphicsItem *> QGraphicsScene::items(const QRectF &rectangle, Qt::ItemSe
     \since 4.3
 
     This convenience function is equivalent to calling items(QRectF(\a x, \a y, \a w, \a h), \a mode).
-    
+
     This function is deprecated and returns incorrect results if the scene
     contains items that ignore transformations. Use the overload that takes
     a QTransform instead.
@@ -4903,17 +4903,7 @@ void QGraphicsScenePrivate::markDirty(QGraphicsItem *item, const QRectF &rect, b
     if (ignoreOpacity)
         item->d_ptr->ignoreOpacity = 1;
 
-    QGraphicsItem *p = item->d_ptr->parent;
-    while (p) {
-        p->d_ptr->dirtyChildren = 1;
-#ifndef QT_NO_GRAPHICSEFFECT
-        if (p->d_ptr->graphicsEffect && p->d_ptr->graphicsEffect->isEnabled()) {
-            p->d_ptr->dirty = 1;
-            p->d_ptr->fullUpdatePending = 1;
-        }
-#endif //QT_NO_GRAPHICSEFFECT
-        p = p->d_ptr->parent;
-    }
+    item->d_ptr->markParentDirty();
 }
 
 static inline bool updateHelper(QGraphicsViewPrivate *view, QGraphicsItemPrivate *item,