From 56d5f3a4f61ce00a1abc4d3c315dcbe15c22efdb Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Mon, 3 Feb 2020 13:29:43 +0200 Subject: [PATCH] [x11] Fix visual artifacts during interactive resize Summary: When a window is being interactively resized, its contents may jump. The reason why that happens is because KWin renders partially resized client window. Composite extension spec says that a window will get a new pixmap each time it is resized or mapped. This applies to the frame window, but not to the client window itself. If the client window is resized, off-screen storage for the frame window won't be reallocated. Therefore, KWin may render partially resized client window if the client doesn't attempt to be in sync with our rendering loop. Currently, the only way to do that is to use extended frame counters, which are not supported by KWin. So, in order to fix visual artifacts during interactive resize, we need somehow forcefully re-allocate off-screen storage for the frame window. Unfortunately, Composite extension doesn't provide any request to do that, so the only option we have is to resize the frame window. BUG: 415839 FIXED-IN: 5.18.0 Reviewers: #kwin Subscribers: davidedmundson, ngraham, alexde, fredrik, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D26914 --- scene.cpp | 12 +++++++++++- scene.h | 3 ++- toplevel.cpp | 2 +- x11client.cpp | 52 ++++++++++++++++++++++++++++++++++++++------------- x11client.h | 2 ++ 5 files changed, 55 insertions(+), 16 deletions(-) diff --git a/scene.cpp b/scene.cpp index 80ff338175..11714a846c 100644 --- a/scene.cpp +++ b/scene.cpp @@ -716,7 +716,7 @@ void Scene::Window::unreferencePreviousPixmap() } } -void Scene::Window::pixmapDiscarded() +void Scene::Window::discardPixmap() { if (!m_currentPixmap.isNull()) { if (m_currentPixmap->isValid()) { @@ -728,6 +728,16 @@ void Scene::Window::pixmapDiscarded() } } +void Scene::Window::updatePixmap() +{ + if (m_currentPixmap.isNull()) { + m_currentPixmap.reset(createWindowPixmap()); + } + if (!m_currentPixmap->isValid()) { + m_currentPixmap->create(); + } +} + void Scene::Window::discardShape() { // it is created on-demand and cached, simply diff --git a/scene.h b/scene.h index cb18317327..243fb1ef71 100644 --- a/scene.h +++ b/scene.h @@ -297,7 +297,8 @@ public: // perform the actual painting of the window virtual void performPaint(int mask, QRegion region, WindowPaintData data) = 0; // do any cleanup needed when the window's composite pixmap is discarded - void pixmapDiscarded(); + void discardPixmap(); + void updatePixmap(); int x() const; int y() const; int width() const; diff --git a/toplevel.cpp b/toplevel.cpp index 33c9020122..56d4668f9e 100644 --- a/toplevel.cpp +++ b/toplevel.cpp @@ -292,7 +292,7 @@ void Toplevel::discardWindowPixmap() { addDamageFull(); if (effectWindow() != nullptr && effectWindow()->sceneWindow() != nullptr) - effectWindow()->sceneWindow()->pixmapDiscarded(); + effectWindow()->sceneWindow()->discardPixmap(); } void Toplevel::damageNotifyEvent() diff --git a/x11client.cpp b/x11client.cpp index de643c904d..451139136a 100644 --- a/x11client.cpp +++ b/x11client.cpp @@ -2708,6 +2708,14 @@ QSize X11Client::clientSizeToFrameSize(const QSize &size) const return QSize(width, height); } +QRect X11Client::frameRectToBufferRect(const QRect &rect) const +{ + if (isDecorated()) { + return rect; + } + return frameRectToClientRect(rect); +} + Xcb::Property X11Client::fetchShowOnScreenEdge() const { return Xcb::Property(false, window(), atoms->kde_screen_edge_show, XCB_ATOM_CARDINAL, 0, 1); @@ -2865,6 +2873,7 @@ void X11Client::handleSync() m_syncRequest.timeout->stop(); } performMoveResize(); + updateWindowPixmap(); } else // setReadyForPainting does as well, but there's a small chance for resize syncs after the resize ended addRepaintFull(); } @@ -4156,7 +4165,6 @@ void X11Client::setFrameGeometry(int x, int y, int w, int h, ForceGeometry_t for // for example using X11Client::clientSize() QRect frameGeometry(x, y, w, h); - QRect bufferGeometry; if (shade_geometry_change) ; // nothing @@ -4170,11 +4178,7 @@ void X11Client::setFrameGeometry(int x, int y, int w, int h, ForceGeometry_t for } else { m_clientGeometry = frameRectToClientRect(frameGeometry); } - if (isDecorated()) { - bufferGeometry = frameGeometry; - } else { - bufferGeometry = m_clientGeometry; - } + const QRect bufferGeometry = frameRectToBufferRect(frameGeometry); if (!areGeometryUpdatesBlocked() && frameGeometry != rules()->checkGeometry(frameGeometry)) { qCDebug(KWIN_CORE) << "forced geometry fail:" << frameGeometry << ":" << rules()->checkGeometry(frameGeometry); } @@ -4269,14 +4273,21 @@ void X11Client::plainResize(int w, int h, ForceGeometry_t force) void X11Client::updateServerGeometry() { - if (m_frame.geometry().size() != m_bufferGeometry.size() || pendingGeometryUpdate() == PendingGeometryForced) { + const QRect oldBufferGeometry = bufferGeometryBeforeUpdateBlocking(); + + if (oldBufferGeometry.size() != m_bufferGeometry.size() || pendingGeometryUpdate() == PendingGeometryForced) { resizeDecoration(); - m_frame.setGeometry(m_bufferGeometry); + // If the client is being interactively resized, then the frame window, the wrapper window, + // and the client window have correct geometry at this point, so we don't have to configure + // them again. If the client doesn't support frame counters, always update geometry. + const bool needsGeometryUpdate = !isResize() || m_syncRequest.counter == XCB_NONE; + if (needsGeometryUpdate) { + m_frame.setGeometry(m_bufferGeometry); + } if (!isShade()) { - QSize cs = clientSize(); - m_wrapper.setGeometry(QRect(clientPos(), cs)); - if (!isResize() || m_syncRequest.counter == XCB_NONE) { - m_client.setGeometry(0, 0, cs.width(), cs.height()); + if (needsGeometryUpdate) { + m_wrapper.setGeometry(QRect(clientPos(), clientSize())); + m_client.resize(clientSize()); } // SELI - won't this be too expensive? // THOMAS - yes, but gtk+ clients will not resize without ... @@ -4757,7 +4768,15 @@ void X11Client::doResizeSync() m_syncRequest.timeout->start(33); // and the client, the mouse is still moved at full speed } // and no human can control faster resizes anyway const QRect moveResizeClientGeometry = frameRectToClientRect(moveResizeGeometry()); - m_client.setGeometry(0, 0, moveResizeClientGeometry.width(), moveResizeClientGeometry.height()); + const QRect moveResizeBufferGeometry = frameRectToBufferRect(moveResizeGeometry()); + + // According to the Composite extension spec, a window will get a new pixmap allocated each time + // it is mapped or resized. Given that we redirect frame windows and not client windows, we have + // to resize the frame window in order to forcefully reallocate offscreen storage. If we don't do + // this, then we might render partially updated client window. I know, it sucks. + m_frame.setGeometry(moveResizeBufferGeometry); + m_wrapper.setGeometry(QRect(clientPos(), moveResizeClientGeometry.size())); + m_client.resize(moveResizeClientGeometry.size()); } void X11Client::doPerformMoveResize() @@ -4972,4 +4991,11 @@ void X11Client::damageNotifyEvent() AbstractClient::damageNotifyEvent(); } +void X11Client::updateWindowPixmap() +{ + if (effectWindow() && effectWindow()->sceneWindow()) { + effectWindow()->sceneWindow()->updatePixmap(); + } +} + } // namespace diff --git a/x11client.h b/x11client.h index 65c4197e32..f8065c157d 100644 --- a/x11client.h +++ b/x11client.h @@ -96,6 +96,7 @@ public: QPoint clientPosToFramePos(const QPoint &point) const override; QSize frameSizeToClientSize(const QSize &size) const override; QSize clientSizeToFrameSize(const QSize &size) const override; + QRect frameRectToBufferRect(const QRect &rect) const; bool isTransient() const override; bool groupTransient() const override; @@ -454,6 +455,7 @@ private: void updateInputShape(); void updateServerGeometry(); + void updateWindowPixmap(); xcb_timestamp_t readUserTimeMapTimestamp(const KStartupInfoId* asn_id, const KStartupInfoData* asn_data, bool session) const;