Painting artifacts when moving an item with partial updates.

Found during manual testing (dndrobotinproxy). The problem was that the
paintedViewBoundingRect was set to an empty rect because the partial
update area didn't intersect with the viewport. The item itself was
partially inside the viewport. Then, when the item was moved, its old
area was not repainted due to this empty paintedViewBoundingRect.

Auto-test included.

3 files changed, 73 insertions, 24 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 85d76d4..2f646f7 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -1821,6 +1821,7 @@ void QGraphicsItemPrivate::setVisibleHelper(bool newVisible, bool explicitly, bo
         if (q_ptr->isSelected())
             q_ptr->setSelected(false);
     } else {
+        geometryChanged = 1;
         if (isWidget && scene) {
             QGraphicsWidget *widget = static_cast<QGraphicsWidget *>(q_ptr);
             if (widget->windowType() == Qt::Popup)
diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 0ca72b7..c7c0865 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -4517,17 +4517,14 @@ void QGraphicsScenePrivate::processDirtyItemsRecursive(QGraphicsItem *item, bool
             item->d_ptr->paintedViewBoundingRectsNeedRepaint = 0;
     }
 
-    if (item->d_ptr->geometryChanged) {
+    if (!hasSceneRect && item->d_ptr->geometryChanged && item->d_ptr->visible) {
         // Update growingItemsBoundingRect.
-        if (!hasSceneRect) {
-            if (item->d_ptr->sceneTransformTranslateOnly) {
-                growingItemsBoundingRect |= item->boundingRect().translated(item->d_ptr->sceneTransform.dx(),
-                                                                            item->d_ptr->sceneTransform.dy());
-            } else {
-                growingItemsBoundingRect |= item->d_ptr->sceneTransform.mapRect(item->boundingRect());
-            }
+        if (item->d_ptr->sceneTransformTranslateOnly) {
+            growingItemsBoundingRect |= item->boundingRect().translated(item->d_ptr->sceneTransform.dx(),
+                                                                        item->d_ptr->sceneTransform.dy());
+        } else {
+            growingItemsBoundingRect |= item->d_ptr->sceneTransform.mapRect(item->boundingRect());
         }
-        item->d_ptr->geometryChanged = 0;
     }
 
     // Process item.
@@ -4552,29 +4549,31 @@ void QGraphicsScenePrivate::processDirtyItemsRecursive(QGraphicsItem *item, bool
             for (int j = 0; j < views.size(); ++j) {
                 QGraphicsView *view = views.at(j);
                 QGraphicsViewPrivate *viewPrivate = view->d_func();
-                if (viewPrivate->fullUpdatePending)
-                    continue;
-                switch (viewPrivate->viewportUpdateMode) {
-                case QGraphicsView::NoViewportUpdate:
-                    continue;
-                case QGraphicsView::FullViewportUpdate:
-                    view->viewport()->update();
-                    viewPrivate->fullUpdatePending = 1;
+                QRect &paintedViewBoundingRect = item->d_ptr->paintedViewBoundingRects[viewPrivate->viewport];
+                if (viewPrivate->fullUpdatePending
+                    || viewPrivate->viewportUpdateMode == QGraphicsView::NoViewportUpdate) {
+                    // Okay, if we have a full update pending or no viewport update, this item's
+                    // paintedViewBoundingRect  will be updated correctly in the next paintEvent if
+                    // it is inside the viewport, but for now we can pretend that it is outside.
+                    paintedViewBoundingRect = QRect(-1, -1, -1, -1);
                     continue;
-                default:
-                    break;
                 }
 
-                QRect &paintedViewBoundingRect = item->d_ptr->paintedViewBoundingRects[viewPrivate->viewport];
-                if (item->d_ptr->paintedViewBoundingRectsNeedRepaint) {
+                if (item->d_ptr->paintedViewBoundingRectsNeedRepaint && !paintedViewBoundingRect.isEmpty()) {
                     paintedViewBoundingRect.translate(viewPrivate->dirtyScrollOffset);
                     if (!viewPrivate->updateRect(paintedViewBoundingRect))
-                        paintedViewBoundingRect = QRect();
+                        paintedViewBoundingRect = QRect(-1, -1, -1, -1); // Outside viewport.
                 }
 
                 if (!item->d_ptr->dirty)
                     continue;
 
+                if (!item->d_ptr->geometryChanged
+                    && paintedViewBoundingRect.x() == -1 && paintedViewBoundingRect.y() == -1
+                    && paintedViewBoundingRect.width() == -1 && paintedViewBoundingRect.height() == -1) {
+                    continue; // Outside viewport.
+                }
+
                 if (uninitializedDirtyRect) {
                     dirtyRect = itemBoundingRect;
                     if (!item->d_ptr->fullUpdatePending) {
@@ -4587,8 +4586,10 @@ void QGraphicsScenePrivate::processDirtyItemsRecursive(QGraphicsItem *item, bool
                 if (dirtyRect.isEmpty())
                     continue; // Discard updates outside the bounding rect.
 
-                if (!updateHelper(viewPrivate, item->d_ptr, dirtyRect, itemIsUntransformable))
-                    paintedViewBoundingRect = QRect();
+                if (!updateHelper(viewPrivate, item->d_ptr, dirtyRect, itemIsUntransformable)
+                    && item->d_ptr->geometryChanged) {
+                    paintedViewBoundingRect = QRect(-1, -1, -1, -1); // Outside viewport.
+                }
             }
         }
     }
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index d689293..2dbd5f1 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -6580,6 +6580,7 @@ public:
 void tst_QGraphicsItem::update()
 {
     QGraphicsScene scene;
+    scene.setSceneRect(-100, -100, 200, 200);
     MyGraphicsView view(&scene);
 
     view.show();
@@ -6636,6 +6637,52 @@ void tst_QGraphicsItem::update()
     QCOMPARE(view.repaints, 1);
     // The entire item's bounding rect (adjusted for antialiasing) should have been painted.
     QCOMPARE(view.paintedRegion, expectedRegion);
+
+    // Make sure item is repainted when shown (after being hidden).
+    view.reset();
+    item->repaints = 0;
+    item->show();
+    qApp->processEvents();
+    QCOMPARE(item->repaints, 1);
+    QCOMPARE(view.repaints, 1);
+    // The entire item's bounding rect (adjusted for antialiasing) should have been painted.
+    QCOMPARE(view.paintedRegion, expectedRegion);
+
+    item->repaints = 0;
+    item->hide();
+    qApp->processEvents();
+    view.reset();
+    const QPointF originalPos = item->pos();
+    item->setPos(5000, 5000);
+    qApp->processEvents();
+    QCOMPARE(item->repaints, 0);
+    QCOMPARE(view.repaints, 0);
+    qApp->processEvents();
+
+    item->setPos(originalPos);
+    qApp->processEvents();
+    QCOMPARE(item->repaints, 0);
+    QCOMPARE(view.repaints, 0);
+    item->show();
+    qApp->processEvents();
+    QCOMPARE(item->repaints, 1);
+    QCOMPARE(view.repaints, 1);
+    // The entire item's bounding rect (adjusted for antialiasing) should have been painted.
+    QCOMPARE(view.paintedRegion, expectedRegion);
+
+    QGraphicsViewPrivate *viewPrivate = static_cast<QGraphicsViewPrivate *>(qt_widget_private(&view));
+    item->setPos(originalPos + QPoint(50, 50));
+    viewPrivate->updateAll();
+    QVERIFY(viewPrivate->fullUpdatePending);
+    QTest::qWait(50);
+    item->repaints = 0;
+    view.reset();
+    item->setPos(originalPos);
+    QTest::qWait(50);
+    qApp->processEvents();
+    QCOMPARE(item->repaints, 1);
+    QCOMPARE(view.repaints, 1);
+    QCOMPARE(view.paintedRegion, expectedRegion + expectedRegion.translated(50, 50));
 }
 
 void tst_QGraphicsItem::setTransformProperties_data()