Fixes: Simplify the cut-offs and be more agressive :-)

AutoTest: Still pass.
Details:  It's easier to read and understand the code now.

3 files changed, 68 insertions, 42 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 23787a7..d27d3cd 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -3613,6 +3613,24 @@ 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
+{
+    // 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.
+    return (!visible && !ignoreVisibleBit)
+           || (dirty && !ignoreDirtyBit)
+           || !scene
+           || (scene->d_func()->updateAll && scene->d_func()->hasSceneRect)
+           || (!ignoreClipping && (childrenClippedToShape() && isClippedAway()))
+           || (childrenCombineOpacity() && isFullyTransparent());
+}
+
+/*!
+    \internal
 
     Asks the scene to mark this item's scene rect as dirty, requesting a
     redraw.  This does not invalidate any cache.
@@ -3625,17 +3643,12 @@ void QGraphicsItemPrivate::updateHelper(const QRectF &rect, bool force, bool may
 {
     // 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.
-    if (dirty)
-        return;
-    if (!scene || (scene && scene->d_func()->updateAll && scene->d_func()->hasSceneRect))
+    if (discardUpdateRequest(/*ignoreClipping=*/maybeDirtyClipPath, /*ignoreVisibleBit=*/force))
         return;
-    if (!maybeDirtyClipPath && discardUpdateRequest())
-        return;
-    if (scene && (visible || force)) {
-        if (rect.isNull())
-            dirty = 1;
-        scene->itemUpdated(q_ptr, rect);
-    }
+
+    if (rect.isNull())
+        dirty = 1;
+    scene->itemUpdated(q_ptr, rect);
 }
 
 /*!
@@ -3645,21 +3658,23 @@ void QGraphicsItemPrivate::updateHelper(const QRectF &rect, bool force, bool may
 */
 void QGraphicsItemPrivate::fullUpdateHelper(bool childrenOnly, bool maybeDirtyClipPath)
 {
-    if (!visible || (!maybeDirtyClipPath && discardUpdateRequest()))
-        return;
-    // 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.
-    if (!scene || (scene && scene->d_func()->updateAll && scene->d_func()->hasSceneRect))
-        return;
-    if (!childrenOnly && !dirty)
-        updateHelper(QRectF(), false, maybeDirtyClipPath);
-    if (children.isEmpty() || dirtyChildren)
+    if (discardUpdateRequest(/*ignoreClipping=*/maybeDirtyClipPath,
+                             /*ignoreVisibleBit=*/false,
+                             /*ignoreDirtyBit=*/true)) {
         return;
-    if (flags & QGraphicsItem::ItemClipsChildrenToShape || children.isEmpty()) {
-        // ### mark all children dirty?
+    }
+
+    if (!childrenOnly && !dirty) {
+        // Effectively the same as updateHelper(QRectF(), false, maybeDirtyClipPath).
+        dirty = 1;
+        scene->itemUpdated(q_ptr, QRectF());
+    }
+
+    if (dirtyChildren || childrenClippedToShape()) {
         // Unnecessary to update children as well.
         return;
     }
+
     if (ancestorFlags & AncestorClipsChildren) {
         Q_Q(QGraphicsItem);
         // Check if we can avoid updating all children.
@@ -3929,22 +3944,23 @@ bool QGraphicsItemPrivate::isProxyWidget() const
 */
 void QGraphicsItem::update(const QRectF &rect)
 {
-    if (d_ptr->dirty || (rect.isEmpty() && !rect.isNull()))
+    if ((rect.isEmpty() && !rect.isNull()) || d_ptr->discardUpdateRequest())
         return;
-    if (d_ptr->discardUpdateRequest())
-        return;
-    if (d_ptr->scene && isVisible()) {
-        if (CacheMode(d_ptr->cacheMode) != NoCache) {
-            QGraphicsItemCache *cache = d_ptr->extraItemCache();
-            if (rect.isNull()) {
-                cache->allExposed = true;
-                cache->exposed.clear();
-            } else {
-                cache->exposed.append(rect);
-            }
+
+    if (CacheMode(d_ptr->cacheMode) != NoCache) {
+        QGraphicsItemCache *cache = d_ptr->extraItemCache();
+        if (rect.isNull()) {
+            cache->allExposed = true;
+            cache->exposed.clear();
+        } else {
+            cache->exposed.append(rect);
         }
-        d_ptr->updateHelper(rect);
     }
+
+    // Effectively the same as updateHelper(rect);
+    if (rect.isNull())
+        d_ptr->dirty = 1;
+    d_ptr->scene->itemUpdated(this, rect);
 }
 
 
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index c795915..a5871a7 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -165,6 +165,9 @@ 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;
     void updateHelper(const QRectF &rect = QRectF(), bool force = false, bool maybeDirtyClipPath = false);
     void fullUpdateHelper(bool childrenOnly = false, bool maybeDirtyClipPath = false);
     void updateEffectiveOpacity();
@@ -260,11 +263,8 @@ public:
     void invalidateCachedClipPathRecursively(bool childrenOnly = false, const QRectF &emptyIfOutsideThisRect = QRectF());
     void updateCachedClipPathFromSetPosHelper(const QPointF &newPos);
 
-    inline bool isInvisible() const
-    {
-        return !visible || (hasEffectiveOpacity
-                            && qFuzzyCompare(q_func()->effectiveOpacity() + 1, qreal(1.0)));
-    }
+    inline bool isFullyTransparent() const
+    { return hasEffectiveOpacity && qFuzzyCompare(q_func()->effectiveOpacity() + 1, qreal(1.0)); }
 
     inline bool childrenCombineOpacity() const
     { return allChildrenCombineOpacity || children.isEmpty(); }
@@ -272,8 +272,15 @@ public:
     inline bool isClippedAway() const
     { return !dirtyClipPath && q_func()->isClipped() && (emptyClipPath || cachedClipPath.isEmpty()); }
 
-    inline bool discardUpdateRequest() const
-    { return ((flags & QGraphicsItem::ItemClipsChildrenToShape) || children.isEmpty()) && isClippedAway(); }
+    inline bool childrenClippedToShape() const
+    { return (flags & QGraphicsItem::ItemClipsChildrenToShape) || children.isEmpty(); }
+
+    inline bool isInvisible() const
+    {
+        return !visible
+               || (childrenClippedToShape() && isClippedAway())
+               || (childrenCombineOpacity() && isFullyTransparent());
+    }
 
     QPainterPath cachedClipPath;
     QPointF pos;
diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp
index 4db2257..f0d360a 100644
--- a/src/gui/graphicsview/qgraphicsview.cpp
+++ b/src/gui/graphicsview/qgraphicsview.cpp
@@ -853,8 +853,11 @@ void QGraphicsViewPrivate::_q_updateLaterSlot()
     const QList<QGraphicsItem *> &dirtyItems = scene->d_func()->dirtyItems;
     for (int i = 0; i < dirtyItems.size(); ++i) {
         const QGraphicsItem *item = dirtyItems.at(i);
-        if (item->d_ptr->discardUpdateRequest())
+        if (item->d_ptr->discardUpdateRequest(/*ignoreClipping=*/false,
+                                              /*ignoreVisibleBit=*/false,
+                                              /*ignoreDirtyBit=*/true)) {
             continue;
+        }
         QTransform x = item->sceneTransform() * viewTransform;
         updateRect(x.mapRect(item->boundingRect()).toAlignedRect() & vr);
     }