Skip to content

fix(shot): relay XRecord events when Dock grab blocks Qt input on X11#845

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-5-21
May 26, 2026
Merged

fix(shot): relay XRecord events when Dock grab blocks Qt input on X11#845
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-5-21

Conversation

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member

When a Dock right-click menu is open, Qt's XGrabPointer routes all pointer events to the Dock process. The screenshot overlay never receives them via the normal Qt event path.

Fix by relaying XRecord events (which intercept at the protocol level, unaffected by grabs) through QApplication::sendEvent in four handlers:

  • onMouseMove: continuously relay during hover phase so window auto-highlight updates as the mouse moves
  • onMouseDrag: relay during drag phase so the selection rectangle tracks the mouse
  • onMousePress: relay the initial click so the drag state is entered
  • onMouseRelease: relay the release so the toolbar appears

State flags (isFirstPressButton, isFirstReleaseButton, m_currentCursor) are used as dedup guards: QueuedConnection ensures XRecord signals arrive after Qt's synchronous event filter, so if Qt already handled the event the guard condition is already true and the relay is skipped.

Log: 修复Dock右键菜单存在时截图工具鼠标事件被X11 Grab拦截的问题

PMS: BUG-330895

Influence: 修复后在Dock右键菜单打开状态下触发截图,窗口悬停检测、
框选拖拽、工具栏显示均可正常工作

When a Dock right-click menu is open, Qt's XGrabPointer routes all
pointer events to the Dock process. The screenshot overlay never
receives them via the normal Qt event path.

Fix by relaying XRecord events (which intercept at the protocol level,
unaffected by grabs) through QApplication::sendEvent in four handlers:

- onMouseMove: continuously relay during hover phase so window
  auto-highlight updates as the mouse moves
- onMouseDrag: relay during drag phase so the selection rectangle
  tracks the mouse
- onMousePress: relay the initial click so the drag state is entered
- onMouseRelease: relay the release so the toolbar appears

State flags (isFirstPressButton, isFirstReleaseButton, m_currentCursor)
are used as dedup guards: QueuedConnection ensures XRecord signals
arrive after Qt's synchronous event filter, so if Qt already handled
the event the guard condition is already true and the relay is skipped.

Log: 修复Dock右键菜单存在时截图工具鼠标事件被X11 Grab拦截的问题

PMS: BUG-330895

Influence: 修复后在Dock右键菜单打开状态下触发截图,窗口悬停检测、
框选拖拽、工具栏显示均可正常工作
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX,很高兴为你审查这份代码。

这份 diff 的核心目的是修复 X11 环境下截屏工具的两大痛点

  1. 窗口列表污染:右键菜单等弹出窗口残留在 windowRects 中,通过将获取时机延迟到 showFullScreen() 之后解决。
  2. X11 Pointer Grab 导致鼠标事件丢失:Dock 栏等持有 Grab 时,Qt 常规事件过滤失效,导致无法框选。通过 XRecord 底层拦截并补发模拟事件解决。

整体思路非常清晰,逻辑严密,对 X11 事件机制的理解很透彻。以下是我从语法逻辑、代码质量、性能和安全四个维度提出的详细审查意见和改进建议:


一、 语法与逻辑

1. refetchWindowInfo() 中的 Wayland 判断存在潜在遗漏
initAttributes() 中,原代码的 else 分支只排除了 Wayland,但 refetchWindowInfo() 中同时排除了 Wayland 和 Treeland。如果在 initAttributes()else 分支中触发了逻辑,说明当前既不是 Wayland 也不是 Treeland,所以 refetchWindowInfo() 的判断是安全的。但建议统一判断逻辑,防止未来维护时出现不一致。

