Skip to content

Bug fix 5 21#844

Open
dengzhongyuan365-dev wants to merge 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-5-21
Open

Bug fix 5 21#844
dengzhongyuan365-dev wants to merge 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-5-21

Conversation

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor

fix(pin_screenshot): adapt toolbar and window management for Treeland compositor

- Add isTreelandMode flag with case-insensitive DDE_CURRENT_COMPOSITOR check
- Use QSurfaceFormat alpha buffer for Treeland before QApplication creation
- Move DGuiApplicationHelper palette setup after QApplication to fix Treeland crash
- Make toolbar a subsurface of the main pin window via QWindow::setParent
- Replace DBlurEffectWidget blur with custom QPainter rounded-rect background
  (DBlur is non-functional under Treeland subsurface)
- Use startSystemMove() / windowHandle()->setPosition() for drag & key move
- Fix geometry() vs mapFromGlobal() coordinate mismatch for toolbar hit-test

TASK: https://pms.uniontech.com/task-view-389563.html

@dengzhongyuan365-dev dengzhongyuan365-dev force-pushed the bug-fix-5-21 branch 3 times, most recently from b00265f to bb39392 Compare May 22, 2026 07:16
… compositor

- Add isTreelandMode flag with case-insensitive DDE_CURRENT_COMPOSITOR check
- Use QSurfaceFormat alpha buffer for Treeland before QApplication creation
- Move DGuiApplicationHelper palette setup after QApplication to fix Treeland crash
- Make toolbar a subsurface of the main pin window via QWindow::setParent
- Replace DBlurEffectWidget blur with custom QPainter rounded-rect background
  (DBlur is non-functional under Treeland subsurface)
- Use startSystemMove() / windowHandle()->setPosition() for drag & key move
- Fix geometry() vs mapFromGlobal() coordinate mismatch for toolbar hit-test

新增 isTreelandMode 标志,通过 DDE_CURRENT_COMPOSITOR 环境变量判断合成器类型。
Treeland 模式下:设置 QSurfaceFormat alpha 缓冲、调整 DGuiApplicationHelper
调色板初始化顺序避免崩溃、将工具栏设为主窗口子表面替代独立窗口、用 QPainter
自定义圆角背景替代 DBlurEffectWidget、使用 startSystemMove() 实现拖拽移动、
修复工具栏命中测试的坐标计算偏差。

Log: 修复截图钉图功能在Treeland合成器下工具栏显示异常和窗口管理问题

PMS: TASK-389563

Influence: 修复后截图钉图功能可在Treeland合成器下正常使用,工具栏正确显示并支持拖拽移动
@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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份 Git Diff 主要针对 deepin-screen-recorder 项目进行了 TreeLand 合成器的适配,涵盖了贴图功能的 D-Bus 异步调用优化、截图区域裁剪逻辑修正、Wayland/TreeLand 窗口标志位与父子层级管理、工具栏自绘替代模糊效果,以及全局环境变量判断的优化。

整体来看,代码结构清晰,对 Wayland 和 TreeLand 的差异化处理比较细致。但经过仔细审查,在语法逻辑、代码质量和安全性方面仍有以下改进意见:


一、 语法与逻辑

1. QWindow::setPosition 在 Wayland/TreeLand 下无效且已被废弃

文件: src/pin_screenshots/mainwindow.cpp:625

const auto applyWindowMove = [this](int nx, int ny) {
    if (PUtils::isTreelandMode && windowHandle()) {
        windowHandle()->setPosition(nx, ny); // 逻辑问题
    } else {
        move(nx, ny);
    }
};

问题: 在 Wayland 协议及 TreeLand 中,客户端无权主动设置窗口的绝对位置(由合成器管理)。QWindow::setPosition() 在 Wayland 上通常会被忽略,且在 Qt6 中已被标记为 deprecated。
改进建议: 对于 TreeLand 下的键盘微调移动,应该同样使用 QWindow::startSystemMove() 结合模拟输入,或者如果 TreeLand 扩展了相关的私有协议,应调用对应协议。若当前仅希望键盘移动生效,建议保留 move() 并依赖 Qt 的 Wayland 集成做本地偏移,或者直接在 TreeLand 下禁用键盘微调移动功能,避免产生不符合预期的行为。

