Improved performance of translating device coordinate graphics effects.

Don't invalidate the cache if we're only translating and the effect rect
is fully contained within the device rect of the painter.

Task-number: QTBUG-6901
Reviewed-by: Bjørn Erik Nilsen

6 files changed, 105 insertions, 18 deletions

diff --git a/src/gui/effects/qgraphicseffect.cpp b/src/gui/effects/qgraphicseffect.cpp
index 90145fe..ad23df3 100644
--- a/src/gui/effects/qgraphicseffect.cpp
+++ b/src/gui/effects/qgraphicseffect.cpp
@@ -374,6 +374,11 @@ QGraphicsEffectSourcePrivate::~QGraphicsEffectSourcePrivate()
     invalidateCache();
 }
 
+void QGraphicsEffectSourcePrivate::setCachedOffset(const QPoint &offset)
+{
+    m_cachedOffset = offset;
+}
+
 void QGraphicsEffectSourcePrivate::invalidateCache(InvalidateReason reason) const
 {
     if (m_cachedMode != QGraphicsEffect::PadToEffectiveBoundingRect
diff --git a/src/gui/effects/qgraphicseffect_p.h b/src/gui/effects/qgraphicseffect_p.h
index 91ad74a..e34dbf9 100644
--- a/src/gui/effects/qgraphicseffect_p.h
+++ b/src/gui/effects/qgraphicseffect_p.h
@@ -129,8 +129,10 @@ public:
                            QGraphicsEffect::PixmapPadMode mode = QGraphicsEffect::PadToTransparentBorder) const = 0;
     virtual void effectBoundingRectChanged() = 0;
 
+    void setCachedOffset(const QPoint &offset);
     void invalidateCache(InvalidateReason reason = SourceChanged) const;
     Qt::CoordinateSystem currentCachedSystem() const { return m_cachedSystem; }
+    QGraphicsEffect::PixmapPadMode currentCachedMode() const { return m_cachedMode; }
 
     friend class QGraphicsScenePrivate;
     friend class QGraphicsItem;
diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 168a9a3..cae9660 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -10715,27 +10715,18 @@ void QGraphicsItemEffectSourcePrivate::draw(QPainter *painter)
     }
 }
 
