Performance issue with QGraphicsItem::ItemClipsChildrenToShape.

If the child rect is bigger than the parent rect and parent has the
ItemClipsChildrenToShape flag set, then by updating the child, the whole
child rect is marked as dirty, resulting in a much larger update area
than required. This has a major impact on performance in Orbit/HB,
where e.g. item-views typically consist of a container item that clips
its children/items to shape. See attached video in QTBUG-9024.

Auto test included.

Task-number: QTBUG-9024

6 files changed, 152 insertions, 5 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index b8c240a185..65edb2ada5 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -1812,7 +1812,7 @@ void QGraphicsItem::setFlags(GraphicsItemFlags flags)
     const quint32 geomChangeFlagsMask = (ItemClipsChildrenToShape | ItemClipsToShape | ItemIgnoresTransformations | ItemIsSelectable);
     bool fullUpdate = (quint32(flags) & geomChangeFlagsMask) != (d_ptr->flags & geomChangeFlagsMask);
     if (fullUpdate)
-        d_ptr->paintedViewBoundingRectsNeedRepaint = 1;
+        d_ptr->updatePaintedViewBoundingRects(/*children=*/true);
 
     // Keep the old flags to compare the diff.
     GraphicsItemFlags oldFlags = GraphicsItemFlags(d_ptr->flags);
@@ -5432,6 +5432,24 @@ void QGraphicsItemPrivate::removeExtraItemCache()
     unsetExtra(ExtraCacheData);
 }
 
+void QGraphicsItemPrivate::updatePaintedViewBoundingRects(bool updateChildren)
+{
+    if (!scene)
+        return;
+
+    for (int i = 0; i < scene->d_func()->views.size(); ++i) {
+        QGraphicsViewPrivate *viewPrivate = scene->d_func()->views.at(i)->d_func();
+        QRect rect = paintedViewBoundingRects.value(viewPrivate->viewport);
+        rect.translate(viewPrivate->dirtyScrollOffset);
+        viewPrivate->updateRect(rect);
+    }
+
+    if (updateChildren) {
+        for (int i = 0; i < children.size(); ++i)
+            children.at(i)->d_ptr->updatePaintedViewBoundingRects(true);
+    }
+}
+
 // Traverses all the ancestors up to the top-level and updates the pointer to
 // always point to the top-most item that has a dirty scene transform.
 // It then backtracks to the top-most dirty item and start calculating the
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index 569a32965d..e812f29e59 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -377,6 +377,7 @@ public:
     QGraphicsItemCache *extraItemCache() const;
     void removeExtraItemCache();
 
+    void updatePaintedViewBoundingRects(bool updateChildren);
     void ensureSceneTransformRecursive(QGraphicsItem **topMostDirtyItem);
     inline void ensureSceneTransform()
     {
diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 36fd5c8c4d..dfba7c914e 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -5116,9 +5116,15 @@ void QGraphicsScenePrivate::processDirtyItemsRecursive(QGraphicsItem *item, bool
 
     // Process children.
     if (itemHasChildren && item->d_ptr->dirtyChildren) {
+        const bool itemClipsChildrenToShape = item->d_ptr->flags & QGraphicsItem::ItemClipsChildrenToShape;
+        if (itemClipsChildrenToShape) {
+            // Make sure child updates are clipped to the item's bounding rect.
+            for (int i = 0; i < views.size(); ++i)
+                views.at(i)->d_func()->setUpdateClip(item);
+        }
         if (!dirtyAncestorContainsChildren) {
             dirtyAncestorContainsChildren = item->d_ptr->fullUpdatePending
-                                            && (item->d_ptr->flags & QGraphicsItem::ItemClipsChildrenToShape);
+                                            && itemClipsChildrenToShape;
         }
         const bool allChildrenDirty = item->d_ptr->allChildrenDirty;
         const bool parentIgnoresVisible = item->d_ptr->ignoreVisible;
@@ -5141,6 +5147,12 @@ void QGraphicsScenePrivate::processDirtyItemsRecursive(QGraphicsItem *item, bool
             }
             processDirtyItemsRecursive(child, dirtyAncestorContainsChildren, opacity);
         }
+
+        if (itemClipsChildrenToShape) {
+            // Reset updateClip.
+            for (int i = 0; i < views.size(); ++i)
+                views.at(i)->d_func()->setUpdateClip(0);
+        }
     } else if (wasDirtyParentSceneTransform) {
         item->d_ptr->invalidateChildrenSceneTransform();
     }
diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp
index 96d5ba18e2..0f951ef1fc 100644
--- a/src/gui/graphicsview/qgraphicsview.cpp
+++ b/src/gui/graphicsview/qgraphicsview.cpp
@@ -337,6 +337,7 @@ QGraphicsViewPrivate::QGraphicsViewPrivate()
       mustAllocateStyleOptions(false),
       mustResizeBackgroundPixmap(true),
       fullUpdatePending(true),
+      hasUpdateClip(false),
       mousePressButton(Qt::NoButton),
       leftIndent(0), topIndent(0),
       lastMouseEvent(QEvent::None, QPoint(), Qt::NoButton, 0, 0),
@@ -880,6 +881,52 @@ static inline void QRect_unite(QRect *rect, const QRect &other)
     }
 }
 
