From e0c66d42cbbd7b0d1b94556ff30c10947a235e61 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Thu, 9 Feb 2023 11:54:56 +0000 Subject: [PATCH] Match pointer/keyboard/touch lifespan to Seat lifespan PointerInterface is a "Server-managed multicasting resource". As in we have one QObject, managed by the server, but internally it represents multiple resources from various clients. We cannot control the lifespan of those resources, they may persist long after we stop having these capabilities on the seat. If we delete our pointer object when we stop advertising a pointer capability we have race conditions with clients calling release, or potentially even having a seat_get_pointer in flight. It's easier and safer just to have PointerInterface last as long as the Seat. If we don't have a mouse no-one should try to bind, and even if they did or remained bound long after we stop having a mouse it won't do any harm as there are no mouse events to broadcast. --- src/pointer_input.cpp | 18 +++------- src/pointer_input.h | 1 - src/wayland/keyboard_interface.h | 1 + src/wayland/pointer_interface.h | 2 +- src/wayland/seat_interface.cpp | 57 ++++++++++---------------------- src/wayland/touch_interface.h | 2 +- 6 files changed, 26 insertions(+), 55 deletions(-) diff --git a/src/pointer_input.cpp b/src/pointer_input.cpp index 01d99c65ff..b807f59af8 100644 --- a/src/pointer_input.cpp +++ b/src/pointer_input.cpp @@ -891,8 +891,6 @@ CursorImage::CursorImage(PointerInputRedirection *parent) m_decoration.cursor = std::make_unique(); m_serverCursor.cursor = std::make_unique(); - connect(waylandServer()->seat(), &KWaylandServer::SeatInterface::hasPointerChanged, - this, &CursorImage::handlePointerChanged); #if KWIN_BUILD_SCREENLOCKER if (waylandServer()->hasScreenLockerIntegration()) { connect(ScreenLocker::KSldApp::self(), &ScreenLocker::KSldApp::lockStateChanged, this, &CursorImage::reevaluteSource); @@ -924,7 +922,11 @@ CursorImage::CursorImage(PointerInputRedirection *parent) m_decoration.cursor->setTheme(m_waylandImage.theme()); }); - handlePointerChanged(); + KWaylandServer::PointerInterface *pointer = waylandServer()->seat()->pointer(); + + connect(pointer, &KWaylandServer::PointerInterface::focusedSurfaceChanged, + this, &CursorImage::handleFocusedSurfaceChanged); + reevaluteSource(); } @@ -950,16 +952,6 @@ void CursorImage::markAsRendered(std::chrono::milliseconds timestamp) cursorSurface->frameRendered(timestamp.count()); } -void CursorImage::handlePointerChanged() -{ - KWaylandServer::PointerInterface *pointer = waylandServer()->seat()->pointer(); - - if (pointer) { - connect(pointer, &KWaylandServer::PointerInterface::focusedSurfaceChanged, - this, &CursorImage::handleFocusedSurfaceChanged); - } -} - void CursorImage::handleFocusedSurfaceChanged() { KWaylandServer::PointerInterface *pointer = waylandServer()->seat()->pointer(); diff --git a/src/pointer_input.h b/src/pointer_input.h index 9a519fd782..d224d3d64b 100644 --- a/src/pointer_input.h +++ b/src/pointer_input.h @@ -226,7 +226,6 @@ private: void updateDecorationCursor(); void updateMoveResize(); - void handlePointerChanged(); void handleFocusedSurfaceChanged(); PointerInputRedirection *m_pointer; diff --git a/src/wayland/keyboard_interface.h b/src/wayland/keyboard_interface.h index e653d33874..332205d7f2 100644 --- a/src/wayland/keyboard_interface.h +++ b/src/wayland/keyboard_interface.h @@ -61,6 +61,7 @@ public: private: void setFocusedSurface(SurfaceInterface *surface, quint32 serial); friend class SeatInterface; + friend class SeatInterfacePrivate; friend class KeyboardInterfacePrivate; explicit KeyboardInterface(SeatInterface *seat); diff --git a/src/wayland/pointer_interface.h b/src/wayland/pointer_interface.h index a68336172e..07d628404f 100644 --- a/src/wayland/pointer_interface.h +++ b/src/wayland/pointer_interface.h @@ -77,7 +77,7 @@ private: explicit PointerInterface(SeatInterface *seat); std::unique_ptr d; - friend class SeatInterface; + friend class SeatInterfacePrivate; friend class PointerInterfacePrivate; }; diff --git a/src/wayland/seat_interface.cpp b/src/wayland/seat_interface.cpp index 871d38b605..e7f6754ea6 100644 --- a/src/wayland/seat_interface.cpp +++ b/src/wayland/seat_interface.cpp @@ -53,6 +53,9 @@ SeatInterfacePrivate::SeatInterfacePrivate(SeatInterface *q, Display *display) textInputV1 = new TextInputV1Interface(q); textInputV2 = new TextInputV2Interface(q); textInputV3 = new TextInputV3Interface(q); + pointer.reset(new PointerInterface(q)); + keyboard.reset(new KeyboardInterface(q)); + touch.reset(new TouchInterface(q)); } void SeatInterfacePrivate::seat_bind_resource(Resource *resource) @@ -66,38 +69,20 @@ void SeatInterfacePrivate::seat_bind_resource(Resource *resource) void SeatInterfacePrivate::seat_get_pointer(Resource *resource, uint32_t id) { - if (!(accumulatedCapabilities & capability_pointer)) { - wl_resource_post_error(resource->handle, 0, "wl_pointer capability is missing"); - return; - } - if (pointer) { - PointerInterfacePrivate *pointerPrivate = PointerInterfacePrivate::get(pointer.get()); - pointerPrivate->add(resource->client(), id, resource->version()); - } + PointerInterfacePrivate *pointerPrivate = PointerInterfacePrivate::get(pointer.get()); + pointerPrivate->add(resource->client(), id, resource->version()); } void SeatInterfacePrivate::seat_get_keyboard(Resource *resource, uint32_t id) { - if (!(accumulatedCapabilities & capability_keyboard)) { - wl_resource_post_error(resource->handle, 0, "wl_keyboard capability is missing"); - return; - } - if (keyboard) { - KeyboardInterfacePrivate *keyboardPrivate = KeyboardInterfacePrivate::get(keyboard.get()); - keyboardPrivate->add(resource->client(), id, resource->version()); - } + KeyboardInterfacePrivate *keyboardPrivate = KeyboardInterfacePrivate::get(keyboard.get()); + keyboardPrivate->add(resource->client(), id, resource->version()); } void SeatInterfacePrivate::seat_get_touch(Resource *resource, uint32_t id) { - if (!(accumulatedCapabilities & capability_touch)) { - wl_resource_post_error(resource->handle, 0, "wl_touch capability is missing"); - return; - } - if (touch) { - TouchInterfacePrivate *touchPrivate = TouchInterfacePrivate::get(touch.get()); - touchPrivate->add(resource->client(), id, resource->version()); - } + TouchInterfacePrivate *touchPrivate = TouchInterfacePrivate::get(touch.get()); + touchPrivate->add(resource->client(), id, resource->version()); } void SeatInterfacePrivate::seat_release(Resource *resource) @@ -334,56 +319,50 @@ void SeatInterfacePrivate::sendCapabilities() void SeatInterface::setHasKeyboard(bool has) { - if (!d->keyboard != has) { + if (hasKeyboard() == has) { return; } if (has) { d->capabilities |= SeatInterfacePrivate::capability_keyboard; - d->keyboard.reset(new KeyboardInterface(this)); } else { d->capabilities &= ~SeatInterfacePrivate::capability_keyboard; - d->keyboard.reset(); } d->accumulatedCapabilities |= d->capabilities; d->sendCapabilities(); - Q_EMIT hasKeyboardChanged(d->keyboard != nullptr); + Q_EMIT hasKeyboardChanged(has); } void SeatInterface::setHasPointer(bool has) { - if (!d->pointer != has) { + if (hasPointer() == has) { return; } if (has) { d->capabilities |= SeatInterfacePrivate::capability_pointer; - d->pointer.reset(new PointerInterface(this)); } else { d->capabilities &= ~SeatInterfacePrivate::capability_pointer; - d->pointer.reset(); } d->accumulatedCapabilities |= d->capabilities; d->sendCapabilities(); - Q_EMIT hasPointerChanged(d->pointer != nullptr); + Q_EMIT hasPointerChanged(has); } void SeatInterface::setHasTouch(bool has) { - if (!d->touch != has) { + if (hasTouch() == has) { return; } if (has) { d->capabilities |= SeatInterfacePrivate::capability_touch; - d->touch.reset(new TouchInterface(this)); } else { d->capabilities &= ~SeatInterfacePrivate::capability_touch; - d->touch.reset(); } d->accumulatedCapabilities |= d->capabilities; d->sendCapabilities(); - Q_EMIT hasTouchChanged(d->touch != nullptr); + Q_EMIT hasTouchChanged(has); } void SeatInterface::setName(const QString &name) @@ -410,17 +389,17 @@ QString SeatInterface::name() const bool SeatInterface::hasPointer() const { - return d->pointer != nullptr; + return d->capabilities & SeatInterfacePrivate::capability_pointer; } bool SeatInterface::hasKeyboard() const { - return d->keyboard != nullptr; + return d->capabilities & SeatInterfacePrivate::capability_keyboard; } bool SeatInterface::hasTouch() const { - return d->touch != nullptr; + return d->capabilities & SeatInterfacePrivate::capability_touch; } Display *SeatInterface::display() const diff --git a/src/wayland/touch_interface.h b/src/wayland/touch_interface.h index 9684672233..0aec515f20 100644 --- a/src/wayland/touch_interface.h +++ b/src/wayland/touch_interface.h @@ -41,7 +41,7 @@ private: explicit TouchInterface(SeatInterface *seat); std::unique_ptr d; - friend class SeatInterface; + friend class SeatInterfacePrivate; friend class TouchInterfacePrivate; };