2. 鼠标事件补发的状态机时序风险
onMousePressonMouseMoveonMouseRelease 中,利用了 isFirstPressButtonisFirstReleaseButton 来防重复触发,并依赖 QueuedConnection 的时序。这在大多数情况下有效,但存在极端的竞态风险:

  • XRecord 回调本身是异步的,如果底层事件到来极快(如高DPI鼠标微操),sendSimulatedMouseEvent 发出的事件在 Qt 事件队列中的排队顺序可能与底层事件顺序不一致。
  • 建议:在 sendSimulatedMouseEvent 中增加对 QApplication::sendEvent 返回值的检查,或者考虑在截图模式下,针对 XRecord 的补发事件增加一个微小的防抖/去重机制(目前 onMouseMove 中基于 m_currentCursor 的去重做得很好,建议将这个去重逻辑也应用到 onMouseDrag 中)。

3. onMousePress 中未更新内部光标状态
onMouseMove 中,你通过 m_currentCursor != pos 进行了去重。但在 onMousePress 补发事件时,没有更新 m_currentCursor。如果后续紧接着有一个相同坐标的 MouseMove 到达,可能会因为去重逻辑被意外吞掉。

  • 建议:在补发 PressRelease 事件时,同步更新 m_currentCursor = pos;

二、 代码质量

1. sendSimulatedMouseEvent 的封装与参数传递
原代码中 new QMouseEvent 后手动 delete,你改用 QScopedPointer 避免内存泄漏,这非常棒。但 QMouseEvent 的构造函数在 Qt5 和 Qt6 中发生了变化(Qt6 引入了 QPointF 和全局坐标分离)。当前代码在 Qt5/Qt6 兼容性上可能存在隐患。

  • 建议:为了更好的兼容性和健壮性,建议补全全局坐标(通常与局部坐标一致),并明确传入时间戳:
    void MainWindow::sendSimulatedMouseEvent(QEvent::Type type, const QPoint &pos,
                                             Qt::MouseButton button, Qt::MouseButtons buttons)
    {
        // Qt6 推荐使用 QMouseEvent 构造函数提供 globalPosition, localPos 等
        QScopedPointer<QMouseEvent> event(new QMouseEvent(type, pos, pos, button, buttons, Qt::NoModifier));
        event->setTimestamp(QDateTime::currentMSecsSinceEpoch()); // 增加时间戳,防止 Qt 忽略陈旧事件
        QApplication::sendEvent(this, event.data());
    }

2. 魔法条件提取为语义化变量
onMouseDragonMousePress 等函数中,出现了 status::shot == m_functionType && isFirstPressButton && !isFirstReleaseButton 这样的长条件判断。

  • 建议:将其提取为语义更清晰的布尔变量,提升可读性:
    // 例如在 onMouseDrag 中:
    bool isInShotDragging = (status::shot == m_functionType) && isFirstPressButton && !isFirstReleaseButton;
    if (isInShotDragging) { ... }

3. 注释质量
注释写得很详尽,特别是解释了“为什么这样做”(如 Dock Grab 导致事件路由到 Dock 而非截图工具),这对后续维护非常友好。保持这个好习惯。


三、 代码性能

1. refetchWindowInfo() 的调用频率
目前 refetchWindowInfo()topWindowsavePathnoNotify 等多个入口函数中都被调用,且内部调用了 Utils::getAllWindowInfo。X11 窗口列表获取通常需要遍历 X11 树,属于相对昂贵的同步 IO 操作。

  • 建议:虽然目前是在 showFullScreen() 之后单次调用,性能影响可控,但如果未来有频繁重置窗口的需求,建议增加脏数据标记(如 m_isWindowInfoDirty),仅在确实有弹窗消失时才触发重新获取。

2. XRecord 回调中的对象构造
onMouseDragonMouseMove 等函数在 X11 底层回调中触发,属于高频调用路径。每次调用都通过 sendSimulatedMouseEvent 在堆上 new QMouseEvent(即使有 QScopedPointer 保护,仍有堆分配开销)。

  • 建议:对于高频触发的 MouseMoveDrag,可以考虑使用栈上的局部变量(但要注意 sendEvent 内部不能 delete 栈对象,Qt 文档说明 sendEvent 不会 delete 事件,所以栈对象是安全的):
    // 高频事件优化示例(如果确认 Qt 版本 sendEvent 不做 delete 操作)
    QMouseEvent event(QEvent::MouseMove, pos, pos, Qt::NoButton, Qt::LeftButton, Qt::NoModifier);
    QApplication::sendEvent(this, &event);
    这样可以彻底消除堆分配的开销。

