Skip to content

fix: show prev/next buttons for single multi-page TIF file#294

Merged
wyu71 merged 2 commits into
linuxdeepin:masterfrom
wyu71:master
May 18, 2026
Merged

fix: show prev/next buttons for single multi-page TIF file#294
wyu71 merged 2 commits into
linuxdeepin:masterfrom
wyu71:master

Conversation

@wyu71
Copy link
Copy Markdown
Contributor

@wyu71 wyu71 commented May 18, 2026

Fix visibility condition and async loading timing issue:

  • QML: use hasPreviousImage/hasNextImage instead of imageCount > 1
  • C++: connect infoChanged to checkSwitchEnable for async frame data

修复单文件多页TIF图片工具栏上一张/下一张按钮不显示的问题,
包含QML可见性条件修复和C++异步加载时序修复。

Log: 修复多页TIF工具栏按钮不显示
PMS: BUG-323847
Influence: 修复后打开单张多页TIF图片时,工具栏上一张/下一张按钮正常显示,
多文件场景和单张普通图片场景不受影响。

Fix visibility condition and async loading timing issue:
- QML: use hasPreviousImage/hasNextImage instead of imageCount > 1
- C++: connect infoChanged to checkSwitchEnable for async frame data

修复单文件多页TIF图片工具栏上一张/下一张按钮不显示的问题,
包含QML可见性条件修复和C++异步加载时序修复。

Log: 修复多页TIF工具栏按钮不显示
PMS: BUG-323847
Influence: 修复后打开单张多页TIF图片时,工具栏上一张/下一张按钮正常显示,
多文件场景和单张普通图片场景不受影响。
@wyu71 wyu71 force-pushed the master branch 2 times, most recently from 3f5124c to 287307c Compare May 18, 2026 07:09
Guard against null model and empty state in checkSwitchEnable() to
handle cases where async infoChanged fires after model is cleared.

增加checkSwitchEnable的防御性检查,防止异步回调时模型已清空
导致的越界访问风险。

Log: 增加checkSwitchEnable防御性检查
PMS: BUG-323847
Influence: 提升异步回调场景下的健壮性,防止模型清空后的越界访问。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改主要涉及图片查看器中缩略图列表显示逻辑的优化、切换按钮状态检查的防抖处理、边界条件的安全加固以及多帧图片(如动图)判断逻辑的修复。

整体来看,代码质量有所提升,特别是引入防抖机制和移除Q_ASSERT改用优雅降级的做法非常值得肯定。以下是针对语法逻辑、代码质量、代码性能和代码安全四个维度的详细审查意见:

1. 语法与逻辑

  • 版权声明年份修改 (跨文件)
    • 观察: 将 2023 修改为 2023 - 2026
    • 意见: 逻辑上没有问题,但请注意,如果当前年份尚未到达2026年,提前写未来的结束年份在开源许可中并不常见(通常结束年份是当前年份或使用 "present")。如果这是贵公司的特定规范则无妨,否则建议确认是否应写为 2023 - 2024(当前年份)。
  • 多帧图片判断逻辑修复 (globalcontrol.cpp)
    • 观察: 原逻辑 curFrameIndex < (currentImage.frameCount() - 1),新逻辑增加了 frameCount > 1 的前置判断:(frameCount > 1 && curFrameIndex < (frameCount - 1))
    • 意见: 这是一个非常优秀的Bug修复。如果 frameCount 为 0 或 1,原逻辑中 frameCount - 1 会产生负数或0,导致单帧图片或无帧信息时错误地判定为“有下一帧”。增加 frameCount > 1 彻底堵住了这个逻辑漏洞。
  • QML可见性逻辑变更 (ThumbnailListView.qml)
    • 观察: 缩略图列表的可见性从 imageCount > 1 变更为 hasPreviousImage || hasNextImage
    • 意见: 逻辑上与C++端的新增防抖和状态检查对齐。但需注意语义上的细微差别:如果当前仅有一张多帧图片(如GIF),imageCount 为 1,但 hasNextImage 可能为 true。此修改使得多帧图片能够正常显示缩略图切换栏,逻辑更加合理。

2. 代码质量

  • 移除 Q_ASSERT 并进行优雅降级 (globalcontrol.cpp)
    • 观察: 移除了 Q_ASSERT(sourceModel);,改为 if (!sourceModel || sourceModel->rowCount() <= 0) 并重置状态后提前返回。
    • 意见: 极佳的改进Q_ASSERT 在 Release 构建中会被忽略,如果 sourceModel 为空,会导致空指针解引用引发崩溃。改为防御性编程,保证了Release环境下的安全性。
  • 防抖定时器的引入 (globalcontrol.h, globalcontrol.cpp)
    • 观察: 引入 switchCheckTimer 并在 infoChanged 信号触发时启动 50ms 防抖。
    • 意见: 代码结构清晰,复用了Qt底层的 QBasicTimertimerEvent,比直接使用 QTimer 性能更好。注释也写得很清楚,提升了代码可读性。
  • 变量内联优化 (globalcontrol.cpp)
    • 观察: 引入了 const int frameCount = currentImage.frameCount();
    • 意见: 避免了在复杂的条件判断中多次调用函数,不仅让代码更整洁,也避免了潜在的重复计算开销。

3. 代码性能

  • 信号连接与防抖 (globalcontrol.cpp)
    • 观察: connect(&currentImage, &ImageInfo::infoChanged, this, [this]() { switchCheckTimer.start(50, this); });
    • 意见: 性能优化很到位。infoChanged 信号在图片加载、旋转等过程中可能高频触发,50ms的防抖有效避免了UI状态的频繁计算和重绘(QML端频繁响应 hasNextImageChanged 等)。
    • 潜在风险提示: QBasicTimer::start 如果在定时器已经激活时调用,会自动停止旧定时器并重启。这意味着如果 infoChanged 以极高频率持续触发(间隔小于50ms),checkSwitchEnable() 将永远无法执行。对于图片查看器而言,50ms的延迟通常难以察觉,但需确保在关键流程(如窗口关闭、对象销毁)前,如果需要最终状态,能有机会执行一次。目前的业务场景下,这是可接受的。

4. 代码安全

  • 空指针与越界访问防范 (globalcontrol.cpp)
    • 观察: if (!sourceModel || sourceModel->rowCount() <= 0) 以及对 frameCount 的前置判断。
    • 意见: 显著提升了代码安全性。防止了在模型为空或数据未加载完成时,访问非法内存或产生整数下溢(rowCount() - 1rowCount() 为0时下溢为极大的正数或-1,取决于类型)。
  • .gitignore 更新
    • 观察: 新增了 .claude/, .trellis/, AGENTS.md 的忽略规则。
    • 意见: 合理。防止了开发环境中使用的AI辅助工具(如Claude)或特定Agent配置文件被意外提交到仓库中,避免了潜在的开发环境信息泄露或仓库污染。

总结与改进建议

本次代码变更质量很高,修复了潜在的越界/空指针崩溃问题,并通过防抖机制优化了高频事件下的性能表现。代码风格统一,逻辑严谨。

唯一建议修改的地方:关于版权年份 2026 的问题,如果这不是贵团队的硬性规范,建议改为当前年份或动态范围(如 2023 - present),以符合开源社区的常规做法。除此之外,无需进一步修改,可以安全合入。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, wyu71

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

@wyu71 wyu71 merged commit 988ce90 into linuxdeepin:master May 18, 2026
19 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