Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support Qt 6.8 #611

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

justforlxz
Copy link
Member

support qt 6.8

Log:

support qt 6.8

Log:
@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: justforlxz

The full list of commands accepted by this bot can be found here.

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
Contributor

deepin pr auto review

代码审查意见:

  1. 代码重复:在DLabel::paintEvent函数中,#if#else块存在大量重复代码,建议提取公共逻辑到一个单独的函数中,以减少代码重复和提高可维护性。

  2. 内存管理:在DLabel::paintEvent函数中,d->cachedimaged->scaledpixmap的内存管理存在问题。在#if QT_VERSION < QT_VERSION_CHECK(6, 4, 2)分支中,d->cachedimage被重新分配,但没有释放之前的内存,可能导致内存泄漏。建议在重新分配之前释放之前的内存。

  3. 类型转换:在DSimpleListViewmouseMoveEventmouseReleaseEvent函数中,将QPointF转换为QPoint时,使用了toPoint()方法。这种方法会丢失小数部分,可能会导致鼠标位置计算不准确。建议在需要精确位置时,保留QPointF类型。

  4. 版本检查:在DLabel::paintEventDSimpleListViewmouseMoveEventmouseReleaseEvent函数中,使用了大量的版本检查宏(如QT_VERSION_CHECK)。建议将这些宏定义集中管理,以便于维护和更新。

  5. 注释和文档:代码中没有足够的注释和文档,特别是对于复杂的逻辑和版本特定的代码块。建议添加注释和文档,以便其他开发者更好地理解代码。

  6. 错误处理:在DLabel::paintEvent函数中,如果d->pixmap->toImage()失败,可能会导致d->cachedimage为空指针,进而引发未定义行为。建议添加错误处理逻辑,以防止这种情况发生。

  7. 性能优化:在DLabel::paintEvent函数中,d->cachedimage->scaled方法可能会创建一个新的QImage对象,这可能会导致性能问题。如果d->scaledpixmap已经存在且大小相同,可以考虑直接使用现有的QPixmap对象,而不是重新创建。

以上是针对代码提交的审查意见,希望能够对代码的改进有所帮助。

deepin-ci-robot added a commit to linuxdeepin/dtk6widget that referenced this pull request Oct 14, 2024
Synchronize source files from linuxdeepin/dtkwidget.

Source-pull-request: linuxdeepin/dtkwidget#611
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.

2 participants