-QPixmap QGraphicsItemEffectSourcePrivate::pixmap(Qt::CoordinateSystem system, QPoint *offset,
-                                                 QGraphicsEffect::PixmapPadMode mode) const
+QRect QGraphicsItemEffectSourcePrivate::paddedEffectRect(Qt::CoordinateSystem system, QGraphicsEffect::PixmapPadMode mode, const QRectF &sourceRect, bool *unpadded) const
 {
-    const bool deviceCoordinates = (system == Qt::DeviceCoordinates);
-    if (!info && deviceCoordinates) {
-        // Device coordinates without info not yet supported.
-        qWarning("QGraphicsEffectSource::pixmap: Not yet implemented, lacking device context");
-        return QPixmap();
-    }
-    if (!item->d_ptr->scene)
-        return QPixmap();
-    QGraphicsScenePrivate *scened = item->d_ptr->scene->d_func();
-
-    const QRectF sourceRect = boundingRect(system);
     QRectF effectRectF;
 
-    bool unpadded = false;
+    if (unpadded)
+        *unpadded = false;
+
     if (mode == QGraphicsEffect::PadToEffectiveBoundingRect) {
         if (info) {
             effectRectF = item->graphicsEffect()->boundingRectFor(boundingRect(Qt::DeviceCoordinates));
-            unpadded = (effectRectF.size() == sourceRect.size());
+            if (unpadded)
+                *unpadded = (effectRectF.size() == sourceRect.size());
             if (info && system == Qt::LogicalCoordinates)
                 effectRectF = info->painter->worldTransform().inverted().mapRect(effectRectF);
         } else {
@@ -10747,10 +10738,29 @@ QPixmap QGraphicsItemEffectSourcePrivate::pixmap(Qt::CoordinateSystem system, QP
         effectRectF = sourceRect.adjusted(-1.5, -1.5, 1.5, 1.5);
     } else {
         effectRectF = sourceRect;
-        unpadded = true;
+        if (unpadded)
+            *unpadded = true;
+    }
+
+    return effectRectF.toAlignedRect();
+}
+
+QPixmap QGraphicsItemEffectSourcePrivate::pixmap(Qt::CoordinateSystem system, QPoint *offset,
+                                                 QGraphicsEffect::PixmapPadMode mode) const
+{
+    const bool deviceCoordinates = (system == Qt::DeviceCoordinates);
+    if (!info && deviceCoordinates) {
+        // Device coordinates without info not yet supported.
+        qWarning("QGraphicsEffectSource::pixmap: Not yet implemented, lacking device context");
+        return QPixmap();
     }
+    if (!item->d_ptr->scene)
+        return QPixmap();
+    QGraphicsScenePrivate *scened = item->d_ptr->scene->d_func();
 
-    QRect effectRect = effectRectF.toAlignedRect();
+    bool unpadded;
+    const QRectF sourceRect = boundingRect(system);
+    QRect effectRect = paddedEffectRect(system, mode, sourceRect, &unpadded);
 
     if (offset)
         *offset = effectRect.topLeft();
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index c8d2061..2d34b80 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -609,6 +609,7 @@ public:
     QPixmap pixmap(Qt::CoordinateSystem system,
                    QPoint *offset,
                    QGraphicsEffect::PixmapPadMode mode) const;
+    QRect paddedEffectRect(Qt::CoordinateSystem system, QGraphicsEffect::PixmapPadMode mode, const QRectF &sourceRect, bool *unpadded = 0) const;
 
     QGraphicsItem *item;
     QGraphicsItemPaintInfo *info;
diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 5c6a8ae..cea723c 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -4686,8 +4686,31 @@ void QGraphicsScenePrivate::drawSubtreeRecursive(QGraphicsItem *item, QPainter *
         if (sourced->currentCachedSystem() != Qt::LogicalCoordinates
             && sourced->lastEffectTransform != painter->worldTransform())
         {
+            bool unclipped = false;
+            if (sourced->lastEffectTransform.type() <= QTransform::TxTranslate
+                && painter->worldTransform().type() <= QTransform::TxTranslate)
+            {
+                QRectF itemRect = item->boundingRect();
+                if (!item->d_ptr->children.isEmpty())
+                    itemRect |= item->childrenBoundingRect();
+
+                QRectF oldSourceRect = sourced->lastEffectTransform.mapRect(itemRect);
+                QRectF newSourceRect = painter->worldTransform().mapRect(itemRect);
+
+                QRect oldEffectRect = sourced->paddedEffectRect(sourced->currentCachedSystem(), sourced->currentCachedMode(), oldSourceRect);
+                QRect newEffectRect = sourced->paddedEffectRect(sourced->currentCachedSystem(), sourced->currentCachedMode(), newSourceRect);
+
+                QRect deviceRect(0, 0, painter->device()->width(), painter->device()->height());
+                if (deviceRect.contains(oldEffectRect) && deviceRect.contains(newEffectRect)) {
+                    sourced->setCachedOffset(newEffectRect.topLeft());
+                    unclipped = true;
+                }
+            }
+
             sourced->lastEffectTransform = painter->worldTransform();
-            sourced->invalidateCache(QGraphicsEffectSourcePrivate::TransformChanged);
+
+            if (!unclipped)
+                sourced->invalidateCache(QGraphicsEffectSourcePrivate::TransformChanged);
         }
 
         item->d_ptr->graphicsEffect->draw(painter);
diff --git a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
index 95de70e..51e2a57 100644
--- a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
+++ b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
@@ -70,6 +70,7 @@ private slots:
     void grayscale();
     void colorize();
     void drawPixmapItem();
+    void deviceCoordinateTranslateCaching();
 };
 
 void tst_QGraphicsEffect::initTestCase()
@@ -514,6 +515,51 @@ void tst_QGraphicsEffect::drawPixmapItem()
     QTRY_VERIFY(effect->repaints >= 2);
 }
 
+class DeviceEffect : public QGraphicsEffect
+{
+public:
+    QRectF boundingRectFor(const QRectF &rect) const
+    { return rect; }
+
+    void draw(QPainter *painter)
+    {
+        QPoint offset;
+        QPixmap pixmap = sourcePixmap(Qt::DeviceCoordinates, &offset, QGraphicsEffect::NoPad);
+
+        if (pixmap.isNull())
+            return;
+
+        painter->save();
+        painter->setWorldTransform(QTransform());
+        painter->drawPixmap(offset, pixmap);
+        painter->restore();
+    }
+};
+
+void tst_QGraphicsEffect::deviceCoordinateTranslateCaching()
+{
+    QGraphicsScene scene;
+    CustomItem *item = new CustomItem(0, 0, 10, 10);
+    scene.addItem(item);
+    scene.setSceneRect(0, 0, 50, 0);
+
+    item->setGraphicsEffect(new DeviceEffect);
+    item->setPen(Qt::NoPen);
+    item->setBrush(Qt::red);
+
+    QGraphicsView view(&scene);
+    view.show();
+    QTest::qWaitForWindowShown(&view);
+
+    QTRY_VERIFY(item->numRepaints >= 1);
+    int numRepaints = item->numRepaints;
+
+    item->translate(10, 0);
+    QTest::qWait(50);
+
+    QVERIFY(item->numRepaints == numRepaints);
+}
+
 QTEST_MAIN(tst_QGraphicsEffect)
 #include "tst_qgraphicseffect.moc"