fix: cache printer names to avoid repeated queries - #776
Conversation
Reviewer's GuideThe preview dialog now caches QPrinterInfo results per dialog session, clearing the cache when the dialog finishes, which avoids repeated printer queries while retaining refreshed results for subsequent sessions. The PR also adapts checkbox signal connections and state handling to Qt 6 while maintaining Qt 5 compatibility. Sequence diagram for printer name caching in the preview dialogsequenceDiagram
participant Dialog as DPrintPreviewDialog
participant Private as DPrintPreviewDialogPrivate
participant Qt as QPrinterInfo
Dialog->>Private: initdata()
Private->>Private: availablePrinterNames()
alt printerNames is empty
Private->>Qt: availablePrinterNames()
Qt-->>Private: QStringList
Private->>Private: printerNames = result
end
Private-->>Dialog: addItems(printerNames)
Private->>Private: isActualPrinter(name)
Private->>Private: availablePrinterNames()
Private-->>Private: printerNames.contains(name)
Dialog-->>Private: finished
Private->>Private: printerNames.clear()
Sequence diagram for Qt 6 checkbox state handlingsequenceDiagram
participant CheckBox as DCheckBox
participant Dialog as DPrintPreviewDialog
participant Private as DPrintPreviewDialogPrivate
CheckBox-->>Dialog: checkStateChanged(Qt::CheckState)
Dialog->>Private: _q_checkStateChanged(int(state))
CheckBox-->>Dialog: checkStateChanged(Qt::CheckState)
alt state is Qt::Unchecked
Dialog->>Private: setPageLayoutEnable(false)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/widgets/dprintpreviewdialog.cpp" line_range="2255-2257" />
<code_context>
+QStringList DPrintPreviewDialogPrivate::availablePrinterNames()
+{
+ if (printerNames.isEmpty()) {
+ qDebug(dPrintPreview) << "Get printer names from QPrinterInfo.";
+ printerNames = QPrinterInfo::availablePrinterNames();
+ qDebug(dPrintPreview) << "Available printer names:" << printerNames;
+ }
</code_context>
<issue_to_address>
**nitpick (performance):** An empty result is treated as an uninitialized cache, so `availablePrinterNames()` calls `QPrinterInfo::availablePrinterNames()` on every invocation when the system has no printers. Repeated `isActualPrinter()` checks therefore retain the system-query overhead the cache was intended to remove.
**Triggers:** When the system has no available printers and multiple preview controls trigger `isActualPrinter()`.
**Suggested fix:** Track cache initialization separately from the list contents, such as with a boolean flag, so an empty printer list is cached as a valid result.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
c143ca1 to
303d3c8
Compare
393ead4 to
d4623b5
Compare
1. Replace direct QPrinterInfo::availablePrinterNames() calls with a cached wrapper 2. Add printerNames and printerNamesInited members to DPrintPreviewDialogPrivate 3. Implement availablePrinterNames() to fetch printer names only once and cache them 4. Clear cached printer names when the dialog is finished to prevent stale data 5. Update isActualPrinter() to use the cached printer names list This change improves performance by avoiding repeated calls to QPrinterInfo::availablePrinterNames(), which can be expensive, especially when the printer list is large. The cache is properly invalidated when the dialog closes, ensuring fresh data on the next dialog opening. Influence: 1. Open the print preview dialog and verify the printer list loads correctly 2. Add or remove a printer while the dialog is open and verify the list does not refresh (expected behavior with caching) 3. Close the dialog and reopen it to verify printer list updates with new system printers 4. Test print-to-PDF and save-as-image options still appear correctly in the dropdown 5. Verify print job submission works correctly with cached printer names style: 重构打印机名称获取逻辑,使用缓存列表 1. 将直接的 QPrinterInfo::availablePrinterNames() 调用替换为带缓存的封装 方法 2. 在 DPrintPreviewDialogPrivate 中添加 printerNames 和 printerNamesInited 成员变量 3. 实现 availablePrinterNames() 方法,仅获取一次打印机名称并进行缓存 4. 对话框结束时清除缓存的打印机名称,防止数据过期 5. 更新 isActualPrinter() 方法以使用缓存的打印机名称列表 此更改通过避免重复调用 QPrinterInfo::availablePrinterNames() 来提升性 能,该调用在打印机列表较大时可能消耗较多资源。对话框关闭时会正确失效缓 存,确保下次打开时获取最新数据。 Influence: 1. 打开打印预览对话框,验证打印机列表能正确加载 2. 在对话框打开期间添加或删除打印机,验证列表不会刷新(缓存行为的预期 表现) 3. 关闭对话框后重新打开,验证打印机列表会随系统打印机变化而更新 4. 测试打印到 PDF 和另存为图片选项在下拉列表中仍然正常显示 5. 验证使用缓存的打印机名称提交打印任务能正常工作 PMS: BUG-375053
d4623b5 to
6c22fba
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 代码语法正确,懒加载缓存模式实现清晰。lambda 捕获 this 指针安全(连接接收者为 q 即 DPrintPreviewDialog 对象,Qt 会在对象销毁时自动断开连接,不存在悬空指针风险)。initdata() 和 isActualPrinter() 中的调用替换正确,逻辑一致。 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 建议为 availablePrinterNames() 方法添加注释,说明其缓存机制、惰性初始化行为以及缓存清除时机(对话框关闭时)。调试日志(qDebug)符合 PR 描述中添加日志的目的,属于合规代码。 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 建议将 availablePrinterNames() 返回类型改为 const QStringList &,以明确表达不修改意图并避免潜在的隐式共享 detach。本次缓存优化本身效果显著,将 QPrinterInfo::availablePrinterNames() 的昂贵系统调用从多次降为一次。 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 未发现安全漏洞。调试日志仅输出打印机名称列表,不包含敏感信息。无用户输入处理变更,无注入风险,无硬编码凭据。代码安全合规。 💡 改进建议代码示例// 建议优化:返回 const 引用避免潜在拷贝
const QStringList &DPrintPreviewDialogPrivate::availablePrinterNames()
{
if (!printerNamesInited) {
qDebug(dPrintPreview) << "Get printer names from QPrinterInfo.";
printerNamesInited = true;
printerNames = QPrinterInfo::availablePrinterNames();
qDebug(dPrintPreview) << "Available printer names:" << printerNames;
}
return printerNames;
}
// 头文件声明也需同步修改
// const QStringList &availablePrinterNames();本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
fix: cache printer names to improve print preview performance
availablePrinterNames()method that stores printernames in
printerNamesmember variableprinterNamesInitedflag to avoidrepeated expensive calls to
QPrinterInfo::availablePrinterNames()fresh data on next dialog opening
QPrinterInfo::availablePrinterNames()ininitdata()andisActualPrinter()with the cached methodLog: Optimized print preview performance by caching printer names
Influence:
confirm printer names refresh properly
verify cache behavior
the list
printers
fix: 缓存打印机名称以提升打印预览性能
availablePrinterNames()方法,将打印机名称缓存在printerNames成员变量中printerNamesInited标志实现惰性初始化,避免重复调用昂贵的QPrinterInfo::availablePrinterNames()接口initdata()和isActualPrinter()中的直接调用替换为缓存方法Log: 通过缓存打印机名称优化打印预览性能
Influence:
PMS: BUG-375053
Summary by Sourcery
Cache printer names within each print preview session to improve performance while clearing the cache when the dialog closes.
Bug Fixes:
Enhancements:
Chores: