diff --git a/src/gui/kernel/qhighdpiscaling_p.h b/src/gui/kernel/qhighdpiscaling_p.h index 89856570ce6..85527fe5983 100644 --- a/src/gui/kernel/qhighdpiscaling_p.h +++ b/src/gui/kernel/qhighdpiscaling_p.h @@ -219,10 +219,9 @@ inline QRegion scale(const QRegion ®ion, qreal scaleFactor, QPoint origin = Q if (!QHighDpiScaling::isActive()) return region; - QRegion scaled; - for (const QRect &rect : region) - scaled += scale(QRectF(rect), scaleFactor, origin).toRect(); - return scaled; + QRegion scaled = region.translated(-origin); + scaled = QTransform::fromScale(scaleFactor, scaleFactor).map(scaled); + return scaled.translated(origin); } template diff --git a/src/gui/painting/qmath_p.h b/src/gui/painting/qmath_p.h index 0191f8381f0..6691665f419 100644 --- a/src/gui/painting/qmath_p.h +++ b/src/gui/painting/qmath_p.h @@ -53,12 +53,27 @@ #include #include +#include QT_BEGIN_NAMESPACE static const qreal Q_PI = qreal(M_PI); // pi static const qreal Q_MM_PER_INCH = 25.4; +inline QRect qt_mapFillRect(const QRectF &rect, const QTransform &xf) +{ + // Only for xf <= scaling or 90 degree rotations + Q_ASSERT(xf.type() <= QTransform::TxScale + || (xf.type() == QTransform::TxRotate && qFuzzyIsNull(xf.m11()) && qFuzzyIsNull(xf.m22()))); + // Transform the corners instead of the rect to avoid hitting numerical accuracy limit + // when transforming topleft and size separately and adding afterwards, + // as that can sometimes be slightly off around the .5 point, leading to wrong rounding + QPoint pt1 = xf.map(rect.topLeft()).toPoint(); + QPoint pt2 = xf.map(rect.bottomRight()).toPoint(); + // Normalize and adjust for the QRect vs. QRectF bottomright + return QRect::span(pt1, pt2).adjusted(0, 0, -1, -1); +} + QT_END_NAMESPACE #endif // QMATH_P_H diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp index b044b485df8..6fbedabdbdd 100644 --- a/src/gui/painting/qpaintengine_raster.cpp +++ b/src/gui/painting/qpaintengine_raster.cpp @@ -49,7 +49,7 @@ #include #include #include -#include +#include "qmath_p.h" #include // #include @@ -1195,7 +1195,7 @@ void QRasterPaintEngine::clip(const QVectorPath &path, Qt::ClipOperation op) #endif const qreal *points = path.points(); QRectF r(points[0], points[1], points[4]-points[0], points[5]-points[1]); - if (setClipRectInDeviceCoords(s->matrix.mapRect(r).toAlignedRect(), op)) + if (setClipRectInDeviceCoords(qt_mapFillRect(r, s->matrix), op)) return; } } @@ -1255,7 +1255,7 @@ void QRasterPaintEngine::clip(const QRect &rect, Qt::ClipOperation op) QPaintEngineEx::clip(rect, op); return; - } else if (!setClipRectInDeviceCoords(s->matrix.mapRect(QRectF(rect)).toRect(), op)) { + } else if (!setClipRectInDeviceCoords(qt_mapFillRect(rect, s->matrix), op)) { QPaintEngineEx::clip(rect, op); return; } diff --git a/src/gui/painting/qtransform.cpp b/src/gui/painting/qtransform.cpp index 9d4f28c8954..c3189582078 100644 --- a/src/gui/painting/qtransform.cpp +++ b/src/gui/painting/qtransform.cpp @@ -45,7 +45,7 @@ #include "qpainterpath.h" #include "qpainterpath_p.h" #include "qvariant.h" -#include +#include "qmath_p.h" #include #include @@ -1486,12 +1486,12 @@ QRegion QTransform::map(const QRegion &r) const QRegion res; if (m11() < 0 || m22() < 0) { for (const QRect &rect : r) - res += mapRect(QRectF(rect)).toRect(); + res += qt_mapFillRect(QRectF(rect), *this); } else { QVarLengthArray rects; rects.reserve(r.rectCount()); for (const QRect &rect : r) { - QRect nr = mapRect(QRectF(rect)).toRect(); + QRect nr = qt_mapFillRect(QRectF(rect), *this); if (!nr.isEmpty()) rects.append(nr); } diff --git a/src/opengl/qopenglpaintengine.cpp b/src/opengl/qopenglpaintengine.cpp index c385b4c62b6..98ab71dad5a 100644 --- a/src/opengl/qopenglpaintengine.cpp +++ b/src/opengl/qopenglpaintengine.cpp @@ -2526,7 +2526,7 @@ void QOpenGL2PaintEngineEx::clip(const QVectorPath &path, Qt::ClipOperation op) && qFuzzyIsNull(state()->matrix.m11()) && qFuzzyIsNull(state()->matrix.m22()))) { - state()->rectangleClip = state()->rectangleClip.intersected(state()->matrix.mapRect(rect).toAlignedRect()); + state()->rectangleClip &= qt_mapFillRect(rect, state()->matrix); d->updateClipScissorTest(); return; } diff --git a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp index cfcaf5b67e1..c8bcd2f030f 100644 --- a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp +++ b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp @@ -69,6 +69,9 @@ Q_OBJECT public: tst_QPainter(); + enum ClipType { ClipRect, ClipRectF, ClipRegionSingle, ClipRegionMulti, ClipPathR, ClipPath }; + Q_ENUM(ClipType); + private slots: void cleanupTestCase(); void getSetCheck(); @@ -155,6 +158,8 @@ private slots: void clipBoundingRect(); void transformedClip(); + void scaledClipConsistency_data(); + void scaledClipConsistency(); void setOpacity_data(); void setOpacity(); @@ -1752,10 +1757,11 @@ void tst_QPainter::setClipRect() /* Verify that the clipping works correctly. - The red outline should be covered by the blue rect on top and left, - while it should be clipped on the right and bottom and thus the red outline be visible + Just like fillRect, cliprect should snap rightwards and downwards in case of .5 coordinates. + The red outline should be covered by the blue rect on top, + while it should be clipped on the other edges and thus the red outline be visible - See: QTBUG-83229 + See: QTBUG-83229, modified by QTBUG-100329 */ void tst_QPainter::clipRect() { @@ -1781,7 +1787,7 @@ void tst_QPainter::clipRect() p.end(); QCOMPARE(image.pixelColor(clipRect.left() + 1, clipRect.top()), QColor(Qt::blue)); - QCOMPARE(image.pixelColor(clipRect.left(), clipRect.top() + 1), QColor(Qt::blue)); + QCOMPARE(image.pixelColor(clipRect.left(), clipRect.top() + 1), QColor(Qt::red)); QCOMPARE(image.pixelColor(clipRect.left() + 1, clipRect.bottom()), QColor(Qt::red)); QCOMPARE(image.pixelColor(clipRect.right(), clipRect.top() + 1), QColor(Qt::red)); } @@ -4583,6 +4589,96 @@ void tst_QPainter::transformedClip() } } +void tst_QPainter::scaledClipConsistency_data() +{ + QTest::addColumn("clipType"); + + QTest::newRow("clipRect") << ClipRect; + QTest::newRow("clipRectF") << ClipRectF; + QTest::newRow("clipRegionSingle") << ClipRegionSingle; + QTest::newRow("clipRegionMulti") << ClipRegionMulti; + QTest::newRow("clipPathR") << ClipPathR; + QTest::newRow("clipPath") << ClipPath; +} + +void tst_QPainter::scaledClipConsistency() +{ + QFETCH(ClipType, clipType); + + const QList clipRects = { + // Varying odd and even coordinates and width/height + QRect(1, 1, 7, 8), + QRect(8, 0, 8, 9), + QRect(0, 9, 8, 7), + QRect(8, 9, 8, 7), + }; + // Assert that these are edge to edge: + QPointF center = QRectF(clipRects[0]).bottomRight(); + Q_ASSERT(QRectF(clipRects[1]).bottomLeft() == center); + Q_ASSERT(QRectF(clipRects[2]).topRight() == center); + Q_ASSERT(QRectF(clipRects[3]).topLeft() == center); + + QRegion multiRegion; + for (const QRect &clipRect : clipRects) + multiRegion += clipRect; + + QColor fillColor(Qt::black); + fillColor.setAlphaF(0.5); + + for (int i = 100; i <= 300; i++) { + qreal dpr = qreal(i) / 100.0; + QImage img(QSize(16, 16) * dpr, QImage::Format_RGB32); + img.fill(Qt::white); + img.setDevicePixelRatio(dpr); + + for (const QRect &clipRect : clipRects) { + QPainter p(&img); + switch (clipType) { + case ClipRect: + p.setClipRect(clipRect); + break; + case ClipRectF: + p.setClipRect(QRectF(clipRect)); + break; + case ClipRegionSingle: + p.setClipRegion(QRegion(clipRect)); + break; + case ClipRegionMulti: + p.setClipRegion(multiRegion); + break; + case ClipPath: + p.rotate(0.001); // Avoid the path being optimized to a rectf + Q_FALLTHROUGH(); + case ClipPathR: { + QPainterPath path; + path.addRect(clipRect); // Will be recognized and converted back to a rectf + p.setClipPath(path); + break; + } + default: + Q_ASSERT(false); + break; + } + p.fillRect(p.window(), fillColor); + if (clipType == ClipRegionMulti) + break; // once is enough, we're not using the clipRect anyway + } + + int qtWidth = img.width() / 4; + int qtHeight = img.height() / 4; + QPoint imgCenter = img.rect().center(); + const QRgb targetColor = img.pixel(qtWidth, qtHeight); + + // Test that there are no gaps or overlaps where the cliprects meet + for (int offset = -2; offset <= 2; offset++) { + QCOMPARE(img.pixel(imgCenter.x() + offset, qtHeight), targetColor); + QCOMPARE(img.pixel(imgCenter.x() + offset, img.height() - qtHeight), targetColor); + QCOMPARE(img.pixel(qtWidth, imgCenter.y() + offset), targetColor); + QCOMPARE(img.pixel(img.width() - qtWidth, imgCenter.y() + offset), targetColor); + } + } +} + #if defined(Q_OS_MAC) // Only Mac supports sub pixel positions in raster engine currently void tst_QPainter::drawText_subPixelPositionsInRaster_qtbug5053() diff --git a/tests/auto/gui/qopengl/tst_qopengl.cpp b/tests/auto/gui/qopengl/tst_qopengl.cpp index e2e7f153a0f..a0a1ecca177 100644 --- a/tests/auto/gui/qopengl/tst_qopengl.cpp +++ b/tests/auto/gui/qopengl/tst_qopengl.cpp @@ -1695,10 +1695,11 @@ void tst_QOpenGL::nullTextureInitializtion() /* Verify that the clipping works correctly. - The red outline should be covered by the blue rect on top and left, - while it should be clipped on the right and bottom and thus the red outline be visible + Just like fillRect, cliprect should snap rightwards and downwards in case of .5 coordinates. + The red outline should be covered by the blue rect on top, + while it should be clipped on the other edges and thus the red outline be visible - See: QTBUG-83229 + See: QTBUG-83229, modified by QTBUG-100329 */ void tst_QOpenGL::clipRect() { @@ -1754,7 +1755,7 @@ void tst_QOpenGL::clipRect() QCOMPARE(fb.size(), size); QCOMPARE(fb.pixelColor(clipRect.left() + 1, clipRect.top()), QColor(Qt::blue)); - QCOMPARE(fb.pixelColor(clipRect.left(), clipRect.top() + 1), QColor(Qt::blue)); + QCOMPARE(fb.pixelColor(clipRect.left(), clipRect.top() + 1), QColor(Qt::red)); QCOMPARE(fb.pixelColor(clipRect.left() + 1, clipRect.bottom()), QColor(Qt::red)); // Enable this once QTBUG-85286 is fixed