2. showEvent 中可能引发死循环或重复创建

文件: src/pin_screenshots/mainwindow.cpp:989

void MainWindow::showEvent(QShowEvent *event)
{
    DWidget::showEvent(event);
    // ...
    m_toolBar->windowHandle()->setParent(windowHandle());
    m_toolBar->hide();
    m_toolBar->show(); // 逻辑风险
    // ...
}

问题: showEvent 在窗口每次显示时都会触发(例如窗口被遮挡后再次激活、切换虚拟桌面等)。在其中调用 hide()show() 可能会导致事件循环或触发额外的 showEvent。此外,如果 toolbarAttachedToWindow() 已经为 true,虽然前面有 return,但 setParent 操作本身也可能引发合成器的重新映射。
改进建议: 增加 static bool 或成员变量标识,确保 setParent 逻辑只执行一次;或者在 initMainWindow 阶段提前调用 createWinId() 完成父子绑定,而不是放在 showEvent 中。

// 推荐增加防重入判断
if (m_isToolbarParentSet) return;
// ... setParent logic ...
m_isToolbarParentSet = true;

3. Copyright 年份逻辑错误

文件: 多个文件头部

-// Copyright (C) 2020 ~ 2021 Uniontech Software Technology Co.,Ltd.
+// Copyright (C) 2020 ~ 2026 Uniontech Software Technology Co.,Ltd.

问题: 版权声明中的结束年份通常指代码首次发布的年份或最后修改的年份,写为 2026(未来年份)在法律和规范上是不严谨的,除非是特定协议要求,否则应使用当前年份或修改年份。


二、 代码质量

1. 大量硬编码的魔术数字

文件: src/pin_screenshots/ui/toolbarwidget.cpp:170 & src/main_window.cpp:3829

// toolbarwidget.cpp
const qreal radius = 16.0;
const QColor backgroundColor = ... ? QColor(237, 237, 237, 255) : QColor(48, 48, 48, 255);

// main_window.cpp
QTimer::singleShot(2, [=] { exitApp(); }); // 旧代码虽已修改,但 D-Bus 回调前的逻辑也需注意

问题: 颜色值、圆角半径等直接硬编码,不利于主题切换和后期维护。
改进建议: 将颜色和圆角提取为常量或从 DTK 主题中读取(如 DGuiApplicationHelper::instance()->applicationPalette()),保持与系统主题的一致性。

2. 日志级别使用不当

文件: src/main_window.cpp:3829

if (watcher->isError()) {
    qCWarning(dsrApp) << "... failed ...";
} else {
    qCWarning(dsrApp) << "... succeeded"; // 质量问题
}

问题: 成功的执行信息使用了 qCWarning(警告级别),这会导致在正常使用时产生大量无谓的警告日志,干扰真正问题的排查。
改进建议: 成功信息应降级为 qCInfoqCDebug

qCInfo(dsrApp) << "[PIN_DIAG] D-Bus openImageAndName succeeded";

3. 工具栏位置计算的坐标系混乱

文件: src/pin_screenshots/mainwindow.cpp:920

QPoint toolbarPos(x, y);
if (toolbarAttachedToWindow()) {
    toolbarPos = mapFromGlobal(toolbarPos);
}

问题: xy 是基于屏幕全局坐标计算出来的,当工具栏作为 Subsurface 时,其坐标应相对于父窗口。mapFromGlobal 虽然能转换坐标,但前提是父窗口的几何信息准确。在 Wayland 下,全局坐标常常是不可靠的。
改进建议: 在 TreeLand 模式下,应直接基于主窗口的尺寸和工具栏的尺寸计算相对偏移量,而不必经过全局坐标的转换,这样逻辑更清晰且避免 Wayland 坐标偏移问题。


三、 代码性能

1. QImage 的隐式共享与不必要的深拷贝风险

文件: src/main_window.cpp:3822

QImage pinImage = m_resultPixmap.toImage();
QDBusPendingCall pendingCall = m_pinInterface->openImageAndName(pinImage, ...);

问题: QImage 通过值传递给 D-Bus 接口。Qt D-Bus 在序列化 QImage 时,可能会进行数据的内存拷贝。如果 m_resultPixmap 很大(如 4K 截图),这里可能发生性能损耗。
改进建议: 虽然 Qt 的隐式共享机制(COW)会尽量避免不必要的拷贝,但在跨线程的 QDBusPendingCallWatcher 中使用时,最好确保数据不被意外修改。当前写法可以接受,但如果发现 D-Bus 传输卡顿,可考虑传递文件路径而非 QImage 本体(看接口定义是否允许)。

2. 事件过滤器和鼠标移动中的频繁坐标映射

文件: src/pin_screenshots/mainwindow.cpp:714

if (m_toolBar->isActiveWindow() || m_toolBar->rect().contains(m_toolBar->mapFromGlobal(globalPos)))

问题: mapFromGlobal 在高频事件(如 mouseMoveEventeventFilter)中调用,在 Wayland 环境下可能引发一定的计算开销。
改进建议: 逻辑上没有大问题,但可以缓存工具栏的可见性状态或区域,避免在鼠标每移动一个像素时都进行映射和碰撞检测。


四、 代码安全

1. 环境变量伪造导致的安全与功能降级风险

文件: src/pin_screenshots/main.cpp:34

const QString compositor = e.value(QStringLiteral("DDE_CURRENT_COMPOSITOR"));
if (compositor.compare(QStringLiteral("treeland"), Qt::CaseInsensitive) == 0) {
    PUtils::isTreelandMode = true;
}

问题: 环境变量很容易被用户或第三方脚本篡改。如果用户在 X11 环境下故意设置 DDE_CURRENT_COMPOSITOR=treeland,代码会走入 TreeLand 分支(如禁用模糊效果、改变窗口 Flags 等),导致在 X11 下界面显示异常甚至功能失效。
改进建议: 在判断 TreeLand 的同时,必须结合底层的显示服务器协议进行交叉验证。例如,确认当前确实是 Wayland 环境(通过 QGuiApplication::platformName() == "wayland" 或类似机制)。

const QString compositor = e.value(QStringLiteral("DDE_CURRENT_COMPOSITOR"));
const bool isWaylandPlatform = (qgetenv("XDG_SESSION_TYPE") == "wayland");
if (compositor.compare(QStringLiteral("treeland"), Qt::CaseInsensitive) == 0 && isWaylandPlatform) {
    PUtils::isTreelandMode = true;
}

2. D-Bus 调用失败时的程序退出逻辑

文件: src/main_window.cpp:3834

connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, watcher]() {
    if (watcher->isError()) {
        qCWarning(dsrApp) << "... failed ...";
    } else { ... }
    watcher->deleteLater();
    exitApp(); // 无论成功失败都会退出
});

问题: 如果 D-Bus 调用失败(例如贴图服务未启动、崩溃),用户点击贴图后,程序依然会默默调用 exitApp() 退出,导致截图丢失且无任何反馈。
改进建议: 在 isError() 分支中,应该给用户弹窗提示(如 "贴图服务不可用"),并阻止 exitApp() 的执行,让用户有机会保存截图或重试。


总结

此 PR 的核心逻辑(TreeLand 适配)是合理的,特别是将 QTimer::singleShot 改为 QDBusPendingCallWatcher 的异步处理,提升了稳定性。但在窗口移动的 Wayland 协议合规性、showEvent 的生命周期管理、以及环境变量校验的安全性上需要进行微调。建议重点修复 D-Bus 失败时的退出逻辑(安全/体验)setPosition 的废弃调用(逻辑/兼容性)

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