Skip to content

fix: grub preview flicker on theme toggle - #3413

Open
52cyb wants to merge 1 commit into
linuxdeepin:masterfrom
52cyb:fix/grub-background-flicker
Open

fix: grub preview flicker on theme toggle#3413
52cyb wants to merge 1 commit into
linuxdeepin:masterfrom
52cyb:fix/grub-background-flicker

Conversation

@52cyb

@52cyb 52cyb commented Aug 13, 2026

Copy link
Copy Markdown
Contributor
  1. Add same-value guard to setGrubThemePath to stop redundant emits
  2. Add grubBackgroundRevision counter for cache-busting same-path content
  3. Add pending flag to distinguish self-initiated background changes
  4. Replace Date.now() with grubBackgroundRevision in BootPage.qml

Log: Fixed background preview flicker when toggling boot menu theme or changing background image

Influence:

  1. Test theme off→on: preview refreshes once, no later flicker
  2. Test theme on→off: preview clears with no bounce-back
  3. Test dragging new background with theme on (BUG-281399 not regressed)
  4. Test dragging background with theme off (auto-enable theme preserved)
  5. Test deleting/restoring default background and first page entry

fix: 修复启动菜单主题切换后背景预览闪烁

  1. 为 setGrubThemePath 增加同值守卫,避免冗余 emit
  2. 新增 grubBackgroundRevision 计数器,用于同路径内容变更的缓存失效
  3. 新增 pending 标志,区分主动发起的背景变更
  4. BootPage.qml 中用 grubBackgroundRevision 替换 Date.now()

Log: 修复启动菜单主题切换或更换背景图后背景预览区闪烁问题

Influence:

  1. 测试主题关→开:背景预览只刷新一次,稍后不再闪
  2. 测试主题开→关:预览清空,无回跳
  3. 测试主题开启态拖拽换背景(验证 BUG-281399 未回归)
  4. 测试主题关闭态拖拽背景(验证自动开主题原行为保留)
  5. 测试删除/恢复默认背景及首次进入页面

PMS: BUG-371485

Summary by Sourcery

Prevent boot menu background preview flicker when toggling the GRUB theme or changing the background image.

Bug Fixes:

  • Ensure theme toggle and background changes no longer cause duplicate preview reloads or unintended re-enabling of the boot menu theme.

Enhancements:

  • Introduce a grub background revision counter exposed to QML for cache-busting same-path background updates without relying on timestamps.
  • Add a guard on grub theme path updates to avoid redundant change notifications and unnecessary image reloads.
  • Track self-initiated background changes with a pending flag so only user-driven updates refresh the preview and adjust theme state.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 52cyb

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @52cyb, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

1. Add same-value guard to setGrubThemePath to stop redundant emits
2. Add grubBackgroundRevision counter for cache-busting same-path content
3. Add pending flag to distinguish self-initiated background changes
4. Replace Date.now() with grubBackgroundRevision in BootPage.qml

Log: Fixed background preview flicker when toggling boot menu theme or changing background image

Influence:
1. Test theme off→on: preview refreshes once, no later flicker
2. Test theme on→off: preview clears with no bounce-back
3. Test dragging new background with theme on (BUG-281399 not regressed)
4. Test dragging background with theme off (auto-enable theme preserved)
5. Test deleting/restoring default background and first page entry

fix: 修复启动菜单主题切换后背景预览闪烁

1. 为 setGrubThemePath 增加同值守卫,避免冗余 emit
2. 新增 grubBackgroundRevision 计数器,用于同路径内容变更的缓存失效
3. 新增 pending 标志,区分主动发起的背景变更
4. BootPage.qml 中用 grubBackgroundRevision 替换 Date.now()

Log: 修复启动菜单主题切换或更换背景图后背景预览区闪烁问题

Influence:
1. 测试主题关→开:背景预览只刷新一次,稍后不再闪
2. 测试主题开→关:预览清空,无回跳
3. 测试主题开启态拖拽换背景(验证 BUG-281399 未回归)
4. 测试主题关闭态拖拽背景(验证自动开主题原行为保留)
5. 测试删除/恢复默认背景及首次进入页面

PMS: BUG-371485
@52cyb
52cyb force-pushed the fix/grub-background-flicker branch from b7f0a83 to df96348 Compare August 13, 2026 03:17
@sourcery-ai

sourcery-ai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Reviewer's Guide

Implements a more precise grub background refresh mechanism by guarding redundant theme path updates, tracking background content revisions, and distinguishing user-initiated background changes so the QML preview reloads only when necessary and without flicker after theme toggles or image changes.

Sequence diagram for grub background change and preview refresh