+/*
+   Calling this function results in update rects being clipped to the item's
+   bounding rect. Note that updates prior to this function call is not clipped.
+   The clip is removed by passing 0.
+*/
+void QGraphicsViewPrivate::setUpdateClip(QGraphicsItem *item)
+{
+    Q_Q(QGraphicsView);
+    // We simply ignore the request if the update mode is either FullViewportUpdate
+    // or NoViewportUpdate; in that case there's no point in clipping anything.
+    if (!item || viewportUpdateMode == QGraphicsView::NoViewportUpdate
+        || viewportUpdateMode == QGraphicsView::FullViewportUpdate) {
+        hasUpdateClip = false;
+        return;
+    }
+
+    // Calculate the clip (item's bounding rect in view coordinates).
+    // Optimized version of:
+    // QRect clip = item->deviceTransform(q->viewportTransform())
+    //              .mapRect(item->boundingRect()).toAlignedRect();
+    QRect clip;
+    if (item->d_ptr->itemIsUntransformable()) {
+        QTransform xform = item->deviceTransform(q->viewportTransform());
+        clip = xform.mapRect(item->boundingRect()).toAlignedRect();
+    } else if (item->d_ptr->sceneTransformTranslateOnly && identityMatrix) {
+        QRectF r(item->boundingRect());
+        r.translate(item->d_ptr->sceneTransform.dx() - horizontalScroll(),
+                    item->d_ptr->sceneTransform.dy() - verticalScroll());
+        clip = r.toAlignedRect();
+    } else if (!q->isTransformed()) {
+        clip = item->d_ptr->sceneTransform.mapRect(item->boundingRect()).toAlignedRect();
+    } else {
+        QTransform xform = item->d_ptr->sceneTransform;
+        xform *= q->viewportTransform();
+        clip = xform.mapRect(item->boundingRect()).toAlignedRect();
+    }
+
+    if (hasUpdateClip) {
+        // Intersect with old clip.
+        updateClip &= clip;
+    } else {
+        updateClip = clip;
+        hasUpdateClip = true;
+    }
+}
+
 bool QGraphicsViewPrivate::updateRegion(const QRectF &rect, const QTransform &xform)
 {
     if (rect.isEmpty())
@@ -910,6 +957,8 @@ bool QGraphicsViewPrivate::updateRegion(const QRectF &rect, const QTransform &xf
             viewRect.adjust(-1, -1, 1, 1);
         else
             viewRect.adjust(-2, -2, 2, 2);
+        if (hasUpdateClip)
+            viewRect &= updateClip;
         dirtyRegion += viewRect;
     }
 
@@ -931,7 +980,10 @@ bool QGraphicsViewPrivate::updateRect(const QRect &r)
         viewport->update();
         break;
     case QGraphicsView::BoundingRectViewportUpdate:
-        QRect_unite(&dirtyBoundingRect, r);
+        if (hasUpdateClip)
+            QRect_unite(&dirtyBoundingRect, r & updateClip);
+        else
+            QRect_unite(&dirtyBoundingRect, r);
         if (containsViewport(dirtyBoundingRect, viewport->width(), viewport->height())) {
             fullUpdatePending = true;
             viewport->update();
@@ -939,7 +991,10 @@ bool QGraphicsViewPrivate::updateRect(const QRect &r)
         break;
     case QGraphicsView::SmartViewportUpdate: // ### DEPRECATE
     case QGraphicsView::MinimalViewportUpdate:
-        dirtyRegion += r;
+        if (hasUpdateClip)
+            dirtyRegion += r & updateClip;
+        else
+            dirtyRegion += r;
         break;
     default:
         break;
diff --git a/src/gui/graphicsview/qgraphicsview_p.h b/src/gui/graphicsview/qgraphicsview_p.h
index 1239ca4f22..7bd9ecb918 100644
--- a/src/gui/graphicsview/qgraphicsview_p.h
+++ b/src/gui/graphicsview/qgraphicsview_p.h
@@ -91,7 +91,8 @@ public:
     quint32 mustAllocateStyleOptions : 1;
     quint32 mustResizeBackgroundPixmap : 1;
     quint32 fullUpdatePending : 1;
-    quint32 padding : 19;
+    quint32 hasUpdateClip : 1;
+    quint32 padding : 18;
 
     QRectF sceneRect;
     void updateLastCenterPoint();
@@ -102,6 +103,7 @@ public:
     QRectF mapRectToScene(const QRect &rect) const;
     QRectF mapRectFromScene(const QRectF &rect) const;
 
+    QRect updateClip;
     QPointF mousePressItemPoint;
     QPointF mousePressScenePoint;
     QPoint mousePressViewPoint;
@@ -195,6 +197,8 @@ public:
 #endif
     }
 
+    void setUpdateClip(QGraphicsItem *);
+
     inline bool updateRectF(const QRectF &rect)
     {
         if (rect.isEmpty())
diff --git a/tests/auto/qgraphicsview/tst_qgraphicsview.cpp b/tests/auto/qgraphicsview/tst_qgraphicsview.cpp
index 1df9a373b0..b8df7f6b84 100644
--- a/tests/auto/qgraphicsview/tst_qgraphicsview.cpp
+++ b/tests/auto/qgraphicsview/tst_qgraphicsview.cpp
@@ -218,6 +218,7 @@ private slots:
     void update();
     void update2_data();
     void update2();
+    void update_ancestorClipsChildrenToShape();
     void inputMethodSensitivity();
     void inputContextReset();
     void indirectPainting();
@@ -3758,6 +3759,62 @@ void tst_QGraphicsView::update2()
 #endif
 }
 
+void tst_QGraphicsView::update_ancestorClipsChildrenToShape()
+{
+    QGraphicsScene scene(-150, -150, 300, 300);
+
+    /*
+    Add three rects:
+
+    +------------------+
+    | child            |
+    | +--------------+ |
+    | | parent       | |
+    | |  +-----------+ |
+    | |  |grandParent| |
+    | |  +-----------+ |
+    | +--------------+ |
+    +------------------+
+
+    ... where both the parent and the grand parent clips children to shape.
+    */
+    QApplication::processEvents(); // Get rid of pending update.
+
+    QGraphicsRectItem *grandParent = static_cast<QGraphicsRectItem *>(scene.addRect(0, 0, 50, 50));
+    grandParent->setBrush(Qt::black);
+    grandParent->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+
+    QGraphicsRectItem *parent = static_cast<QGraphicsRectItem *>(scene.addRect(-50, -50, 100, 100));
+    parent->setBrush(QColor(0, 0, 255, 125));
+    parent->setParentItem(grandParent);
+    parent->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+
+    QGraphicsRectItem *child = static_cast<QGraphicsRectItem *>(scene.addRect(-100, -100, 200, 200));
+    child->setBrush(QColor(255, 0, 0, 125));
+    child->setParentItem(parent);
+
+    CustomView view(&scene);
+    view.show();
+    QTest::qWaitForWindowShown(&view);
+    QTRY_VERIFY(view.painted);
+
+    view.lastUpdateRegions.clear();
+    view.painted = false;
+
+    // Call child->update() and make sure the updated area is within the ancestors' clip.
+    QRectF expected = child->deviceTransform(view.viewportTransform()).mapRect(child->boundingRect());
+    expected &= grandParent->deviceTransform(view.viewportTransform()).mapRect(grandParent->boundingRect());
+
+    child->update();
+    QTRY_VERIFY(view.painted);
+
+#ifndef QT_MAC_USE_COCOA //cocoa doesn't support drawing regions
+    QTRY_VERIFY(view.painted);
+    QCOMPARE(view.lastUpdateRegions.size(), 1);
+    QCOMPARE(view.lastUpdateRegions.at(0), QRegion(expected.toAlignedRect()));
+#endif
+}
+
 class FocusItem : public QGraphicsRectItem
 {
 public: