Skip to content

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Jan 15, 2026

  1. Added explicit call to setRetainWhileLoading(true) for the internal imageItem in DQuickDciIconImagePrivate constructor
  2. This ensures that when Qt version is 6.8.0 or higher, the icon image retains its previous visual state while loading new content
  3. The change prevents visual flickering or blank states during icon loading transitions

Log: Fixed icon flickering issue during loading by enabling retainWhileLoading property

Influence:

  1. Test icon loading transitions to ensure no flickering occurs
  2. Verify that icons maintain their previous appearance while loading new content
  3. Test with various DCI icon sources and loading states
  4. Confirm compatibility with Qt 6.8.0 and higher versions

fix: 为 DQuickDciIconImage 设置 retainWhileLoading 为 true

  1. 在 DQuickDciIconImagePrivate 构造函数中为内部 imageItem 显式调用 setRetainWhileLoading(true)
  2. 确保当 Qt 版本为 6.8.0 或更高时,图标图像在加载新内容时保留其先前的视 觉状态
  3. 此更改防止图标加载过渡期间出现视觉闪烁或空白状态

Log: 通过启用 retainWhileLoading 属性修复图标加载时的闪烁问题

Influence:

  1. 测试图标加载过渡,确保不会出现闪烁
  2. 验证图标在加载新内容时是否保持其先前的外观
  3. 使用各种 DCI 图标源和加载状态进行测试
  4. 确认与 Qt 6.8.0 及更高版本的兼容性

PMS: BUG-346973

Summary by Sourcery

Bug Fixes:

  • Prevent icon flickering or blank states during loading transitions by retaining the previous icon appearance while new content is being loaded on supported Qt versions.

deepin-ci-robot added a commit to linuxdeepin/dtk6declarative that referenced this pull request Jan 15, 2026
Synchronize source files from linuxdeepin/dtkdeclarative.

Source-pull-request: linuxdeepin/dtkdeclarative#557
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 15, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR ensures that DQuickDciIconImage’s internal DQuickIconImage retains its previous visual content while loading new data on Qt 6.8.0+ by explicitly enabling the retainWhileLoading property in the private class constructor, preventing flicker/blank states during icon reloads.

Sequence diagram for icon reload with retainWhileLoading enabled

sequenceDiagram
    actor User
    participant DQuickDciIconImage as DQuickDciIconImage
    participant DQuickDciIconImagePrivate as DQuickDciIconImagePrivate
    participant DQuickIconImage as DQuickIconImage
    participant IconSource as IconSource

    User->>DQuickDciIconImage: requestIconChange(newSource)
    DQuickDciIconImage->>DQuickDciIconImagePrivate: setSource(newSource)
    DQuickDciIconImagePrivate->>DQuickIconImage: setSource(newSource)

    Note over DQuickIconImage: Qt version >= 6.8.0

    DQuickDciIconImagePrivate->>DQuickIconImage: setRetainWhileLoading(true)

    DQuickIconImage->>IconSource: load(newSource)
    activate IconSource
    DQuickIconImage-->>DQuickDciIconImage: retainWhileLoadingChanged(true)
    DQuickDciIconImage-->>User: icon visually unchanged while loading

    IconSource-->>DQuickIconImage: new image data
    deactivate IconSource

    DQuickIconImage-->>DQuickDciIconImage: cacheChanged()
    DQuickDciIconImage-->>User: display updated icon without flicker
Loading

File-Level Changes

Change Details Files
Enable retainWhileLoading on the internal image item for DQuickDciIconImage when built with Qt >= 6.8.0 to prevent icon flicker during reloads.
  • In the DQuickDciIconImagePrivate constructor, after wiring up cacheChanged and retainWhileLoadingChanged signal connections, call setRetainWhileLoading(true) on the internal imageItem when QT_VERSION is at least 6.8.0.
  • Keep the behavior gated under the existing Qt version check so older Qt versions remain unaffected.
src/private/dquickdciiconimage.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@mhduiy mhduiy force-pushed the retainWhileLoading branch from b2d4bd2 to c933302 Compare January 15, 2026 08:22
deepin-ci-robot added a commit to linuxdeepin/dtk6declarative that referenced this pull request Jan 15, 2026
Synchronize source files from linuxdeepin/dtkdeclarative.

Source-pull-request: linuxdeepin/dtkdeclarative#557
1. Added explicit call to setRetainWhileLoading(true) for the internal
imageItem in DQuickDciIconImagePrivate constructor
2. This ensures that when Qt version is 6.8.0 or higher, the icon image
retains its previous visual state while loading new content
3. The change prevents visual flickering or blank states during icon
loading transitions

Log: Fixed icon flickering issue during loading by enabling
retainWhileLoading property

Influence:
1. Test icon loading transitions to ensure no flickering occurs
2. Verify that icons maintain their previous appearance while loading
new content
3. Test with various DCI icon sources and loading states
4. Confirm compatibility with Qt 6.8.0 and higher versions

fix: 为 DQuickDciIconImage 设置 retainWhileLoading 为 true

1. 在 DQuickDciIconImagePrivate 构造函数中为内部 imageItem 显式调用
setRetainWhileLoading(true)
2. 确保当 Qt 版本为 6.8.0 或更高时,图标图像在加载新内容时保留其先前的视
觉状态
3. 此更改防止图标加载过渡期间出现视觉闪烁或空白状态

