Setting QGraphicsItem::ItemClipsChildrenToShape forces ItemClipsToShape

Problem was that setting ItemClipsChildrenToShape on an item resulted
in item itself being clipped to its own shape (ItemClipsToShape).
This commit also reduces state changes on the painter and re-uses the
clip whenever possible, which in turn means better performance for
items that clip children to shape.

Auto test included.

Task-number: QTBUG-12760

2 files changed, 267 insertions, 45 deletions

diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 54fdf3f..38612a8 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -4814,6 +4814,27 @@ void QGraphicsScenePrivate::drawSubtreeRecursive(QGraphicsItem *item, QPainter *
     }
 }
 
+static inline void setClip(QPainter *painter, QGraphicsItem *item)
+{
+    painter->save();
+    QRectF clipRect;
+    const QPainterPath clipPath(item->shape());
+    if (QPathClipper::pathToRect(clipPath, &clipRect))
+        painter->setClipRect(clipRect, Qt::IntersectClip);
+    else
+        painter->setClipPath(clipPath, Qt::IntersectClip);
+}
+
+static inline void setWorldTransform(QPainter *painter, const QTransform *const transformPtr,
+                                     const QTransform *effectTransform)
+{
+    Q_ASSERT(transformPtr);
+    if (effectTransform)
+        painter->setWorldTransform(*transformPtr * *effectTransform);
+    else
+        painter->setWorldTransform(*transformPtr);
+}
+
 void QGraphicsScenePrivate::draw(QGraphicsItem *item, QPainter *painter, const QTransform *const viewTransform,
                                  const QTransform *const transformPtr, QRegion *exposedRegion, QWidget *widget,
                                  qreal opacity, const QTransform *effectTransform,
@@ -4822,36 +4843,37 @@ void QGraphicsScenePrivate::draw(QGraphicsItem *item, QPainter *painter, const Q
     const bool itemIsFullyTransparent = QGraphicsItemPrivate::isOpacityNull(opacity);
     const bool itemClipsChildrenToShape = (item->d_ptr->flags & QGraphicsItem::ItemClipsChildrenToShape);
     const bool itemHasChildren = !item->d_ptr->children.isEmpty();
+    bool setChildClip = itemClipsChildrenToShape;
+    bool itemHasChildrenStackedBehind = false;
 
     int i = 0;
     if (itemHasChildren) {
+        if (itemClipsChildrenToShape)
+            setWorldTransform(painter, transformPtr, effectTransform);
+
         item->d_ptr->ensureSortedChildren();
+        // Items with the 'ItemStacksBehindParent' flag are put in front of the list
+        // so all we have to do is to check the first item.
+        itemHasChildrenStackedBehind = (item->d_ptr->children.at(0)->d_ptr->flags
+                                        & QGraphicsItem::ItemStacksBehindParent);
 
-        if (itemClipsChildrenToShape) {
-            painter->save();
-            Q_ASSERT(transformPtr);
-            if (effectTransform)
-                painter->setWorldTransform(*transformPtr * *effectTransform);
-            else
-                painter->setWorldTransform(*transformPtr);
-            QRectF clipRect;
-            const QPainterPath clipPath(item->shape());
-            if (QPathClipper::pathToRect(clipPath, &clipRect))
-                painter->setClipRect(clipRect, Qt::IntersectClip);
-            else
-                painter->setClipPath(clipPath, Qt::IntersectClip);
-        }
+        if (itemHasChildrenStackedBehind) {
+            if (itemClipsChildrenToShape) {
+                setClip(painter, item);
+                setChildClip = false;
+            }
 
-        // Draw children behind
-        for (i = 0; i < item->d_ptr->children.size(); ++i) {
-            QGraphicsItem *child = item->d_ptr->children.at(i);
-            if (wasDirtyParentSceneTransform)
-                child->d_ptr->dirtySceneTransform = 1;
-            if (!(child->d_ptr->flags & QGraphicsItem::ItemStacksBehindParent))
-                break;
-            if (itemIsFullyTransparent && !(child->d_ptr->flags & QGraphicsItem::ItemIgnoresParentOpacity))
-                continue;
-            drawSubtreeRecursive(child, painter, viewTransform, exposedRegion, widget, opacity, effectTransform);
+            // Draw children behind
+            for (i = 0; i < item->d_ptr->children.size(); ++i) {
+                QGraphicsItem *child = item->d_ptr->children.at(i);
+                if (wasDirtyParentSceneTransform)
+                    child->d_ptr->dirtySceneTransform = 1;
+                if (!(child->d_ptr->flags & QGraphicsItem::ItemStacksBehindParent))
+                    break;
+                if (itemIsFullyTransparent && !(child->d_ptr->flags & QGraphicsItem::ItemIgnoresParentOpacity))
+                    continue;
+                drawSubtreeRecursive(child, painter, viewTransform, exposedRegion, widget, opacity, effectTransform);
+            }
         }
     }
 
@@ -4864,38 +4886,50 @@ void QGraphicsScenePrivate::draw(QGraphicsItem *item, QPainter *painter, const Q
                                      ? *exposedRegion : QRegion(), exposedRegion == 0);
 
         const bool itemClipsToShape = item->d_ptr->flags & QGraphicsItem::ItemClipsToShape;
-        const bool savePainter = itemClipsToShape || painterStateProtection;
-        if (savePainter)
-            painter->save();
+        bool restorePainterClip = false;
 
         if (!itemHasChildren || !itemClipsChildrenToShape) {
-            if (effectTransform)
-                painter->setWorldTransform(*transformPtr * *effectTransform);
-            else
-                painter->setWorldTransform(*transformPtr);
+            // Item does not have children or clip children to shape.
+            setWorldTransform(painter, transformPtr, effectTransform);
+            if ((restorePainterClip = itemClipsToShape))
+                setClip(painter, item);
+        } else if (itemHasChildrenStackedBehind){
+            // Item clips children to shape and has children stacked behind, which means
+            // the painter is already clipped to the item's shape.
+            if (itemClipsToShape) {
+                // The clip is already correct. Ensure correct world transform.
+                setWorldTransform(painter, transformPtr, effectTransform);
+            } else {
+                // Remove clip (this also ensures correct world transform).
+                painter->restore();
+                setChildClip = true;
+            }
+        } else if (itemClipsToShape) {
+            // Item clips children and itself to shape. It does not have hildren stacked
+            // behind, which means the clip has not yet been set. We set it now and re-use it
+            // for the children.
+            setClip(painter, item);
+            setChildClip = false;
         }
 
-        if (itemClipsToShape) {
-            QRectF clipRect;
-            const QPainterPath clipPath(item->shape());
-            if (QPathClipper::pathToRect(clipPath, &clipRect))
-                painter->setClipRect(clipRect, Qt::IntersectClip);
-            else
-                painter->setClipPath(clipPath, Qt::IntersectClip);
-        }
-        painter->setOpacity(opacity);
+        if (painterStateProtection && !restorePainterClip)
+            painter->save();
 
+        painter->setOpacity(opacity);
         if (!item->d_ptr->cacheMode && !item->d_ptr->isWidget)
             item->paint(painter, &styleOptionTmp, widget);
         else
             drawItemHelper(item, painter, &styleOptionTmp, widget, painterStateProtection);
 
-        if (savePainter)
+        if (painterStateProtection || restorePainterClip)
             painter->restore();
     }
 
     // Draw children in front
     if (itemHasChildren) {
+        if (setChildClip)
+            setClip(painter, item);
+
         for (; i < item->d_ptr->children.size(); ++i) {
             QGraphicsItem *child = item->d_ptr->children.at(i);
             if (wasDirtyParentSceneTransform)
@@ -4904,11 +4938,11 @@ void QGraphicsScenePrivate::draw(QGraphicsItem *item, QPainter *painter, const Q
                 continue;
             drawSubtreeRecursive(child, painter, viewTransform, exposedRegion, widget, opacity, effectTransform);
         }
-    }
 
-    // Restore child clip
-    if (itemHasChildren && itemClipsChildrenToShape)
-        painter->restore();
+        // Restore child clip
+        if (itemClipsChildrenToShape)
+            painter->restore();
+    }
 }
 
 void QGraphicsScenePrivate::markDirty(QGraphicsItem *item, const QRectF &rect, bool invalidateChildren,
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index 4476084..25ec040 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -377,6 +377,7 @@ private slots:
     void itemClipsChildrenToShape2();
     void itemClipsChildrenToShape3();
     void itemClipsChildrenToShape4();
+    void itemClipsChildrenToShape5();
     void itemClipsTextChildToShape();
     void itemClippingDiscovery();
     void ancestorFlags();
@@ -5464,6 +5465,193 @@ void tst_QGraphicsItem::itemClipsChildrenToShape4()
     QTRY_COMPARE(innerWidget->painted, true);
 }
 
+//#define DEBUG_ITEM_CLIPS_CHILDREN_TO_SHAPE_5
+static inline void renderSceneToImage(QGraphicsScene *scene, QImage *image, const QString &filename)
+{
+    image->fill(0);
+    QPainter painter(image);
+    scene->render(&painter);
+    painter.end();
+#ifdef DEBUG_ITEM_CLIPS_CHILDREN_TO_SHAPE_5
+    image->save(filename);
+#else
+    Q_UNUSED(filename);
+#endif
+}
+
+void tst_QGraphicsItem::itemClipsChildrenToShape5()
+{
+    class ParentItem : public QGraphicsRectItem
+    {
+    public:
+        ParentItem(qreal x, qreal y, qreal width, qreal height)
+            : QGraphicsRectItem(x, y, width, height) {}
+
+        QPainterPath shape() const
+        {
+            QPainterPath path;
+            path.addRect(50, 50, 200, 200);
+            return path;
+        }
+    };
+
+    ParentItem *parent = new ParentItem(0, 0, 300, 300);
+    parent->setBrush(Qt::blue);
+    parent->setOpacity(0.5);
+
+    const QRegion parentRegion(0, 0, 300, 300);
+    const QRegion clippedParentRegion = parentRegion & QRect(50, 50, 200, 200);
+    QRegion childRegion;
+    QRegion grandChildRegion;
+
+    QGraphicsRectItem *topLeftChild = new QGraphicsRectItem(0, 0, 100, 100);
+    topLeftChild->setBrush(Qt::red);
+    topLeftChild->setParentItem(parent);
+    childRegion += QRect(0, 0, 100, 100);
+
+    QGraphicsRectItem *topRightChild = new QGraphicsRectItem(0, 0, 100, 100);
+    topRightChild->setBrush(Qt::red);
+    topRightChild->setParentItem(parent);
+    topRightChild->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+    topRightChild->setPos(200, 0);
+    childRegion += QRect(200, 0, 100, 100);
+
+    QGraphicsRectItem *topRightGrandChild = new QGraphicsRectItem(0, 0, 100, 100);
+    topRightGrandChild->setBrush(Qt::green);
+    topRightGrandChild->setParentItem(topRightChild);
+    topRightGrandChild->setPos(-40, 40);
+    grandChildRegion += QRect(200 - 40, 0 + 40, 100, 100) & QRect(200, 0, 100, 100);
+
+    QGraphicsRectItem *bottomLeftChild = new QGraphicsRectItem(0, 0, 100, 100);
+    bottomLeftChild->setBrush(Qt::red);
+    bottomLeftChild->setParentItem(parent);
+    bottomLeftChild->setFlag(QGraphicsItem::ItemClipsToShape);
+    bottomLeftChild->setPos(0, 200);
+    childRegion += QRect(0, 200, 100, 100);
+
+    QGraphicsRectItem *bottomLeftGrandChild = new QGraphicsRectItem(0, 0, 160, 160);
+    bottomLeftGrandChild->setBrush(Qt::green);
+    bottomLeftGrandChild->setParentItem(bottomLeftChild);
+    bottomLeftGrandChild->setFlag(QGraphicsItem::ItemClipsToShape);
+    bottomLeftGrandChild->setPos(0, -60);
+    grandChildRegion += QRect(0, 200 - 60, 160, 160);
+
+    QGraphicsRectItem *bottomRightChild = new QGraphicsRectItem(0, 0, 100, 100);
+    bottomRightChild->setBrush(Qt::red);
+    bottomRightChild->setParentItem(parent);
+    bottomRightChild->setPos(200, 200);
+    childRegion += QRect(200, 200, 100, 100);
+
+    QPoint controlPoints[17] = {
+        QPoint(5, 5)  , QPoint(95, 5)  , QPoint(205, 5)  , QPoint(295, 5)  ,
+        QPoint(5, 95) , QPoint(95, 95) , QPoint(205, 95) , QPoint(295, 95) ,
+                             QPoint(150, 150),
+        QPoint(5, 205), QPoint(95, 205), QPoint(205, 205), QPoint(295, 205),
+        QPoint(5, 295), QPoint(95, 295), QPoint(205, 295), QPoint(295, 295),
+    };
+
+    const QRegion clippedChildRegion = childRegion & QRect(50, 50, 200, 200);
+    const QRegion clippedGrandChildRegion = grandChildRegion & QRect(50, 50, 200, 200);
+
+    QGraphicsScene scene;
+    scene.addItem(parent);
+    QImage sceneImage(300, 300, QImage::Format_ARGB32);
+
+#define VERIFY_CONTROL_POINTS(pRegion, cRegion, gRegion) \
+    for (int i = 0; i < 17; ++i) { \
+        QPoint controlPoint = controlPoints[i]; \
+        QRgb pixel = sceneImage.pixel(controlPoint.x(), controlPoint.y()); \
+        if (pRegion.contains(controlPoint)) \
+            QVERIFY(qBlue(pixel) != 0); \
+        else \
+            QVERIFY(qBlue(pixel) == 0); \
+        if (cRegion.contains(controlPoint)) \
+            QVERIFY(qRed(pixel) != 0); \
+        else \
+            QVERIFY(qRed(pixel) == 0); \
+        if (gRegion.contains(controlPoint)) \
+            QVERIFY(qGreen(pixel) != 0); \
+        else \
+            QVERIFY(qGreen(pixel) == 0); \
+    }
+
+    const QList<QGraphicsItem *> children = parent->childItems();
+    const int childrenCount = children.count();
+
+    for (int i = 0; i < 5; ++i) {
+        QString clipString;
+        QString childString;
+        switch (i) {
+        case 0:
+            // All children stacked in front.
+            childString = QLatin1String("ChildrenInFront.png");
+            foreach (QGraphicsItem *child, children)
+                child->setFlag(QGraphicsItem::ItemStacksBehindParent, false);
+            break;
+        case 1:
+            // All children stacked behind.
+            childString = QLatin1String("ChildrenBehind.png");
+            foreach (QGraphicsItem *child, children)
+                child->setFlag(QGraphicsItem::ItemStacksBehindParent, true);
+            break;
+        case 2:
+            // First half of the children behind, second half in front.
+            childString = QLatin1String("FirstHalfBehind_SecondHalfInFront.png");
+            for (int j = 0; j < childrenCount; ++j) {
+                QGraphicsItem *child = children.at(j);
+                child->setFlag(QGraphicsItem::ItemStacksBehindParent, (j < childrenCount / 2));
+            }
+            break;
+        case 3:
+            // First half of the children in front, second half behind.
+            childString = QLatin1String("FirstHalfInFront_SecondHalfBehind.png");
+            for (int j = 0; j < childrenCount; ++j) {
+                QGraphicsItem *child = children.at(j);
+                child->setFlag(QGraphicsItem::ItemStacksBehindParent, (j >= childrenCount / 2));
+            }
+            break;
+        case 4:
+            // Child2 and child4 behind, rest in front.
+            childString = QLatin1String("Child2And4Behind_RestInFront.png");
+            for (int j = 0; j < childrenCount; ++j) {
+                QGraphicsItem *child = children.at(j);
+                if (j == 1 || j == 3)
+                    child->setFlag(QGraphicsItem::ItemStacksBehindParent, true);
+                else
+                    child->setFlag(QGraphicsItem::ItemStacksBehindParent, false);
+            }
+            break;
+        default:
+            qFatal("internal error");
+        }
+
+        // Nothing is clipped.
+        parent->setFlag(QGraphicsItem::ItemClipsChildrenToShape, false);
+        parent->setFlag(QGraphicsItem::ItemClipsToShape, false);
+        clipString = QLatin1String("nothingClipped_");
+        renderSceneToImage(&scene, &sceneImage, clipString + childString);
+        VERIFY_CONTROL_POINTS(parentRegion, childRegion, grandChildRegion);
+
+        // Parent clips children to shape.
+        parent->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+        clipString = QLatin1String("parentClipsChildrenToShape_");
+        renderSceneToImage(&scene, &sceneImage, clipString + childString);
+        VERIFY_CONTROL_POINTS(parentRegion, clippedChildRegion, clippedGrandChildRegion);
+
+        // Parent clips itself and children to shape.
+        parent->setFlag(QGraphicsItem::ItemClipsToShape);
+        clipString = QLatin1String("parentClipsItselfAndChildrenToShape_");
+        renderSceneToImage(&scene, &sceneImage, clipString + childString);
+        VERIFY_CONTROL_POINTS(clippedParentRegion, clippedChildRegion, clippedGrandChildRegion);
+
+        // Parent clips itself to shape.
+        parent->setFlag(QGraphicsItem::ItemClipsChildrenToShape, false);
+        clipString = QLatin1String("parentClipsItselfToShape_");
+        renderSceneToImage(&scene, &sceneImage, clipString + childString);
+        VERIFY_CONTROL_POINTS(clippedParentRegion, childRegion, grandChildRegion);
+    }
+}
+
 void tst_QGraphicsItem::itemClipsTextChildToShape()
 {
     // Construct a scene with a rect that clips its children, with one text