Fixed memory leaks when removing a QGraphicsEffect from a QGraphicsItem or QWidget

with setGraphicsEffect(0).

The effect was not deleted in that case, problem solved for both QGraphicsItem
and QWidget.

Autotest included.

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

7 files changed, 45 insertions, 28 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index f93ffdf..9e8d587 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -2563,32 +2563,19 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect)
     if (d_ptr->graphicsEffect == effect)
         return;
 
-    if (d_ptr->graphicsEffect && effect) {
+    if (d_ptr->graphicsEffect) {
         delete d_ptr->graphicsEffect;
         d_ptr->graphicsEffect = 0;
     }
 
-    if (!effect) {
-        // Unset current effect.
-        QGraphicsEffectPrivate *oldEffectPrivate = d_ptr->graphicsEffect->d_func();
-        d_ptr->graphicsEffect = 0;
-        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(), /*invalidateChildren=*/false,
-                                                  /*force=*/false, /*ignoreOpacity=*/false,
-                                                  /*removeItemFromScene=*/true);
-            }
-        }
-    } else {
+    if (effect) {
         // Set new effect.
         QGraphicsEffectSourcePrivate *sourced = new QGraphicsItemEffectSourcePrivate(this);
         QGraphicsEffectSource *source = new QGraphicsEffectSource(*sourced);
         d_ptr->graphicsEffect = effect;
         effect->d_func()->setGraphicsEffectSource(source);
+        prepareGeometryChange();
     }
-
-    prepareGeometryChange();
 }
 #endif //QT_NO_GRAPHICSEFFECT
 
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index d6ffb1a..8f9fe54 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -562,7 +562,10 @@ public:
     {}
 
     inline void detach()
-    { item->setGraphicsEffect(0); }
+    {
+        item->d_ptr->graphicsEffect = 0;
+        item->prepareGeometryChange();
+    }
 
     inline const QGraphicsItem *graphicsItem() const
     { return item; }
diff --git a/src/gui/kernel/qwidget.cpp b/src/gui/kernel/qwidget.cpp
index d19c574..00c7dc0 100644
--- a/src/gui/kernel/qwidget.cpp
+++ b/src/gui/kernel/qwidget.cpp
@@ -5059,28 +5059,22 @@ void QWidget::setGraphicsEffect(QGraphicsEffect *effect)
     if (d->graphicsEffect == effect)
         return;
 
-    if (d->graphicsEffect && effect) {
+    if (d->graphicsEffect) {
+        d->invalidateBuffer(rect());
         delete d->graphicsEffect;
         d->graphicsEffect = 0;
     }
 
-    if (!effect) {
-        // Unset current effect.
-        QGraphicsEffectPrivate *oldEffectPrivate = d->graphicsEffect->d_func();
-        d->graphicsEffect = 0;
-        if (oldEffectPrivate) {
-            oldEffectPrivate->setGraphicsEffectSource(0); // deletes the current source.
-        }
-    } else {
+    if (effect) {
         // Set new effect.
         QGraphicsEffectSourcePrivate *sourced = new QWidgetEffectSourcePrivate(this);
         QGraphicsEffectSource *source = new QGraphicsEffectSource(*sourced);
         d->graphicsEffect = effect;
         effect->d_func()->setGraphicsEffectSource(source);
+        update();
     }
 
     d->updateIsOpaque();
-    update();
 }
 #endif //QT_NO_GRAPHICSEFFECT
 
diff --git a/src/gui/kernel/qwidget.h b/src/gui/kernel/qwidget.h
index 5ba1d23..0824db3 100644
--- a/src/gui/kernel/qwidget.h
+++ b/src/gui/kernel/qwidget.h
@@ -744,6 +744,7 @@ private:
     friend struct QWidgetExceptionCleaner;
     friend class QGestureManager;
     friend class QWinNativePanGestureRecognizer;
+    friend class QWidgetEffectSourcePrivate;
 
 #ifdef Q_WS_MAC
     friend class QCoreGraphicsPaintEnginePrivate;
diff --git a/src/gui/kernel/qwidget_p.h b/src/gui/kernel/qwidget_p.h
index 04cf4bb..5d73951 100644
--- a/src/gui/kernel/qwidget_p.h
+++ b/src/gui/kernel/qwidget_p.h
@@ -823,7 +823,7 @@ public:
     {}
 
     inline void detach()
-    { m_widget->setGraphicsEffect(0); }
+    { m_widget->d_func()->graphicsEffect = 0; }
 
     inline const QGraphicsItem *graphicsItem() const
     { return 0; }
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index 565a3e7..6266933 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -7962,6 +7962,22 @@ void tst_QGraphicsItem::setGraphicsEffect()
     delete item;
     QVERIFY(!blurEffect);
     delete anotherItem;
+
+    // Ensure the effect is uninstalled when deleting it
+    item = new QGraphicsRectItem(0, 0, 10, 10);
+    blurEffect = new QGraphicsBlurEffect;
+    item->setGraphicsEffect(blurEffect);
+    delete blurEffect;
+    QVERIFY(!item->graphicsEffect());
+
+    // Ensure the existing effect is uninstalled and deleted when setting a null effect
+    blurEffect = new QGraphicsBlurEffect;
+    item->setGraphicsEffect(blurEffect);
+    item->setGraphicsEffect(0);
+    QVERIFY(!item->graphicsEffect());
+    QVERIFY(!blurEffect);
+
+    delete item;
 }
 
 void tst_QGraphicsItem::panel()
diff --git a/tests/auto/qwidget/tst_qwidget.cpp b/tests/auto/qwidget/tst_qwidget.cpp
index 9960f47..3e14b56 100644
--- a/tests/auto/qwidget/tst_qwidget.cpp
+++ b/tests/auto/qwidget/tst_qwidget.cpp
@@ -9549,6 +9549,22 @@ void tst_QWidget::setGraphicsEffect()
     delete widget;
     QVERIFY(!blurEffect);
     delete anotherWidget;
+
+    // Ensure the effect is uninstalled when deleting it
+    widget = new QWidget;
+    blurEffect = new QGraphicsBlurEffect;
+    widget->setGraphicsEffect(blurEffect);
+    delete blurEffect;
+    QVERIFY(!widget->graphicsEffect());
+
+    // Ensure the existing effect is uninstalled and deleted when setting a null effect
+    blurEffect = new QGraphicsBlurEffect;
+    widget->setGraphicsEffect(blurEffect);
+    widget->setGraphicsEffect(0);
+    QVERIFY(!widget->graphicsEffect());
+    QVERIFY(!blurEffect);
+
+    delete widget;
 }
 
 void tst_QWidget::activateWindow()