Skip to content

Conversation

@LiHua000
Copy link
Contributor

  • safeguard SheetBrowser’s fit logic against zero-sized page metrics to keep dual-page mode stable for XPS documents
  • initialise XPS DocSheets with the standard thumbnail/catalog/bookmark sidebar
  • allow TitleWidget controls (sidebar toggle, zoom menu) for XPS files now that the underlying features are wired
  • keep XPS render path improvements aligned with existing viewer behaviour

task: https://pms.uniontech.com/task-view-383459.html

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, 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

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Nov 13, 2025

TAG Bot

New tag: 6.5.42
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #203

- safeguard SheetBrowser’s fit logic against zero-sized page metrics to keep dual-page mode stable for XPS documents
- initialise XPS DocSheets with the standard thumbnail/catalog/bookmark sidebar
- allow TitleWidget controls (sidebar toggle, zoom menu) for XPS files now that the underlying features are wired
- keep XPS render path improvements aligned with existing viewer behaviour

task: https://pms.uniontech.com/task-view-383459.html
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码进行详细审查:

  1. BrowserPage.cpp 中的改进:
bool supportedType = (Dr::PDF == m_sheet->fileType());
#ifdef XPS_SUPPORT_ENABLED
supportedType = supportedType || (Dr::XPS == m_sheet->fileType());
#endif

优点:

  • 使用条件编译支持XPS格式
  • 代码结构更清晰,便于后续扩展支持其他格式

建议:

  • 可以考虑将支持的文件类型定义为一个常量或枚举集合,便于统一管理
  1. SheetBrowser.cpp 中的改进:
const qreal safeMaxWidth = qFuzzyIsNull(m_maxWidth) ? 1.0 : m_maxWidth;
const qreal safeMaxHeight = qFuzzyIsNull(m_maxHeight) ? (qFuzzyIsNull(m_maxWidth) ? 1.0 : m_maxWidth) : m_maxHeight;

优点:

  • 添加了零值检查,避免除零错误
  • 使用qFuzzyIsNull进行浮点数比较更安全

建议:

  • 可以将默认值1.0定义为常量,便于统一修改
  • safeMaxHeight的默认值逻辑可以简化,直接使用1.0即可
  1. XpsDocumentAdapter.cpp 中的改进:
if (logicalSize.isEmpty() || logicalSize.width() <= 0.0 || logicalSize.height() <= 0.0) {
    qCWarning(appLog) << "Invalid logical size for XPS page" << logicalSize;
    return QImage();
}

优点:

  • 添加了更严格的参数验证
  • 使用了更精确的错误日志记录
  • 改进了渲染矩阵的计算方式

建议:

  • 可以将错误消息提取为常量,便于国际化
  • 考虑添加更多的边界检查,如最大尺寸限制
  1. DocSheet.cpp 中的改进:
#ifdef XPS_SUPPORT_ENABLED
else if (Dr::XPS == fileType)
    m_sidebar = new SheetSidebar(this, PREVIEW_THUMBNAIL | PREVIEW_CATALOG | PREVIEW_BOOKMARK);
#endif

优点:

  • 使用条件编译支持XPS格式
  • 为XPS文档配置了合适的侧边栏功能

建议:

  • 可以考虑使用工厂模式创建不同类型的侧边栏,使代码更易维护
  1. TitleWidget.cpp 中的改进:
if (Dr::PDF == m_curSheet->fileType()
        || Dr::DOCX == m_curSheet->fileType()
#ifdef XPS_SUPPORT_ENABLED
        || Dr::XPS == m_curSheet->fileType()
#endif
    ) {

优点:

  • 使用条件编译支持XPS格式
  • 代码格式更清晰

建议:

  • 可以将支持的文件类型检查提取为单独的函数,提高代码复用性

总体建议:

  1. 考虑添加更多的单元测试,特别是边界条件
  2. 可以将一些魔法数字(如1000、25.0等)定义为命名常量
  3. 考虑使用RAII模式管理资源
  4. 可以添加更多的错误处理和日志记录
  5. 考虑使用智能指针管理动态分配的内存

这些改进主要提高了代码的安全性、可维护性和可扩展性,同时修复了一些潜在的问题。

@LiHua000
Copy link
Contributor Author

/forcemerge

@deepin-bot deepin-bot bot merged commit beea693 into linuxdeepin:master Nov 13, 2025
6 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