Skip to content

refactor: Optimize font deletion process in DFontPreviewListView#194

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master
Mar 7, 2026
Merged

refactor: Optimize font deletion process in DFontPreviewListView#194
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:master

Conversation

@dengzhongyuan365-dev

@dengzhongyuan365-dev dengzhongyuan365-dev commented Mar 6, 2026

Copy link
Copy Markdown
Member
  • Introduced QSet for efficient file existence checks during font deletion.
  • Emitted additional signals to improve UI responsiveness and state management.
  • Enhanced performance by avoiding full model rebuild after each deletion.

bug: https://pms.uniontech.com/bug-view-351365.html https://pms.uniontech.com/bug-view-321575.html

Summary by Sourcery

Optimize the font deletion workflow in DFontPreviewListView to improve performance and keep the UI state in sync without rebuilding the entire model.

Bug Fixes:

  • Ensure UI signals and tab status are correctly updated after font deletions to address inconsistent state issues.

Enhancements:

  • Use a QSet to speed up font file existence checks during batch deletion.
  • Emit more granular signals (item removal, row count changes, delete completion, spinner and focus updates) instead of triggering a full model update after deletions.
  • Integrate performance monitoring for the font deletion process.

@sourcery-ai

sourcery-ai Bot commented Mar 6, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors DFontPreviewListView::deleteCurFonts to optimize font deletion performance and improve UI/state updates by using a QSet for file lookups, emitting more granular signals on deletion, and avoiding a full model rebuild.

Sequence diagram for optimized deleteCurFonts interaction flow

sequenceDiagram
    actor User
    participant DFontPreviewListView
    participant DFontDataThread
    participant DFMDBManager
    participant SignalManager
    participant PerformanceMonitor

    User ->> DFontPreviewListView: deleteCurFonts(files, force)
    activate DFontPreviewListView

    DFontPreviewListView ->> DFontPreviewListView: construct QSet fileSet from files

    alt not force
        DFontPreviewListView ->> DFontPreviewListView: updateSpinner(Delete, false)
    end

    DFontPreviewListView ->> DFontDataThread: getFontModelList()
    activate DFontDataThread
    DFontDataThread -->> DFontPreviewListView: list of DFontPreviewItemData
    deactivate DFontDataThread

    loop for each itemData in m_fontModelList
        DFontPreviewListView ->> DFontPreviewListView: check fileSet.contains(itemData.fontInfo.filePath)
        alt file in fileSet
            DFontPreviewListView ->> DFontPreviewListView: enableFont(filePath)
            DFontPreviewListView ->> DFMDBManager: deleteFontInfo(itemData)
            activate DFMDBManager
            DFMDBManager -->> DFontPreviewListView: return
            deactivate DFMDBManager

            DFontPreviewListView ->> DFontDataThread: removePathWatcher(filePath)
            DFontPreviewListView ->> DFontDataThread: erase item from m_fontModelList
            DFontPreviewListView ->> DFontPreviewListView: m_recoverSelectStateUserfontMultiMap.remove(filePath)
            DFontPreviewListView ->> User: itemRemoved(itemData)
        else file not in fileSet
            DFontPreviewListView ->> DFontPreviewListView: skip item
        end
    end

    DFontPreviewListView ->> DFontPreviewListView: enableFonts()

    DFontPreviewListView ->> SignalManager: fontSizeRequestToSlider()

    alt m_FontViewHasFocus
        DFontPreviewListView ->> DFontPreviewListView: refreshFocuses()
        DFontPreviewListView ->> DFontPreviewListView: setFontViewHasFocus(false)
    end

    DFontPreviewListView ->> User: requestShowSpinner(false, true, Delete)
    DFontPreviewListView ->> User: rowCountChanged()
    DFontPreviewListView ->> User: deleteFinished()
    DFontPreviewListView ->> DFontPreviewListView: syncTabStatus()
    DFontPreviewListView ->> DFontPreviewListView: m_recoveryTabFocusState = false

    DFontPreviewListView ->> PerformanceMonitor: deleteFontFinish(deleteCount)
    activate PerformanceMonitor
    PerformanceMonitor -->> DFontPreviewListView: return
    deactivate PerformanceMonitor

    DFontPreviewListView ->> DFontDataThread: onAutoDirWatchers()

    deactivate DFontPreviewListView
Loading

Class diagram for updated DFontPreviewListView deleteCurFonts workflow

