Skip to content

fix: xembed tray popup window position correction for stashed container#1608

Merged
BLumia merged 2 commits into
linuxdeepin:masterfrom
BLumia:xembed-treeland-stash
May 28, 2026
Merged

fix: xembed tray popup window position correction for stashed container#1608
BLumia merged 2 commits into
linuxdeepin:masterfrom
BLumia:xembed-treeland-stash

Conversation

@BLumia

@BLumia BLumia commented May 26, 2026

Copy link
Copy Markdown
Member

之前托盘位置总是相对于任务栏的 wl_surface 的,会导致向上收起区域的偏移
位置不正确。此提交增加了托盘位于哪个区域的区分,并根据对应的位置决定
所基于的 wl_surface 是哪个。

PMS: BUG-360349

@BLumia BLumia requested review from 18202781743 and tsic404 May 26, 2026 12:10
@deepin-ci-robot

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @BLumia, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@BLumia BLumia marked this pull request as ready for review May 26, 2026 12:11
@BLumia BLumia force-pushed the xembed-treeland-stash branch 3 times, most recently from 7f02618 to 22b3fc9 Compare May 27, 2026 08:04
之前托盘位置总是相对于任务栏的 wl_surface 的,会导致向上收起区域的偏移
位置不正确。此提交增加了托盘位于哪个区域的区分,并根据对应的位置决定
所基于的 wl_surface 是哪个。

PMS: BUG-360349
Log:
@BLumia BLumia force-pushed the xembed-treeland-stash branch from 22b3fc9 to 1083e55 Compare May 27, 2026 09:34
tsic404
tsic404 previously approved these changes May 28, 2026
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia

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

1 similar comment
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia

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-ci-robot

Copy link
Copy Markdown

deepin pr auto review

这份代码变更主要实现了将 XEmbed 窗口移动的参考坐标系从固定的 Dock 面板窗口改为动态的“锚点窗口”,并引入了基于 wl_callback 的 Wayland 异步移动结果回调机制。

整体设计思路清晰,但在线程安全、内存管理、类型转换和代码健壮性方面存在一些需要重点关注的问题。以下是详细的审查意见:

1. 严重问题:线程安全与对象生命周期

文件: waylanddockhelper.cpp

struct CallbackData { uint32_t wid; WaylandDockHelper *helper; };
static struct wl_callback_listener s_callbackListener = {
    .done = [](void *data, wl_callback *callback, uint32_t callback_data) {
        auto *d = static_cast<CallbackData *>(data);
        bool success = (callback_data == 0);
        Q_EMIT d->helper->xembedWindowMoveResult(d->wid, success);
        wl_callback_destroy(callback);
        delete d;
    }
};
wl_callback_add_listener(cb, &s_callbackListener, new CallbackData{wid, this});
  • 问题: Wayland 的事件分发通常在独立的事件循环(C 库层面)中触发。如果在 wl_callback 触发前,WaylandDockHelper 对象已经被销毁,d->helper 将成为悬空指针,导致程序崩溃。同时,在非 Qt 事件循环中直接 Q_EMIT 信号,如果该信号跨线程连接,可能会引发 Qt 的线程断言错误。
  • 改进建议:
    1. 使用 QPointer<WaylandDockHelper> 来守护 helper 指针,在回调中检查其是否为空。
    2. 确保 Q_EMIT 在 Qt 的主事件循环中执行,可以使用 QMetaObject::invokeMethod 结合 Lambda 来安全地发射信号。

修改后代码示例:

struct CallbackData { 
    uint32_t wid; 
    QPointer<WaylandDockHelper> helper; // 使用 QPointer 防止悬空指针
};
static struct wl_callback_listener s_callbackListener = {
    .done = [](void *data, wl_callback *callback, uint32_t callback_data) {
        auto *d = static_cast<CallbackData *>(data);
        bool success = (callback_data == 0);
        
        // 安全地将信号发射转移到 Qt 主线程
        if (d->helper) {
            QMetaObject::invokeMethod(d->helper, [helper = d->helper, wid = d->wid, success]() {
                Q_EMIT helper->xembedWindowMoveResult(wid, success);
            }, Qt::QueuedConnection);
        }
        
        wl_callback_destroy(callback);
        delete d;
    }
};
wl_callback_add_listener(cb, &s_callbackListener, new CallbackData{wid, QPointer<WaylandDockHelper>(this)});

2. 严重问题:错误的类型转换

文件: waylanddockhelper.cpp

auto waylandWindow = dynamic_cast<QtWaylandClient::QWaylandWindow*>(anchorWindow->handle());
  • 问题: QQuickWindow 是用于 QML 场景渲染的窗口类(属于服务端/合成器侧),而 QtWaylandClient::QWaylandWindow 是用于 Wayland 客户端的底层窗口类。在 Dock 这种作为 Wayland 合成器的进程中,anchorWindow->handle() 返回的通常是 QWaylandQuickWindowQQuickRenderControl 相关的句柄,绝对不可能是 QtWaylandClient::QWaylandWindow。这个 dynamic_cast 必定返回 nullptr,导致锚点窗口逻辑失效,永远回退到 m_dockWlSurface
  • 改进建议: 作为 Wayland 合成器,要获取内部 QQuickWindow 对应的 wl_surface,需要使用 QtWayland Compositor 的 API。通常的做法是通过 QWaylandSurfaceQWaylandQuickItem 来获取原生的 wl_surface

修改思路示例:

