Fix QGraphicsItem::deviceTransform() to also work with normal items.

QGraphicsItem::deviceTransform() returns the item-to-device transform,
provided with the device-to-scene transform, and combining it with the
item's scene transform. This function is meant to handle items that
enable ItemIgnoresTransformations, but it happened to not work properly
for items that _don't_ enable that flag. Unfortunately this bug is
hard to work around for users from the outside, as it requires you to
check if the item or any ancestor enables ItemIgnoresTransformations.

The fix also removes unnecessary branchs inside QGV so that we use the
same function for all items.

Reviewed-by: bnilsen

4 files changed, 120 insertions, 28 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 30c15bc..4908296 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -2572,6 +2572,10 @@ QTransform QGraphicsItem::sceneTransform() const
 */
 QTransform QGraphicsItem::deviceTransform(const QTransform &viewportTransform) const
 {
+    // Ensure we return the standard transform if we're not untransformable.
+    if (!d_ptr->itemIsUntransformable())
+        return sceneTransform() * viewportTransform;
+
     // Find the topmost item that ignores view transformations.
     const QGraphicsItem *untransformedAncestor = this;
     QList<const QGraphicsItem *> parents;
diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 13f70e5..b89e352 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -2289,12 +2289,7 @@ void QGraphicsScene::render(QPainter *painter, const QRectF &target, const QRect
         // Calculate a simple level-of-detail metric.
         // ### almost identical code in QGraphicsView::paintEvent()
         //     and QGraphicsView::render() - consider refactoring
-        QTransform itemToDeviceTransform;
-        if (item->d_ptr->itemIsUntransformable()) {
-            itemToDeviceTransform = item->deviceTransform(painterTransform);
-        } else {
-            itemToDeviceTransform = item->sceneTransform() * painterTransform;
-        }
+        QTransform itemToDeviceTransform = item->deviceTransform(painterTransform);
 
         option.levelOfDetail = qSqrt(itemToDeviceTransform.map(v1).length() * itemToDeviceTransform.map(v2).length());
         option.matrix = itemToDeviceTransform.toAffine(); //### discards perspective
@@ -5078,11 +5073,7 @@ void QGraphicsScene::drawItems(QPainter *painter,
                 // optimization, but it's hit very rarely.
                 for (int i = clippers.size() - 1; i >= 0; --i) {
                     QGraphicsItem *clipper = clippers[i];
-                    if (clipper->d_ptr->itemIsUntransformable()) {
-                        painter->setWorldTransform(clipper->deviceTransform(viewTransform), false);
-                    } else {
-                        painter->setWorldTransform(clipper->sceneTransform() * viewTransform, false);
-                    }
+                    painter->setWorldTransform(clipper->deviceTransform(viewTransform), false);
 
                     childClippers.append(clipper);
                     painter->save();
@@ -5093,12 +5084,8 @@ void QGraphicsScene::drawItems(QPainter *painter,
         }
 
         // Set up the painter transform
-        if (item->d_ptr->itemIsUntransformable()) {
-            painter->setWorldTransform(item->deviceTransform(viewTransform), false);
-        } else {
-            painter->setWorldTransform(item->sceneTransform() * viewTransform, false);
-        }
-       
+        painter->setWorldTransform(item->deviceTransform(viewTransform), false);
+
         // Save painter
         bool saveState = (d->painterStateProtection || (item->flags() & QGraphicsItem::ItemClipsToShape));
         if (saveState)
diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp
index 8b133f3..a795fb4 100644
--- a/src/gui/graphicsview/qgraphicsview.cpp
+++ b/src/gui/graphicsview/qgraphicsview.cpp
@@ -1153,11 +1153,7 @@ void QGraphicsViewPrivate::generateStyleOptions(const QList<QGraphicsItem *> &it
         // Calculate a simple level-of-detail metric.
         // ### almost identical code in QGraphicsScene::render()
         //     and QGraphicsView::render() - consider refactoring
-        if (item->d_ptr->itemIsUntransformable()) {
-            itemToViewportTransform = item->deviceTransform(worldTransform);
-        } else {
-            itemToViewportTransform = item->sceneTransform() * worldTransform;
-        }
+        itemToViewportTransform = item->deviceTransform(worldTransform);
 
         if (itemToViewportTransform.type() <= QTransform::TxTranslate) {
             // Translation and rotation only? The LOD is 1.
@@ -2160,12 +2156,7 @@ void QGraphicsView::render(QPainter *painter, const QRectF &target, const QRect
         // Calculate a simple level-of-detail metric.
         // ### almost identical code in QGraphicsScene::render()
         //     and QGraphicsView::paintEvent() - consider refactoring
-        QTransform itemToViewportTransform;
-        if (item->d_ptr->itemIsUntransformable()) {
-            itemToViewportTransform = item->deviceTransform(painterMatrix);
-        } else {
-            itemToViewportTransform = item->sceneTransform() * painterMatrix;
-        }
+        QTransform itemToViewportTransform = item->deviceTransform(painterMatrix);
 
         option->levelOfDetail = qSqrt(itemToViewportTransform.map(v1).length() * itemToViewportTransform.map(v2).length());
         option->matrix = itemToViewportTransform.toAffine();
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index 6d150cb..58a17ea 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -215,6 +215,8 @@ private slots:
     void tabChangesFocus_data();
     void cacheMode();
     void updateCachedItemAfterMove();
+    void deviceTransform_data();
+    void deviceTransform();
 
     // task specific tests below me
     void task141694_textItemEnsureVisible();
@@ -6200,5 +6202,113 @@ void tst_QGraphicsItem::updateCachedItemAfterMove()
     QCOMPARE(tester->repaints, 1);
 }
 
+class Track : public QGraphicsRectItem
+{
+public:
+    Track(const QRectF &rect)
+        : QGraphicsRectItem(rect)
+    {
+        setAcceptHoverEvents(true);
+    }
+
+    void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget = 0)
+    {
+        QGraphicsRectItem::paint(painter, option, widget);
+        painter->drawText(boundingRect(), Qt::AlignCenter, QString("%1x%2\n%3x%4").arg(p.x()).arg(p.y()).arg(sp.x()).arg(sp.y()));
+    }
+    
+protected:
+    void hoverMoveEvent(QGraphicsSceneHoverEvent *event)
+    {
+        p = event->pos();
+        sp = event->widget()->mapFromGlobal(event->screenPos());
+        update();
+    }
+private:
+    QPointF p;
+    QPoint sp;
+};
+
+void tst_QGraphicsItem::deviceTransform_data()
+{
+    QTest::addColumn<bool>("untransformable1");
+    QTest::addColumn<bool>("untransformable2");
+    QTest::addColumn<bool>("untransformable3");
+    QTest::addColumn<qreal>("rotation1");
+    QTest::addColumn<qreal>("rotation2");
+    QTest::addColumn<qreal>("rotation3");
+    QTest::addColumn<QTransform>("deviceX");
+    QTest::addColumn<QPointF>("mapResult1");
+    QTest::addColumn<QPointF>("mapResult2");
+    QTest::addColumn<QPointF>("mapResult3");
+
+    QTest::newRow("nil") << false << false << false
+                         << qreal(0.0) << qreal(0.0) << qreal(0.0)
+                         << QTransform()
+                         << QPointF(150, 150) << QPointF(250, 250) << QPointF(350, 350);
+    QTest::newRow("deviceX rot 90") << false << false << false
+                         << qreal(0.0) << qreal(0.0) << qreal(0.0)
+                         << QTransform().rotate(90)
+                         << QPointF(-150, 150) << QPointF(-250, 250) << QPointF(-350, 350);
+    QTest::newRow("deviceX rot 90 100") << true << false << false
+                         << qreal(0.0) << qreal(0.0) << qreal(0.0)
+                         << QTransform().rotate(90)
+                         << QPointF(-50, 150) << QPointF(50, 250) << QPointF(150, 350);
+    QTest::newRow("deviceX rot 90 010") << false << true << false
+                         << qreal(0.0) << qreal(0.0) << qreal(0.0)
+                         << QTransform().rotate(90)
+                         << QPointF(-150, 150) << QPointF(-150, 250) << QPointF(-50, 350);
+    QTest::newRow("deviceX rot 90 001") << false << false << true
+                         << qreal(0.0) << qreal(0.0) << qreal(0.0)
+                         << QTransform().rotate(90)
+                         << QPointF(-150, 150) << QPointF(-250, 250) << QPointF(-250, 350);
+    QTest::newRow("deviceX rot 90 111") << true << true << true
+                         << qreal(0.0) << qreal(0.0) << qreal(0.0)
+                         << QTransform().rotate(90)
+                         << QPointF(-50, 150) << QPointF(50, 250) << QPointF(150, 350);
+    QTest::newRow("deviceX rot 90 101") << true << false << true
+                         << qreal(0.0) << qreal(0.0) << qreal(0.0)
+                         << QTransform().rotate(90)
+                         << QPointF(-50, 150) << QPointF(50, 250) << QPointF(150, 350);
+}
+
+void tst_QGraphicsItem::deviceTransform()
+{
+    QFETCH(bool, untransformable1);
+    QFETCH(bool, untransformable2);
+    QFETCH(bool, untransformable3);
+    QFETCH(qreal, rotation1);
+    QFETCH(qreal, rotation2);
+    QFETCH(qreal, rotation3);
+    QFETCH(QTransform, deviceX);
+    QFETCH(QPointF, mapResult1);
+    QFETCH(QPointF, mapResult2);
+    QFETCH(QPointF, mapResult3);
+
+    QGraphicsScene scene;
+    Track *rect1 = new Track(QRectF(0, 0, 100, 100));
+    Track *rect2 = new Track(QRectF(0, 0, 100, 100));
+    Track *rect3 = new Track(QRectF(0, 0, 100, 100));
+    rect2->setParentItem(rect1);
+    rect3->setParentItem(rect2);
+    rect1->setPos(100, 100);
+    rect2->setPos(100, 100);
+    rect3->setPos(100, 100);
+    rect1->rotate(rotation1);
+    rect2->rotate(rotation2);
+    rect3->rotate(rotation3);
+    rect1->setFlag(QGraphicsItem::ItemIgnoresTransformations, untransformable1);
+    rect2->setFlag(QGraphicsItem::ItemIgnoresTransformations, untransformable2);
+    rect3->setFlag(QGraphicsItem::ItemIgnoresTransformations, untransformable3);
+    rect1->setBrush(Qt::red);
+    rect2->setBrush(Qt::green);
+    rect3->setBrush(Qt::blue);
+    scene.addItem(rect1);
+
+    QCOMPARE(rect1->deviceTransform(deviceX).map(QPointF(50, 50)), mapResult1);
+    QCOMPARE(rect2->deviceTransform(deviceX).map(QPointF(50, 50)), mapResult2);
+    QCOMPARE(rect3->deviceTransform(deviceX).map(QPointF(50, 50)), mapResult3);
+}
+
 QTEST_MAIN(tst_QGraphicsItem)
 #include "tst_qgraphicsitem.moc"