classDiagram
    class DFontPreviewListView {
        - DFontDataThread* m_dataThread
        - QAbstractItemModel* m_fontPreviewProxyModel
        - SignalManager* m_signalManager
        - bool m_FontViewHasFocus
        - QMultiMap~QString, QVariant~ m_recoverSelectStateUserfontMultiMap
        - bool m_recoveryTabFocusState
        + void deleteCurFonts(QStringList& files, bool force)
        + void updateSpinner(DFontSpinnerWidget::Action action, bool show)
        + void enableFont(QString filePath)
        + void enableFonts()
        + void refreshFocuses()
        + void setFontViewHasFocus(bool hasFocus)
        + void syncTabStatus()
        <<signal>> void itemRemoved(DFontPreviewItemData itemData)
        <<signal>> void requestShowSpinner(bool show, bool delay, DFontSpinnerWidget::Action action)
        <<signal>> void rowCountChanged()
        <<signal>> void deleteFinished()
        <<signal>> void requestUpdateModel(int changedCount, bool needReselect)
    }

    class DFontDataThread {
        + QList~DFontPreviewItemData~ m_fontModelList
        + QList~DFontPreviewItemData~ getFontModelList()
        + void removePathWatcher(QString filePath)
        + void onAutoDirWatchers()
    }

    class DFontPreviewItemData {
        + FontInfo fontInfo
    }

    class FontInfo {
        + QString filePath
    }

    class DFMDBManager {
        + static DFMDBManager* instance()
        + void deleteFontInfo(DFontPreviewItemData itemData)
    }

    class SignalManager {
        + void fontSizeRequestToSlider()
    }

    class PerformanceMonitor {
        + static void deleteFontFinish(int deleteCount)
    }

    class DFontSpinnerWidget {
        <<enumeration>> Action
        Delete
    }

    DFontPreviewListView --> DFontDataThread : uses m_dataThread
    DFontPreviewListView --> DFMDBManager : calls deleteFontInfo
    DFontPreviewListView --> SignalManager : uses m_signalManager
    DFontPreviewListView --> PerformanceMonitor : calls deleteFontFinish
    DFontPreviewListView --> DFontSpinnerWidget : uses Delete
    DFontDataThread --> DFontPreviewItemData : contains
    DFontPreviewItemData --> FontInfo : contains
Loading

File-Level Changes

Change Details Files
Optimize font deletion lookup and item removal behavior in DFontPreviewListView::deleteCurFonts
  • Include QSet and construct a QSet from the incoming files list for O(1) membership checks during deletion
  • Replace repeated QStringList::contains calls with QSet::contains when determining whether a font file should be deleted
  • Emit an itemRemoved signal whenever a font entry is erased from the font model list to notify listeners of per-item deletion
deepin-font-manager/interfaces/dfontpreviewlistview.cpp
Adjust post-deletion UI and state updates to avoid full model rebuild and keep UI responsive
  • Remove requestUpdateModel call after deletion and instead emit fontSizeRequestToSlider to keep preview size in sync with updateModel behavior
  • When the font view previously had focus, refresh focus, clear the focus state flag, and then restore appropriate focus handling
  • Emit requestShowSpinner(false, true, DFontSpinnerWidget::Delete) after deletion to hide the delete spinner and update UI state
  • Emit rowCountChanged and deleteFinished signals so other components can react to the changed model size and completion of the delete operation
  • Call syncTabStatus and reset m_recoveryTabFocusState to false to ensure tab UI and focus recovery state reflect the new model
  • Notify PerformanceMonitor via deleteFontFinish(deleteCount) with the number of deleted fonts, and then re-arm directory watchers with m_dataThread->onAutoDirWatchers
deepin-font-manager/interfaces/dfontpreviewlistview.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

@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.

Hey - I've left some high level feedback:

  • Consider using Q_EMIT consistently (e.g., for m_signalManager->fontSizeRequestToSlider()) to match the existing signal emission style in this class and keep the codebase uniform.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider using `Q_EMIT` consistently (e.g., for `m_signalManager->fontSizeRequestToSlider()`) to match the existing signal emission style in this class and keep the codebase uniform.

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.

- Introduced QSet for efficient file existence checks during font deletion.
- Emitted additional signals to improve UI responsiveness and state management.
- Enhanced performance by avoiding full model rebuild after each deletion.

bug: https://pms.uniontech.com/bug-view-351365.html
https://pms.uniontech.com/bug-view-321575.html
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

Git Diff 代码审查报告

代码变更概述

该修改主要针对 DFontPreviewListView::deleteCurFonts 函数进行了优化和功能增强,主要包括:

  1. 使用 QSet 优化文件路径查找性能
  2. 添加 itemRemoved 信号通知
  3. 删除后添加了更多状态更新和信号发射

代码质量与性能分析

1. 性能优化改进

优点

