QGraphicsItem::setOpacity(0.0) does not trigger an update.

The problem was that we discarded update requests for fully
transparent items, which is correct, but we even did that
when the update was issued from QGraphicsItem::setOpacity.
We don't have to, and shouldn't, consider the opacity in
that case. Whenever we reach the fullUpdateHelper call in
setOpacity it means we have to do an update regardless of
the current opacity (oldOpacity was not 0.0 if the
currentOpacity is 0.0).

Auto-test included.

Task-number: 252913
Reviewed-by: Andreas

4 files changed, 102 insertions, 16 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 3e8d38f..11ec24b 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -1921,7 +1921,7 @@ void QGraphicsItem::setOpacity(qreal opacity)
     itemChange(ItemOpacityHasChanged, newOpacity);
 
     // Update.
-    d_ptr->fullUpdateHelper();
+    d_ptr->fullUpdateHelper(/*childrenOnly=*/false, /*maybeDirtyClipPath=*/false, /*ignoreOpacity=*/true);
 }
 
 /*!
@@ -3621,9 +3621,8 @@ void QGraphicsItem::setBoundingRegionGranularity(qreal granularity)
     \internal
     Returns true if we can discard an update request; otherwise false.
 */
-bool QGraphicsItemPrivate::discardUpdateRequest(bool ignoreClipping,
-                                                bool ignoreVisibleBit,
-                                                bool ignoreDirtyBit) const
+bool QGraphicsItemPrivate::discardUpdateRequest(bool ignoreClipping, bool ignoreVisibleBit,
+                                                bool ignoreDirtyBit, bool ignoreOpacity) const
 {
     // No scene, or if the scene is updating everything, means we have nothing
     // to do. The only exception is if the scene tracks the growing scene rect.
@@ -3632,7 +3631,7 @@ bool QGraphicsItemPrivate::discardUpdateRequest(bool ignoreClipping,
            || !scene
            || (scene->d_func()->updateAll && scene->d_func()->hasSceneRect)
            || (!ignoreClipping && (childrenClippedToShape() && isClippedAway()))
-           || (childrenCombineOpacity() && isFullyTransparent());
+           || (!ignoreOpacity && childrenCombineOpacity() && isFullyTransparent());
 }
 
 /*!
@@ -3662,11 +3661,10 @@ void QGraphicsItemPrivate::updateHelper(const QRectF &rect, bool force, bool may
 
     Propagates updates to \a item and all its children.
 */
-void QGraphicsItemPrivate::fullUpdateHelper(bool childrenOnly, bool maybeDirtyClipPath)
+void QGraphicsItemPrivate::fullUpdateHelper(bool childrenOnly, bool maybeDirtyClipPath, bool ignoreOpacity)
 {
-    if (discardUpdateRequest(/*ignoreClipping=*/maybeDirtyClipPath,
-                             /*ignoreVisibleBit=*/false,
-                             /*ignoreDirtyBit=*/true)) {
+    if (discardUpdateRequest(/*ignoreClipping=*/maybeDirtyClipPath, /*ignoreVisibleBit=*/false,
+                             /*ignoreDirtyBit=*/true, ignoreOpacity)) {
         return;
     }
 
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index a5871a7..9ce1bbf 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -165,11 +165,10 @@ public:
     void setPosHelper(const QPointF &pos);
     void setVisibleHelper(bool newVisible, bool explicitly, bool update = true);
     void setEnabledHelper(bool newEnabled, bool explicitly, bool update = true);
-    bool discardUpdateRequest(bool ignoreClipping = false,
-                              bool ignoreVisibleBit = false,
-                              bool ignoreDirtyBit = false) const;
+    bool discardUpdateRequest(bool ignoreClipping = false, bool ignoreVisibleBit = false,
+                              bool ignoreDirtyBit = false, bool ignoreOpacity = false) const;
     void updateHelper(const QRectF &rect = QRectF(), bool force = false, bool maybeDirtyClipPath = false);
-    void fullUpdateHelper(bool childrenOnly = false, bool maybeDirtyClipPath = false);
+    void fullUpdateHelper(bool childrenOnly = false, bool maybeDirtyClipPath = false, bool ignoreOpacity = false);
     void updateEffectiveOpacity();
     void resolveEffectiveOpacity(qreal effectiveParentOpacity);
     void resolveDepth(int parentDepth);
diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp
index b5a1bdf..05e4907 100644
--- a/src/gui/graphicsview/qgraphicsview.cpp
+++ b/src/gui/graphicsview/qgraphicsview.cpp
@@ -1071,8 +1071,12 @@ QList<QGraphicsItem *> QGraphicsViewPrivate::findItems(const QRegion &exposedReg
         QList<QGraphicsItem *> itemList(scene->items());
         int i = 0;
         while (i < itemList.size()) {
+            const QGraphicsItem *item = itemList.at(i);
             // But we only want to include items that are visible
-            if (!itemList.at(i)->isVisible())
+            // The following check is basically the same as item->d_ptr->isInvisible(), except
+            // that we don't check whether the item clips children to shape or propagates its
+            // opacity (we loop through all items, so those checks are wrong in this context).
+            if (!item->isVisible() || item->d_ptr->isClippedAway() || item->d_ptr->isFullyTransparent())
                 itemList.removeAt(i);
             else
                 ++i;
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index 77b7948..6d150cb 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -78,8 +78,7 @@ Q_DECLARE_METATYPE(QRectF)
 class EventTester : public QGraphicsItem
 {
 public:
-    EventTester()
-        : repaints(0)
+    EventTester(QGraphicsItem *parent = 0) : QGraphicsItem(parent), repaints(0)
     { br = QRectF(-10, -10, 20, 20); }
 
     void setGeometry(const QRectF &rect)
@@ -207,6 +206,7 @@ private slots:
     void itemTransform_unrelated();
     void opacity_data();
     void opacity();
+    void opacity2();
     void itemStacksBehindParent();
     void nestedClipping();
     void nestedClippingTransforms();
@@ -5570,6 +5570,91 @@ void tst_QGraphicsItem::opacity()
     QCOMPARE(c3->effectiveOpacity(), c3_effectiveOpacity);
 }
 
+void tst_QGraphicsItem::opacity2()
+{
+    EventTester *parent = new EventTester;
+    EventTester *child = new EventTester(parent);
+    EventTester *grandChild = new EventTester(child);
+
+    QGraphicsScene scene;
+    scene.addItem(parent);
+
+    class MyGraphicsView : public QGraphicsView
+    { public:
+        int repaints;
+        MyGraphicsView(QGraphicsScene *scene) : QGraphicsView(scene), repaints(0) {}
+        void paintEvent(QPaintEvent *e) { ++repaints; QGraphicsView::paintEvent(e); }
+    };
+
+    MyGraphicsView view(&scene);
+    view.show();
+#ifdef Q_WS_X11
+    qt_x11_wait_for_window_manager(&view);
+#endif
+    QTest::qWait(250);
+
+#define RESET_REPAINT_COUNTERS \
+    parent->repaints = 0; \
+    child->repaints = 0; \
+    grandChild->repaints = 0; \
+    view.repaints = 0;
+
+    RESET_REPAINT_COUNTERS
+
+    child->setOpacity(0.0);
+    QTest::qWait(100);
+    QCOMPARE(view.repaints, 1);
+    QCOMPARE(parent->repaints, 1);
+    QCOMPARE(child->repaints, 0);
+    QCOMPARE(grandChild->repaints, 0);
+
+    RESET_REPAINT_COUNTERS
+
+    child->setOpacity(1.0);
+    QTest::qWait(100);
+    QCOMPARE(view.repaints, 1);
+    QCOMPARE(parent->repaints, 1);
+    QCOMPARE(child->repaints, 1);
+    QCOMPARE(grandChild->repaints, 1);
+
+    RESET_REPAINT_COUNTERS
+
+    parent->setOpacity(0.0);
+    QTest::qWait(100);
+    QCOMPARE(view.repaints, 1);
+    QCOMPARE(parent->repaints, 0);
+    QCOMPARE(child->repaints, 0);
+    QCOMPARE(grandChild->repaints, 0);
+
+    RESET_REPAINT_COUNTERS
+
+    parent->setOpacity(1.0);
+    QTest::qWait(100);
+    QCOMPARE(view.repaints, 1);
+    QCOMPARE(parent->repaints, 1);
+    QCOMPARE(child->repaints, 1);
+    QCOMPARE(grandChild->repaints, 1);
+
+    grandChild->setFlag(QGraphicsItem::ItemIgnoresParentOpacity);
+    RESET_REPAINT_COUNTERS
+
+    child->setOpacity(0.0);
+    QTest::qWait(100);
+    QCOMPARE(view.repaints, 1);
+    QCOMPARE(parent->repaints, 1);
+    QCOMPARE(child->repaints, 0);
+    QCOMPARE(grandChild->repaints, 1);
+
+    RESET_REPAINT_COUNTERS
+
+    child->setOpacity(0.0); // Already 0.0; no change.
+    QTest::qWait(100);
+    QCOMPARE(view.repaints, 0);
+    QCOMPARE(parent->repaints, 0);
+    QCOMPARE(child->repaints, 0);
+    QCOMPARE(grandChild->repaints, 0);
+}
+
 void tst_QGraphicsItem::itemStacksBehindParent()
 {
     QGraphicsRectItem *parent1 = new QGraphicsRectItem(QRectF(0, 0, 100, 50));