From 2e78ae2b6dc2a05061e0e729595d22dbd97eb52d Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 14 Dec 2023 15:19:54 +0200 Subject: [PATCH] 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 | 1 + src/core/pixelgrid.h | 38 +++++++++++ src/core/rendertarget.cpp | 20 ++++++ src/core/rendertarget.h | 2 + src/core/renderviewport.cpp | 65 ++++++++----------- src/core/renderviewport.h | 4 +- .../startupfeedback/startupfeedback.cpp | 10 +-- src/scene/itemrenderer_opengl.cpp | 14 ++-- 8 files changed, 95 insertions(+), 59 deletions(-) create mode 100644 src/core/pixelgrid.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 43767ba662..a802750514 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/core/pixelgrid.h b/src/core/pixelgrid.h new file mode 100644 index 0000000000..015d2f76c4 --- /dev/null +++ b/src/core/pixelgrid.h @@ -0,0 +1,38 @@ +/* + SPDX-FileCopyrightText: 2023 Vlad Zahorodnii + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#pragma once + +#include "kwin_export.h" + +#include + +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 diff --git a/src/core/rendertarget.cpp b/src/core/rendertarget.cpp index 4d53cd129d..6db2d1d3b7 100644 --- a/src/core/rendertarget.cpp +++ b/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; diff --git a/src/core/rendertarget.h b/src/core/rendertarget.h index b59e898522..5342f155a5 100644 --- a/src/core/rendertarget.h +++ b/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; diff --git a/src/core/renderviewport.cpp b/src/core/renderviewport.cpp index 13b84fb2fd..dc25e37aaf 100644 --- a/src/core/renderviewport.cpp +++ b/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)); - 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); + ret.ortho(rect); return ret; } RenderViewport::RenderViewport(const QRectF &renderRect, double scale, const RenderTarget &renderTarget) - : m_renderRect(renderRect) - , m_projectionMatrix(createProjectionMatrix(renderRect, scale, renderTarget)) - , m_logicalToLocal(createLogicalToLocalMatrix(renderRect, renderTarget)) - , m_logicalToLocalTexture(createLogicalToLocalTextureMatrix(renderRect, scale)) + : m_renderTarget(&renderTarget) + , m_renderRect(renderRect) + , m_deviceRenderRect(snapToPixelGrid(scaledRect(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; } diff --git a/src/core/renderviewport.h b/src/core/renderviewport.h index e7ee2ccd18..23d8efee3e 100644 --- a/src/core/renderviewport.h +++ b/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; }; diff --git a/src/plugins/startupfeedback/startupfeedback.cpp b/src/plugins/startupfeedback/startupfeedback.cpp index 79b690e862..6681c5a400 100644 --- a/src/plugins/startupfeedback/startupfeedback.cpp +++ b/src/plugins/startupfeedback/startupfeedback.cpp @@ -25,6 +25,7 @@ #include // 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 ®ion, 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); diff --git a/src/scene/itemrenderer_opengl.cpp b/src/scene/itemrenderer_opengl.cpp index 4a60031e18..22f256edfe 100644 --- a/src/scene/itemrenderer_opengl.cpp +++ b/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());