QPainter::worldTransform() does not return identity matrix.

QPainter::worldTransform() does not return identity matrix when created
on a redirected widget. It should always be identity by default, and
should only change as a result of QPainter::setWorldTransform. The
reason it didn't return identity for redirected widgets, was that we
translated the shared painter's world matrix directly.

Since we cannot modify the world matrix directly, we have to store
the shared painter's current world transform in a separate matrix
(redirectedMatrix), reset the world transform to identity, and later
combine the redirectedMatrix with world transforms set on the painter.

Note that redirection_offset was in negative coordinates before,
and that redirectionMatrix now is in positive coordinates, hence opposite
signs around.

Auto-test included.

Reviewed-by: lars
Reviewed-by: Samuel

4 files changed, 115 insertions, 31 deletions

diff --git a/src/gui/painting/qpaintengineex.cpp b/src/gui/painting/qpaintengineex.cpp
index 3cf5ff9..d2671c8 100644
--- a/src/gui/painting/qpaintengineex.cpp
+++ b/src/gui/painting/qpaintengineex.cpp
@@ -151,8 +151,7 @@ void QPaintEngineExPrivate::replayClipOperations()
 
     QTransform transform = q->state()->matrix;
 
-    QTransform redirection;
-    redirection.translate(-q->state()->redirection_offset.x(), -q->state()->redirection_offset.y());
+    const QTransform &redirection = q->state()->redirectionMatrix;
 
     for (int i = 0; i <  clipInfo.size(); ++i) {
         const QPainterClipInfo &info = clipInfo.at(i);
diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp
index 4744f14..0ece498 100644
--- a/src/gui/painting/qpainter.cpp
+++ b/src/gui/painting/qpainter.cpp
@@ -281,10 +281,14 @@ bool QPainterPrivate::attachPainterPrivate(QPainter *q, QPaintDevice *pdev)
     q->d_ptr->state->wh = q->d_ptr->state->vh = widget->height();
 
     // Update matrix.
-    if (q->d_ptr->state->WxF)
-        q->d_ptr->state->worldMatrix.translate(-offset.x(), -offset.y());
-    else
-        q->d_ptr->state->redirection_offset = offset;
+    if (q->d_ptr->state->WxF) {
+        q->d_ptr->state->redirectionMatrix *= q->d_ptr->state->worldMatrix;
+        q->d_ptr->state->redirectionMatrix.translate(-offset.x(), -offset.y());
+        q->d_ptr->state->worldMatrix = QTransform();
+        q->d_ptr->state->WxF = false;
+    } else {
+        q->d_ptr->state->redirectionMatrix = QTransform::fromTranslate(-offset.x(), -offset.y());
+    }
     q->d_ptr->updateMatrix();
 
     QPaintEnginePrivate *enginePrivate = q->d_ptr->engine->d_func();
@@ -410,7 +414,7 @@ void QPainterPrivate::draw_helper(const QPainterPath &originalPath, DrawOperatio
             bool old_txinv = txinv;
             QTransform old_invMatrix = invMatrix;
             txinv = true;
-            invMatrix = QTransform().translate(-state->redirection_offset.x(), -state->redirection_offset.y());
+            invMatrix = state->redirectionMatrix;
             QPainterPath clipPath = q->clipPath();
             QRectF r = clipPath.boundingRect().intersected(absPathRect);
             absPathRect = r.toAlignedRect();
@@ -634,20 +638,7 @@ void QPainterPrivate::updateMatrix()
         state->matrix *= viewTransform();
 
     txinv = false;                                // no inverted matrix
-    if (!state->redirection_offset.isNull()) {
-        // We want to translate in dev space so we do the adding of the redirection
-        // offset manually.
-        if (state->matrix.isAffine()) {
-            state->matrix = QTransform(state->matrix.m11(), state->matrix.m12(),
-                                       state->matrix.m21(), state->matrix.m22(),
-                                       state->matrix.dx()-state->redirection_offset.x(),
-                                       state->matrix.dy()-state->redirection_offset.y());
-        } else {
-            QTransform temp;
-            temp.translate(-state->redirection_offset.x(), -state->redirection_offset.y());
-            state->matrix *= temp;
-        }
-    }
+    state->matrix *= state->redirectionMatrix;
     if (extended)
         extended->transformChanged();
     else
@@ -1572,10 +1563,8 @@ void QPainter::restore()
         // replay the list of clip states,
         for (int i=0; i<d->state->clipInfo.size(); ++i) {
             const QPainterClipInfo &info = d->state->clipInfo.at(i);
-            tmp->matrix.setMatrix(info.matrix.m11(), info.matrix.m12(), info.matrix.m13(),
-                                  info.matrix.m21(), info.matrix.m22(), info.matrix.m23(),
-                                  info.matrix.dx() - d->state->redirection_offset.x(),
-                                  info.matrix.dy() - d->state->redirection_offset.y(), info.matrix.m33());
+            tmp->matrix = info.matrix;
+            tmp->matrix *= d->state->redirectionMatrix;
             tmp->clipOperation = info.operation;
             if (info.clipType == QPainterClipInfo::RectClip) {
                 tmp->dirtyFlags = QPaintEngine::DirtyClipRegion | QPaintEngine::DirtyTransform;
@@ -1689,7 +1678,7 @@ bool QPainter::begin(QPaintDevice *pd)
     d->state->painter = this;
     d->states.push_back(d->state);
 
-    d->state->redirection_offset = redirectionOffset;
+    d->state->redirectionMatrix.translate(-redirectionOffset.x(), -redirectionOffset.y());
     d->state->brushOrigin = QPointF();
 
     if (!d->engine) {
@@ -1723,7 +1712,8 @@ bool QPainter::begin(QPaintDevice *pd)
             // Adjust offset for alien widgets painting outside the paint event.
             if (!inPaintEvent && paintOutsidePaintEvent && !widget->internalWinId()
                 && widget->testAttribute(Qt::WA_WState_Created)) {
-                d->state->redirection_offset -= widget->mapTo(widget->nativeParentWidget(), QPoint());
+                const QPoint offset = widget->mapTo(widget->nativeParentWidget(), QPoint());
+                d->state->redirectionMatrix.translate(offset.x(), offset.y());
             }
             break;
         }
@@ -1805,11 +1795,12 @@ bool QPainter::begin(QPaintDevice *pd)
         d->state->wh = d->state->vh = pd->metric(QPaintDevice::PdmHeight);
     }
 
-    d->state->redirection_offset += d->engine->coordinateOffset();
+    const QPoint coordinateOffset = d->engine->coordinateOffset();
+    d->state->redirectionMatrix.translate(-coordinateOffset.x(), -coordinateOffset.y());
 
     Q_ASSERT(d->engine->isActive());
 
-    if (!d->state->redirection_offset.isNull())
+    if (!d->state->redirectionMatrix.isIdentity())
         d->updateMatrix();
 
     Q_ASSERT(d->engine->isActive());
@@ -7704,7 +7695,7 @@ QPainterState::QPainterState(const QPainterState *s)
       clipRegion(s->clipRegion), clipPath(s->clipPath),
       clipOperation(s->clipOperation),
       renderHints(s->renderHints), clipInfo(s->clipInfo),
-      worldMatrix(s->worldMatrix), matrix(s->matrix), redirection_offset(s->redirection_offset),
+      worldMatrix(s->worldMatrix), matrix(s->matrix), redirectionMatrix(s->redirectionMatrix),
       wx(s->wx), wy(s->wy), ww(s->ww), wh(s->wh),
       vx(s->vx), vy(s->vy), vw(s->vw), vh(s->vh),
       opacity(s->opacity), WxF(s->WxF), VxF(s->VxF),
diff --git a/src/gui/painting/qpainter_p.h b/src/gui/painting/qpainter_p.h
index 6c8821a..8d4e6c5 100644
--- a/src/gui/painting/qpainter_p.h
+++ b/src/gui/painting/qpainter_p.h
@@ -158,7 +158,7 @@ public:
     QList<QPainterClipInfo> clipInfo; // ### Make me smaller and faster to copy around...
     QTransform worldMatrix;       // World transformation matrix, not window and viewport
     QTransform matrix;            // Complete transformation matrix,
-    QPoint redirection_offset;
+    QTransform redirectionMatrix;
     int wx, wy, ww, wh;         // window rectangle
     int vx, vy, vw, vh;         // viewport rectangle
     qreal opacity;
diff --git a/tests/auto/qwidget/tst_qwidget.cpp b/tests/auto/qwidget/tst_qwidget.cpp
index 041aa7a..85d7de1 100644
--- a/tests/auto/qwidget/tst_qwidget.cpp
+++ b/tests/auto/qwidget/tst_qwidget.cpp
@@ -288,6 +288,7 @@ private slots:
     void render_systemClip3_data();
     void render_systemClip3();
     void render_task252837();
+    void render_worldTransform();
 
     void setContentsMargins();
 
@@ -7151,6 +7152,99 @@ void tst_QWidget::render_task252837()
     // Please do not crash.
     widget.render(&painter);
 }
+
+void tst_QWidget::render_worldTransform()
+{
+    class MyWidget : public QWidget
+    { public:
+        void paintEvent(QPaintEvent *)
+        {
+            QPainter painter(this);
+            // Make sure world transform is identity.
+            QCOMPARE(painter.worldTransform(), QTransform());
+
+            // Make sure device transform is correct.
+            const QPoint widgetOffset = geometry().topLeft();
+            QTransform expectedDeviceTransform = QTransform::fromTranslate(105, 5);
+            expectedDeviceTransform.rotate(90);
+            expectedDeviceTransform.translate(widgetOffset.x(), widgetOffset.y());
+            QCOMPARE(painter.deviceTransform(), expectedDeviceTransform);
+
+            // Set new world transform.
+            QTransform newWorldTransform = QTransform::fromTranslate(10, 10);
+            newWorldTransform.rotate(90);
+            painter.setWorldTransform(newWorldTransform);
+            QCOMPARE(painter.worldTransform(), newWorldTransform);
+
+            // Again, check device transform.
+            expectedDeviceTransform.translate(10, 10);
+            expectedDeviceTransform.rotate(90);
+            QCOMPARE(painter.deviceTransform(), expectedDeviceTransform);
+
+            painter.fillRect(QRect(0, 0, 20, 10), Qt::green);
+        }
+    };
+
+    MyWidget widget;
+    widget.setFixedSize(100, 100);
+    widget.setPalette(Qt::red);
+    widget.setAutoFillBackground(true);
+
+    MyWidget child;
+    child.setParent(&widget);
+    child.move(50, 50);
+    child.setFixedSize(50, 50);
+    child.setPalette(Qt::blue);
+    child.setAutoFillBackground(true);
+
+    QImage image(QSize(110, 110), QImage::Format_RGB32);
+    image.fill(QColor(Qt::black).rgb());
+
+    QPainter painter(&image);
+    painter.translate(105, 5);
+    painter.rotate(90);
+
+    const QTransform worldTransform = painter.worldTransform();
+    const QTransform deviceTransform = painter.deviceTransform();
+
+    // Render widgets onto image.
+    widget.render(&painter);
+#ifdef RENDER_DEBUG
+    image.save("render_worldTransform_image.png");
+#endif
+
+    // Ensure the transforms are unchanged after render.
+    QCOMPARE(painter.worldTransform(), painter.worldTransform());
+    QCOMPARE(painter.deviceTransform(), painter.deviceTransform());
+    painter.end();
+
+    // Paint expected image.
+    QImage expected(QSize(110, 110), QImage::Format_RGB32);
+    expected.fill(QColor(Qt::black).rgb());
+
+    QPainter expectedPainter(&expected);
+    expectedPainter.translate(105, 5);
+    expectedPainter.rotate(90);
+    expectedPainter.save();
+    expectedPainter.fillRect(widget.rect(),Qt::red);
+    expectedPainter.translate(10, 10);
+    expectedPainter.rotate(90);
+    expectedPainter.fillRect(QRect(0, 0, 20, 10), Qt::green);
+    expectedPainter.restore();
+    expectedPainter.translate(50, 50);
+    expectedPainter.fillRect(child.rect(),Qt::blue);
+    expectedPainter.translate(10, 10);
+    expectedPainter.rotate(90);
+    expectedPainter.fillRect(QRect(0, 0, 20, 10), Qt::green);
+    expectedPainter.end();
+
+#ifdef RENDER_DEBUG
+    expected.save("render_worldTransform_expected.png");
+#endif
+
+    QCOMPARE(image, expected);
+}
+
 void tst_QWidget::setContentsMargins()
 {
     QLabel label("why does it always rain on me?");