From 1cc38c929a100337547abd42e0c7ccc133878877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Thu, 24 Aug 2017 16:53:40 +0200 Subject: [PATCH] Move updateXTime into the X11 standalone platform Summary: KWin::updateXTime only delegates into the platform API where the method is a no-op. The actual implementation is moved into the X11 standalone platform as it uses QX11Info which is non functional except on the X11 standalone platform. This change exposes a problem with timestamp handling: on Wayland the X11 timestamp does not get updated at all, causing e.g. window sync not work correctly (c.f. bug 374881). We cannot implement the updating in the same way as QX11Info/Qt xcb platform does it as that would introduce a blocking roundtrip to XWayland which is dangerous. As a side-effect this change removes linking to Qt5::X11Extras in kwin core as it's no longer needed. Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D7515 --- CMakeLists.txt | 3 +-- platform.cpp | 4 ++++ platform.h | 9 +++++++++ .../platforms/x11/standalone/x11_platform.cpp | 17 +++++++++++++++++ plugins/platforms/x11/standalone/x11_platform.h | 2 ++ utils.cpp | 15 ++------------- 6 files changed, 35 insertions(+), 15 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4860c20c56..97313160d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -532,7 +532,6 @@ set(kwin_QT_LIBS Qt5::DBus Qt5::Quick Qt5::Script - Qt5::X11Extras ) set(kwin_KDE_LIBS @@ -614,7 +613,7 @@ generate_export_header(kwin EXPORT_FILE_NAME kwin_export.h) target_link_libraries(kwin kwinglutils ${epoxy_LIBRARY}) kf5_add_kdeinit_executable(kwin_x11 main_x11.cpp) -target_link_libraries(kdeinit_kwin_x11 kwin KF5::Crash) +target_link_libraries(kdeinit_kwin_x11 kwin KF5::Crash Qt5::X11Extras) install(TARGETS kwin ${INSTALL_TARGETS_DEFAULT_ARGS} LIBRARY NAMELINK_SKIP ) install(TARGETS kdeinit_kwin_x11 ${INSTALL_TARGETS_DEFAULT_ARGS} ) diff --git a/platform.cpp b/platform.cpp index 91ac210c55..8472b56cd4 100644 --- a/platform.cpp +++ b/platform.cpp @@ -466,4 +466,8 @@ OverlayWindow *Platform::createOverlayWindow() return nullptr; } +void Platform::updateXTime() +{ +} + } diff --git a/platform.h b/platform.h index 35392bf5d5..1d56933c4e 100644 --- a/platform.h +++ b/platform.h @@ -308,6 +308,15 @@ public: **/ virtual OverlayWindow *createOverlayWindow(); + /** + * Allows a platform to update the X11 timestamp. + * Mostly for the X11 standalone platform to interact with QX11Info. + * + * Default implementation does nothing. This means code relying on the X timestamp being up to date, + * might not be working. E.g. synced X11 window resizing + **/ + virtual void updateXTime(); + public Q_SLOTS: void pointerMotion(const QPointF &position, quint32 time); void pointerButtonPressed(quint32 button, quint32 time); diff --git a/plugins/platforms/x11/standalone/x11_platform.cpp b/plugins/platforms/x11/standalone/x11_platform.cpp index 0a938b20e7..bbf9ec4ed9 100644 --- a/plugins/platforms/x11/standalone/x11_platform.cpp +++ b/plugins/platforms/x11/standalone/x11_platform.cpp @@ -315,4 +315,21 @@ OverlayWindow *X11StandalonePlatform::createOverlayWindow() return new OverlayWindowX11(); } +/* + Updates xTime(). This used to simply fetch current timestamp from the server, + but that can cause xTime() to be newer than timestamp of events that are + still in our events queue, thus e.g. making XSetInputFocus() caused by such + event to be ignored. Therefore events queue is searched for first + event with timestamp, and extra PropertyNotify is generated in order to make + sure such event is found. +*/ +void X11StandalonePlatform::updateXTime() +{ + // NOTE: QX11Info::getTimestamp does not yet search the event queue as the old + // solution did. This means there might be regressions currently. See the + // documentation above on how it should be done properly. + kwinApp()->setX11Time(QX11Info::getTimestamp(), Application::TimestampUpdate::Always); +} + + } diff --git a/plugins/platforms/x11/standalone/x11_platform.h b/plugins/platforms/x11/standalone/x11_platform.h index 861b2bf10a..295e20fdce 100644 --- a/plugins/platforms/x11/standalone/x11_platform.h +++ b/plugins/platforms/x11/standalone/x11_platform.h @@ -57,6 +57,8 @@ public: OverlayWindow *createOverlayWindow() override; + void updateXTime() override; + protected: void doHideCursor() override; void doShowCursor() override; diff --git a/utils.cpp b/utils.cpp index 2176ba614f..c58e8b19f5 100644 --- a/utils.cpp +++ b/utils.cpp @@ -35,12 +35,12 @@ along with this program. If not, see . #include #include -#include #include #include #include "atoms.h" +#include "platform.h" #include "workspace.h" #include @@ -72,20 +72,9 @@ StrutRect::StrutRect(const StrutRect& other) #endif #ifndef KCMRULES -/* - Updates xTime(). This used to simply fetch current timestamp from the server, - but that can cause xTime() to be newer than timestamp of events that are - still in our events queue, thus e.g. making XSetInputFocus() caused by such - event to be ignored. Therefore events queue is searched for first - event with timestamp, and extra PropertyNotify is generated in order to make - sure such event is found. -*/ void updateXTime() { - // NOTE: QX11Info::getTimestamp does not yet search the event queue as the old - // solution did. This means there might be regressions currently. See the - // documentation above on how it should be done properly. - kwinApp()->setX11Time(QX11Info::getTimestamp(), Application::TimestampUpdate::Always); + kwinApp()->platform()->updateXTime(); } static int server_grab_count = 0;