nix-stuff/roles/kde/patches/kwin-pr6406.patch
2024-12-10 11:23:16 +01:00

1045 lines
38 KiB
Diff

From 3ea8c3758e75f06c71b0b279c4e51f22ec554954 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Fri, 4 Oct 2024 22:49:16 +0300
Subject: [PATCH 1/9] Resize X11 window immediately if it doesn't support
_NET_WM_SYNC_REQUEST
The fact that X11Window::handleSyncTimeout() is shared by code that runs
when the client supports or doesn't support _NET_WM_SYNC_REQUEST makes
the XSync code difficult to follow and more error prone.
On the other hand, it appears that other window managers, e.g. Mutter and
openbox, don't bother about throttling interactive resize when the client
doesn't opt in into _NET_WM_SYNC_REQUEST, so let's do the same to simplify
the code.
As a backup plan, if it indeed becomes a problem later, we can always
save current monotonic time in X11Window::doInteractiveResizeSync() and
make X11Window::isWaitingForInteractiveResizeSync() check whether enough
of time has passed since the last resize sync.
---
autotests/integration/x11_window_test.cpp | 16 +++--------
src/x11window.cpp | 35 +++++++++--------------
2 files changed, 17 insertions(+), 34 deletions(-)
diff --git a/autotests/integration/x11_window_test.cpp b/autotests/integration/x11_window_test.cpp
index 12f53c4220..a208fa3adb 100644
--- a/autotests/integration/x11_window_test.cpp
+++ b/autotests/integration/x11_window_test.cpp
@@ -1492,7 +1492,6 @@ void X11WindowTest::testNetWmKeyboardResize()
QSignalSpy interactiveMoveResizeStartedSpy(window, &X11Window::interactiveMoveResizeStarted);
QSignalSpy interactiveMoveResizeFinishedSpy(window, &X11Window::interactiveMoveResizeFinished);
QSignalSpy interactiveMoveResizeSteppedSpy(window, &X11Window::interactiveMoveResizeStepped);
- QSignalSpy frameGeometryChangedSpy(window, &X11Window::frameGeometryChanged);
QVERIFY(interactiveMoveResizeStartedSpy.wait());
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 0);
QVERIFY(window->isInteractiveResize());
@@ -1503,7 +1502,6 @@ void X11WindowTest::testNetWmKeyboardResize()
Test::keyboardKeyPressed(KEY_RIGHT, timestamp++);
Test::keyboardKeyReleased(KEY_RIGHT, timestamp++);
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 1);
- QVERIFY(frameGeometryChangedSpy.wait());
QCOMPARE(window->frameGeometry(), originalGeometry.adjusted(0, 0, 8, 0));
// Finish the interactive move.
@@ -1531,7 +1529,6 @@ void X11WindowTest::testNetWmKeyboardResizeCancel()
QSignalSpy interactiveMoveResizeStartedSpy(window, &X11Window::interactiveMoveResizeStarted);
QSignalSpy interactiveMoveResizeFinishedSpy(window, &X11Window::interactiveMoveResizeFinished);
QSignalSpy interactiveMoveResizeSteppedSpy(window, &X11Window::interactiveMoveResizeStepped);
- QSignalSpy frameGeometryChangedSpy(window, &X11Window::frameGeometryChanged);
QVERIFY(interactiveMoveResizeStartedSpy.wait());
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 0);
QVERIFY(window->isInteractiveResize());
@@ -1542,7 +1539,6 @@ void X11WindowTest::testNetWmKeyboardResizeCancel()
Test::keyboardKeyPressed(KEY_RIGHT, timestamp++);
Test::keyboardKeyReleased(KEY_RIGHT, timestamp++);
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 1);
- QVERIFY(frameGeometryChangedSpy.wait());
QCOMPARE(window->frameGeometry(), originalGeometry.adjusted(0, 0, 8, 0));
// Cancel the interactive move.
@@ -1741,7 +1737,6 @@ void X11WindowTest::testNetWmButtonSize()
QSignalSpy interactiveMoveResizeStartedSpy(window, &X11Window::interactiveMoveResizeStarted);
QSignalSpy interactiveMoveResizeFinishedSpy(window, &X11Window::interactiveMoveResizeFinished);
QSignalSpy interactiveMoveResizeSteppedSpy(window, &X11Window::interactiveMoveResizeStepped);
- QSignalSpy frameGeometryChangedSpy(window, &X11Window::frameGeometryChanged);
QVERIFY(interactiveMoveResizeStartedSpy.wait());
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 0);
QVERIFY(window->isInteractiveResize());
@@ -1750,7 +1745,6 @@ void X11WindowTest::testNetWmButtonSize()
// Resize the window a tiny bit.
Test::pointerMotionRelative(directionToVector(direction, QSizeF(8, 8)), timestamp++);
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 1);
- QVERIFY(frameGeometryChangedSpy.wait());
QCOMPARE(window->frameGeometry(), expandRect(originalGeometry, direction, QSizeF(8, 8)));
// Finish the interactive move.
@@ -1803,7 +1797,6 @@ void X11WindowTest::testNetWmButtonSizeCancel()
QSignalSpy interactiveMoveResizeStartedSpy(window, &X11Window::interactiveMoveResizeStarted);
QSignalSpy interactiveMoveResizeFinishedSpy(window, &X11Window::interactiveMoveResizeFinished);
QSignalSpy interactiveMoveResizeSteppedSpy(window, &X11Window::interactiveMoveResizeStepped);
- QSignalSpy frameGeometryChangedSpy(window, &X11Window::frameGeometryChanged);
QVERIFY(interactiveMoveResizeStartedSpy.wait());
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 0);
QVERIFY(window->isInteractiveResize());
@@ -1811,7 +1804,6 @@ void X11WindowTest::testNetWmButtonSizeCancel()
// Resize the window a tiny bit.
Test::pointerMotionRelative(QPointF(-8, -8), timestamp++);
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 1);
- QVERIFY(frameGeometryChangedSpy.wait());
QCOMPARE(window->frameGeometry(), originalGeometry.adjusted(-8, -8, 0, 0));
// Cancel the interactive resize.
@@ -1888,7 +1880,7 @@ void X11WindowTest::testMinimumSize()
window->updateInteractiveMoveResize(KWin::Cursors::self()->mouse()->pos(), Qt::KeyboardModifiers());
QCOMPARE(KWin::Cursors::self()->mouse()->pos(), cursorPos + QPoint(8, 0));
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 1);
- QVERIFY(frameGeometryChangedSpy.wait());
+ QVERIFY(!frameGeometryChangedSpy.wait(10));
// whilst X11 window size goes through scale, the increment is a logical value kwin side
QCOMPARE(window->clientSize().width(), 100 / scale + 8);
@@ -1910,7 +1902,7 @@ void X11WindowTest::testMinimumSize()
window->updateInteractiveMoveResize(KWin::Cursors::self()->mouse()->pos(), Qt::KeyboardModifiers());
QCOMPARE(KWin::Cursors::self()->mouse()->pos(), cursorPos + QPoint(8, 8));
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 2);
- QVERIFY(frameGeometryChangedSpy.wait());
+ QVERIFY(!frameGeometryChangedSpy.wait(10));
QCOMPARE(window->clientSize().height(), 200 / scale + 8);
// Finish the resize operation.
@@ -1993,7 +1985,7 @@ void X11WindowTest::testMaximumSize()
window->updateInteractiveMoveResize(KWin::Cursors::self()->mouse()->pos(), Qt::KeyboardModifiers());
QCOMPARE(KWin::Cursors::self()->mouse()->pos(), cursorPos + QPoint(-8, 0));
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 1);
- QVERIFY(frameGeometryChangedSpy.wait());
+ QVERIFY(!frameGeometryChangedSpy.wait(10));
QCOMPARE(window->clientSize().width(), 100 / scale - 8);
window->keyPressEvent(Qt::Key_Down);
@@ -2014,7 +2006,7 @@ void X11WindowTest::testMaximumSize()
window->updateInteractiveMoveResize(KWin::Cursors::self()->mouse()->pos(), Qt::KeyboardModifiers());
QCOMPARE(KWin::Cursors::self()->mouse()->pos(), cursorPos + QPoint(-8, -8));
QCOMPARE(interactiveMoveResizeSteppedSpy.count(), 2);
- QVERIFY(frameGeometryChangedSpy.wait());
+ QVERIFY(!frameGeometryChangedSpy.wait(10));
QCOMPARE(window->clientSize().height(), 200 / scale - 8);
// Finish the resize operation.
diff --git a/src/x11window.cpp b/src/x11window.cpp
index b71a9d3640..780ccfbc7e 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -4857,36 +4857,27 @@ void X11Window::doInteractiveResizeSync(const QRectF &rect)
return;
}
- setMoveResizeGeometry(moveResizeFrameGeometry);
- setAllowCommits(false);
+ if (m_syncRequest.counter == XCB_NONE) {
+ moveResize(rect);
+ } else {
+ if (!m_syncRequest.timeout) {
+ m_syncRequest.timeout = new QTimer(this);
+ connect(m_syncRequest.timeout, &QTimer::timeout, this, &X11Window::handleSyncTimeout);
+ m_syncRequest.timeout->setSingleShot(true);
+ }
- if (!m_syncRequest.timeout) {
- m_syncRequest.timeout = new QTimer(this);
- connect(m_syncRequest.timeout, &QTimer::timeout, this, &X11Window::handleSyncTimeout);
- m_syncRequest.timeout->setSingleShot(true);
- }
+ setMoveResizeGeometry(moveResizeFrameGeometry);
+ setAllowCommits(false);
- if (m_syncRequest.counter != XCB_NONE) {
- m_syncRequest.timeout->start(250);
sendSyncRequest();
- } else {
- // For clients not supporting the XSYNC protocol, we limit the resizes to 30Hz
- // to take pointless load from X11 and the client, the mouse is still moved at
- // full speed and no human can control faster resizes anyway.
- m_syncRequest.isPending = true;
- m_syncRequest.interactiveResize = true;
- m_syncRequest.timeout->start(33);
- }
+ configure(nativeFrameGeometry, nativeWrapperGeometry, nativeClientGeometry);
- configure(nativeFrameGeometry, nativeWrapperGeometry, nativeClientGeometry);
+ m_syncRequest.timeout->start(250);
+ }
}
void X11Window::handleSyncTimeout()
{
- if (m_syncRequest.counter == XCB_NONE) { // client w/o XSYNC support. allow the next resize event
- m_syncRequest.isPending = false; // NEVER do this for clients with a valid counter
- m_syncRequest.interactiveResize = false; // (leads to sync request races in some clients)
- }
performInteractiveResize();
}
--
2.47.0
From 1be3dfa4b40732997ecb1bd1959ba775331d603a Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Fri, 4 Oct 2024 22:55:44 +0300
Subject: [PATCH 2/9] Rename X11Window::handleSync,handleSyncTimeout
---
src/syncalarmx11filter.cpp | 2 +-
src/x11window.cpp | 6 +++---
src/x11window.h | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/syncalarmx11filter.cpp b/src/syncalarmx11filter.cpp
index f4b4d49caa..133c3c6b99 100644
--- a/src/syncalarmx11filter.cpp
+++ b/src/syncalarmx11filter.cpp
@@ -28,7 +28,7 @@ bool SyncAlarmX11Filter::event(xcb_generic_event_t *event)
return alarmEvent->alarm == syncRequest.alarm && alarmEvent->counter_value.hi == syncRequest.value.hi && alarmEvent->counter_value.lo == syncRequest.value.lo;
});
if (client) {
- client->handleSync();
+ client->ackSync();
}
return false;
}
diff --git a/src/x11window.cpp b/src/x11window.cpp
index 780ccfbc7e..14d45abb27 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -3059,7 +3059,7 @@ void X11Window::checkApplicationMenuObjectPath()
readApplicationMenuObjectPath(property);
}
-void X11Window::handleSync()
+void X11Window::ackSync()
{
setReadyForPainting();
m_syncRequest.isPending = false;
@@ -4862,7 +4862,7 @@ void X11Window::doInteractiveResizeSync(const QRectF &rect)
} else {
if (!m_syncRequest.timeout) {
m_syncRequest.timeout = new QTimer(this);
- connect(m_syncRequest.timeout, &QTimer::timeout, this, &X11Window::handleSyncTimeout);
+ connect(m_syncRequest.timeout, &QTimer::timeout, this, &X11Window::ackSyncTimeout);
m_syncRequest.timeout->setSingleShot(true);
}
@@ -4876,7 +4876,7 @@ void X11Window::doInteractiveResizeSync(const QRectF &rect)
}
}
-void X11Window::handleSyncTimeout()
+void X11Window::ackSyncTimeout()
{
performInteractiveResize();
}
diff --git a/src/x11window.h b/src/x11window.h
index 4f3b43c8b0..b54db23345 100644
--- a/src/x11window.h
+++ b/src/x11window.h
@@ -293,8 +293,8 @@ public:
return m_syncRequest;
}
bool wantsSyncCounter() const;
- void handleSync();
- void handleSyncTimeout();
+ void ackSync();
+ void ackSyncTimeout();
bool allowWindowActivation(xcb_timestamp_t time = -1U, bool focus_in = false);
--
2.47.0
From ea53098c38b64cb5870ef4f3b47fb53f509e03a4 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Fri, 4 Oct 2024 22:59:34 +0300
Subject: [PATCH 3/9] Rename X11Window::SyncRequest::isPending
---
src/x11window.cpp | 12 ++++++------
src/x11window.h | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/x11window.cpp b/src/x11window.cpp
index 14d45abb27..2ebb4a5957 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -309,7 +309,7 @@ X11Window::X11Window()
m_syncRequest.counter = m_syncRequest.alarm = XCB_NONE;
m_syncRequest.timeout = m_syncRequest.failsafeTimeout = nullptr;
m_syncRequest.lastTimestamp = xTime();
- m_syncRequest.isPending = false;
+ m_syncRequest.pending = false;
m_syncRequest.interactiveResize = false;
// Set the initial mapping state
@@ -2585,7 +2585,7 @@ void X11Window::getSyncCounter()
*/
void X11Window::sendSyncRequest()
{
- if (m_syncRequest.counter == XCB_NONE || m_syncRequest.isPending) {
+ if (m_syncRequest.counter == XCB_NONE || m_syncRequest.pending) {
return; // do NOT, NEVER send a sync request when there's one on the stack. the clients will just stop respoding. FOREVER! ...
}
@@ -2599,7 +2599,7 @@ void X11Window::sendSyncRequest()
return;
}
// failed during resize
- m_syncRequest.isPending = false;
+ m_syncRequest.pending = false;
m_syncRequest.interactiveResize = false;
m_syncRequest.counter = XCB_NONE;
m_syncRequest.alarm = XCB_NONE;
@@ -2630,7 +2630,7 @@ void X11Window::sendSyncRequest()
// Send the message to client
sendClientMessage(window(), atoms->wm_protocols, atoms->net_wm_sync_request,
m_syncRequest.value.lo, m_syncRequest.value.hi);
- m_syncRequest.isPending = true;
+ m_syncRequest.pending = true;
m_syncRequest.interactiveResize = isInteractiveResize();
m_syncRequest.lastTimestamp = xTime();
}
@@ -3062,7 +3062,7 @@ void X11Window::checkApplicationMenuObjectPath()
void X11Window::ackSync()
{
setReadyForPainting();
- m_syncRequest.isPending = false;
+ m_syncRequest.pending = false;
if (m_syncRequest.failsafeTimeout) {
m_syncRequest.failsafeTimeout->stop();
}
@@ -4840,7 +4840,7 @@ void X11Window::leaveInteractiveMoveResize()
bool X11Window::isWaitingForInteractiveResizeSync() const
{
- return m_syncRequest.isPending && m_syncRequest.interactiveResize;
+ return m_syncRequest.pending && m_syncRequest.interactiveResize;
}
void X11Window::doInteractiveResizeSync(const QRectF &rect)
diff --git a/src/x11window.h b/src/x11window.h
index b54db23345..10fa57eaf2 100644
--- a/src/x11window.h
+++ b/src/x11window.h
@@ -285,7 +285,7 @@ public:
xcb_sync_alarm_t alarm;
xcb_timestamp_t lastTimestamp;
QTimer *timeout, *failsafeTimeout;
- bool isPending;
+ bool pending;
bool interactiveResize;
};
const SyncRequest &syncRequest() const
--
2.47.0
From 49319a3f3e2a1d468e12bdf3dfd7eeb2e3db0899 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Fri, 4 Oct 2024 23:12:44 +0300
Subject: [PATCH 4/9] Make XSync timeout more permissive
There are two timeouts: a soft one and a hard one. If the soft timeout
occurs, then the scheduled operation will be performed. For example,
the window will be marked as ready for painting or resized. If the hard
timeout occurs, _NET_WM_SYNC_REQUEST will be disabled.
Such an arrangement is fine in its current present form, but in order
to make Xwayland window resizing less glitchy, _NET_WM_SYNC_REQUEST
code needs to interact with wl_surface event flow, which in its turn
means that the timeout code also needs to be prepared for that. It is
very challenging to make the timeout code not break, so this change
seeks to re-arrange the timeout handling so it's easier to integrate
with wl_surface commits.
With the proposed changes, there is going to be only one timer: timeout.
If it fires, the window will be resized immediately and XSync is going
to be kept disabled until an acknowledgement arrives from the client.
---
src/x11window.cpp | 101 +++++++++++++++++-----------------------------
src/x11window.h | 5 ++-
2 files changed, 41 insertions(+), 65 deletions(-)
diff --git a/src/x11window.cpp b/src/x11window.cpp
index 2ebb4a5957..1d25ab82b1 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -307,8 +307,9 @@ X11Window::X11Window()
// TODO: Do all as initialization
m_syncRequest.counter = m_syncRequest.alarm = XCB_NONE;
- m_syncRequest.timeout = m_syncRequest.failsafeTimeout = nullptr;
+ m_syncRequest.timeout = nullptr;
m_syncRequest.lastTimestamp = xTime();
+ m_syncRequest.enabled = false;
m_syncRequest.pending = false;
m_syncRequest.interactiveResize = false;
@@ -460,9 +461,6 @@ void X11Window::releaseWindow(bool on_shutdown)
ungrabXServer();
}
- if (m_syncRequest.failsafeTimeout) {
- m_syncRequest.failsafeTimeout->stop();
- }
if (m_syncRequest.timeout) {
m_syncRequest.timeout->stop();
}
@@ -516,9 +514,6 @@ void X11Window::destroyWindow()
m_frame.reset();
}
- if (m_syncRequest.failsafeTimeout) {
- m_syncRequest.failsafeTimeout->stop();
- }
if (m_syncRequest.timeout) {
m_syncRequest.timeout->stop();
}
@@ -2550,6 +2545,7 @@ void X11Window::getSyncCounter()
Xcb::Property syncProp(false, window(), atoms->net_wm_sync_request_counter, XCB_ATOM_CARDINAL, 0, 1);
const xcb_sync_counter_t counter = syncProp.value<xcb_sync_counter_t>(XCB_NONE);
if (counter != XCB_NONE) {
+ m_syncRequest.enabled = true;
m_syncRequest.counter = counter;
m_syncRequest.value.hi = 0;
m_syncRequest.value.lo = 0;
@@ -2585,35 +2581,16 @@ void X11Window::getSyncCounter()
*/
void X11Window::sendSyncRequest()
{
- if (m_syncRequest.counter == XCB_NONE || m_syncRequest.pending) {
+ if (!m_syncRequest.enabled || m_syncRequest.pending) {
return; // do NOT, NEVER send a sync request when there's one on the stack. the clients will just stop respoding. FOREVER! ...
}
- if (!m_syncRequest.failsafeTimeout) {
- m_syncRequest.failsafeTimeout = new QTimer(this);
- connect(m_syncRequest.failsafeTimeout, &QTimer::timeout, this, [this]() {
- // client does not respond to XSYNC requests in reasonable time, remove support
- if (!ready_for_painting) {
- // failed on initial pre-show request
- setReadyForPainting();
- return;
- }
- // failed during resize
- m_syncRequest.pending = false;
- m_syncRequest.interactiveResize = false;
- m_syncRequest.counter = XCB_NONE;
- m_syncRequest.alarm = XCB_NONE;
- delete m_syncRequest.timeout;
- delete m_syncRequest.failsafeTimeout;
- m_syncRequest.timeout = nullptr;
- m_syncRequest.failsafeTimeout = nullptr;
- m_syncRequest.lastTimestamp = XCB_CURRENT_TIME;
- });
- m_syncRequest.failsafeTimeout->setSingleShot(true);
+ if (!m_syncRequest.timeout) {
+ m_syncRequest.timeout = new QTimer(this);
+ m_syncRequest.timeout->setSingleShot(true);
+ connect(m_syncRequest.timeout, &QTimer::timeout, this, &X11Window::ackSyncTimeout);
}
- // if there's no response within 10 seconds, sth. went wrong and we remove XSYNC support from this client.
- // see events.cpp X11Window::syncEvent()
- m_syncRequest.failsafeTimeout->start(ready_for_painting ? 10000 : 1000);
+ m_syncRequest.timeout->start(ready_for_painting ? 10000 : 1000);
// We increment before the notify so that after the notify
// syncCounterSerial will equal the value we are expecting
@@ -3061,29 +3038,40 @@ void X11Window::checkApplicationMenuObjectPath()
void X11Window::ackSync()
{
- setReadyForPainting();
+ // Note that a sync request can be ack'ed after the timeout. If that happens, just re-enable
+ // XSync back and do nothing more.
m_syncRequest.pending = false;
- if (m_syncRequest.failsafeTimeout) {
- m_syncRequest.failsafeTimeout->stop();
+ if (!m_syncRequest.enabled) {
+ m_syncRequest.enabled = true;
+ return;
}
- // Sync request can be acknowledged shortly after finishing resize.
+ m_syncRequest.timeout->stop();
+
+ finishSync();
+}
+
+void X11Window::ackSyncTimeout()
+{
+ // If a sync request times out, disable XSync temporarily until the client comes back to its senses.
+ m_syncRequest.enabled = false;
+
+ finishSync();
+}
+
+void X11Window::finishSync()
+{
+ setReadyForPainting();
+
if (m_syncRequest.interactiveResize) {
m_syncRequest.interactiveResize = false;
- if (m_syncRequest.timeout) {
- m_syncRequest.timeout->stop();
- }
- performInteractiveResize();
+
+ moveResize(moveResizeGeometry());
updateWindowPixmap();
+ setAllowCommits(true);
}
}
-void X11Window::performInteractiveResize()
-{
- resize(moveResizeGeometry().size());
- setAllowCommits(true);
-}
-
bool X11Window::belongToSameApplication(const X11Window *c1, const X11Window *c2, SameApplicationChecks checks)
{
bool same_app = false;
@@ -4840,7 +4828,7 @@ void X11Window::leaveInteractiveMoveResize()
bool X11Window::isWaitingForInteractiveResizeSync() const
{
- return m_syncRequest.pending && m_syncRequest.interactiveResize;
+ return m_syncRequest.enabled && m_syncRequest.pending && m_syncRequest.interactiveResize;
}
void X11Window::doInteractiveResizeSync(const QRectF &rect)
@@ -4857,30 +4845,17 @@ void X11Window::doInteractiveResizeSync(const QRectF &rect)
return;
}
- if (m_syncRequest.counter == XCB_NONE) {
+ if (!m_syncRequest.enabled) {
moveResize(rect);
} else {
- if (!m_syncRequest.timeout) {
- m_syncRequest.timeout = new QTimer(this);
- connect(m_syncRequest.timeout, &QTimer::timeout, this, &X11Window::ackSyncTimeout);
- m_syncRequest.timeout->setSingleShot(true);
- }
-
setMoveResizeGeometry(moveResizeFrameGeometry);
setAllowCommits(false);
sendSyncRequest();
configure(nativeFrameGeometry, nativeWrapperGeometry, nativeClientGeometry);
-
- m_syncRequest.timeout->start(250);
}
}
-void X11Window::ackSyncTimeout()
-{
- performInteractiveResize();
-}
-
NETExtendedStrut X11Window::strut() const
{
NETExtendedStrut ext = info->extendedStrut();
@@ -4991,7 +4966,7 @@ void X11Window::damageNotifyEvent()
Q_ASSERT(kwinApp()->operationMode() == Application::OperationModeX11);
if (!readyForPainting()) { // avoid "setReadyForPainting()" function calling overhead
- if (m_syncRequest.counter == XCB_NONE) { // cannot detect complete redraw, consider done now
+ if (!m_syncRequest.enabled) { // cannot detect complete redraw, consider done now
setReadyForPainting();
}
}
@@ -5025,7 +5000,7 @@ void X11Window::updateWindowPixmap()
void X11Window::associate()
{
auto handleMapped = [this]() {
- if (syncRequest().counter == XCB_NONE) { // cannot detect complete redraw, consider done now
+ if (!m_syncRequest.enabled) { // cannot detect complete redraw, consider done now
setReadyForPainting();
}
};
diff --git a/src/x11window.h b/src/x11window.h
index 10fa57eaf2..6b430b861a 100644
--- a/src/x11window.h
+++ b/src/x11window.h
@@ -284,7 +284,8 @@ public:
xcb_sync_int64_t value;
xcb_sync_alarm_t alarm;
xcb_timestamp_t lastTimestamp;
- QTimer *timeout, *failsafeTimeout;
+ QTimer *timeout;
+ bool enabled;
bool pending;
bool interactiveResize;
};
@@ -295,6 +296,7 @@ public:
bool wantsSyncCounter() const;
void ackSync();
void ackSyncTimeout();
+ void finishSync();
bool allowWindowActivation(xcb_timestamp_t time = -1U, bool focus_in = false);
@@ -385,7 +387,6 @@ private:
void getSyncCounter();
void sendSyncRequest();
void leaveInteractiveMoveResize() override;
- void performInteractiveResize();
void establishCommandWindowGrab(uint8_t button);
void establishCommandAllGrab(uint8_t button);
--
2.47.0
From 79e1853e9ecd20c1d9549399b46d434475b753bd Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Fri, 4 Oct 2024 23:20:03 +0300
Subject: [PATCH 5/9] Disable Xwayland surface commits for all sync requests
---
src/x11window.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/x11window.cpp b/src/x11window.cpp
index 1d25ab82b1..5688ea6da8 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -2604,7 +2604,7 @@ void X11Window::sendSyncRequest()
kwinApp()->updateXTime();
}
- // Send the message to client
+ setAllowCommits(false);
sendClientMessage(window(), atoms->wm_protocols, atoms->net_wm_sync_request,
m_syncRequest.value.lo, m_syncRequest.value.hi);
m_syncRequest.pending = true;
@@ -3049,6 +3049,7 @@ void X11Window::ackSync()
m_syncRequest.timeout->stop();
finishSync();
+ setAllowCommits(true);
}
void X11Window::ackSyncTimeout()
@@ -3057,6 +3058,7 @@ void X11Window::ackSyncTimeout()
m_syncRequest.enabled = false;
finishSync();
+ setAllowCommits(true);
}
void X11Window::finishSync()
@@ -3068,7 +3070,6 @@ void X11Window::finishSync()
moveResize(moveResizeGeometry());
updateWindowPixmap();
- setAllowCommits(true);
}
}
@@ -4849,8 +4850,6 @@ void X11Window::doInteractiveResizeSync(const QRectF &rect)
moveResize(rect);
} else {
setMoveResizeGeometry(moveResizeFrameGeometry);
- setAllowCommits(false);
-
sendSyncRequest();
configure(nativeFrameGeometry, nativeWrapperGeometry, nativeClientGeometry);
}
--
2.47.0
From 038a798c8be170eb5b7132b823d77474d118ca42 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Fri, 4 Oct 2024 23:40:41 +0300
Subject: [PATCH 6/9] Make Xwayland resizing less glitchy
XSYNC requests and wl_surface commits are unsynchronized. This results
in the window bouncing when it's being resized.
Let's consider a concrete example. Assume that there is a window with 0,0
100x100 geometry and it is being resized by dragging the left edge. If
the left edge is dragged by 10px, the following will occur:
- wl_surface commits will be blocked (it is needed to ensure the
consistent order of XSync acknowledgements and wl_surface commits)
- an XSync request will be sent
- a ConfigureNotify event will be sent
- a client processes the ConfigureNotify by repainting the window and
acknowledges the XSync request
- kwin notices that the XSync request has been acked and updates the
window position to 10,0 and unblocks surface commits
The problem is that it can take a while for Xwayland to attach a new
buffer to the surface with a size of 90x100. If kwin composes a frame in
meanwhile, the effective geometry of the window will be 10,0 100x100,
i.e. the right window edge will stick by 10px. Some time later, Xwayland
would attach a buffer with the right size (90x100), and the right window
edge will be put back in the right place.
In order to address the bouncing, this change reworks how the
synchronization is performed:
- when a window is asked to resize, kwin will freeze wl_surface commits,
send a sync request, and configure the window
- after the client repaints the window, it acks the sync request
- when kwin sees that the sync request has been acked, it will unfreeze
the wl_surface commits. And here's the most important part: it will NOT
update the window position until Xwayland commits something
- when Xwayland commits the wl_surface, kwin will update the window geometry
It's important to wait until Xwayland commits the wl_surface because, as
it was said previously, it can take a while until Xwayland commits the
wl_surface and kwin could compose a frame with the new position but old
surface size in meanwhile.
BUG: 486464
---
src/x11window.cpp | 40 +++++++++++++++++++++++++++++-----------
src/x11window.h | 2 ++
2 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/src/x11window.cpp b/src/x11window.cpp
index 5688ea6da8..b22de557d8 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -311,6 +311,7 @@ X11Window::X11Window()
m_syncRequest.lastTimestamp = xTime();
m_syncRequest.enabled = false;
m_syncRequest.pending = false;
+ m_syncRequest.acked = false;
m_syncRequest.interactiveResize = false;
// Set the initial mapping state
@@ -3046,9 +3047,13 @@ void X11Window::ackSync()
return;
}
+ m_syncRequest.acked = true;
m_syncRequest.timeout->stop();
- finishSync();
+ // With Xwayland, the sync request will be completed after the wl_surface is committed.
+ if (!waylandServer()) {
+ finishSync();
+ }
setAllowCommits(true);
}
@@ -3071,6 +3076,8 @@ void X11Window::finishSync()
moveResize(moveResizeGeometry());
updateWindowPixmap();
}
+
+ m_syncRequest.acked = false;
}
bool X11Window::belongToSameApplication(const X11Window *c1, const X11Window *c2, SameApplicationChecks checks)
@@ -3945,6 +3952,19 @@ void X11Window::handleXwaylandScaleChanged()
resize(moveResizeGeometry().size());
}
+void X11Window::handleCommitted()
+{
+ if (surface()->isMapped()) {
+ if (m_syncRequest.acked) {
+ finishSync();
+ }
+
+ if (!m_syncRequest.enabled) {
+ setReadyForPainting();
+ }
+ }
+}
+
void X11Window::setAllowCommits(bool allow)
{
if (!waylandServer()) {
@@ -4829,7 +4849,7 @@ void X11Window::leaveInteractiveMoveResize()
bool X11Window::isWaitingForInteractiveResizeSync() const
{
- return m_syncRequest.enabled && m_syncRequest.pending && m_syncRequest.interactiveResize;
+ return m_syncRequest.enabled && m_syncRequest.interactiveResize && (m_syncRequest.pending || m_syncRequest.acked);
}
void X11Window::doInteractiveResizeSync(const QRectF &rect)
@@ -4998,19 +5018,17 @@ void X11Window::updateWindowPixmap()
void X11Window::associate()
{
- auto handleMapped = [this]() {
- if (!m_syncRequest.enabled) { // cannot detect complete redraw, consider done now
- setReadyForPainting();
+ if (surface()->isMapped()) {
+ if (m_syncRequest.acked) {
+ finishSync();
}
- };
- if (surface()->isMapped()) {
- handleMapped();
- } else {
- connect(surface(), &SurfaceInterface::mapped, this, handleMapped);
+ if (!m_syncRequest.enabled) {
+ setReadyForPainting();
+ }
}
- m_pendingSurfaceId = 0;
+ connect(surface(), &SurfaceInterface::committed, this, &X11Window::handleCommitted);
}
QWindow *X11Window::findInternalWindow() const
diff --git a/src/x11window.h b/src/x11window.h
index 6b430b861a..73d4c23a4c 100644
--- a/src/x11window.h
+++ b/src/x11window.h
@@ -287,6 +287,7 @@ public:
QTimer *timeout;
bool enabled;
bool pending;
+ bool acked;
bool interactiveResize;
};
const SyncRequest &syncRequest() const
@@ -438,6 +439,7 @@ private:
void checkOutput();
void associate();
void handleXwaylandScaleChanged();
+ void handleCommitted();
void setAllowCommits(bool allow);
--
2.47.0
From 10c5298221f5dc3cc90255846580b608f6f3c18c Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Thu, 3 Oct 2024 00:43:57 +0300
Subject: [PATCH 7/9] scene: Use standard wl_surface item for Xwayland surfaces
Besides unifying the code, it fixes some visual glitches caused by the
opaque region getting out of sync.
Xwayland MR: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1698
---
src/scene/surfaceitem_wayland.cpp | 36 -------------------------------
src/scene/surfaceitem_wayland.h | 20 -----------------
src/scene/windowitem.cpp | 2 +-
3 files changed, 1 insertion(+), 57 deletions(-)
diff --git a/src/scene/surfaceitem_wayland.cpp b/src/scene/surfaceitem_wayland.cpp
index 4a307565c0..04961952b9 100644
--- a/src/scene/surfaceitem_wayland.cpp
+++ b/src/scene/surfaceitem_wayland.cpp
@@ -12,11 +12,6 @@
#include "wayland/linuxdmabufv1clientbuffer.h"
#include "wayland/subcompositor.h"
#include "wayland/surface.h"
-#include "window.h"
-
-#if KWIN_BUILD_X11
-#include "x11window.h"
-#endif
namespace KWin
{
@@ -247,37 +242,6 @@ bool SurfacePixmapWayland::isValid() const
return m_bufferRef;
}
-#if KWIN_BUILD_X11
-SurfaceItemXwayland::SurfaceItemXwayland(X11Window *window, Item *parent)
- : SurfaceItemWayland(window->surface(), parent)
- , m_window(window)
-{
- connect(window, &X11Window::shapeChanged, this, &SurfaceItemXwayland::discardQuads);
-}
-
-QList<QRectF> SurfaceItemXwayland::shape() const
-{
- QList<QRectF> shape = m_window->shapeRegion();
- for (QRectF &shapePart : shape) {
- shapePart = shapePart.intersected(rect());
- }
- return shape;
-}
-
-QRegion SurfaceItemXwayland::opaque() const
-{
- QRegion shapeRegion;
- for (const QRectF &shapePart : shape()) {
- shapeRegion += shapePart.toRect();
- }
- if (!m_window->hasAlpha()) {
- return shapeRegion;
- } else {
- return m_window->opaqueRegion() & shapeRegion;
- }
- return QRegion();
-}
-#endif
} // namespace KWin
#include "moc_surfaceitem_wayland.cpp"
diff --git a/src/scene/surfaceitem_wayland.h b/src/scene/surfaceitem_wayland.h
index f769284172..fbc7498446 100644
--- a/src/scene/surfaceitem_wayland.h
+++ b/src/scene/surfaceitem_wayland.h
@@ -16,7 +16,6 @@ namespace KWin
class GraphicsBuffer;
class SubSurfaceInterface;
class SurfaceInterface;
-class X11Window;
/**
* The SurfaceItemWayland class represents a Wayland surface in the scene.
@@ -83,23 +82,4 @@ private:
SurfaceItemWayland *m_item;
};
-#if KWIN_BUILD_X11
-/**
- * The SurfaceItemXwayland class represents an Xwayland surface in the scene.
- */
-class KWIN_EXPORT SurfaceItemXwayland : public SurfaceItemWayland
-{
- Q_OBJECT
-
-public:
- explicit SurfaceItemXwayland(X11Window *window, Item *parent = nullptr);
-
- QRegion opaque() const override;
- QList<QRectF> shape() const override;
-
-private:
- X11Window *m_window;
-};
-#endif
-
} // namespace KWin
diff --git a/src/scene/windowitem.cpp b/src/scene/windowitem.cpp
index 4283051e7e..913d2708f5 100644
--- a/src/scene/windowitem.cpp
+++ b/src/scene/windowitem.cpp
@@ -323,7 +323,7 @@ void WindowItemX11::initialize()
if (!window()->surface()) {
updateSurfaceItem(nullptr);
} else {
- updateSurfaceItem(std::make_unique<SurfaceItemXwayland>(static_cast<X11Window *>(window()), this));
+ updateSurfaceItem(std::make_unique<SurfaceItemWayland>(window()->surface(), this));
}
break;
case Application::OperationModeWaylandOnly:
--
2.47.0
From 91ca5127fbf590a383fe311b76da8dce553e2a8a Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Mon, 14 Oct 2024 00:34:28 +0300
Subject: [PATCH 8/9] Send the initial sync request before mapping the frame
window
When the wl_surface commits are blocked by the initial sync request, it
is really important that Xwayland doesn't commit the surface until the
sync request is acked. However, with the current arrangement of the code,
Xwayland may render something before the sync request is acked.
It can happen because the frame window is mapped before the initial sync
request is sent. There are many places where it can happen, e.g. in the
setupCompositing() function, or the setMinimized() function, etc.
In order to ensure that Xwayland won't render the wl_surface while we
are waiting for the sync request to get acked, this change moves the initial
sync request all the way to the top of the manage() function so the
surface commits are blocked before the frame window is mapped.
---
src/x11window.cpp | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/src/x11window.cpp b/src/x11window.cpp
index b22de557d8..be35fe07fd 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -655,6 +655,18 @@ bool X11Window::manage(xcb_window_t w, bool isMapped)
getSyncCounter();
setCaption(readName());
+ if (Compositor::compositing()) {
+ // Sending ConfigureNotify is done when setting mapping state below, getting the
+ // first sync response means window is ready for compositing.
+ //
+ // The sync request will block wl_surface commits, and with Xwayland, it is really
+ // important that wl_surfaces commits are blocked before the frame window is mapped.
+ // Otherwise Xwayland can attach a buffer before the sync request is acked.
+ sendSyncRequest();
+ } else {
+ ready_for_painting = true; // set to true in case compositing is turned on later
+ }
+
setupWindowRules();
connect(this, &X11Window::windowClassChanged, this, &X11Window::evaluateWindowRules);
@@ -1093,14 +1105,6 @@ bool X11Window::manage(xcb_window_t w, bool isMapped)
workspace()->restoreSessionStackingOrder(this);
}
- if (Compositor::compositing()) {
- // Sending ConfigureNotify is done when setting mapping state below,
- // Getting the first sync response means window is ready for compositing
- sendSyncRequest();
- } else {
- ready_for_painting = true; // set to true in case compositing is turned on later. bug #160393
- }
-
if (isShown()) {
bool allow;
if (session) {
--
2.47.0
From cffaa20e83b7d9de69dcc835f6ab5d2609a49be5 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Thu, 17 Oct 2024 19:21:32 +0300
Subject: [PATCH 9/9] Block interactive resizing if there is a pending XSync
request
There cannot be two XSync requests in flight, the type of the sync
requests doesn't matter either, e.g. one for interactive resize or
one to determine whether the client has painted the initial frame.
---
src/x11window.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/x11window.cpp b/src/x11window.cpp
index be35fe07fd..3e989f29bd 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -4853,7 +4853,7 @@ void X11Window::leaveInteractiveMoveResize()
bool X11Window::isWaitingForInteractiveResizeSync() const
{
- return m_syncRequest.enabled && m_syncRequest.interactiveResize && (m_syncRequest.pending || m_syncRequest.acked);
+ return m_syncRequest.enabled && (m_syncRequest.pending || m_syncRequest.acked);
}
void X11Window::doInteractiveResizeSync(const QRectF &rect)
--
2.47.0