From ac4dce1c20a1cbfd3974bf8da6dd7557d35a4ca1 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 28 Nov 2019 14:00:58 +0200 Subject: [PATCH] [scene] Fix decoration texture bleeding Summary: Quite long time ago, window decorations were painted on real X11 windows. The nicest thing about that approach is that we get both contents of the client and the frame window at the same time. However, somewhere around KDE 4.2 - 4.3 times, decoration rendering architecture had been changed to what we have now. I've mentioned the previous decoration rendering design because it didn't have a problem that the new design has, namely the texture bleeding issue. In the name of better performance, opengl scene puts all decoration parts to an atlas. This is totally reasonable, however we must be super cautious about things such as the GL_LINEAR filter. The GL_LINEAR filter may need to sample a couple of neighboring texels in order to produce the final texel value. However, since all decoration parts now live in a single texture, we have to make sure that we don't sample texels that belong to another decoration part. This patch fixes the texture bleeding problem by padding each individual decoration part in the atlas. There is another solution for this problem though. We could render a window into an offscreen texture and then map that texture on the transformed window geometry. This would work well and we definitely need an offscreen rendering path in the opengl scene, however it's not feasible at the moment since we need to break the window quads API. Also, it would be great to have as less as possible stuff going on between invocation of Scene::Window::performPaint() and getting the corresponding pixel data on the screen. There is a good chance that the new padding stuff may make you vomit. If it does so, I'm all ears for the suggestions how to make the code more nicer. BUG: 257566 BUG: 360549 CCBUG: 412573 FIXED-IN: 5.18.0 Reviewers: #kwin Subscribers: fredrik, kwin, fvogt Tags: #kwin Differential Revision: https://phabricator.kde.org/D25611 --- decorations/decorationrenderer.cpp | 7 +- decorations/decorationrenderer.h | 1 + plugins/scenes/opengl/scene_opengl.cpp | 117 +++++++++++++++++++++++-- scene.cpp | 15 +++- 4 files changed, 126 insertions(+), 14 deletions(-) diff --git a/decorations/decorationrenderer.cpp b/decorations/decorationrenderer.cpp index 3efbceba30..ef753f27c4 100644 --- a/decorations/decorationrenderer.cpp +++ b/decorations/decorationrenderer.cpp @@ -74,10 +74,15 @@ QImage Renderer::renderToImage(const QRect &geo) p.setRenderHint(QPainter::Antialiasing); p.setWindow(QRect(geo.topLeft(), geo.size() * dpr)); p.setClipRect(geo); - client()->decoration()->paint(&p, geo); + renderToPainter(&p, geo); return image; } +void Renderer::renderToPainter(QPainter *painter, const QRect &rect) +{ + client()->decoration()->paint(painter, rect); +} + void Renderer::reparent(Deleted *deleted) { setParent(deleted); diff --git a/decorations/decorationrenderer.h b/decorations/decorationrenderer.h index ac3aa7e83f..86651f403d 100644 --- a/decorations/decorationrenderer.h +++ b/decorations/decorationrenderer.h @@ -73,6 +73,7 @@ protected: m_imageSizesDirty = false; } QImage renderToImage(const QRect &geo); + void renderToPainter(QPainter *painter, const QRect &rect); private: DecoratedClientImpl *m_client; diff --git a/plugins/scenes/opengl/scene_opengl.cpp b/plugins/scenes/opengl/scene_opengl.cpp index f6f5d9c6c6..2c6e2a1ddc 100644 --- a/plugins/scenes/opengl/scene_opengl.cpp +++ b/plugins/scenes/opengl/scene_opengl.cpp @@ -4,6 +4,7 @@ Copyright (C) 2006 Lubos Lunak Copyright (C) 2009, 2010, 2011 Martin Gräßlin +Copyright (C) 2019 Vlad Zahorodnii Based on glcompmgr code by Felix Bellaby. Using code from Compiz and Beryl. @@ -2518,6 +2519,52 @@ static QImage rotate(const QImage &srcImage, const QRect &srcRect) return image; } +static void clamp_row(int left, int width, int right, const uint32_t *src, uint32_t *dest) +{ + std::fill_n(dest, left, *src); + std::copy(src, src + width, dest + left); + std::fill_n(dest + left + width, right, *(src + width - 1)); +} + +static void clamp_sides(int left, int width, int right, const uint32_t *src, uint32_t *dest) +{ + std::fill_n(dest, left, *src); + std::fill_n(dest + left + width, right, *(src + width - 1)); +} + +static void clamp(QImage &image, const QRect &viewport) +{ + Q_ASSERT(image.depth() == 32); + + const QRect rect = image.rect(); + + const int left = viewport.left() - rect.left(); + const int top = viewport.top() - rect.top(); + const int right = rect.right() - viewport.right(); + const int bottom = rect.bottom() - viewport.bottom(); + + const int width = rect.width() - left - right; + const int height = rect.height() - top - bottom; + + const uint32_t *firstRow = reinterpret_cast(image.scanLine(top)); + const uint32_t *lastRow = reinterpret_cast(image.scanLine(top + height - 1)); + + for (int i = 0; i < top; ++i) { + uint32_t *dest = reinterpret_cast(image.scanLine(i)); + clamp_row(left, width, right, firstRow + left, dest); + } + + for (int i = 0; i < height; ++i) { + uint32_t *dest = reinterpret_cast(image.scanLine(top + i)); + clamp_sides(left, width, right, dest + left, dest); + } + + for (int i = 0; i < bottom; ++i) { + uint32_t *dest = reinterpret_cast(image.scanLine(top + height + i)); + clamp_row(left, width, right, lastRow + left, dest); + } +} + void SceneOpenGLDecorationRenderer::render() { const QRegion scheduled = getScheduled(); @@ -2540,21 +2587,68 @@ void SceneOpenGLDecorationRenderer::render() const QRect geometry = dirty ? QRect(QPoint(0, 0), client()->client()->size()) : scheduled.boundingRect(); - auto renderPart = [this](const QRect &geo, const QRect &partRect, const QPoint &offset, bool rotated = false) { + // We pad each part in the decoration atlas in order to avoid texture bleeding. + const int padding = 1; + + auto renderPart = [=](const QRect &geo, const QRect &partRect, const QPoint &position, bool rotated = false) { if (!geo.isValid()) { return; } - QImage image = renderToImage(geo); + + QRect rect = geo; + + // We allow partial decoration updates and it might just so happen that the dirty region + // is completely contained inside the decoration part, i.e. the dirty region doesn't touch + // any of the decoration's edges. In that case, we should **not** pad the dirty region. + if (rect.left() == partRect.left()) { + rect.setLeft(rect.left() - padding); + } + if (rect.top() == partRect.top()) { + rect.setTop(rect.top() - padding); + } + if (rect.right() == partRect.right()) { + rect.setRight(rect.right() + padding); + } + if (rect.bottom() == partRect.bottom()) { + rect.setBottom(rect.bottom() + padding); + } + + QRect viewport = geo.translated(-rect.x(), -rect.y()); + const qreal devicePixelRatio = client()->client()->screenScale(); + + QImage image(rect.size() * devicePixelRatio, QImage::Format_ARGB32_Premultiplied); + image.setDevicePixelRatio(devicePixelRatio); + image.fill(Qt::transparent); + + QPainter painter(&image); + painter.setRenderHint(QPainter::Antialiasing); + painter.setViewport(QRect(viewport.topLeft(), viewport.size() * devicePixelRatio)); + painter.setWindow(QRect(geo.topLeft(), geo.size() * devicePixelRatio)); + painter.setClipRect(geo); + renderToPainter(&painter, geo); + painter.end(); + + clamp(image, QRect(viewport.topLeft(), viewport.size() * devicePixelRatio)); + if (rotated) { // TODO: get this done directly when rendering to the image - image = rotate(image, QRect(geo.topLeft() - partRect.topLeft(), geo.size())); + image = rotate(image, QRect(QPoint(), rect.size())); + viewport = QRect(viewport.y(), viewport.x(), viewport.height(), viewport.width()); } - m_texture->update(image, (geo.topLeft() - partRect.topLeft() + offset) * image.devicePixelRatio()); + + const QPoint dirtyOffset = geo.topLeft() - partRect.topLeft(); + m_texture->update(image, (position + dirtyOffset - viewport.topLeft()) * image.devicePixelRatio()); }; - renderPart(left.intersected(geometry), left, QPoint(0, top.height() + bottom.height() + 2), true); - renderPart(top.intersected(geometry), top, QPoint(0, 0)); - renderPart(right.intersected(geometry), right, QPoint(0, top.height() + bottom.height() + left.width() + 3), true); - renderPart(bottom.intersected(geometry), bottom, QPoint(0, top.height() + 1)); + + const QPoint topPosition(padding, padding); + const QPoint bottomPosition(padding, topPosition.y() + top.height() + 2 * padding); + const QPoint leftPosition(padding, bottomPosition.y() + bottom.height() + 2 * padding); + const QPoint rightPosition(padding, leftPosition.y() + left.width() + 2 * padding); + + renderPart(left.intersected(geometry), left, leftPosition, true); + renderPart(top.intersected(geometry), top, topPosition); + renderPart(right.intersected(geometry), right, rightPosition, true); + renderPart(bottom.intersected(geometry), bottom, bottomPosition); } static int align(int value, int align) @@ -2571,7 +2665,12 @@ void SceneOpenGLDecorationRenderer::resizeTexture() size.rwidth() = qMax(qMax(top.width(), bottom.width()), qMax(left.height(), right.height())); size.rheight() = top.height() + bottom.height() + - left.width() + right.width() + 3; + left.width() + right.width(); + + // Reserve some space for padding. We pad decoration parts to avoid texture bleeding. + const int padding = 1; + size.rwidth() += 2 * padding; + size.rheight() += 4 * 2 * padding; size.rwidth() = align(size.width(), 128); diff --git a/scene.cpp b/scene.cpp index 1c65625ef0..b9fb7aa755 100644 --- a/scene.cpp +++ b/scene.cpp @@ -880,11 +880,18 @@ WindowQuadList Scene::Window::makeDecorationQuads(const QRect *rects, const QReg { WindowQuadList list; + const int padding = 1; + + const QPoint topSpritePosition(padding, padding); + const QPoint bottomSpritePosition(padding, topSpritePosition.y() + rects[1].height() + 2 * padding); + const QPoint leftSpritePosition(bottomSpritePosition.y() + rects[3].height() + 2 * padding, padding); + const QPoint rightSpritePosition(leftSpritePosition.x() + rects[0].width() + 2 * padding, padding); + const QPoint offsets[4] = { - QPoint(-rects[0].x() + rects[1].height() + rects[3].height() + 2, -rects[0].y()), // Left - QPoint(-rects[1].x(), -rects[1].y()), // Top - QPoint(-rects[2].x() + rects[1].height() + rects[3].height() + rects[0].width() + 3, -rects[2].y()), // Right - QPoint(-rects[3].x(), -rects[3].y() + rects[1].height() + 1) // Bottom + QPoint(-rects[0].x(), -rects[0].y()) + leftSpritePosition, + QPoint(-rects[1].x(), -rects[1].y()) + topSpritePosition, + QPoint(-rects[2].x(), -rects[2].y()) + rightSpritePosition, + QPoint(-rects[3].x(), -rects[3].y()) + bottomSpritePosition, }; const Qt::Orientation orientations[4] = {