四、 代码安全

1. X11 环境变量与权限检查
代码依赖 X11 的 XRecord 扩展。在某些严格的安全沙箱(如 Flatpak/Snap)或无权限的 SSH 转发环境中,XRecord 可能无法初始化或静默失败。

  • 建议:在 refetchWindowInfo 和鼠标事件补发逻辑中,确保 Utils::getAllWindowInfo 和 XRecord 相关 API 有做 nullptr 或返回值异常处理,防止在异常环境下引发 Crash。

2. 模拟事件的无限循环风险
sendSimulatedMouseEvent 将事件发送给 this。必须确保 this 的事件过滤器不会再次将这些模拟事件识别为 X11 底层事件并重新触发 XRecord 回调,从而形成死循环。

  • 分析:从代码逻辑看,XRecord 捕获的是 X Server 的真实物理设备事件,而 QApplication::sendEvent 是 Qt 层的软件注入,通常不会再次触发 XRecord 回调,所以这里是安全的。但建议在代码中加一行断言或注释,明确说明此安全前提。

综合改进代码示例

结合上述建议,以下是优化后的核心代码片段:

void MainWindow::refetchWindowInfo()
{
    qCDebug(dsrApp) << "refetchWindowInfo() called.";
    // 统一判断逻辑,与 initAttributes 对齐
    if (Utils::isWaylandMode || Utils::isTreelandMode) {
        return;
    }

    windowRects.clear();
    windowNames.clear();
    Utils::getAllWindowInfo(static_cast<quint32>(this->winId()), m_screenWidth, m_screenHeight, windowRects, windowNames);
    qCDebug(dsrApp) << "refetchWindowInfo: got" << windowRects.size() << "windows";

    if (!isFirstMove) {
        qCDebug(dsrApp) << "发送鼠标事件!";
        sendSimulatedMouseEvent(QEvent::MouseMove, this->cursor().pos(), Qt::NoButton, Qt::NoButton);
    }
}

void MainWindow::sendSimulatedMouseEvent(QEvent::Type type, const QPoint &pos,
                                         Qt::MouseButton button, Qt::MouseButtons buttons)
{
    // 使用栈对象避免堆分配,提升高频事件性能(sendEvent 不会 delete 事件对象)
    // 同时补全 localPos 和 globalPos,兼容 Qt5/Qt6
    QMouseEvent event(type, pos, pos, button, buttons, Qt::NoModifier);
    event.setTimestamp(QDateTime::currentMSecsSinceEpoch());
    QApplication::sendEvent(this, &event);
    
    // 同步更新光标位置,防止后续 Move 去重逻辑误判
    m_currentCursor = pos; 
}

// onMouseDrag 优化示例
void MainWindow::onMouseDrag(int x, int y)
{
    if (status::record == m_functionType) {
        showDragFeedback(x, y);
    }

    bool isInShotDragging = (status::shot == m_functionType) && isFirstPressButton && !isFirstReleaseButton;
    if (isInShotDragging) {
        QPoint pos(x, y);
        // 统一应用去重逻辑
        if (m_currentCursor != pos) {
            sendSimulatedMouseEvent(QEvent::MouseMove, pos, Qt::NoButton, Qt::LeftButton);
        }
    }
}

总结:这是一份高质量的 Bugfix PR,对 X11 事件机制和 Qt 事件循环的交互理解很深。主要建议集中在防重复触发的健壮性增强高频事件的堆分配优化以及状态变量的同步更新上。采纳上述建议后,代码将更加健壮和高效。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

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

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/forcemerge

@deepin-bot

deepin-bot Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit 8f43444 into linuxdeepin:master May 26, 2026
9 of 10 checks passed
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