Fixed rendering bugs when scrolling graphics items with drop shadows.

We can't clip source pixmaps to the device rect, as there's no way of
knowing which parts of the source pixmap are needed for the part of the
graphics effect that's unclipped.

Reviewed-by: Bjørn Erik Nilsen

3 files changed, 31 insertions, 43 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index bd214e1..368af58 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -10810,6 +10810,7 @@ void QGraphicsItemEffectSourcePrivate::draw(QPainter *painter)
     }
 }
 
+// sourceRect must be in the given coordinate system
 QRect QGraphicsItemEffectSourcePrivate::paddedEffectRect(Qt::CoordinateSystem system, QGraphicsEffect::PixmapPadMode mode, const QRectF &sourceRect, bool *unpadded) const
 {
     QRectF effectRectF;
@@ -10819,7 +10820,8 @@ QRect QGraphicsItemEffectSourcePrivate::paddedEffectRect(Qt::CoordinateSystem sy
 
     if (mode == QGraphicsEffect::PadToEffectiveBoundingRect) {
         if (info) {
-            effectRectF = item->graphicsEffect()->boundingRectFor(boundingRect(Qt::DeviceCoordinates));
+            QRectF deviceRect = system == Qt::DeviceCoordinates ? sourceRect : info->painter->worldTransform().mapRect(sourceRect);
+            effectRectF = item->graphicsEffect()->boundingRectFor(deviceRect);
             if (unpadded)
                 *unpadded = (effectRectF.size() == sourceRect.size());
             if (info && system == Qt::LogicalCoordinates)
@@ -10868,30 +10870,6 @@ QPixmap QGraphicsItemEffectSourcePrivate::pixmap(Qt::CoordinateSystem system, QP
         return static_cast<QGraphicsPixmapItem *>(item)->pixmap();
     }
 
-    if (deviceCoordinates) {
-        // Clip to viewport rect.
-        int left, top, right, bottom;
-        effectRect.getCoords(&left, &top, &right, &bottom);
-        if (left < 0) {
-            if (offset)
-                offset->rx() += -left;
-            effectRect.setX(0);
-        }
-        if (top < 0) {
-            if (offset)
-                offset->ry() += -top;
-            effectRect.setY(0);
-        }
-        // NB! We use +-1 for historical reasons (see QRect documentation).
-        QPaintDevice *device = info->painter->device();
-        const int deviceWidth = device->width();
-        const int deviceHeight = device->height();
-        if (right + 1 > deviceWidth)
-            effectRect.setRight(deviceWidth - 1);
-        if (bottom + 1 > deviceHeight)
-            effectRect.setBottom(deviceHeight -1);
-
-    }
     if (effectRect.isEmpty())
         return QPixmap();
 
diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index afea082..4ee2301 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -4714,31 +4714,18 @@ 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 sourceRect = sourced->boundingRect(Qt::DeviceCoordinates);
+                QRect effectRect = sourced->paddedEffectRect(Qt::DeviceCoordinates, sourced->currentCachedMode(), sourceRect);
 
-                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->setCachedOffset(effectRect.topLeft());
+            } else {
+                sourced->invalidateCache(QGraphicsEffectSourcePrivate::TransformChanged);
             }
 
             sourced->lastEffectTransform = painter->worldTransform();
-
-            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 795431b..1007d61 100644
--- a/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
+++ b/tests/auto/qgraphicseffect/tst_qgraphicseffect.cpp
@@ -72,6 +72,7 @@ private slots:
     void drawPixmapItem();
     void deviceCoordinateTranslateCaching();
     void inheritOpacity();
+    void dropShadowClipping();
 };
 
 void tst_QGraphicsEffect::initTestCase()
@@ -590,6 +591,28 @@ void tst_QGraphicsEffect::inheritOpacity()
     QTRY_VERIFY(item->numRepaints > numRepaints);
 }
 
+void tst_QGraphicsEffect::dropShadowClipping()
+{
+    QImage img(128, 128, QImage::Format_ARGB32_Premultiplied);
+    img.fill(0xffffffff);
+
+    QGraphicsScene scene;
+    QGraphicsRectItem *item = new QGraphicsRectItem(-5, -500, 10, 1000);
+    item->setGraphicsEffect(new QGraphicsDropShadowEffect);
+    item->setPen(Qt::NoPen);
+    item->setBrush(Qt::red);
+
+    scene.addItem(item);
+
+    QPainter p(&img);
+    scene.render(&p, img.rect(), QRect(-64, -64, 128, 128));
+    p.end();
+
+    for (int y = 1; y < img.height(); ++y)
+        for (int x = 0; x < img.width(); ++x)
+            QCOMPARE(img.pixel(x, y), img.pixel(x, y-1));
+}
+
 QTEST_MAIN(tst_QGraphicsEffect)
 #include "tst_qgraphicseffect.moc"