QGraphicsView drawing artifacts due to rounding errors.

Found during investigation of QTBUG-8820, and clearly visible in
examples/graphicsview/diagramscene when slowly moving an item out of the
viewport (left and top edge). Caused by two problems:
1) Using QRectF::toRect() instead of QRectF::toAlignedRect().
2) Didn't adjust the item's bounding rect properly in drawSubtree().

QRectF::toRect() is completely useless since all the coordinates are rounded
to the nearest integer. E.g. QRectF(-0.4, -0.4, 10.4, 10.4).toRect()
-> QRect(0, 0, 10, 10), whereas toAlignedRect() returns QRect(-1, -1, 11, 11).

Then when we have a proper aligned rect, we have to adjust it by 2
pixels in all directions (or 1 pixel in case of
QGraphicsView::DontAdjustForAntialiasing). At first glance this
adjustment seems too much, since one would assume adjusing the QRectF by
0.5 before using toAlignedRect() would be sufficient. That's sufficient
in an untransformed world with pens using BevelJoin. However, the story
is completely different as soon as the world is transformed or the pens
use a different join. It's basically complicated (in some cases
impossible) to calculate a pixel perfect aligned QRect, so instead we
adjust by the amount of pixels required in the worst case.

This commit also includes some optimizations for QRegion updates (since
I anyways had to change the code). There's no point in using QRegion
granularity if the viewport update mode is either FullViewportUpdate or
BoundingRectViewportUpdate.

Auto tests adjusted and new ones included.

Task-number: QTBUG-10338

4 files changed, 76 insertions, 61 deletions

diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 8cee021..0d4e48a 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -297,6 +297,7 @@ QGraphicsScenePrivate::QGraphicsScenePrivate()
       scenePosDescendantsUpdatePending(false),
       stickyFocus(false),
       hasFocus(false),
+      rectAdjust(2),
       focusItem(0),
       lastFocusItem(0),
       tabFocusFirst(0),
