core: Fix pixel grid snapping in RenderViewport

Our painting code is assumed to work as following: scale the geometry,
snap it to the pixel grid, apply the render transform, e.g. flip the
geometry vertically.

However, with QMatrix4x4 in RenderViewport, we have a slightly different
order: scale the geometry, apply the render transform, snap to the pixel
grid. It results in mapToRenderTarget() not properly mapping logical
geometry to the device geometry, which later manifests as glitches.

BUG: 477455

src/CMakeLists.txt

@@ -504,6 +504,7 @@ install(FILES
 install(FILES
     core/colorspace.h
     core/output.h
+    core/pixelgrid.h
     core/renderloop.h
     core/rendertarget.h
     core/renderviewport.h

src/core/pixelgrid.h

@@ -0,0 +1,38 @@
+/*
+    SPDX-FileCopyrightText: 2023 Vlad Zahorodnii <vlad.zahorodnii@kde.org>
+
+    SPDX-License-Identifier: GPL-2.0-or-later
+*/
+
+#pragma once
+
+#include "kwin_export.h"
+
+#include <QRect>
+
+namespace KWin
+{
+
+KWIN_EXPORT inline QPoint snapToPixelGrid(const QPointF &point)
+{
+    return QPoint(std::round(point.x()), std::round(point.y()));
+}
+
+KWIN_EXPORT inline QPointF snapToPixelGridF(const QPointF &point)
+{
+    return QPointF(std::round(point.x()), std::round(point.y()));
+}
+
+KWIN_EXPORT inline QRect snapToPixelGrid(const QRectF &rect)
+{
+    const QPoint topLeft = snapToPixelGrid(rect.topLeft());
+    const QPoint bottomRight = snapToPixelGrid(rect.bottomRight());
+    return QRect(topLeft.x(), topLeft.y(), bottomRight.x() - topLeft.x(), bottomRight.y() - topLeft.y());
+}
+
+KWIN_EXPORT inline QRectF snapToPixelGridF(const QRectF &rect)
+{
+    return QRectF(snapToPixelGridF(rect.topLeft()), snapToPixelGridF(rect.bottomRight()));
+}
+
+} // namespace KWin

src/core/rendertarget.cpp

@@ -49,6 +49,26 @@ QRect RenderTarget::applyTransformation(const QRect &rect, const QRect &viewport
     return applyTransformation(QRectF(rect), QRectF(viewport)).toRect();
 }
 
+QPointF RenderTarget::applyTransformation(const QPointF &point, const QRectF &viewport) const
+{
+    const auto center = viewport.center();
+    QMatrix4x4 relativeTransformation;
+    relativeTransformation.translate(center.x(), center.y());
+    relativeTransformation *= m_transformation;
+    relativeTransformation.translate(-center.x(), -center.y());
+    return relativeTransformation.map(point);
+}
+
+QPoint RenderTarget::applyTransformation(const QPoint &point, const QRect &viewport) const
+{
+    const auto center = viewport.center();
+    QMatrix4x4 relativeTransformation;
+    relativeTransformation.translate(center.x(), center.y());
+    relativeTransformation *= m_transformation;
+    relativeTransformation.translate(-center.x(), -center.y());
+    return relativeTransformation.map(point);
+}
+
 QMatrix4x4 RenderTarget::transformation() const
 {
     return m_transformation;

src/core/rendertarget.h

@@ -29,6 +29,8 @@ public:
     const ColorDescription &colorDescription() const;
     QRectF applyTransformation(const QRectF &rect, const QRectF &viewport) const;
     QRect applyTransformation(const QRect &rect, const QRect &viewport) const;
+    QPointF applyTransformation(const QPointF &point, const QRectF &viewport) const;
+    QPoint applyTransformation(const QPoint &point, const QRect &viewport) const;
 
     QImage *image() const;
     GLFramebuffer *framebuffer() const;

src/core/renderviewport.cpp

@@ -4,46 +4,25 @@
     SPDX-License-Identifier: GPL-2.0-or-later
 */
 #include "core/renderviewport.h"
+#include "core/pixelgrid.h"
 #include "core/rendertarget.h"
+#include "effect/globals.h"
 
 namespace KWin
 {
 
-static QMatrix4x4 createProjectionMatrix(const QRectF &renderRect, double scale, const RenderTarget &renderTarget)
+static QMatrix4x4 createProjectionMatrix(const RenderTarget &renderTarget, const QRect &rect)
 {
     QMatrix4x4 ret = renderTarget.transformation();
-    ret.ortho(QRectF(renderRect.left() * scale, renderRect.top() * scale, renderRect.width() * scale, renderRect.height() * scale));
+    ret.ortho(rect);
-    return ret;
-}
-
-static QMatrix4x4 createLogicalToLocalMatrix(const QRectF &renderRect, const RenderTarget &renderTarget)
-{
-    // map the normalized device coordinates [-1, 1] from the projection matrix (without scaling) to pixels
-    QMatrix4x4 ret;
-    ret.scale(renderTarget.size().width(), renderTarget.size().height());
-    ret.translate(0.5, 0.5);
-    ret.scale(0.5, -0.5);
-    ret *= renderTarget.transformation();
-    ret.ortho(renderRect);
-    return ret;
-}
-
-static QMatrix4x4 createLogicalToLocalTextureMatrix(const QRectF &renderRect, double scale)
-{
-    // map the normalized device coordinates [-1, 1] from the projection matrix (without scaling) to pixels
-    QMatrix4x4 ret;
-    ret.scale(renderRect.width() * scale, renderRect.height() * scale);
-    ret.translate(0.5, 0.5);
-    ret.scale(0.5, -0.5);
-    ret.ortho(renderRect);
     return ret;
 }
 
 RenderViewport::RenderViewport(const QRectF &renderRect, double scale, const RenderTarget &renderTarget)
-    : m_renderRect(renderRect)
+    : m_renderTarget(&renderTarget)
-    , m_projectionMatrix(createProjectionMatrix(renderRect, scale, renderTarget))
+    , m_renderRect(renderRect)
-    , m_logicalToLocal(createLogicalToLocalMatrix(renderRect, renderTarget))
+    , m_deviceRenderRect(snapToPixelGrid(scaledRect(renderRect, scale)))
-    , m_logicalToLocalTexture(createLogicalToLocalTextureMatrix(renderRect, scale))
+    , m_projectionMatrix(createProjectionMatrix(renderTarget, m_deviceRenderRect))
     , m_scale(scale)
 {
 }
@@ -65,58 +44,66 @@ double RenderViewport::scale() const
 
 QRectF RenderViewport::mapToRenderTarget(const QRectF &logicalGeometry) const
 {
-    return m_logicalToLocal.mapRect(logicalGeometry);
+    const QRectF deviceGeometry = scaledRect(logicalGeometry, m_scale)
+                                      .translated(-m_deviceRenderRect.topLeft());
+    return m_renderTarget->applyTransformation(deviceGeometry, QRectF(QPointF(), m_renderTarget->size()));
 }
 
 QRect RenderViewport::mapToRenderTarget(const QRect &logicalGeometry) const
 {
-    return m_logicalToLocal.mapRect(logicalGeometry);
+    const QRect deviceGeometry = snapToPixelGrid(scaledRect(logicalGeometry, m_scale))
+                                     .translated(-m_deviceRenderRect.topLeft());
+    return m_renderTarget->applyTransformation(deviceGeometry, QRect(QPoint(), m_renderTarget->size()));
 }
 
 QPoint RenderViewport::mapToRenderTarget(const QPoint &logicalGeometry) const
 {
-    return m_logicalToLocal.map(logicalGeometry);
+    const QPoint devicePoint = snapToPixelGrid(QPointF(logicalGeometry) * m_scale) - m_deviceRenderRect.topLeft();
+    return m_renderTarget->applyTransformation(devicePoint, QRect(QPoint(), m_renderTarget->size()));
 }
 
 QPointF RenderViewport::mapToRenderTarget(const QPointF &logicalGeometry) const
 {
-    return m_logicalToLocal.map(logicalGeometry);
+    const QPointF devicePoint = logicalGeometry * m_scale - m_deviceRenderRect.topLeft();
+    return m_renderTarget->applyTransformation(devicePoint, QRectF(QPointF(), m_renderTarget->size()));
 }
 
 QRegion RenderViewport::mapToRenderTarget(const QRegion &logicalGeometry) const
 {
     QRegion ret;
     for (const auto &rect : logicalGeometry) {
-        ret |= m_logicalToLocal.mapRect(rect);
+        ret |= mapToRenderTarget(rect);
     }
     return ret;
 }
 
 QRectF RenderViewport::mapToRenderTargetTexture(const QRectF &logicalGeometry) const
 {
-    return m_logicalToLocalTexture.mapRect(logicalGeometry);
+    return scaledRect(logicalGeometry, m_scale)
+        .translated(-m_deviceRenderRect.topLeft());
 }
 
 QRect RenderViewport::mapToRenderTargetTexture(const QRect &logicalGeometry) const
 {
-    return m_logicalToLocalTexture.mapRect(logicalGeometry);
+    return snapToPixelGrid(scaledRect(logicalGeometry, m_scale))
+        .translated(-m_deviceRenderRect.topLeft());
 }
 
 QPoint RenderViewport::mapToRenderTargetTexture(const QPoint &logicalGeometry) const
 {
-    return m_logicalToLocalTexture.map(logicalGeometry);
+    return snapToPixelGrid(QPointF(logicalGeometry) * m_scale) - m_deviceRenderRect.topLeft();
 }
 
 QPointF RenderViewport::mapToRenderTargetTexture(const QPointF &logicalGeometry) const
 {
-    return m_logicalToLocalTexture.map(logicalGeometry);
+    return logicalGeometry * m_scale - m_deviceRenderRect.topLeft();
 }
 
 QRegion RenderViewport::mapToRenderTargetTexture(const QRegion &logicalGeometry) const
 {
     QRegion ret;
     for (const auto &rect : logicalGeometry) {
-        ret |= m_logicalToLocalTexture.mapRect(rect);
+        ret |= mapToRenderTargetTexture(rect);
     }
     return ret;
 }

src/core/renderviewport.h

@@ -39,10 +39,10 @@ public:
     QRegion mapToRenderTargetTexture(const QRegion &logicalGeometry) const;
 
 private:
+    const RenderTarget *m_renderTarget;
     const QRectF m_renderRect;
+    const QRect m_deviceRenderRect;
     const QMatrix4x4 m_projectionMatrix;
-    const QMatrix4x4 m_logicalToLocal;
-    const QMatrix4x4 m_logicalToLocalTexture;
     const double m_scale;
 };
 

src/plugins/startupfeedback/startupfeedback.cpp

@@ -25,6 +25,7 @@
 #include <KWindowSystem>
 // KWin
 #include "core/output.h"
+#include "core/pixelgrid.h"
 #include "core/rendertarget.h"
 #include "core/renderviewport.h"
 #include "effect/effecthandler.h"
@@ -198,13 +199,6 @@ void StartupFeedbackEffect::prePaintScreen(ScreenPrePaintData &data, std::chrono
     effects->prePaintScreen(data, presentTime);
 }
 
-static QRectF snapToPixelGrid(const QRectF &rect, qreal devicePixelRatio)
-{
-    const QVector2D topLeft = roundVector(QVector2D(rect.topLeft()) * devicePixelRatio);
-    const QVector2D bottomRight = roundVector(QVector2D(rect.bottomRight()) * devicePixelRatio);
-    return QRectF(topLeft.x(), topLeft.y(), bottomRight.x() - topLeft.x(), bottomRight.y() - topLeft.y());
-}
-
 void StartupFeedbackEffect::paintScreen(const RenderTarget &renderTarget, const RenderViewport &viewport, int mask, const QRegion &region, Output *screen)
 {
     effects->paintScreen(renderTarget, viewport, mask, region, screen);
@@ -235,7 +229,7 @@ void StartupFeedbackEffect::paintScreen(const RenderTarget &renderTarget, const
         } else {
             shader = ShaderManager::instance()->pushShader(ShaderTrait::MapTexture | ShaderTrait::TransformColorspace);
         }
-        const QRectF pixelGeometry = snapToPixelGrid(m_currentGeometry, viewport.scale());
+        const QRectF pixelGeometry = snapToPixelGridF(scaledRect(m_currentGeometry, viewport.scale()));
         QMatrix4x4 mvp = viewport.projectionMatrix();
         mvp.translate(pixelGeometry.x(), pixelGeometry.y());
         shader->setUniform(GLShader::ModelViewProjectionMatrix, mvp);

src/scene/itemrenderer_opengl.cpp

@@ -5,6 +5,7 @@
 */
 
 #include "scene/itemrenderer_opengl.h"
+#include "core/pixelgrid.h"
 #include "core/rendertarget.h"
 #include "core/renderviewport.h"
 #include "effect/effect.h"
@@ -95,12 +96,6 @@ static OpenGLSurfaceContents bindSurfaceTexture(SurfaceItem *surfaceItem)
     return platformSurfaceTexture->texture();
 }
 
-static QRectF logicalRectToDeviceRect(const QRectF &logical, qreal deviceScale)
-{
-    return QRectF(QPointF(std::round(logical.left() * deviceScale), std::round(logical.top() * deviceScale)),
-                  QPointF(std::round(logical.right() * deviceScale), std::round(logical.bottom() * deviceScale)));
-}
-
 static RenderGeometry clipQuads(const Item *item, const ItemRendererOpenGL::RenderContext *context)
 {
     const WindowQuadList quads = item->quads();
@@ -116,10 +111,10 @@ static RenderGeometry clipQuads(const Item *item, const ItemRendererOpenGL::Rend
     for (const WindowQuad &quad : std::as_const(quads)) {
         if (context->clip != infiniteRegion() && !context->hardwareClipping) {
             // Scale to device coordinates, rounding as needed.
-            QRectF deviceBounds = logicalRectToDeviceRect(quad.bounds(), scale);
+            QRectF deviceBounds = snapToPixelGridF(scaledRect(quad.bounds(), scale));
 
             for (const QRect &clipRect : std::as_const(context->clip)) {
-                QRectF deviceClipRect = logicalRectToDeviceRect(clipRect, scale).translated(-worldTranslation);
+                QRectF deviceClipRect = snapToPixelGridF(scaledRect(clipRect, scale)).translated(-worldTranslation);
 
                 const QRectF &intersected = deviceClipRect.intersected(deviceBounds);
                 if (intersected.isValid()) {
@@ -247,8 +242,7 @@ void ItemRendererOpenGL::renderBackground(const RenderTarget &renderTarget, cons
         glClearColor(0, 0, 0, 0);
         glEnable(GL_SCISSOR_TEST);
 
-        const auto targetSize = viewport.mapToRenderTarget(viewport.renderRect());
+        const auto targetSize = renderTarget.size();
-
         for (const QRect &r : region) {
             const auto deviceRect = viewport.mapToRenderTarget(r);
             glScissor(deviceRect.x(), targetSize.height() - (deviceRect.y() + deviceRect.height()), deviceRect.width(), deviceRect.height());