sequenceDiagram
    actor User
    participant BootPageQML
    participant CommonInfoWork
    participant CommonInfoProxy
    participant CommonInfoModel

    User->>BootPageQML: dragBackground
    BootPageQML->>CommonInfoWork: setBackground(path)
    CommonInfoWork->>CommonInfoWork: m_pendingBackgroundRefresh = true
    CommonInfoWork->>CommonInfoProxy: setBackground(path)

    CommonInfoProxy-->>CommonInfoWork: BackgroundChanged
    CommonInfoWork->>CommonInfoWork: pending = m_pendingBackgroundRefresh
    CommonInfoWork->>CommonInfoWork: m_pendingBackgroundRefresh = false

    alt pending && !CommonInfoModel.themeEnabled()
        CommonInfoWork->>CommonInfoWork: setEnableTheme(true)
        CommonInfoWork->>CommonInfoModel: setThemeEnabled(true)
    end

    CommonInfoWork->>CommonInfoModel: setGrubThemePath(backgroundPath)
    CommonInfoModel->>CommonInfoModel: guard same m_grubThemePath

    alt pending
        CommonInfoWork->>CommonInfoModel: bumpGrubBackgroundRevision()
        CommonInfoModel-->>BootPageQML: grubBackgroundRevisionChanged
    end

    CommonInfoModel-->>BootPageQML: grubThemePathChanged
    BootPageQML->>BootPageQML: reload Image.source using grubThemePath and grubBackgroundRevision
Loading

File-Level Changes

Change Details Files
Differentiate user-initiated background changes from passive theme-related updates to control when the preview refreshes and theme auto-enables.
  • Introduce m_pendingBackgroundRefresh flag in CommonInfoWork to mark self-initiated background updates in setBackground
  • Use pending flag in BackgroundChanged lambda to only auto-enable theme and refresh the preview when the change was requested by the user
  • Reset pending flag after handling each BackgroundChanged signal to avoid misclassifying subsequent signals
src/plugin-commoninfo/operation/commoninfowork.cpp
src/plugin-commoninfo/operation/commoninfowork.h
Prevent redundant grub theme path updates and drive cache-busting through a dedicated background revision counter instead of timestamp-based URLs.
  • Add same-value guard in CommonInfoModel::setGrubThemePath to avoid emitting grubThemePathChanged when path is unchanged
  • Introduce grubBackgroundRevision property, getter, and bumpGrubBackgroundRevision() in CommonInfoModel with corresponding signal and member initialization
  • Expose grubBackgroundRevision as a Q_PROPERTY for QML consumption and wire its signal into the model
src/plugin-commoninfo/operation/commoninfomodel.cpp
src/plugin-commoninfo/operation/commoninfomodel.h
Use the background revision counter in QML image source URLs so the preview reloads precisely on real content changes without relying on Date.now().
  • Replace Date.now()-based query parameter in BootPage.qml Image source with grubBackgroundRevision-driven version field
  • Keep existing file path logic but tie cache-busting strictly to model-driven revision increments triggered on real background changes
src/plugin-commoninfo/qml/BootPage.qml
Tie revision bumps to user-requested content changes only so theme toggles no longer cause extra reloads or flicker.
  • Call m_commomModel->bumpGrubBackgroundRevision() only when pending is true in BackgroundChanged handler
  • Document behavior in comments to clarify that content-change revision bumps are for BUG-281399 while duplicate BackgroundChanged after theme toggles become no-ops due to the same-value guard
src/plugin-commoninfo/operation/commoninfowork.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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:79分

■ 【总体评价】

代码有效修复了GRUB背景图片闪烁与主题状态回弹问题,但存在未使用变量的轻微瑕疵
逻辑正确但因存在未使用变量导致代码质量和性能维度各扣5分且触发多维度不通过降分规则

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

修复逻辑严密,通过引入 m_pendingBackgroundRefresh 状态标记精确区分了用户主动触发与后端延迟信号,setGrubThemePath 的相同值拦截与 bumpGrubBackgroundRevision 配合完美解决了同路径内容更新的缓存失效问题

建议:无

  • 2.代码质量(一般)✕

CommonInfoWork::CommonInfoWork 构造函数中的 BackgroundChanged 槽函数内声明了局部变量 QPixmap pix = QPixmap(backgroundPath);,但该变量在后续代码中从未被引用或使用,属于冗余代码
潜在问题:冗余变量降低代码可读性,容易让后续维护者误解其用途
建议:删除未使用的 QPixmap pix 变量声明及相关加载操作

  • 3.代码性能(存在性能问题)✕

BackgroundChanged 槽函数中,无意义的 QPixmap(backgroundPath) 调用会触发磁盘I/O读取以及图片解码操作,但由于结果未被使用,这完全是浪费CPU和I/O资源的开销
潜在问题:每次收到 BackgroundChanged 信号(即使是需要被忽略的重复信号)都会执行一次昂贵的图片加载与解码过程
建议:移除无用的 QPixmap 构造代码,避免不必要的资源消耗

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码未引入新的安全风险,文件路径来源于受信任的本地后端DBus接口,临时目录清理操作基于内部可控变量,无命令注入或路径遍历风险

  • 建议:保持现有的输入源信任边界管理

■ 【改进建议代码示例】

--- a/src/plugin-commoninfo/operation/commoninfowork.cpp
+++ b/src/plugin-commoninfo/operation/commoninfowork.cpp
@@ -209,8 +209,6 @@ CommonInfoWork::CommonInfoWork(CommonInfoModel *model, QObject *parent)
         }
 
         QString backgroundPath = m_commonInfoProxy->Background();
-        QPixmap pix = QPixmap(backgroundPath);
         m_commomModel->setGrubThemePath(backgroundPath);
 
         QDir dir(m_tmpBackgroundPath);

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