@@ -3217,7 +3218,10 @@ void QGraphicsScene::update(const QRectF &rect)
             // Update all views.
             for (int i = 0; i < d->views.size(); ++i) {
                 QGraphicsView *view = d->views.at(i);
-                view->d_func()->updateRegion(QRegion(view->mapFromScene(rect).boundingRect()));
+                if (view->isTransformed())
+                    view->d_func()->updateRectF(view->viewportTransform().mapRect(rect));
+                else
+                    view->d_func()->updateRectF(rect);
             }
         } else {
             d->updatedRects << rect;
@@ -4701,11 +4705,11 @@ void QGraphicsScenePrivate::drawSubtreeRecursive(QGraphicsItem *item, QPainter *
     if (drawItem) {
         const QRectF brect = adjustedItemEffectiveBoundingRect(item);
         ENSURE_TRANSFORM_PTR
-        QRect viewBoundingRect = translateOnlyTransform ? brect.translated(transformPtr->dx(), transformPtr->dy()).toRect()
-                                                        : transformPtr->mapRect(brect).toRect();
+        QRect viewBoundingRect = translateOnlyTransform ? brect.translated(transformPtr->dx(), transformPtr->dy()).toAlignedRect()
+                                                        : transformPtr->mapRect(brect).toAlignedRect();
+        viewBoundingRect.adjust(-rectAdjust, -rectAdjust, rectAdjust, rectAdjust);
         if (widget)
             item->d_ptr->paintedViewBoundingRects.insert(widget, viewBoundingRect);
-        viewBoundingRect.adjust(-1, -1, 1, 1);
         drawItem = exposedRegion ? exposedRegion->intersects(viewBoundingRect)
                                  : !viewBoundingRect.normalized().isEmpty();
         if (!drawItem) {
@@ -4954,34 +4958,29 @@ static inline bool updateHelper(QGraphicsViewPrivate *view, QGraphicsItemPrivate
     if (itemIsUntransformable) {
         const QTransform xform = itemq->deviceTransform(viewq->viewportTransform());
         if (!item->hasBoundingRegionGranularity)
-            return view->updateRect(xform.mapRect(rect).toRect());
-        return view->updateRegion(xform.map(QRegion(rect.toRect())));
+            return view->updateRectF(xform.mapRect(rect));
+        return view->updateRegion(rect, xform);
     }
 
     if (item->sceneTransformTranslateOnly && view->identityMatrix) {
         const qreal dx = item->sceneTransform.dx();
         const qreal dy = item->sceneTransform.dy();
-        if (!item->hasBoundingRegionGranularity) {
-            QRectF r(rect);
-            r.translate(dx - view->horizontalScroll(), dy - view->verticalScroll());
-            return view->updateRect(r.toRect());
-        }
-        QRegion r(rect.toRect());
-        r.translate(qRound(dx) - view->horizontalScroll(), qRound(dy) - view->verticalScroll());
-        return view->updateRegion(r);
+        QRectF r(rect);
+        r.translate(dx - view->horizontalScroll(), dy - view->verticalScroll());
+        return view->updateRectF(r);
     }
 
     if (!viewq->isTransformed()) {
         if (!item->hasBoundingRegionGranularity)
-            return view->updateRect(item->sceneTransform.mapRect(rect).toRect());
-        return view->updateRegion(item->sceneTransform.map(QRegion(rect.toRect())));
+            return view->updateRectF(item->sceneTransform.mapRect(rect));
+        return view->updateRegion(rect, item->sceneTransform);
     }
 
     QTransform xform = item->sceneTransform;
     xform *= viewq->viewportTransform();
     if (!item->hasBoundingRegionGranularity)
-        return view->updateRect(xform.mapRect(rect).toRect());
-    return view->updateRegion(xform.map(QRegion(rect.toRect())));
+        return view->updateRectF(xform.mapRect(rect));
+    return view->updateRegion(rect, xform);
 }
 
 void QGraphicsScenePrivate::processDirtyItemsRecursive(QGraphicsItem *item, bool dirtyAncestorContainsChildren,
@@ -5197,8 +5196,14 @@ void QGraphicsScene::drawItems(QPainter *painter,
     // Determine view, expose and flags.
     QGraphicsView *view = widget ? qobject_cast<QGraphicsView *>(widget->parentWidget()) : 0;
     QRegion *expose = 0;
-    if (view)
+    const quint32 oldRectAdjust = d->rectAdjust;
+    if (view) {
         expose = &view->d_func()->exposedRegion;
+        if (view->d_func()->optimizationFlags & QGraphicsView::DontAdjustForAntialiasing)
+            d->rectAdjust = 1;
+        else
+            d->rectAdjust = 2;
+    }
 
     // Find all toplevels, they are already sorted.
     QList<QGraphicsItem *> topLevelItems;
@@ -5211,6 +5216,7 @@ void QGraphicsScene::drawItems(QPainter *painter,
         }
     }
 
+    d->rectAdjust = oldRectAdjust;
     // Reset discovery bits.
     for (int i = 0; i < topLevelItems.size(); ++i)
         topLevelItems.at(i)->d_ptr->itemDiscovered = 0;
diff --git a/src/gui/graphicsview/qgraphicsscene_p.h b/src/gui/graphicsview/qgraphicsscene_p.h
index 11e250e..0a85f0e 100644
--- a/src/gui/graphicsview/qgraphicsscene_p.h
+++ b/src/gui/graphicsview/qgraphicsscene_p.h
@@ -136,8 +136,10 @@ public:
     QBrush backgroundBrush;
     QBrush foregroundBrush;
 
-    bool stickyFocus;
-    bool hasFocus;
+    quint32 stickyFocus : 1;
+    quint32 hasFocus : 1;
+    quint32 padding : 30;
+    quint32 rectAdjust;
     QGraphicsItem *focusItem;
     QGraphicsItem *lastFocusItem;
     QGraphicsWidget *tabFocusFirst;
diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp
index c951dce..0bba7e9 100644
--- a/src/gui/graphicsview/qgraphicsview.cpp
+++ b/src/gui/graphicsview/qgraphicsview.cpp
@@ -854,10 +854,7 @@ void QGraphicsViewPrivate::processPendingUpdates()
     if (fullUpdatePending) {
         viewport->update();
     } else if (viewportUpdateMode == QGraphicsView::BoundingRectViewportUpdate) {
-        if (optimizationFlags & QGraphicsView::DontAdjustForAntialiasing)
-            viewport->update(dirtyBoundingRect.adjusted(-1, -1, 1, 1));
-        else
-            viewport->update(dirtyBoundingRect.adjusted(-2, -2, 2, 2));
+        viewport->update(dirtyBoundingRect);
     } else {
         viewport->update(dirtyRegion); // Already adjusted in updateRect/Region.
     }
@@ -882,46 +879,44 @@ static inline void QRect_unite(QRect *rect, const QRect &other)
     }
 }
 
-bool QGraphicsViewPrivate::updateRegion(const QRegion &r)
+bool QGraphicsViewPrivate::updateRegion(const QRectF &rect, const QTransform &xform)
 {
-    if (fullUpdatePending || viewportUpdateMode == QGraphicsView::NoViewportUpdate || r.isEmpty())
+    if (rect.isEmpty())
         return false;
 
-    const QRect boundingRect = r.boundingRect();
-    if (!intersectsViewport(boundingRect, viewport->width(), viewport->height()))
-        return false; // Update region outside viewport.
-
-    switch (viewportUpdateMode) {
-    case QGraphicsView::FullViewportUpdate:
-        fullUpdatePending = true;
-        viewport->update();
-        break;
-    case QGraphicsView::BoundingRectViewportUpdate:
-        QRect_unite(&dirtyBoundingRect, boundingRect);
-        if (containsViewport(dirtyBoundingRect, viewport->width(), viewport->height())) {
-            fullUpdatePending = true;
-            viewport->update();
-        }
-        break;
-    case QGraphicsView::SmartViewportUpdate: // ### DEPRECATE
-    case QGraphicsView::MinimalViewportUpdate:
-    {
-        const QVector<QRect> &rects = r.rects();
-        for (int i = 0; i < rects.size(); ++i) {
-            if (optimizationFlags & QGraphicsView::DontAdjustForAntialiasing)
-                dirtyRegion += rects.at(i).adjusted(-1, -1, 1, 1);
-            else
-                dirtyRegion += rects.at(i).adjusted(-2, -2, 2, 2);
-        }
-        break;
+    if (viewportUpdateMode != QGraphicsView::MinimalViewportUpdate
+        && viewportUpdateMode != QGraphicsView::SmartViewportUpdate) {
+        // No point in updating with QRegion granularity; use the rect instead.
+        return updateRectF(xform.mapRect(rect));
     }
-    default:
-        break;
+
+    // Update mode is either Minimal or Smart, so we have to do a potentially slow operation,
+    // which is clearly documented here: QGraphicsItem::setBoundingRegionGranularity.
+    const QRegion region = xform.map(QRegion(rect.toAlignedRect()));
+    QRect viewRect = region.boundingRect();
+    const bool dontAdjustForAntialiasing = optimizationFlags & QGraphicsView::DontAdjustForAntialiasing;
+    if (dontAdjustForAntialiasing)
+        viewRect.adjust(-1, -1, 1, 1);
+    else
+        viewRect.adjust(-2, -2, 2, 2);
+    if (!intersectsViewport(viewRect, viewport->width(), viewport->height()))
+        return false; // Update region for sure outside viewport.
+
+    const QVector<QRect> &rects = region.rects();
+    for (int i = 0; i < rects.size(); ++i) {
+        viewRect = rects.at(i);
+        if (dontAdjustForAntialiasing)
+            viewRect.adjust(-1, -1, 1, 1);
+        else
+            viewRect.adjust(-2, -2, 2, 2);
+        dirtyRegion += viewRect;
     }
 
     return true;
 }
 
+// NB! Assumes the rect 'r' is already aligned and adjusted for antialiasing.
+// For QRectF use updateRectF(const QRectF &) to ensure proper adjustments.
 bool QGraphicsViewPrivate::updateRect(const QRect &r)
 {
     if (fullUpdatePending || viewportUpdateMode == QGraphicsView::NoViewportUpdate
@@ -943,10 +938,7 @@ bool QGraphicsViewPrivate::updateRect(const QRect &r)
         break;
     case QGraphicsView::SmartViewportUpdate: // ### DEPRECATE
     case QGraphicsView::MinimalViewportUpdate:
-        if (optimizationFlags & QGraphicsView::DontAdjustForAntialiasing)
-            dirtyRegion += r.adjusted(-1, -1, 1, 1);
-        else
-            dirtyRegion += r.adjusted(-2, -2, 2, 2);
+        dirtyRegion += r;
         break;
     default:
         break;
@@ -3403,8 +3395,14 @@ void QGraphicsView::paintEvent(QPaintEvent *event)
 
     // Items
     if (!(d->optimizationFlags & IndirectPainting)) {
+        const quint32 oldRectAdjust = d->scene->d_func()->rectAdjust;
+        if (d->optimizationFlags & QGraphicsView::DontAdjustForAntialiasing)
+            d->scene->d_func()->rectAdjust = 1;
+        else
+            d->scene->d_func()->rectAdjust = 2;
         d->scene->d_func()->drawItems(&painter, viewTransformed ? &viewTransform : 0,
                                       &d->exposedRegion, viewport());
+        d->scene->d_func()->rectAdjust = oldRectAdjust;
         // Make sure the painter's world transform is restored correctly when
         // drawing without painter state protection (DontSavePainterState).
         // We only change the worldTransform() so there's no need to do a full-blown
diff --git a/src/gui/graphicsview/qgraphicsview_p.h b/src/gui/graphicsview/qgraphicsview_p.h
index 80e3ec1..aeff28a 100644
--- a/src/gui/graphicsview/qgraphicsview_p.h
+++ b/src/gui/graphicsview/qgraphicsview_p.h
@@ -192,8 +192,17 @@ public:
 #endif
     }
 
+    inline bool updateRectF(const QRectF &rect)
+    {
+        if (rect.isEmpty())
+            return false;
+        if (optimizationFlags & QGraphicsView::DontAdjustForAntialiasing)
+            return updateRect(rect.toAlignedRect().adjusted(-1, -1, 1, 1));
+        return updateRect(rect.toAlignedRect().adjusted(-2, -2, 2, 2));
+    }
+
     bool updateRect(const QRect &rect);
-    bool updateRegion(const QRegion &region);
+    bool updateRegion(const QRectF &rect, const QTransform &xform);
     bool updateSceneSlotReimplementedChecked;
     QRegion exposedRegion;