Skip to content

Conversation

@yixinshark
Copy link
Contributor

@yixinshark yixinshark commented Feb 5, 2026

… protocol.

add support in plugin-manager-v1.xml

Log: Add support for modifying the plugin popup cursor shape via the protocol.

Summary by Sourcery

Add support for changing the plugin popup cursor shape via the Wayland plugin manager protocol and integrate it with widget event handling.

New Features:

  • Allow plugin popups to request cursor shape changes through the Wayland plugin manager protocol.

Enhancements:

  • Propagate widget cursor change events to plugin popups so they can notify the compositor of updated cursor shapes.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 5, 2026

Reviewer's Guide

Adds Wayland protocol and plumbing to allow plugins to request cursor shape changes for popup widgets, wiring widget cursor changes through PluginPopup to the plugin-manager-v1 set_cursor request.

Sequence diagram for plugin popup cursor shape change flow

sequenceDiagram
    actor User
    participant PopupWidget
    participant EventFilter
    participant PluginPopup
    participant PluginPopupSurface
    participant PluginManagerV1
    participant WaylandCompositor

    User->>PopupWidget: Move pointer over popup
    PopupWidget->>EventFilter: QEvent::CursorChange
    EventFilter->>EventFilter: handleCursorChange(widget)
    EventFilter->>PluginPopup: requestSetCursor(cursorShape)
    PluginPopup->>PluginPopupSurface: signal requestSetCursor(cursorShape)
    PluginPopupSurface->>PluginPopupSurface: set_cursor(cursorShape)
    PluginPopupSurface->>PluginManagerV1: request set_cursor(cursor_shape)
    PluginManagerV1->>WaylandCompositor: set_cursor(cursor_shape)
    WaylandCompositor-->>User: Updated cursor shape
Loading

Class diagram for updated PluginPopup and EventFilter cursor handling

classDiagram
    class QObject
    class QWidget
    class QWindow
    class QEvent
    class QHelpEvent
    class QTimer

    class EventFilter {
        +bool eventFilter(QObject watched, QEvent event)
        -void updateToolTipPosition(QPoint globalPos)
        -void handleCursorChange(QWidget widget)
    }

    class PluginPopup {
        -QWindow window
        +int x()
        +int y()
        +void xChanged()
        +void yChanged()
        +void pluginPosChanged(QPoint point)
        +void requestSetCursor(int cursorShape)
        -PluginPopup(QWindow window)
        +static PluginPopup getWithoutCreating(QWindow window)
    }

    class PluginPopupSurface {
        -PluginManagerIntegration manager
        -QtWaylandClientShellSurface shellSurface
        -PluginPopup popup
        -QTimer m_dirtyTimer
        +PluginPopupSurface(PluginManagerIntegration manager, QtWaylandClientShellSurface shellSurface, PluginPopup popup)
        +~PluginPopupSurface()
        -void set_position(int x, int y)
        -void set_cursor(int cursorShape)
    }

    QObject <|-- EventFilter
    QObject <|-- PluginPopup
    QWidget <|-- PopupWidget
    QWindow <.. PluginPopup
    PluginPopupSurface o-- PluginPopup
    PluginPopupSurface o-- QTimer

    EventFilter ..> QWidget : handleCursorChange
    EventFilter ..> PluginPopup : getWithoutCreating
    EventFilter ..> PluginPopup : requestSetCursor
    PluginPopupSurface ..> PluginPopup : requestSetCursor signal
    PluginPopupSurface ..> QTimer : timeout connection
Loading

File-Level Changes

Change Details Files
Forward widget cursor shape changes from the popup widget to the plugin popup abstraction.
  • Extend the EventFilter to handle QEvent::CursorChange in addition to tooltip events
  • On cursor change, obtain the QWidget from the watched object, resolve its window handle, and fetch the associated PluginPopup via Plugin::PluginPopup::getWithoutCreating
  • Emit a new requestSetCursor(int cursorShape) signal on the PluginPopup with the current widget cursor shape
src/loader/widgetplugin.cpp
src/tray-wayland-integration/plugin.h
Extend the Wayland plugin-manager protocol and surface integration to support cursor shape updates requested by PluginPopup.
  • Add a set_cursor request with an int cursor_shape argument to plugin-manager-v1.xml, documenting that it follows Qt::CursorShape enum values
  • Add a requestSetCursor(int) signal to PluginPopup in the Wayland tray plugin interface
  • In PluginPopupSurface, connect the PluginPopup::requestSetCursor signal to the Wayland surface set_cursor request so cursor changes are propagated to the compositor
src/protocol/plugin-manager-v1.xml
src/tray-wayland-integration/pluginsurface.cpp
src/tray-wayland-integration/plugin.h

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • The handleCursorChange path assumes any QEvent::CursorChange should be forwarded; consider validating that the cursor shape actually changed from the last sent value to avoid spamming set_cursor requests during rapid cursor updates.
  • Since the protocol now relies on integer values matching Qt::CursorShape, it may be safer to introduce an explicit mapping/validation layer (e.g. a small switch or helper) rather than sending static_cast<int>(cursorShape) directly, to guard against enum changes or unexpected values.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `handleCursorChange` path assumes any `QEvent::CursorChange` should be forwarded; consider validating that the cursor shape actually changed from the last sent value to avoid spamming `set_cursor` requests during rapid cursor updates.