// 假设 anchorWindow 对应的 backend 是 QWaylandQuickSurface
#include <QtWaylandCompositor/QWaylandQuickSurface>
// ...
if (anchorWindow) {
    // 正确的合成器侧转换方式,具体取决于你的 Qt 版本和窗口创建方式
    auto waylandSurface = dynamic_cast<QtWaylandCompositor::QWaylandQuickSurface*>(anchorWindow->handle());
    if (waylandSurface && waylandSurface->waylandSurface()) {
        anchorSurface = waylandSurface->waylandSurface()->object();
    } else {
        qWarning() << "Failed to cast anchorWindow to QWaylandQuickSurface";
    }
}

3. 代码质量:QML 中的匿名函数连接与内存泄漏风险

文件: panels/dock/package/main.qml

Panel.xembedWindowMoveResult.connect(function(wid, success) {
    DockCompositor.notifyXEmbedWindowMoveResult(wid, success)
})
  • 问题: 在 QML 中使用 connect(function) 会在每次组件初始化时创建一个新的连接,且无法通过 disconnect() 解除。如果该组件由于 Dock 面板重建等原因被多次创建/销毁,会导致信号槽堆积,同一个结果被通知多次,甚至引用已销毁的上下文导致崩溃。
  • 改进建议: 使用具名函数或者在组件销毁时断开连接。推荐使用具名函数:

修改后代码示例:

// 在 Window 或 Item 中定义函数
function handleXEmbedWindowMoveResult(wid, success) {
    DockCompositor.notifyXEmbedWindowMoveResult(wid, success)
}

// 连接时传入函数引用
Panel.xembedWindowMoveResult.connect(handleXEmbedWindowMoveResult)

// 在 Component.onDestruction 中断开
Component.onDestruction: {
    Panel.xembedWindowMoveResult.disconnect(handleXEmbedWindowMoveResult)
}

4. 代码健壮性:PluginSurface 中 AnchorWindow 的更新与同步

文件: pluginmanagerextension.cppActionLegacyTrayPluginDelegate.qml

pluginItem.plugin.setAnchorWindow(pluginItem.Window.window)
pluginItem.plugin.updatePluginGeometry(...)
void PluginSurface::setAnchorWindow(QQuickWindow *window) {
    m_anchorWindow = window;
}
  • 问题:
    1. m_anchorWindow 使用了 QPointer,当窗口被销毁时会自动置空,这是好习惯。但是 setAnchorWindow 没有做任何判空或防重入处理,也没有发出信号通知变更。
    2. 在 QML 中,是在 updatePluginGeometry 之前调用 setAnchorWindow,但如果 anchorWindow 在移动请求期间发生改变或被销毁,可能会导致 C++ 层持有了错误的窗口指针。
  • 改进建议: 在 C++ 层增加判空保护和调试日志,确保状态一致性。
void PluginSurface::setAnchorWindow(QQuickWindow *window)
{
    if (m_anchorWindow == window)
        return;
        
    if (!window) {
        qWarning() << "PluginSurface::setAnchorWindow: setting null anchor window for" << m_pluginId;
    }
    
    m_anchorWindow = window;
}

5. 代码性能:QML 中不必要的日志输出

文件: DockCompositor.qml

onMoveXEmbedWindowRequested: (wid, pluginId, itemKey, dx, dy, anchorWindow) => {
    console.log("move xembed window requested:", wid, pluginId, itemKey, dx, dy, "anchorWindow:", anchorWindow)
  • 问题: 拖动窗口或插件时,该信号可能会以极高的频率(如 60fps/120fps)触发。console.log 在 QML 中性能开销极大,高频触发会导致严重的性能下降和日志刷屏。
  • 改进建议: 移除该 console.log,或者改为只在调试模式下输出(例如通过一个全局的 verboseDebug 属性控制)。

6. 代码安全与逻辑:异步回调与并发请求

文件: pluginmanagerextension.cpp & waylanddockhelper.cpp

// pluginmanagerextension.cpp
m_pendingXEmbedCallbacks[xembed_winid] = {callback, resource->handle};
  • 问题: 代码中使用了 m_pendingXEmbedCallbacks 来支持并发请求。但在 WaylandDockHelper 中,wl_callback 的回调只携带了 wid。如果同一个 wid 在极短时间内发起了多次移动请求(例如快速拖动),m_pendingXEmbedCallbacks 中可能存在多条记录,当 xembedWindowMoveResult 信号触发时,notifyXEmbedWindowMoveResult 如何确保清理正确的 callback?
  • 改进建议: 确认 notifyXEmbedWindowMoveResult 的底层实现,如果它仅仅根据 widtake 第一个 callback,那么多条并发请求可能会导致部分 Wayland client 的 callback 永远不被触发(内存泄漏和客户端挂起)。建议在 WaylandDockHelper 的回调中,如果支持并发,可能需要传递一个唯一的 Request ID 而不仅仅是 wid;如果不支持并发,应在发起第二次请求时主动取消/触发上一次的 pending callback。

总结

最关键且必须修复的是 第2点(类型转换错误),这会导致整个 anchorWindow 功能完全失效。其次是 第1点(线程与生命周期),这在复杂场景下会引发崩溃。其他点属于代码质量和性能优化,建议一并修复以提升代码的健壮性。

@BLumia BLumia merged commit 4d8c9f8 into linuxdeepin:master May 28, 2026
10 of 12 checks passed
@BLumia BLumia deleted the xembed-treeland-stash branch May 28, 2026 03:37
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