Skip to content

Conversation

@deepin-ci-robot
Copy link
Contributor

Synchronize source files from linuxdeepin/qt5platform-plugins.

Source-pull-request: linuxdeepin/qt5platform-plugins#311

@deepin-ci-robot
Copy link
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot

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
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot

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 deepin-ci-robot force-pushed the sync-pr-311-nosync branch 2 times, most recently from ad0533a to 16c33f8 Compare December 3, 2025 03:52
Synchronize source files from linuxdeepin/qt5platform-plugins.

Source-pull-request: linuxdeepin/qt5platform-plugins#311
@mhduiy mhduiy merged commit 2feee5c into master Dec 4, 2025
28 of 31 checks passed
@mhduiy mhduiy deleted the sync-pr-311-nosync branch December 4, 2025 11:33
@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

我来对这个代码变更进行审查:

  1. 语法逻辑:
  • 代码语法正确,没有明显的语法错误。
  • 新增的代码逻辑清晰,通过清理和重建光标上下文来处理光标主题变化。
  1. 代码质量:
  • 优点:
    • 添加了空指针检查,提高了代码健壮性
    • 使用了清晰的注释说明了代码的用途
    • 代码结构合理,易于理解
  • 可改进点:
    • 代码中有重复的检查逻辑(screen && screen->handle() && screen->handle()->cursor()),可以提取为一个单独的函数
    • 新增的代码没有使用Qt的信号槽机制,而是直接操作底层对象,可能破坏了Qt的事件循环模型
  1. 代码性能:
  • 性能方面存在一些问题:
    • 每次主题变化都会遍历所有屏幕和窗口,可能会造成不必要的性能开销
    • 对每个屏幕都重建光标上下文,即使某些屏幕的光标可能没有变化
    • 建议添加一个标志位来跟踪实际需要更新的屏幕,避免不必要的重建
  1. 代码安全:
  • 安全性考虑:
    • 代码中有适当的空指针检查,避免了潜在的空指针访问
    • 使用了Qt的类型转换(static_cast),相对安全
    • 建议在操作xcb_cursor_context时添加更多的错误检查

改进建议:

  1. 提取公共函数:
static bool isValidScreenCursor(QScreen* screen) {
    return screen && screen->handle() && screen->handle()->cursor();
}
  1. 优化性能:
static void updateCursorContext(QScreen* screen) {
    if (!isValidScreenCursor(screen)) return;
    
    QXcbCursor* xcb_cursor = static_cast<QXcbCursor*>(screen->handle()->cursor());
    if (!xcb_cursor) return;

    xcb_cursor->m_cursorHash.clear();
    
    if (xcb_cursor->m_cursorContext) {
        xcb_cursor_context_free(xcb_cursor->m_cursorContext);
    }
    xcb_cursor->m_cursorContext = nullptr;
    
    xcb_connection_t* conn = xcb_cursor->xcb_connection();
    if (xcb_cursor_context_new(conn, xcb_cursor->m_screen->screen(), &xcb_cursor->m_cursorContext) < 0) {
        qWarning() << "Failed to create cursor context for screen" << screen;
        xcb_cursor->m_cursorContext = nullptr;
    }
}
  1. 使用Qt的信号槽机制:
// 在类中添加信号
signals:
    void cursorThemeChanged();

// 在initialize中连接
connect(this, &DPlatformIntegration::cursorThemeChanged, 
        this, &DPlatformIntegration::handleCursorThemeChange, Qt::QueuedConnection);

// 修改回调函数
static void cursorThemePropertyChanged(xcb_connection_t* connection, const QByteArray& property, const QVariant& value, void* handle) {
    Q_UNUSED(connection);
    Q_UNUSED(property);
    Q_UNUSED(value);
    Q_UNUSED(handle);
    
    QMetaObject::invokeMethod(qApp, [](){
        emit static_cast<DPlatformIntegration*>(qApp)->cursorThemeChanged();
    }, Qt::QueuedConnection);
}
  1. 添加更多的错误处理和日志:
if (xcb_cursor_context_new(conn, xcb_cursor->m_screen->screen(), &xcb_cursor->m_cursorContext) < 0) {
    qWarning() << "Failed to create new cursor context for screen" << screen 
               << "Error:" << strerror(errno);
    xcb_cursor->m_cursorContext = nullptr;
}

这些改进可以:

  1. 提高代码的可维护性和可读性
  2. 减少重复代码
  3. 提供更好的错误处理
  4. 保持与Qt框架的一致性
  5. 提供更好的性能优化机会

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