- Since the protocol now relies on integer values matching `Qt::CursorShape`, it may be safer to introduce an explicit mapping/validation layer (e.g. a small switch or helper) rather than sending `static_cast<int>(cursorShape)` directly, to guard against enum changes or unexpected values.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@deepin-bot
Copy link

deepin-bot bot commented Feb 5, 2026

TAG Bot

New tag: 2.0.25
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #426

… protocol.

add support in plugin-manager-v1.xml

Log: Add support for modifying the plugin popup cursor shape via the protocol.
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的diff主要实现了在Wayland环境下,当插件弹窗内的鼠标光标形状发生变化时,将其同步到合成器的功能。以下是对这段代码的审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 类型转换与检查
    EventFilter::eventFilter 中,event->type() == QEvent::CursorChange 分支内使用了 qobject_cast<QWidget*>(watched)。逻辑上是正确的,因为 QCursorChange 事件通常只发送给 QWidget。但在 handleCursorChange 函数中,又对 widget 进行了空指针检查(!widget)。虽然这增加了鲁棒性,但考虑到 eventFilter 中已经做过 qobject_cast,这里的 !widget 检查显得略微冗余,除非 handleCursorChange 设计为可能被外部单独调用。
  • 事件类型匹配
    代码逻辑清晰,正确捕获了 QEvent::CursorChange 事件。
  • 移除 Q_UNUSED
    EventFilter::eventFilter 中移除了 Q_UNUSED(watched) 是正确的,因为新逻辑使用了 watched 对象。

2. 代码质量

  • 函数职责
    将光标处理逻辑提取到 handleCursorChange 私有成员函数中是很好的做法,保持了 eventFilter 的简洁,符合单一职责原则。
  • 空指针检查链
    handleCursorChange 中的 if (!widget || !widget->window() || !widget->window()->windowHandle()) 写法紧凑,但为了可读性,建议拆分或添加注释,说明为什么需要检查 windowHandle(因为 Wayland 协议交互需要它)。
  • Magic Number (隐式)
    plugin-manager-v1.xml 中,cursor_shape 被定义为 int 类型,代码中通过 static_cast<int>(cursorShape) 传递。这依赖于客户端和合成器双方对 Qt::CursorShape 枚举值的共识。虽然没有显式的 Magic Number,但这种基于枚举值强转的协议耦合度较高。建议在协议文档中明确指出该 int 值对应 Qt 的枚举。

3. 代码性能

  • 信号槽连接
    PluginPopupSurface 构造函数中使用了 Lambda 表达式连接信号 requestSetCursor。这是高效的,避免了创建额外的私有槽函数。
  • 事件过滤频率
    QEvent::CursorChange 事件通常不会像 MouseMove 那样高频触发,因此在 eventFilter 中处理它不会造成明显的性能瓶颈。
  • 对象查找
    Plugin::PluginPopup::getWithoutCreating(windowHandle) 涉及哈希表或映射查找,但在光标改变这个低频操作下是可以接受的。

4. 代码安全

  • 类型安全
    XML 协议中使用 int 传递枚举值,破坏了类型安全。如果合成器端对传入的 int 值没有进行边界检查(例如,传入了一个不是 Qt::CursorShape 的值),可能会导致未定义行为或崩溃。
    • 建议:在接收端(合成器端或协议处理层)应增加对 cursor_shape 的范围校验,确保其是有效的 Qt::CursorShape 枚举值。
  • 空指针解引用风险
    虽然代码中有多层空指针检查,但在 handleCursorChange 中,widget->window()->windowHandle() 的调用链如果在多线程环境下(尽管 Qt GUI 通常在主线程),对象状态可能发生变化。不过,Qt 的事件循环通常在主线程,风险较低。
  • 协议安全性
    协议中添加了 set_cursor 请求。恶意插件可能会频繁发送此请求导致合成器资源消耗。建议在合成器端实现该请求时,考虑增加频率限制或去重逻辑(如果光标形状未变则不处理)。

改进建议总结

  1. 增强协议文档:在 XML 注释中明确引用 Qt::CursorShape,并说明值的范围。
  2. 防御性编程:在 set_cursor 的接收端(C++ 实现侧),增加对 cursorShape 参数的有效性检查。
  3. 代码可读性
    // 建议修改 handleCursorChange 中的检查逻辑,增加注释
    void handleCursorChange(QWidget *widget)
    {
        // Ensure the widget has a valid window and window handle for Wayland communication
        if (!widget || !widget->window() || !widget->window()->windowHandle()) {
            return;
        }
        // ... 后续逻辑
    }
  4. 宏定义一致性:确保 Q_DECL_HIDDEN 在项目中定义一致,用于隐藏类实现细节,这里的使用是正确的。

总体来说,这段代码逻辑清晰,实现了功能需求,但在协议层的类型安全和边界检查上可以进一步加强。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants