Skip to content

Conversation

@deepin-ci-robot
Copy link
Contributor

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#36

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.

Sorry @deepin-ci-robot, your pull request is larger than the review limit of 150000 diff characters

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#36
@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

我对这段代码进行了审查,以下是我的改进意见:

  1. 代码逻辑方面:
  • m_resetGeometryTimer 的 timeout 连接中,添加了几何比较逻辑是个好改进,避免了不必要的重绘
  • 但是 shutdownkModeChanged 信号连接了两次,这可能会导致重复操作,建议合并为一个连接
  1. 代码质量方面:
  • 变量命名可以更加清晰,如 m_geometryRect 可以改为 m_targetGeometry 更能表达其用途
  • 在设置 m_shutdownBlackWidget 的大小时,直接使用 this->size() 可能不够灵活,建议考虑使用布局管理
  • 日志输出格式不统一,建议使用统一的日志格式
  1. 代码性能方面:
  • m_resetGeometryTimer 的超时时间从 2005 增加到 4005,这可能会影响界面响应的及时性,建议根据实际需求调整
  • setddeGeometry 函数中,使用 QTimer::singleShot 来停止定时器,可以考虑使用更直接的方式
  1. 代码安全方面:
  • 在创建 m_shutdownBlackWidget 时,没有进行空指针检查,虽然代码中已经检查了 !m_shutdownBlackWidget,但建议增加异常处理机制
  • shutdownkModeChanged 信号处理中,直接使用 setFixedSize 可能会导致在高DPI屏幕上显示问题,建议考虑使用 devicePixelRatio

改进建议:

// 合并重复的信号连接
connect(m_model, &SessionBaseModel::shutdownkModeChanged, this, [this](bool value) {
    if (!m_shutdownBlackWidget) {
        try {
            m_shutdownBlackWidget = new ShutdownBlackWidget(this);
        } catch (const std::exception& e) {
            qCCritical(DDE_SHELL) << "Failed to create ShutdownBlackWidget:" << e.what();
            return;
        }
    }
    
    qCInfo(DDE_SHELL) << "FullScreenBackground size:" << size();
    const QSizeF sizeF(size() * devicePixelRatioF());
    m_shutdownBlackWidget->setFixedSize(sizeF.toSize());
    m_shutdownBlackWidget->setBlackMode(value);
});

// 改进 setddeGeometry 函数
void FullScreenBackground::setddeGeometry(const QRect &rect)
{
    setGeometry(rect);
    m_geometryRect = rect;
    m_resetGeometryTimer->start(200);
    // 使用更直接的方式停止定时器
    QMetaObject::invokeMethod(m_resetGeometryTimer, "stop", Qt::QueuedConnection);
}

这些改进可以提高代码的可维护性、性能和安全性。特别是在处理UI相关的操作时,考虑高DPI支持和异常处理是非常重要的。

@deepin-ci-robot
Copy link
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot, 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

@yixinshark yixinshark merged commit ba34f47 into master Sep 12, 2025
25 of 28 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