Fix QGraphicsView::mapToScene(QRect) to avoid extra adjustments.

The QGraphicsView::mapToScene(QRect) function assumes that QRect and
QRectF share semantics for ::bottomRight(). However, since QRect follows
Qt 3 semantics (the rect is based on viewport pixels and (0,0,1,1) is
equivalent to one pixel, topleft = bottomright), this function gives
unexpected behavior: map(0,0,1,1) gives you an empty polygon! To work
around this, users of the function need to adjust their rectangles
with (0,0,1,1) to get the correct behavior, matching what QRectF does.

QRectF(0,0,1,1).bottomRight() == QPointF(1,1)
QRect(0,0,1,1).bottomRight() == QPoint(0,0)

Reviewed-by: TrustMe

2 files changed, 49 insertions, 18 deletions

diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp
index bffbf4d..a9d585c 100644
--- a/src/gui/graphicsview/qgraphicsview.cpp
+++ b/src/gui/graphicsview/qgraphicsview.cpp
@@ -1075,7 +1075,7 @@ QList<QGraphicsItem *> QGraphicsViewPrivate::findItems(const QRegion &exposedReg
 
     // Step 1) If all items are contained within the expose region, then
     // return a list of all visible items.
-    const QRectF exposedRegionSceneBounds = q->mapToScene(exposedRegion.boundingRect().adjusted(-1, -1, 2, 2))
+    const QRectF exposedRegionSceneBounds = q->mapToScene(exposedRegion.boundingRect().adjusted(-1, -1, 1, 1))
                                             .boundingRect();
     if (exposedRegionSceneBounds.contains(scene->d_func()->growingItemsBoundingRect)) {
         Q_ASSERT(allItems);
@@ -2313,7 +2313,7 @@ QList<QGraphicsItem *> QGraphicsView::items(const QPoint &pos) const
             QTransform xinv = viewportTransform().inverted();
             return d->scene->items(xinv.mapRect(QRectF(pos.x(), pos.y(), 1, 1)));
         }
-        return d->scene->items(mapToScene(pos.x(), pos.y(), 2, 2));
+        return d->scene->items(mapToScene(pos.x(), pos.y(), 1, 1));
     }
 
     QPainterPath path;
@@ -2475,10 +2475,11 @@ QPolygonF QGraphicsView::mapToScene(const QRect &rect) const
         return QPolygonF();
 
     QPointF scrollOffset(d->horizontalScroll(), d->verticalScroll());
-    QPointF tl = scrollOffset + rect.topLeft();
-    QPointF tr = scrollOffset + rect.topRight();
-    QPointF br = scrollOffset + rect.bottomRight();
-    QPointF bl = scrollOffset + rect.bottomLeft();
+    QRect r = rect.adjusted(0, 0, 1, 1);
+    QPointF tl = scrollOffset + r.topLeft();
+    QPointF tr = scrollOffset + r.topRight();
+    QPointF br = scrollOffset + r.bottomRight();
+    QPointF bl = scrollOffset + r.bottomLeft();
 
     QPolygonF poly;
     poly.resize(4);
@@ -3471,7 +3472,7 @@ void QGraphicsView::paintEvent(QPaintEvent *event)
         exposedRegion = viewport()->rect();
     else if (d->viewportUpdateMode == BoundingRectViewportUpdate)
         exposedRegion = event->rect();
-    QRectF exposedSceneRect = mapToScene(exposedRegion.boundingRect().adjusted(0, 0, 1, 1)).boundingRect();
+    QRectF exposedSceneRect = mapToScene(exposedRegion.boundingRect()).boundingRect();
 
     // Set up the painter
     QPainter painter(viewport());
