Skip to content

Conversation

@LiHua000
Copy link
Contributor

Replace custom Document API approach with pdftocairo process for better print quality and to resolve character encoding issues. The new method converts Linearized PDFs to embedded vector PDF format before printing.

Log: fix pdf printing issue.
Bug: https://pms.uniontech.com/bug-view-348017.html
cherry-pick to: https://pms.uniontech.com/bug-view-348169.html

Replace custom Document API approach with pdftocairo process for better
print quality and to resolve character encoding issues. The new method
converts Linearized PDFs to embedded vector PDF format before printing.

Log: fix pdf printing issue.
Bug: https://pms.uniontech.com/bug-view-348017.html
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要是为了解决 PDF 打印时的乱码和模糊问题,将原来的“Linearized PDF 检测并转换”逻辑替换为“强制使用 pdftocairo 进行转换”的逻辑。

以下是对该 git diff 的详细审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 改进建议:检查 pdftocairo 的可用性

    • 问题:代码直接调用 process.start("pdftocairo", ...),假设系统环境变量 PATH 中一定存在 pdftocairo 命令。如果用户环境未安装 poppler-utils 或相关依赖,或者该命令不在 PATH 中,process.waitForFinished() 会执行完毕,但 exitCode 非零,导致打印直接静默失败(虽然代码有打印警告,但用户体验不佳)。
    • 建议:在执行前,最好检查命令是否存在,或者提供一个降级方案(例如:如果 pdftocairo 失败,尝试回退到原来的逻辑或直接打印原文件)。
  • 改进建议:waitForFinished 的阻塞时间

    • 问题process.waitForFinished() 默认没有参数,这意味着它会无限期等待直到进程结束。如果 pdftocairo 处理一个巨大的 PDF 文件卡死,主界面(UI 线程)将会冻结,导致用户无法操作程序。
    • 建议:设置一个合理的超时时间,例如 process.waitForFinished(30000)(30秒)。如果超时,则 kill 进程并提示用户。
  • 改进建议:临时文件路径处理

    • 问题QString tmpPdfPath = convertedFileDir() + "/pdftocairoPrint.pdf"; 使用了硬编码的文件名。
    • 建议:如果用户连续快速点击打印,或者程序多线程处理,可能会发生文件名冲突。建议使用 QTemporaryFile 或包含时间戳/随机数的文件名来确保唯一性。

2. 代码质量

  • 改进建议:资源清理

    • 问题tmpPdfPath 是一个临时文件,打印预览对话框关闭或打印完成后,这个文件应该被删除以避免占用磁盘空间。目前的代码没有看到清理逻辑。
    • 建议:确保在打印任务结束后(例如在 DocSheet 的析构函数中,或者在打印完成的槽函数中)删除该临时文件。或者使用 QTemporaryFile 对象,利用其 autoRemove 特性。
  • 改进建议:日志级别

    • 问题qWarning() << "pdftocairo failed:" << ... 使用了 qWarning,这是合适的。但在成功时缺少日志。
    • 建议:建议在转换成功时也输出 qInfo(),方便排查问题。
  • 改进建议:硬编码字符串

    • 问题"pdftocairo""LinearizedConverted.pdf"(旧代码)以及 "/pdftocairoPrint.pdf" 都是硬编码。
    • 建议:将命令字符串定义为常量,便于维护。

3. 代码性能

  • 改进建议:转换开销
    • 分析:引入 pdftocairo 进行全量转换确实能解决乱码和清晰度问题,但这是一个重 I/O 和 CPU 操作。对于小文件还好,对于几百 MB 的 PDF,用户点击打印到看到预览界面的时间会显著增加(因为必须先等待转换完成)。
    • 建议:目前的逻辑是同步阻塞的。如果可能,考虑显示一个加载提示(如“正在处理文档以优化打印效果...”),让用户知道程序没有卡死。

4. 代码安全

  • 改进建议:命令注入风险

    • 问题:虽然 QProcess::start 的参数形式(QStringList)通常能规避 shell 注入,但需要确保 pdfPath(即 filePath())不包含恶意构造的字符。不过 QProcess 通常能正确处理包含空格的路径,这一点比 system() 安全得多。
    • 现状:当前使用 QStringList 传参是安全的做法,无需修改,但需注意保持这种风格,不要改用拼接字符串的方式执行命令。
  • 改进建议:临时文件权限

    • 问题:生成的临时文件 pdftocairoPrint.pdf 继承了默认的 umask 权限。
    • 建议:通常问题不大,但如果系统环境敏感,应确保临时文件权限仅对当前用户可读。

代码重构建议

综合以上意见,建议修改后的代码逻辑如下:

    //pdf若是Linearized类型的,需要另存为Normal类型,然后打印
    if (Dr::PDF == fileType()) {
        QString pdfPath = filePath();
        // 使用 pdftocairo 转换为嵌入矢量pdf,解决打印乱码和模糊问题
        
        // 1. 构造唯一的临时文件路径
        QString tmpPdfPath = convertedFileDir() + "/pdftocairoPrint_" + QString::number(QDateTime::currentMSecsSinceEpoch()) + ".pdf";

        // 2. 检查命令是否存在 (可选,视具体部署环境而定,或者直接捕获错误)
        QProcess process;
        
        // 3. 启动转换
        // 建议添加 -q (quiet) 参数减少不必要的控制台输出
        process.start("pdftocairo", QStringList() << "-pdf" << "-q" << pdfPath << tmpPdfPath);
        
        // 4. 等待完成,设置 30 秒超时
        if (!process.waitForFinished(30000)) {
            qWarning() << "pdftocairo timeout or crashed:" << process.errorString();
            process.kill();
            process.waitForFinished();
            return; // 或者考虑回退逻辑:直接使用 pdfPath
        }

        // 5. 检查退出码
        if (process.exitCode() != 0) {
            QString err = process.readAllStandardError();
            qWarning() << "pdftocairo failed with exit code" << process.exitCode() << ":" << err;
            
            // 安全建议:如果转换失败,是否应该回退到直接打印原文件?
            // preview->setPrintFromPath(pdfPath); 
            return;
        }

        qInfo() << "pdftocairo convert success, saved to:" << tmpPdfPath;
        preview->setPrintFromPath(tmpPdfPath);
        
        // 注意:需要在合适的地方(如析构或打印结束)添加 QFile::remove(tmpPdfPath);
    }

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, max-lvs

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

@LiHua000
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 26, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 9cdbe75 into linuxdeepin:release/eagle Jan 26, 2026
5 of 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.

4 participants