From ea52ef9e572b54bce64c0c18303295f4a0f15aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Mon, 17 Oct 2016 16:12:21 +0200 Subject: [PATCH] Add a PlatformCursorImage to Platform and EffectsHandler Summary: There are several effects (screenshot, zoom) which need access to the cursor image and cursor hotspot. So far these effects used X11 unconditionally to get the cursor which obviously does not work on Wayland. This change adds a new class PlatformCursorImage to kwinglobals which wraps what a cursor is (image and hotspot) and adds a new virtual method to Platform to provide such a PlatformCursorImage. By default it's the cursor image the Platform tracks. On X11/standalone platform this new virtual method is overriden and provides a PlatformCursorImage from X11 using the code previously used in screenshot effect. Screenshot effect and zoom are adjusted to use the new API instead of X11. Test Plan: Zoom effect tested on Wayland, now gets the proper cursor icon. X11 functionality not yet tested. Reviewers: #kwin, #plasma_on_wayland Subscribers: plasma-devel, kwin Tags: #plasma_on_wayland, #kwin Differential Revision: https://phabricator.kde.org/D3093 --- autotests/mock_effectshandler.h | 4 +++ effects.cpp | 5 +++ effects.h | 2 ++ effects/screenshot/screenshot.cpp | 13 ++------ effects/zoom/zoom.cpp | 20 ++++-------- libkwineffects/CMakeLists.txt | 2 +- libkwineffects/kwineffects.h | 7 ++++ libkwineffects/kwinglobals.h | 32 +++++++++++++++++++ platform.cpp | 5 +++ platform.h | 12 +++++++ .../platforms/x11/standalone/x11_platform.cpp | 17 ++++++++++ .../platforms/x11/standalone/x11_platform.h | 2 ++ 12 files changed, 97 insertions(+), 24 deletions(-) diff --git a/autotests/mock_effectshandler.h b/autotests/mock_effectshandler.h index ef1320acc6..d19f3796ca 100644 --- a/autotests/mock_effectshandler.h +++ b/autotests/mock_effectshandler.h @@ -237,6 +237,10 @@ public: m_animationsSuported = set; } + KWin::PlatformCursorImage cursorImage() const override { + return KWin::PlatformCursorImage(); + } + private: bool m_animationsSuported = true; }; diff --git a/effects.cpp b/effects.cpp index f50c96b744..cd687bf5c4 100644 --- a/effects.cpp +++ b/effects.cpp @@ -1559,6 +1559,11 @@ void EffectsHandlerImpl::highlightWindows(const QVector &windows e->perform(Effect::HighlightWindows, QVariantList{QVariant::fromValue(windows)}); } +PlatformCursorImage EffectsHandlerImpl::cursorImage() const +{ + return kwinApp()->platform()->cursorImage(); +} + //**************************************** // EffectWindowImpl //**************************************** diff --git a/effects.h b/effects.h index 0cab4b9527..ccacc80eeb 100644 --- a/effects.h +++ b/effects.h @@ -228,6 +228,8 @@ public: bool animationsSupported() const override; + PlatformCursorImage cursorImage() const override; + Scene *scene() const { return m_scene; } diff --git a/effects/screenshot/screenshot.cpp b/effects/screenshot/screenshot.cpp index d50e21a998..d91ef46ad0 100644 --- a/effects/screenshot/screenshot.cpp +++ b/effects/screenshot/screenshot.cpp @@ -369,20 +369,13 @@ QImage ScreenShotEffect::blitScreenshot(const QRect &geometry) } void ScreenShotEffect::grabPointerImage(QImage& snapshot, int offsetx, int offsety) -// Uses the X11_EXTENSIONS_XFIXES_H extension to grab the pointer image, and overlays it onto the snapshot. { - QScopedPointer cursor( - xcb_xfixes_get_cursor_image_reply(xcbConnection(), - xcb_xfixes_get_cursor_image_unchecked(xcbConnection()), - NULL)); - if (cursor.isNull()) + const auto cursor = effects->cursorImage(); + if (cursor.image().isNull()) return; - QImage qcursorimg((uchar *) xcb_xfixes_get_cursor_image_cursor_image(cursor.data()), cursor->width, cursor->height, - QImage::Format_ARGB32_Premultiplied); - QPainter painter(&snapshot); - painter.drawImage(QPointF(cursor->x - cursor->xhot - offsetx, cursor->y - cursor ->yhot - offsety), qcursorimg); + painter.drawImage(effects->cursorPos() - cursor.hotSpot() - QPoint(offsetx, offsety), cursor.image()); } void ScreenShotEffect::convertFromGLImage(QImage &img, int w, int h) diff --git a/effects/zoom/zoom.cpp b/effects/zoom/zoom.cpp index 9e76864077..6c9865bd2c 100644 --- a/effects/zoom/zoom.cpp +++ b/effects/zoom/zoom.cpp @@ -188,23 +188,17 @@ void ZoomEffect::hideCursor() void ZoomEffect::recreateTexture() { effects->makeOpenGLContextCurrent(); - // load the cursor-theme image from the Xcursor-library - xcb_xfixes_get_cursor_image_cookie_t keks = xcb_xfixes_get_cursor_image_unchecked(xcbConnection()); - xcb_xfixes_get_cursor_image_reply_t *ximg = xcb_xfixes_get_cursor_image_reply(xcbConnection(), keks, 0); - if (ximg) { - // turn the XcursorImage into a QImage that will be used to create the GLTexture/XRenderPicture. - imageWidth = ximg->width; - imageHeight = ximg->height; - cursorHotSpot = QPoint(ximg->xhot, ximg->yhot); - uint32_t *bits = xcb_xfixes_get_cursor_image_cursor_image(ximg); - QImage img((uchar*)bits, imageWidth, imageHeight, QImage::Format_ARGB32_Premultiplied); + const auto cursor = effects->cursorImage(); + if (!cursor.image().isNull()) { + imageWidth = cursor.image().width(); + imageHeight = cursor.image().height(); + cursorHotSpot = cursor.hotSpot(); if (effects->isOpenGLCompositing()) - texture.reset(new GLTexture(img)); + texture.reset(new GLTexture(cursor.image())); #ifdef KWIN_HAVE_XRENDER_COMPOSITING if (effects->compositingType() == XRenderCompositing) - xrenderPicture.reset(new XRenderPicture(img)); + xrenderPicture.reset(new XRenderPicture(cursor.image())); #endif - free(ximg); } else { qCDebug(KWINEFFECTS) << "Falling back to proportional mouse tracking!"; diff --git a/libkwineffects/CMakeLists.txt b/libkwineffects/CMakeLists.txt index d22c1db904..a0e1ccf043 100644 --- a/libkwineffects/CMakeLists.txt +++ b/libkwineffects/CMakeLists.txt @@ -5,7 +5,7 @@ ecm_setup_version(${PROJECT_VERSION} VARIABLE_PREFIX KWINEFFECTS VERSION_HEADER "${CMAKE_CURRENT_BINARY_DIR}/kwineffects_version.h" PACKAGE_VERSION_FILE "${CMAKE_CURRENT_BINARY_DIR}/KWinEffectsConfigVersion.cmake" - SOVERSION 9 + SOVERSION 10 ) ### xrenderutils lib ### diff --git a/libkwineffects/kwineffects.h b/libkwineffects/kwineffects.h index 869e552d92..e96a777ea2 100644 --- a/libkwineffects/kwineffects.h +++ b/libkwineffects/kwineffects.h @@ -1185,6 +1185,13 @@ public: **/ virtual bool animationsSupported() const = 0; + /** + * The current cursor image of the Platform. + * @see cursorPos + * @since 5.9 + **/ + virtual PlatformCursorImage cursorImage() const = 0; + /** * @return @ref KConfigGroup which holds given effect's config options **/ diff --git a/libkwineffects/kwinglobals.h b/libkwineffects/kwinglobals.h index 2926402596..e755da1e9c 100644 --- a/libkwineffects/kwinglobals.h +++ b/libkwineffects/kwinglobals.h @@ -22,6 +22,8 @@ along with this program. If not, see . #define KWIN_LIB_KWINGLOBALS_H #include +#include +#include #include #include #include @@ -197,6 +199,36 @@ KWIN_EXPORT int displayHeight() return screen ? screen->height_in_pixels : 0; } +/** + * Short wrapper for a cursor image provided by the Platform. + * @since 5.9 + **/ +class PlatformCursorImage { +public: + explicit PlatformCursorImage() + : m_image() + , m_hotSpot() + { + } + explicit PlatformCursorImage(const QImage &image, const QPoint &hotSpot) + : m_image(image) + , m_hotSpot(hotSpot) + { + } + virtual ~PlatformCursorImage() = default; + + QImage image() const { + return m_image; + } + QPoint hotSpot() const { + return m_hotSpot; + } + +private: + QImage m_image; + QPoint m_hotSpot; +}; + } // namespace #define KWIN_SINGLETON_VARIABLE(ClassName, variableName) \ diff --git a/platform.cpp b/platform.cpp index 7a7751be67..bc0853ffa1 100644 --- a/platform.cpp +++ b/platform.cpp @@ -54,6 +54,11 @@ QPoint Platform::softwareCursorHotspot() const return input()->pointer()->cursorHotSpot(); } +PlatformCursorImage Platform::cursorImage() const +{ + return PlatformCursorImage(softwareCursor(), softwareCursorHotspot()); +} + Screens *Platform::createScreens(QObject *parent) { Q_UNUSED(parent) diff --git a/platform.h b/platform.h index 3991f5fe34..1d2a190231 100644 --- a/platform.h +++ b/platform.h @@ -20,6 +20,7 @@ along with this program. If not, see . #ifndef KWIN_PLATFORM_H #define KWIN_PLATFORM_H #include +#include #include #include #include @@ -161,6 +162,17 @@ public: QPoint softwareCursorHotspot() const; void markCursorAsRendered(); + /** + * Returns a PlatformCursorImage. By default this is created by softwareCursor and + * softwareCursorHotspot. An implementing subclass can use this to provide a better + * suited PlatformCursorImage. + * + * @see softwareCursor + * @see softwareCursorHotspot + * @since 5.9 + **/ + virtual PlatformCursorImage cursorImage() const; + bool handlesOutputs() const { return m_handlesOutputs; } diff --git a/plugins/platforms/x11/standalone/x11_platform.cpp b/plugins/platforms/x11/standalone/x11_platform.cpp index 8212d1e0e0..e3cb89043f 100644 --- a/plugins/platforms/x11/standalone/x11_platform.cpp +++ b/plugins/platforms/x11/standalone/x11_platform.cpp @@ -208,4 +208,21 @@ void X11StandalonePlatform::createOpenGLSafePoint(OpenGLSafePoint safePoint) group.sync(); } +PlatformCursorImage X11StandalonePlatform::cursorImage() const +{ + auto c = kwinApp()->x11Connection(); + QScopedPointer cursor( + xcb_xfixes_get_cursor_image_reply(c, + xcb_xfixes_get_cursor_image_unchecked(c), + nullptr)); + if (cursor.isNull()) { + return PlatformCursorImage(); + } + + QImage qcursorimg((uchar *) xcb_xfixes_get_cursor_image_cursor_image(cursor.data()), cursor->width, cursor->height, + QImage::Format_ARGB32_Premultiplied); + // deep copy of image as the data is going to be freed + return PlatformCursorImage(qcursorimg.copy(), QPoint(cursor->xhot, cursor->yhot)); +} + } diff --git a/plugins/platforms/x11/standalone/x11_platform.h b/plugins/platforms/x11/standalone/x11_platform.h index e3a670e0bf..f01d56d266 100644 --- a/plugins/platforms/x11/standalone/x11_platform.h +++ b/plugins/platforms/x11/standalone/x11_platform.h @@ -49,6 +49,8 @@ public: bool openGLCompositingIsBroken() const override; void createOpenGLSafePoint(OpenGLSafePoint safePoint) override; + PlatformCursorImage cursorImage() const override; + private: /** * Tests whether GLX is supported and returns @c true