diff --git a/tests/auto/qgraphicsview/tst_qgraphicsview.cpp b/tests/auto/qgraphicsview/tst_qgraphicsview.cpp
index 9368630..ca88afc 100644
--- a/tests/auto/qgraphicsview/tst_qgraphicsview.cpp
+++ b/tests/auto/qgraphicsview/tst_qgraphicsview.cpp
@@ -72,6 +72,7 @@ Q_DECLARE_METATYPE(QList<QRectF>)
 Q_DECLARE_METATYPE(QMatrix)
 Q_DECLARE_METATYPE(QPainterPath)
 Q_DECLARE_METATYPE(QPointF)
+Q_DECLARE_METATYPE(QPolygonF)
 Q_DECLARE_METATYPE(QRectF)
 Q_DECLARE_METATYPE(Qt::ScrollBarPolicy)
 
@@ -159,6 +160,7 @@ private slots:
     void itemAt2();
     void mapToScene();
     void mapToScenePoint();
+    void mapToSceneRect_data();
     void mapToSceneRect();
     void mapToScenePoly();
     void mapToScenePath();
@@ -1612,23 +1614,51 @@ void tst_QGraphicsView::mapToScenePoint()
              view.mapToScene(center) + QPointF(0, -10));
 }
 
+void tst_QGraphicsView::mapToSceneRect_data()
+{
+    QTest::addColumn<QRect>("viewRect");
+    QTest::addColumn<QPolygonF>("scenePoly");
+    QTest::addColumn<qreal>("rotation");
+
+    QTest::newRow("nil") << QRect() << QPolygonF() << qreal(0);
+    QTest::newRow("0, 0, 1, 1") << QRect(0, 0, 1, 1) << QPolygonF(QRectF(0, 0, 1, 1)) << qreal(0);
+    QTest::newRow("0, 0, 10, 10") << QRect(0, 0, 10, 10) << QPolygonF(QRectF(0, 0, 10, 10)) << qreal(0);
+    QTest::newRow("nil") << QRect() << QPolygonF() << qreal(90);
+    QPolygonF p;
+    p << QPointF(0, 0) << QPointF(0, -1) << QPointF(1, -1) << QPointF(1, 0) << QPointF(0, 0);
+    QTest::newRow("0, 0, 1, 1") << QRect(0, 0, 1, 1)
+                                << p
+                                << qreal(90);
+    p.clear();
+    p << QPointF(0, 0) << QPointF(0, -10) << QPointF(10, -10) << QPointF(10, 0) << QPointF(0, 0);
+    QTest::newRow("0, 0, 10, 10") << QRect(0, 0, 10, 10)
+                                  << p
+                                  << qreal(90);
+}
+
 void tst_QGraphicsView::mapToSceneRect()
 {
-    QGraphicsScene scene;
+    QFETCH(QRect, viewRect);
+    QFETCH(QPolygonF, scenePoly);
+    QFETCH(qreal, rotation);
+
+    QGraphicsScene scene(-1000, -1000, 2000, 2000);
+    scene.addRect(25, -25, 50, 50);
     QGraphicsView view(&scene);
-    view.rotate(90);
-    view.setFixedSize(117, 117);
+    view.setFrameStyle(0);
+    view.setAlignment(Qt::AlignTop | Qt::AlignLeft);
+    view.setFixedSize(200, 200);
+    view.setTransformationAnchor(QGraphicsView::NoAnchor);
+    view.setResizeAnchor(QGraphicsView::NoAnchor);
     view.show();
-    QPoint center = view.viewport()->rect().center();
-    QRect rect(center + QPoint(10, 0), QSize(10, 10));
 
-    QPolygonF poly;
-    poly << view.mapToScene(rect.topLeft());
-    poly << view.mapToScene(rect.topRight());
-    poly << view.mapToScene(rect.bottomRight());
-    poly << view.mapToScene(rect.bottomLeft());
+    view.rotate(rotation);
+
+    QPolygonF poly = view.mapToScene(viewRect);
+    if (!poly.isEmpty())
+        poly << poly[0];
 
-    QCOMPARE(view.mapToScene(rect), poly);
+    QCOMPARE(poly, scenePoly);
 }
 
 void tst_QGraphicsView::mapToScenePoly()