diff options
author | Tor Arne Vestbø <[email protected]> | 2025-07-06 11:18:03 +0200 |
---|---|---|
committer | Tor Arne Vestbø <[email protected]> | 2025-07-08 13:56:07 +0200 |
commit | 0bbdccb3061c76d43c3ce35c4f9187ab9b468657 (patch) | |
tree | c31e9fdd76fbfc44cac9d6ac7a47f91b467496b2 | |
parent | 83f2d606f1498d520a1bc4e00030fbebdaaf402b (diff) |
The implementation of QRhiBackingStore::flush uses rhiFlush, which
assumed that the incoming region and offset were in logical coordinates,
and then applied the sourceDevicePixelRatio to map them to device
coordinates.
This works for QWidgetRepaintManager::flush, which passes on the region
and offset in logical coordinates, but QRhiBackingStore::flush has logic
to pre-transform the coordinates from device independent to native for
the case where QtGui is doing the high-DPI scaling (Windows, Wayland,
Android, etc, as opposed to macOS/iOS where the OS does the scaling).
As a result QRhiBackingStore::flush would get pre-transformed
coordinates that it then passed on to rhiFlush, mapping it as
if it was in logical coordinates.
Fixing this by inverting the transform in QRhiBackingStore::flush is not
trivial due to fractional DPR accumulation, which QBackingStore::flush
already takes care of.
Instead we pass on the DPR coming from the platform window, which will
typically be 1 on platforms like Windows and Android, but may be non-1
when testing with QT_SCALE_FACTOR on e.g. macOS. This results in the
right transformation applied in QBackingStoreDefaultCompositor::flush.
We still need to pass the source DPR, as QBackingStoreDefaultCompositor
relies on the source DPR to compute the window-relative source rect,
which needs to reflect the DPR of the raw source pixels.
Tested with every combination of QT_WIDGETS_RHI, QT_SCALE_FACTOR,
QT_WIDGETS_HIGHDPI_DOWNSCALE, on 1x and 2x screens, with both integer
and fractional scale factors.
Note, in testing this it was revealed that we have an issue with
rounding errors when using both QtGui level high-DPI scaling, e.g.
via QT_SCALE_FACTOR=1.25 and OS-level scaling (e.g. 2x retina on
macOS). In this case QBackingStore applies the QtGui scale, but
rounds before passing things on to the platform, where we then
multiply the rounding error. The reason we don't see any visual
artifacts for this right now with QT_WIDGETS_RHI is that even though
we clear too much in beginPaint, the rhiFlush code path used by
QWidgetRepaintManager directly does not do any QHighDpiScaling,
so we end up not flushing the wrongly cleared pixels.
Solving this is not straight forward, as the QPlatformBackingStore
API surface uses QRegion extensively, and we surprisingly don't
have a QRegionF.
Fixes: QTBUG-128794
Fixes: QTBUG-133549
Fixes: QTBUG-128457
Fixes: QTBUG-128422
Fixes: QTBUG-121757
Task-number: QTBUG-138261
Pick-to: 6.10 6.9 6.8
Change-Id: I47884c28ef0ba8223ee16229d21e9c38801ae11f
Reviewed-by: Assam Boudjelthia <[email protected]>
-rw-r--r-- | src/gui/painting/qbackingstoredefaultcompositor.cpp | 17 | ||||
-rw-r--r-- | src/gui/painting/qbackingstoredefaultcompositor_p.h | 3 | ||||
-rw-r--r-- | src/gui/painting/qplatformbackingstore.cpp | 5 | ||||
-rw-r--r-- | src/gui/painting/qplatformbackingstore.h | 3 | ||||
-rw-r--r-- | src/gui/painting/qrhibackingstore.cpp | 17 | ||||
-rw-r--r-- | src/opengl/qopenglcompositorbackingstore.cpp | 4 | ||||
-rw-r--r-- | src/opengl/qopenglcompositorbackingstore_p.h | 3 | ||||
-rw-r--r-- | src/plugins/platforms/cocoa/qcocoabackingstore.h | 3 | ||||
-rw-r--r-- | src/plugins/platforms/cocoa/qcocoabackingstore.mm | 6 | ||||
-rw-r--r-- | src/plugins/platforms/xcb/qxcbbackingstore.cpp | 5 | ||||
-rw-r--r-- | src/plugins/platforms/xcb/qxcbbackingstore.h | 3 |
11 files changed, 51 insertions, 18 deletions
diff --git a/src/gui/painting/qbackingstoredefaultcompositor.cpp b/src/gui/painting/qbackingstoredefaultcompositor.cpp index c1452ca7688..41ab7c97f8c 100644 --- a/src/gui/painting/qbackingstoredefaultcompositor.cpp +++ b/src/gui/painting/qbackingstoredefaultcompositor.cpp @@ -460,11 +460,20 @@ QPlatformBackingStore::FlushResult QBackingStoreDefaultCompositor::flush(QPlatfo const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground) + bool translucentBackground, + qreal sourceTransformFactor) { if (!rhi) return QPlatformBackingStore::FlushFailed; + // Note, the sourceTransformFactor is different from the sourceDevicePixelRatio, + // as the former may reflect the fact that the region and offset is pre-transformed, + // in which case we don't need to do a full transform here based on the source DPR. + // In the default case where no explicit source transform has been passed, we fall + // back to the source device pixel ratio. + if (!sourceTransformFactor) + sourceTransformFactor = sourceDevicePixelRatio; + Q_ASSERT(textures); // may be empty if there are no render-to-texture widgets at all, but null it cannot be if (!m_rhi) { @@ -508,7 +517,7 @@ QPlatformBackingStore::FlushResult QBackingStoreDefaultCompositor::flush(QPlatfo const QImage::Format format = QImage::toImageFormat(graphicsBuffer->format()); const QSize size = graphicsBuffer->size(); QImage wrapperImage(graphicsBuffer->data(), size.width(), size.height(), graphicsBuffer->bytesPerLine(), format); - toTexture(wrapperImage, rhi, resourceUpdates, scaledRegion(region, sourceDevicePixelRatio, offset), &flags); + toTexture(wrapperImage, rhi, resourceUpdates, scaledRegion(region, sourceTransformFactor, offset), &flags); gotTextureFromGraphicsBuffer = true; graphicsBuffer->unlock(); if (graphicsBuffer->origin() == QPlatformGraphicsBuffer::OriginBottomLeft) @@ -516,7 +525,7 @@ QPlatformBackingStore::FlushResult QBackingStoreDefaultCompositor::flush(QPlatfo } } if (!gotTextureFromGraphicsBuffer) - toTexture(backingStore, rhi, resourceUpdates, scaledRegion(region, sourceDevicePixelRatio, offset), &flags); + toTexture(backingStore, rhi, resourceUpdates, scaledRegion(region, sourceTransformFactor, offset), &flags); ensureResources(resourceUpdates, swapchain->renderPassDescriptor()); @@ -549,7 +558,7 @@ QPlatformBackingStore::FlushResult QBackingStoreDefaultCompositor::flush(QPlatfo // The backingstore is for the entire tlw. In case of native children, offset tells the position // relative to the tlw. The window rect is scaled by the source device pixel ratio to get // the source rect. - const QPoint sourceWindowOffset = scaledOffset(offset, sourceDevicePixelRatio); + const QPoint sourceWindowOffset = scaledOffset(offset, sourceTransformFactor); const QRect srcRect = toBottomLeftRect(sourceWindowRect.translated(sourceWindowOffset), m_texture->pixelSize().height()); const QMatrix3x3 source = sourceTransform(srcRect, m_texture->pixelSize(), origin); QMatrix4x4 target; // identity diff --git a/src/gui/painting/qbackingstoredefaultcompositor_p.h b/src/gui/painting/qbackingstoredefaultcompositor_p.h index c5a8ffd328e..cdc9d098099 100644 --- a/src/gui/painting/qbackingstoredefaultcompositor_p.h +++ b/src/gui/painting/qbackingstoredefaultcompositor_p.h @@ -41,7 +41,8 @@ public: const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground); + bool translucentBackground, + qreal sourceTransformFactor); private: enum UpdateUniformOption { diff --git a/src/gui/painting/qplatformbackingstore.cpp b/src/gui/painting/qplatformbackingstore.cpp index 21e89d67fd2..2acc5ef7c52 100644 --- a/src/gui/painting/qplatformbackingstore.cpp +++ b/src/gui/painting/qplatformbackingstore.cpp @@ -213,14 +213,15 @@ QPlatformBackingStore::FlushResult QPlatformBackingStore::rhiFlush(QWindow *wind const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground) + bool translucentBackground, + qreal sourceTransformFactor) { auto &surfaceSupport = d_ptr->surfaceSupport[window->surfaceType()]; return surfaceSupport.compositor.flush(this, surfaceSupport.rhiSupport.rhi(), surfaceSupport.rhiSupport.swapChainForWindow(window), window, sourceDevicePixelRatio, region, offset, textures, - translucentBackground); + translucentBackground, sourceTransformFactor); } /*! diff --git a/src/gui/painting/qplatformbackingstore.h b/src/gui/painting/qplatformbackingstore.h index a6cb43b4e66..86035b98bea 100644 --- a/src/gui/painting/qplatformbackingstore.h +++ b/src/gui/painting/qplatformbackingstore.h @@ -151,7 +151,8 @@ public: const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground); + bool translucentBackground, + qreal sourceTransformFactor = 0); virtual QImage toImage() const; diff --git a/src/gui/painting/qrhibackingstore.cpp b/src/gui/painting/qrhibackingstore.cpp index d59cc2d83c5..3d9932e5ee2 100644 --- a/src/gui/painting/qrhibackingstore.cpp +++ b/src/gui/painting/qrhibackingstore.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only #include "qrhibackingstore_p.h" +#include "qpa/qplatformwindow.h" #include <private/qimage_p.h> QT_BEGIN_NAMESPACE @@ -43,10 +44,22 @@ void QRhiBackingStore::flush(QWindow *flushedWindow, const QRegion ®ion, cons createRhi(flushedWindow, rhiConfig); } + // The backing store operates on behalf of its window(), even if we're + // flushing a child window, so pull the source DPR from the window(). + const qreal sourceDevicePixelRatio = window()->devicePixelRatio(); + + // QBackingStore::flush will convert the region and offset from device independent + // pixels to native pixels before calling QPlatformBackingStore::flush, which means + // we can't pass on the window's DPR as the sourceTransformFactor, as that will include + // the Qt scale factor, which has already been applied. Instead we ask the platform + // window, which only reflect the remaining scale factor from the OS. + const qreal sourceTransformFactor = flushedWindow->handle()->devicePixelRatio(); + static QPlatformTextureList emptyTextureList; bool translucentBackground = m_image.hasAlphaChannel(); - rhiFlush(flushedWindow, flushedWindow->devicePixelRatio(), - region, offset, &emptyTextureList, translucentBackground); + rhiFlush(flushedWindow, sourceDevicePixelRatio, + region, offset, &emptyTextureList, translucentBackground, + sourceTransformFactor); } QImage::Format QRhiBackingStore::format() const diff --git a/src/opengl/qopenglcompositorbackingstore.cpp b/src/opengl/qopenglcompositorbackingstore.cpp index 20c86fb8adc..95dd1a562a6 100644 --- a/src/opengl/qopenglcompositorbackingstore.cpp +++ b/src/opengl/qopenglcompositorbackingstore.cpp @@ -163,7 +163,8 @@ QPlatformBackingStore::FlushResult QOpenGLCompositorBackingStore::rhiFlush(QWind const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground) + bool translucentBackground, + qreal sourceTransformFactor) { // QOpenGLWidget/QQuickWidget content provided as textures. The raster content goes on top. @@ -171,6 +172,7 @@ QPlatformBackingStore::FlushResult QOpenGLCompositorBackingStore::rhiFlush(QWind Q_UNUSED(offset); Q_UNUSED(translucentBackground); Q_UNUSED(sourceDevicePixelRatio); + Q_UNUSED(sourceTransformFactor); m_rhi = rhi(window); Q_ASSERT(m_rhi); diff --git a/src/opengl/qopenglcompositorbackingstore_p.h b/src/opengl/qopenglcompositorbackingstore_p.h index 4512e2d589e..b50629000cf 100644 --- a/src/opengl/qopenglcompositorbackingstore_p.h +++ b/src/opengl/qopenglcompositorbackingstore_p.h @@ -48,7 +48,8 @@ public: const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground) override; + bool translucentBackground, + qreal sourceTransformFactor = 0) override; const QPlatformTextureList *textures() const { return m_textures; } diff --git a/src/plugins/platforms/cocoa/qcocoabackingstore.h b/src/plugins/platforms/cocoa/qcocoabackingstore.h index 71b6015a54d..79aed15a1d0 100644 --- a/src/plugins/platforms/cocoa/qcocoabackingstore.h +++ b/src/plugins/platforms/cocoa/qcocoabackingstore.h @@ -44,7 +44,8 @@ public: const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground) override; + bool translucentBackground, + qreal sourceTransformFactor) override; QImage toImage() const override; QPlatformGraphicsBuffer *graphicsBuffer() const override; diff --git a/src/plugins/platforms/cocoa/qcocoabackingstore.mm b/src/plugins/platforms/cocoa/qcocoabackingstore.mm index 78d23b01dea..186aeaac44d 100644 --- a/src/plugins/platforms/cocoa/qcocoabackingstore.mm +++ b/src/plugins/platforms/cocoa/qcocoabackingstore.mm @@ -457,7 +457,8 @@ QPlatformBackingStore::FlushResult QCALayerBackingStore::rhiFlush(QWindow *windo const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground) + bool translucentBackground, + qreal sourceTransformFactor) { if (!m_buffers.back()) { qCWarning(lcQpaBackingStore) << "Flush requested with no back buffer. Ignoring."; @@ -466,7 +467,8 @@ QPlatformBackingStore::FlushResult QCALayerBackingStore::rhiFlush(QWindow *windo finalizeBackBuffer(); - return QPlatformBackingStore::rhiFlush(window, sourceDevicePixelRatio, region, offset, textures, translucentBackground); + return QPlatformBackingStore::rhiFlush(window, sourceDevicePixelRatio, + region, offset, textures, translucentBackground, sourceTransformFactor); } QImage QCALayerBackingStore::toImage() const diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.cpp b/src/plugins/platforms/xcb/qxcbbackingstore.cpp index 8353fac6a92..fda47944d9d 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.cpp +++ b/src/plugins/platforms/xcb/qxcbbackingstore.cpp @@ -870,7 +870,8 @@ QPlatformBackingStore::FlushResult QXcbBackingStore::rhiFlush(QWindow *window, const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground) + bool translucentBackground, + qreal sourceTransformFactor) { if (!m_image || m_image->size().isEmpty()) return FlushFailed; @@ -878,7 +879,7 @@ QPlatformBackingStore::FlushResult QXcbBackingStore::rhiFlush(QWindow *window, m_image->flushScrolledRegion(true); auto result = QPlatformBackingStore::rhiFlush(window, sourceDevicePixelRatio, region, offset, - textures, translucentBackground); + textures, translucentBackground, sourceTransformFactor); if (result != FlushSuccess) return result; QXcbWindow *platformWindow = static_cast<QXcbWindow *>(window->handle()); diff --git a/src/plugins/platforms/xcb/qxcbbackingstore.h b/src/plugins/platforms/xcb/qxcbbackingstore.h index 674640780eb..5cda9e1ab79 100644 --- a/src/plugins/platforms/xcb/qxcbbackingstore.h +++ b/src/plugins/platforms/xcb/qxcbbackingstore.h @@ -27,7 +27,8 @@ public: const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, - bool translucentBackground) override; + bool translucentBackground, + qreal sourceTransformFactor) override; QImage toImage() const override; QPlatformGraphicsBuffer *graphicsBuffer() const override; |