Skip to content

fix: add PNG/JPEG to BMP DIB conversion for X11 clipboard#712

Open
wyu71 wants to merge 1 commit intolinuxdeepin:masterfrom
wyu71:master
Open

fix: add PNG/JPEG to BMP DIB conversion for X11 clipboard#712
wyu71 wants to merge 1 commit intolinuxdeepin:masterfrom
wyu71:master

Conversation

@wyu71
Copy link
Contributor

@wyu71 wyu71 commented Mar 9, 2026

Add XWindowsClipboardImageConverter to convert PNG/JPEG images to BMP DIB format for cross-platform clipboard compatibility.

添加 XWindowsClipboardImageConverter 实现 PNG/JPEG 图片到 BMP DIB 格式转换,确保跨平台剪贴板兼容性.

Log: 支持 PNG/JPEG 图片剪贴板跨平台传输

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Sorry @wyu71, your pull request is larger than the review limit of 150000 diff characters

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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 force-pushed the master branch 3 times, most recently from a75be70 to 2272f4a Compare March 9, 2026 08:06
Add XWindowsClipboardImageConverter to convert PNG/JPEG images to BMP
DIB format for cross-platform clipboard compatibility.

添加 XWindowsClipboardImageConverter 实现 PNG/JPEG 图片到 BMP DIB
格式转换,确保跨平台剪贴板兼容性.

Log: 支持 PNG/JPEG 图片剪贴板跨平台传输
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查报告

1. 语法与逻辑审查

1.1 整体评价

代码整体语法正确,逻辑清晰,实现了XWindows平台下剪贴板图像格式转换的功能。使用了stb_image库进行图像解码,并正确处理了PNG/JPEG/BMP等常见格式。

1.2 具体问题与建议

  1. XWindowsClipboardImageConverter.cpp中的类型转换安全性

    • 问题:convertToBMPDIB函数中存在多处类型转换,如static_cast<size_t>(imageSize),虽然添加了溢出检查,但建议使用更安全的转换方式。
    • 建议:考虑使用std::numeric_limits进行更严格的范围检查,或使用boost::numeric_cast等类型安全转换工具。
  2. 字符串处理安全性

    • 问题:detectImageFormat函数中直接访问data[0]data[1]等,没有充分检查数据长度。
    • 建议:在访问前确保data.size() >= 8,已部分实现但可以更明确。
  3. 内存管理

    • 优点:使用了RAII(std::unique_ptr)管理stb_image分配的内存,这是良好的实践。
    • 建议:可以考虑使用std::vector替代部分手动内存管理,进一步降低内存泄漏风险。
  4. 错误处理

    • 问题:部分函数在错误情况下仅返回空字符串{},没有提供详细的错误信息。
    • 建议:考虑引入异常机制或更详细的错误码,便于调试和问题追踪。

2. 代码质量

2.1 整体评价

代码结构清晰,使用了现代C++特性,如RAII、智能指针等。注释充分,易于理解。

2.2 具体问题与建议

  1. 代码复用性

    • 问题:icccmFillCachemotifFillCache中存在大量重复代码。
    • 建议:提取公共逻辑到单独的函数中,减少代码重复。
  2. 命名规范

    • 问题:部分变量命名不够描述性,如rawdst
    • 建议:使用更具描述性的名称,如rawImageDatadestinationBuffer
  3. 常量定义

    • 优点:使用了命名常量kMaxClipboardSizeMAX_IMAGE_DIMENSION等。
    • 建议:考虑将这些常量集中管理,例如使用enum classconstexpr变量。
  4. 代码组织

    • 建议:将图像格式检测、转换等不同功能模块化,可以进一步划分为独立的类或命名空间。

3. 代码性能

3.1 整体评价

代码考虑了性能因素,如优先使用PNG/JPEG格式,对大尺寸图像进行了限制。

3.2 兀体问题与建议

  1. 图像转换性能

    • 问题:在convertToBMPDIB中,逐像素进行格式转换,可能不是最优解。
    • 建议:考虑使用SIMD指令集(SSE/AVX)优化像素格式转换,或使用更高效的图像处理库。
  2. 内存分配

    • 问题:convertToBMPDIB中使用了std::vector分配大量内存,对于大图像可能造成性能问题。
    • 建议:考虑使用内存池或预分配策略,减少动态分配次数。
  3. stb_image配置

    • 优点:通过宏定义禁用了不需要的图像格式支持,减少了编译后代码体积。
    • 建议:可以考虑进一步优化stb_image的配置,例如根据实际使用情况调整缓冲区大小。

4. 代码安全

4.1 整体评价

代码考虑了多种安全因素,如整数溢出检查、内存大小限制等。

4.2 具体问题与建议

  1. 整数溢出防护

    • 问题:虽然添加了溢出检查,但部分计算如width * 3仍可能溢出。
    • 建议:使用更安全的数学运算库,如checked_arithmeticboost::numeric_cast
  2. 缓冲区溢出防护

    • 问题:在detectImageFormat中,虽然检查了data.size() >= 8,但后续访问data[2]等仍可能越界。
    • 建议:使用更严格的边界检查,或使用at()方法替代[]访问。
  3. 输入验证

    • 问题:toIClipboard函数中,对BMP格式的offset验证不够严格。
    • 建议:添加更全面的验证逻辑,确保offset在有效范围内,防止读取越界。
  4. 资源管理

    • 优点:使用了RAII和智能指针管理资源。
    • 建议:考虑使用std::shared_ptr管理共享资源,或实现自定义的RAII包装器。

5. 其他建议

  1. 跨平台兼容性

    • 问题:代码主要针对X11平台,未考虑其他平台。
    • 建议:如果需要跨平台支持,可以考虑使用抽象层隔离平台特定代码。
  2. 测试覆盖

    • 建议:增加单元测试和集成测试,特别是针对边界条件和错误情况的测试用例。
  3. 文档完善

    • 建议:补充API文档,特别是公共接口的使用示例和注意事项。

总结

总体而言,这段代码实现了X11平台下剪贴板图像格式转换的核心功能,代码质量较高,但在类型安全、性能优化和错误处理方面仍有改进空间。建议在后续开发中关注上述问题,以提高代码的健壮性、安全性和性能。

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