From ed21b9dafea192ddd39f1d2a4abcb54962d6dfb8 Mon Sep 17 00:00:00 2001 From: Bjoern Erik Nilsen Date: Fri, 24 Apr 2009 17:34:28 +0200 Subject: Sometimes wrong clipping in QWidget::render() when passing a device or an untransformed painter When passing a painter to QWidget::render, we use the painter->paintEngine()->systemClip() as the "system viewport", i.e. all painting triggered by render() should be limited to this area. The only way to achieve this is by always ensuring the system clip is clipped to the same area (systemClip &= systemViewport). The problem however, was that we only did this for transformed painters. We must of course always do it when there's a systemViewport set, regardless of whether the painter is transformed or not. Auto test included. Task-number: 248852 Reviewed-by: Trond --- src/gui/kernel/qwidget.cpp | 11 +++-- src/gui/painting/qpaintengine.cpp | 4 +- src/gui/painting/qpaintengine_p.h | 12 +++-- tests/auto/qwidget/tst_qwidget.cpp | 98 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 11 deletions(-) diff --git a/src/gui/kernel/qwidget.cpp b/src/gui/kernel/qwidget.cpp index 071e1bd..e9fd28b 100644 --- a/src/gui/kernel/qwidget.cpp +++ b/src/gui/kernel/qwidget.cpp @@ -4709,10 +4709,13 @@ void QWidget::render(QPaintDevice *target, const QPoint &targetOffset, if (redirected) { target = redirected; offset -= redirectionOffset; - if (!inRenderWithPainter) { // Clip handled by shared painter (in qpainter.cpp). - const QRegion redirectedSystemClip = redirected->paintEngine()->systemClip(); - if (!redirectedSystemClip.isEmpty()) - paintRegion &= redirectedSystemClip.translated(-offset); + } + + if (!inRenderWithPainter) { // Clip handled by shared painter (in qpainter.cpp). + if (QPaintEngine *targetEngine = target->paintEngine()) { + const QRegion targetSystemClip = targetEngine->systemClip(); + if (!targetSystemClip.isEmpty()) + paintRegion &= targetSystemClip.translated(-offset); } } diff --git a/src/gui/painting/qpaintengine.cpp b/src/gui/painting/qpaintengine.cpp index ad09060..7de1ec4 100644 --- a/src/gui/painting/qpaintengine.cpp +++ b/src/gui/painting/qpaintengine.cpp @@ -949,8 +949,8 @@ void QPaintEngine::setSystemClip(const QRegion ®ion) Q_D(QPaintEngine); d->systemClip = region; // Be backward compatible and only call d->systemStateChanged() - // if we currently have a system transform set. - if (d->hasSystemTransform) { + // if we currently have a system transform/viewport set. + if (d->hasSystemTransform || d->hasSystemViewport) { d->transformSystemClip(); d->systemStateChanged(); } diff --git a/src/gui/painting/qpaintengine_p.h b/src/gui/painting/qpaintengine_p.h index eeba7ec..0b5e175 100644 --- a/src/gui/painting/qpaintengine_p.h +++ b/src/gui/painting/qpaintengine_p.h @@ -83,10 +83,12 @@ public: if (systemClip.isEmpty()) return; - if (systemTransform.type() <= QTransform::TxTranslate) - systemClip.translate(qRound(systemTransform.dx()), qRound(systemTransform.dy())); - else - systemClip = systemTransform.map(systemClip); + if (hasSystemTransform) { + if (systemTransform.type() <= QTransform::TxTranslate) + systemClip.translate(qRound(systemTransform.dx()), qRound(systemTransform.dy())); + else + systemClip = systemTransform.map(systemClip); + } // Make sure we're inside the viewport. if (hasSystemViewport) { @@ -101,7 +103,7 @@ public: inline void setSystemTransform(const QTransform &xform) { systemTransform = xform; - if ((hasSystemTransform = !xform.isIdentity())) + if ((hasSystemTransform = !xform.isIdentity()) || hasSystemViewport) transformSystemClip(); systemStateChanged(); } diff --git a/tests/auto/qwidget/tst_qwidget.cpp b/tests/auto/qwidget/tst_qwidget.cpp index a052034..767553a 100644 --- a/tests/auto/qwidget/tst_qwidget.cpp +++ b/tests/auto/qwidget/tst_qwidget.cpp @@ -283,6 +283,8 @@ private slots: void render_task217815(); void render_windowOpacity(); void render_systemClip(); + void render_systemClip2_data(); + void render_systemClip2(); void setContentsMargins(); @@ -6897,6 +6899,102 @@ void tst_QWidget::render_systemClip() #endif } +void tst_QWidget::render_systemClip2_data() +{ + QTest::addColumn("autoFillBackground"); + QTest::addColumn("usePaintEvent"); + QTest::addColumn("expectedColor"); + + QTest::newRow("Only auto-fill background") << true << false << QColor(Qt::blue); + QTest::newRow("Only draw in paintEvent") << false << true << QColor(Qt::green); + QTest::newRow("Auto-fill background and draw in paintEvent") << true << true << QColor(Qt::green); +} + +void tst_QWidget::render_systemClip2() +{ + QFETCH(bool, autoFillBackground); + QFETCH(bool, usePaintEvent); + QFETCH(QColor, expectedColor); + + Q_ASSERT_X(expectedColor != QColor(Qt::red), Q_FUNC_INFO, + "Qt::red is the reference color for the image, pick another color"); + + class MyWidget : public QWidget + { + public: + bool usePaintEvent; + void paintEvent(QPaintEvent *) + { + if (usePaintEvent) + QPainter(this).fillRect(rect(), Qt::green); + } + }; + + MyWidget widget; + widget.usePaintEvent = usePaintEvent; + widget.setPalette(Qt::blue); + // NB! widget.setAutoFillBackground(autoFillBackground) won't do the + // trick here since the widget is a top-level. The background is filled + // regardless, unless Qt::WA_OpaquePaintEvent or Qt::WA_NoSystemBackground + // is set. We therefore use the opaque attribute to turn off auto-fill. + if (!autoFillBackground) + widget.setAttribute(Qt::WA_OpaquePaintEvent); + widget.resize(100, 100); + + QImage image(widget.size(), QImage::Format_RGB32); + image.fill(QColor(Qt::red).rgb()); + + QPaintEngine *paintEngine = image.paintEngine(); + QVERIFY(paintEngine); + + QRegion systemClip(QRegion(50, 0, 50, 10)); + systemClip += QRegion(90, 10, 10, 40); + paintEngine->setSystemClip(systemClip); + + // Render entire widget directly onto device. + widget.render(&image); + +#ifndef RENDER_DEBUG + image.save("systemclip_with_device.png"); +#endif + // All pixels within the system clip should now be + // the expectedColor, and the rest should be red. + for (int i = 0; i < image.height(); ++i) { + for (int j = 0; j < image.width(); ++j) { + if (systemClip.contains(QPoint(j, i))) + QCOMPARE(image.pixel(j, i), expectedColor.rgb()); + else + QCOMPARE(image.pixel(j, i), QColor(Qt::red).rgb()); + } + } + + // Refill image with red. + image.fill(QColor(Qt::red).rgb()); + + // Do the same with an untransformed painter. + QPainter painter(&image); + //Make sure we're using the same paint engine and has the right clip set. + paintEngine->setSystemClip(systemClip); + QCOMPARE(painter.paintEngine(), paintEngine); + QCOMPARE(paintEngine->systemClip(), systemClip); + + widget.render(&painter); + +#ifndef RENDER_DEBUG + image.save("systemclip_with_untransformed_painter.png"); +#endif + // All pixels within the system clip should now be + // the expectedColor, and the rest should be red. + for (int i = 0; i < image.height(); ++i) { + for (int j = 0; j < image.width(); ++j) { + if (systemClip.contains(QPoint(j, i))) + QCOMPARE(image.pixel(j, i), expectedColor.rgb()); + else + QCOMPARE(image.pixel(j, i), QColor(Qt::red).rgb()); + } + } +} + void tst_QWidget::setContentsMargins() { QLabel label("why does it always rain on me?"); -- cgit v0.12