const QSet<QString> fileSet(files.begin(), files.end());
// ...
if (fileSet.contains(filePath)) {

QStringList 转换为 QSet 进行查找是一个很好的优化:

  • QStringList::contains() 时间复杂度为 O(n)
  • QSet::contains() 时间复杂度为 O(1) 平均情况下
  • 这在循环中多次查找时能显著提高性能,特别是当 files 列表较大时

建议

  • 考虑在函数开始处添加注释说明此优化的目的,便于后续维护

2. 信号发射与状态管理

优点

Q_EMIT itemRemoved(itemData);

添加了 itemRemoved 信号,使其他组件能够及时响应字体删除操作。

潜在问题

Q_EMIT requestShowSpinner(false, true, DFontSpinnerWidget::Delete);
Q_EMIT rowCountChanged();
Q_EMIT deleteFinished();

在短时间内发射多个信号可能会造成性能问题,特别是如果这些信号连接到多个槽函数时。

建议

  • 考虑是否可以合并某些信号,或者使用单一信号携带更多状态信息
  • 评估这些信号是否都需要立即发射,或者可以延迟发射

3. 焦点管理

新增代码

if (m_FontViewHasFocus) {
    refreshFocuses();
    setFontViewHasFocus(false);
}

建议

  • 添加注释说明为什么需要在删除后重置焦点状态
  • 考虑 refreshFocuses() 是否有必要,它可能会引起不必要的重绘

4. 代码组织

建议

  • 将删除后的状态更新和信号发射逻辑封装到一个单独的函数中,如 postDeleteOperations(),提高代码可读性
  • 添加适当的注释解释每个操作的目的

5. 资源管理

观察

m_dataThread->removePathWatcher(filePath);

在删除字体文件前移除文件监视器是正确的做法,避免了不必要的文件系统事件。

安全性考虑

  1. 文件路径验证:代码中直接使用 filePath,建议添加路径验证,确保它是合法的文件路径
  2. 并发访问m_dataThread->m_fontModelList 的修改需要确保线程安全,建议检查是否有适当的互斥锁保护

改进建议代码

void DFontPreviewListView::deleteCurFonts(QStringList &files, bool force)
{
    qDebug() << __FUNCTION__ << " before delete " << m_dataThread->getFontModelList().size() << m_fontPreviewProxyModel->rowCount();

    int deleteCount = files.count();
    // 使用 QSet 优化查找性能,时间复杂度从 O(n) 降低到 O(1)
    const QSet<QString> fileSet(files.begin(), files.end());

    if (!force)
        updateSpinner(DFontSpinnerWidget::Delete, false);

    for (auto iter = m_dataThread->m_fontModelList.begin(); iter != m_dataThread->m_fontModelList.end();) {
        DFontPreviewItemData itemData(*iter);
        QString filePath = itemData.fontInfo.filePath;
        
        // 如果字体文件已经不存在,则从t_manager表中删除
        if (fileSet.contains(filePath)) {
            // 删除字体之前启用字体,防止下次重新安装后就被禁用
            enableFont(itemData.fontInfo.filePath);
            DFMDBManager::instance()->deleteFontInfo(itemData);
            
            // 移除文件监视器,避免不必要的文件系统事件
            m_dataThread->removePathWatcher(filePath);
            iter = m_dataThread->m_fontModelList.erase(iter);
            m_recoverSelectStateUserfontMultiMap.remove(filePath);
            
            // 通知其他组件字体已删除
            Q_EMIT itemRemoved(itemData);
        } else {
            ++iter;
        }
    }

    enableFonts();

    // 执行删除后的操作
    postDeleteOperations(deleteCount, force);
    
    m_dataThread->onAutoDirWatchers();
    qDebug() << "Exiting function: DFontPreviewListView::deleteCurFonts";
}

void DFontPreviewListView::postDeleteOperations(int deleteCount, bool force)
{
    // 设置预览大小(与 updateModel 保持一致)
    emit m_signalManager->fontSizeRequestToSlider();

    // 删除之后设置焦点(与 updateModel 保持一致)
    if (m_FontViewHasFocus) {
        refreshFocuses();
        setFontViewHasFocus(false);
    }

    // 批量发射信号,减少信号发射次数
    Q_EMIT requestShowSpinner(false, true, DFontSpinnerWidget::Delete);
    Q_EMIT rowCountChanged();
    Q_EMIT deleteFinished();
    
    syncTabStatus();
    m_recoveryTabFocusState = false;
    PerformanceMonitor::deleteFontFinish(deleteCount);
}

总结

该代码修改主要优化了字体删除操作的性能,并增强了删除后的状态管理。使用 QSet 替代 QStringList 进行查找是一个很好的性能优化。建议进一步整理代码结构,将相关操作封装到单独的函数中,并添加适当的注释。同时,需要考虑信号发射的频率和线程安全问题。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

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

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/forcemerge

@deepin-bot deepin-bot Bot merged commit 548aa27 into linuxdeepin:master Mar 7, 2026
15 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