Log: 通过启用 retainWhileLoading 属性修复图标加载时的闪烁问题

Influence:
1. 测试图标加载过渡,确保不会出现闪烁
2. 验证图标在加载新内容时是否保持其先前的外观
3. 使用各种 DCI 图标源和加载状态进行测试
4. 确认与 Qt 6.8.0 及更高版本的兼容性

PMS: BUG-346973
@mhduiy mhduiy force-pushed the retainWhileLoading branch from c933302 to b64ba7a Compare January 15, 2026 11:04
deepin-ci-robot added a commit to linuxdeepin/dtk6declarative that referenced this pull request Jan 15, 2026
Synchronize source files from linuxdeepin/dtkdeclarative.

Source-pull-request: linuxdeepin/dtkdeclarative#557
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这段代码修改主要涉及构造函数的逻辑调整和 Qt 6.8 新特性的适配。以下是对该 git diff 的详细审查意见,涵盖语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 严重错误:构造函数递归调用

    • 问题代码
      DQuickIconImage::DQuickIconImage(QQuickItem *parent)
          : DQuickIconImage(*(new DQuickIconImagePrivate), parent) // 错误:调用了自己
    • 分析:在公有构造函数 DQuickIconImage(QQuickItem *parent) 的初始化列表中,代码尝试调用 DQuickIconImage(DQuickIconImagePrivate &, QQuickItem *)。由于构造函数名相同,这会导致无限递归,造成栈溢出,程序必然崩溃。
    • 改进建议:公有构造函数应该调用基类 QQuickImage 的构造函数,而不是调用自身的重载版本。应该还原为原来的逻辑:
      DQuickIconImage::DQuickIconImage(QQuickItem *parent)
          : QQuickImage(*(new DQuickIconImagePrivate), parent)
      {
          // ...
      }
  • 逻辑一致性

    • 问题代码
      // 公有构造函数中
      {
          // setAsynchronous(true); 被移除了
      }
      // ...
      // 受保护构造函数中
      {
          setAsynchronous(true);
      }
    • 分析:原代码中公有构造函数设置了 setAsynchronous(true),修改后将其移除并试图通过调用另一个构造函数来间接实现。但由于上述递归错误,这个逻辑断了。如果修正了递归错误(例如改为调用基类),那么 setAsynchronous(true) 的设置需要在公有构造函数中显式保留,或者确保调用的受保护构造函数逻辑能覆盖到。
    • 改进建议:如果公有构造函数改为调用基类构造函数,请务必保留 setAsynchronous(true);,以保持原有的“默认异步加载”行为。

2. 代码质量

  • 宏版本检查的健壮性

    • 代码#if QT_VERSION >= QT_VERSION_CHECK(6, 8, 0)
    • 意见:使用 QT_VERSION_CHECK 是正确的做法,符合 Qt 编码规范。这确保了代码在旧版本 Qt 上编译时不会因找不到 setRetainWhileLoading 而报错。
    • 建议:确认 setRetainWhileLoading 确实是 Qt 6.8 引入的 API。如果是 6.5 或其他版本引入的,版本号需要修正。
  • 代码可读性

    • 意见:受保护构造函数中添加了 setRetainWhileLoading(true),这有助于提升用户体验(加载时保留旧图标),但缺少注释说明为什么需要这个新属性。
    • 建议:添加注释解释引入该特性的原因,例如:
      // Qt 6.8+:在加载新图标源时保留当前显示的图标,避免闪烁或空白
      setRetainWhileLoading(true);

3. 代码性能

  • 内存分配
    • 代码*(new DQuickIconImagePrivate)
    • 分析:在公有构造函数初始化列表中使用 new 分配 Private 对象。这是 Qt Pimpl 惯用模式的写法。
    • 建议:确保 DQuickIconImage 的析构函数或基类机制能够正确释放这个 Private 对象,防止内存泄漏。通常 QQuickImage 或其父类会处理这个问题,但值得确认。

4. 代码安全

  • 潜在的崩溃风险
    • 分析:如前所述,构造函数的递归调用会导致严重的运行时崩溃(栈溢出)。这是当前 Diff 中最严重的安全隐患。
    • 改进建议:必须修正构造函数调用链。

总结与修改建议

这段代码的主要意图是适配 Qt 6.8 的新 API 并重构构造函数逻辑,但引入了致命的语法错误。

建议修改后的代码如下:

// 修正公有构造函数:调用基类而非自身,并保留异步设置
DQuickIconImage::DQuickIconImage(QQuickItem *parent)
    : QQuickImage(*(new DQuickIconImagePrivate), parent)
{
    setAsynchronous(true); // asynchronous by default
}

// 受保护构造函数保持原样或添加注释
DQuickIconImage::DQuickIconImage(DQuickIconImagePrivate &dd, QQuickItem *parent)
    : QQuickImage(dd, parent)
{
    setAsynchronous(true);

#if QT_VERSION >= QT_VERSION_CHECK(6, 8, 0)
    // 优化加载体验:在加载新资源时保持当前图像显示
    setRetainWhileLoading(true);
#endif
}

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

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

@mhduiy mhduiy merged commit 5279290 into linuxdeepin:master Jan 16, 2026
20 of 21 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