Add PPTExporter with PPTX export support for PAGX documents#3361
Add PPTExporter with PPTX export support for PAGX documents#3361OnionsYu wants to merge 141 commits intoTencent:mainfrom
Conversation
…ile mode instead of stretch mode.
…ect ratio instead of tile mode.
…correct x-y order.
…tch when tileModeX or tileModeY is Repeat or Mirror.
…mage is smaller than the shape.
…mage pattern and shadow code.
1bc505f to
cdaf2a3
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3361 +/- ##
==========================================
+ Coverage 78.41% 80.49% +2.07%
==========================================
Files 532 550 +18
Lines 41448 48626 +7178
Branches 12486 13460 +974
==========================================
+ Hits 32502 39142 +6640
- Misses 6391 6750 +359
- Partials 2555 2734 +179 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ents and shadows.
…ngle-pass glyph walk.
…y inlined in PPTWriter.
…calation, textbox containers, and export-option toggles to raise PPT module coverage from 64% to 87%.
…n bakes render at the requested pixel density.
… no longer produce corrupt pptx files.
…render correctly.
…faulting to off to preserve editability.
…s during PPTX export.
… on exported PPTX files.
…ntour containment scans.
…and AABB pruning.
…in the main path.
…ing text-as-path glyphs inline so WeChat still pins the bbox and PowerPoint stops painting dots at corners.
6018ec9 to
0e5392f
Compare
…rimitive and dispatching writes through early returns.
… the group and multi-group paths share bounds marker handling.
shlzxjp
left a comment
There was a problem hiding this comment.
Code Review 第四轮:新发现 14 个问题(1 Critical、6 Major、7 Minor),详见行级评论。
| if (memcmp(data + offset + 4, "IDAT", 4) == 0) { | ||
| break; | ||
| } | ||
| offset += 12 + chunkLen; |
There was a problem hiding this comment.
[Critical] PNG chunk 遍历存在无限循环风险。offset += 12 + chunkLen 中 12 + chunkLen 是 uint32_t 运算,当畸形 PNG 的 chunkLen 接近 0xFFFFFFF4 时,加法回绕为 0,offset 不变导致死循环。GetPNGDPI 也有相同模式(第496行)。建议转为 size_t 后再加:offset += 12 + static_cast<size_t>(chunkLen);
| return; | ||
| } | ||
|
|
||
| auto* mutableText = const_cast<Text*>(text); |
There was a problem hiding this comment.
[Major] const_cast<Text*> 存在未定义行为风险。此处以及第 2198、2228 行(SVGExporter.cpp 的 899、997 行也有同样问题),TextLayout::Layout() 和 getTextLines() 要求非 const 指针,迫使调用方对 const Text* 做 cast。如果 Text 对象是 const 构造的,任何内部修改均为 UB。建议让 TextLayout::Layout / getTextLines / getTextBounds 接受 const Text*,或让导出器从源头持有非 const 引用。
| if (!run->font || run->glyphs.empty()) { | ||
| continue; | ||
| } | ||
| float scale = run->fontSize / static_cast<float>(run->font->unitsPerEm); |
There was a problem hiding this comment.
[Major] run->font->unitsPerEm 未检查是否为 0。当前第 248 行的跳过条件只检查了 !run->font,但损坏字体数据的 unitsPerEm == 0 会导致此处除零产生 inf/NaN,传播到所有字形坐标和 EMU 转换,生成畸形 OOXML。建议在跳过条件中增加 || run->font->unitsPerEm <= 0。
| generated.reserve(static_cast<size_t>(maxCount)); | ||
| for (int i = 0; i < maxCount; ++i) { | ||
| float progress = static_cast<float>(i) + rep->offset; | ||
| float sx = std::pow(rep->scale.x, progress); |
There was a problem hiding this comment.
[Major] std::pow(负数, 分数) 返回 NaN。rep->offset 使 progress 成为分数,当 rep->scale.x 或 rep->scale.y 为负时,std::pow 返回 NaN 并传播到下游几何计算。建议将 scale 基数钳制为非负,或使用 std::abs 并重新应用符号。
| namespace pagx { | ||
|
|
||
| int64_t PxToEMU(float px) { | ||
| return static_cast<int64_t>(std::round(static_cast<double>(px) * EMU_PER_PX)); |
There was a problem hiding this comment.
[Major] 极端像素值存在未定义行为。当 px 超过 ~9.68e14(INT64_MAX / 9525)时,乘积溢出 int64_t 范围,static_cast<int64_t> 从越界 double 转换是 UB。建议钳制:return static_cast<int64_t>(std::clamp(std::round(v), (double)INT64_MIN, (double)INT64_MAX));
| const std::vector<LayerStyle*>& styles) { | ||
| out.openElement("a:r").closeElementStart(); | ||
| out.openElement("a:rPr") | ||
| .addRequiredAttribute("lang", "en-US") |
There was a problem hiding this comment.
[Minor] 语言标签硬编码为 en-US(此处及第 1959 行、PPTBoilerplate.cpp 等处)。对 CJK 为主的文档,PowerPoint 会对正常文本显示拼写检查红线。建议考虑启发式检测(如文本含 CJK 码点则用 zh-CN),或作为选项暴露。
| for (const auto& verb : verbs) { | ||
| if (verb == PathVerb::Move) { | ||
| contours.emplace_back(); | ||
| contours.back().start = points[ptIndex++]; |
There was a problem hiding this comment.
[Minor] points[ptIndex++] 无越界检查。如果 PathData 的 verbs 和 points 数量不一致,ptIndex 会超出 points.size() 导致越界访问。建议添加 assert(ptIndex < points.size()) 或 bounds check。
| if (_prettyPrint) { | ||
| _buf += '\n'; | ||
| } | ||
| _tags.pop_back(); |
There was a problem hiding this comment.
[Minor] _tags.pop_back() 无空检查(此处及第 208、223 行)。open/close 不匹配时会触发 UB。同时第 305 行 _indent 为负时 static_cast<size_t> 回绕为巨大数导致 OOM。建议添加 assert(!_tags.empty()) 和 assert(_indent >= 0) 做调试期防御。
| std::vector<uint8_t> inside; | ||
| size_t n = 0; | ||
|
|
||
| bool isInside(size_t i, size_t j) const { |
There was a problem hiding this comment.
[Minor] isInside(i, j) 直接以 i * n + j 索引 inside 数组,无越界校验。所有当前调用方均由循环索引保护,但建议添加 assert(i < n && j < n) 防止未来误用。
| // Maps dash-to-stroke-width ratios to OOXML preset dash types (ISO/IEC 29500-1, | ||
| // §20.1.10.48 ST_PresetLineDashVal). Thresholds approximate the boundary between | ||
| // dot (≤1.5×), dash (≤4.5×), and long-dash (>4.5×) categories. | ||
| static const char* MatchPresetDash(const std::vector<float>& dashes, float strokeWidth) { |
There was a problem hiding this comment.
[Minor] MatchPresetDash 中 1.5f、2.0f、4.5f 等阈值用于匹配 OOXML 预设虚线类型,但缺少规范来源注释。建议添加引用 ISO/IEC 29500-1 §20.1.10.48(ST_PresetLineDashVal)说明各阈值的对应关系。
| s.reserve(512); | ||
| s += XML_DECL; | ||
| s += "<cp:coreProperties " | ||
| "xmlns:cp=\"http://schemas.openxmlformats.org/package/2006/metadata/core-properties\" " |
There was a problem hiding this comment.
发现好几个函数中都有增加xmlns,排查下ppt文件中是不是重复冗余的
| << " backdrop beneath the layer, so the blend composites\n" | ||
| << " correctly at the cost of turning native content under\n" | ||
| << " the patch into baked pixels (default: off, the blend\n" | ||
| << " mode is silently dropped and the layer renders Normal)\n" |
There was a problem hiding this comment.
[建议] CLI 的 PPT 参数过多,建议精简。当前 5 个 --ppt-* 参数中有 4 个是关闭默认光栅化行为的调试级选项(关掉后蒙版/裁剪/平铺被静默丢弃,普通用户不会想要这些效果):
| 参数 | 关掉后的用户可见效果 | 面向谁 |
|---|---|---|
--ppt-no-bake-mask |
蒙版被丢弃 | 开发调试 |
--ppt-no-bake-scroll-rect |
裁剪区域被丢弃,内容溢出 | 开发调试 |
--ppt-no-bake-tiled-pattern |
跨应用平铺渲染不一致 | 开发调试 |
--ppt-no-bridge-contours |
部分渲染器填充可能出错 | 开发调试 |
建议:
- CLI 只保留
--text-to-path和--ppt-rasterize-blend(用户真正需要做取舍的选项) - 新增
--ppt-raster-dpi <n>(C++ API 已有rasterDPI但 CLI 未暴露,用户确实可能需要调整导出清晰度/文件大小) - 上述 4 个调试参数从 CLI 移除,仅保留在 C++ API 供高级用户使用
这样 PPT 选项从 14 行 help 文本降到 3 行,与 SVG 的 2 个参数量级一致,用户认知成本更低。
There was a problem hiding this comment.
--ppt-no-bridge-contours 参数还是先去掉吧,小众场景,代码层面兼容的,不用对外暴露命令行参数了
| * | ||
| * All mutating methods return *this to allow chaining. | ||
| */ | ||
| class XMLBuilder { |
There was a problem hiding this comment.
PAGXExporter.cpp里面已经有一个XMLBuilder class了
…rtedBlend, and rasterizeWideGamut into a single rasterizeUnsupported option defaulting to false; tile patterns now always bake.
…m --ppt-no-bridge-contours to --ppt-bridge-contours. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cripts, detect CJK language, split PPTWriter into style and text units, and drop unused xmlns on some boilerplate parts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…conf.h. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| size_t ptIndex = 0; | ||
| for (const auto& verb : verbs) { | ||
| if (verb == PathVerb::Move) { | ||
| assert(ptIndex < points.size()); |
| _indent--; | ||
| writeIndent(); | ||
| } | ||
| assert(!_tags.empty()); |
| size_t ptIndex = 0; | ||
| for (const auto& verb : verbs) { | ||
| if (verb == PathVerb::Move) { | ||
| assert(ptIndex < points.size()); |
There was a problem hiding this comment.
[Critical] assert(ptIndex < points.size()) 在 Release 构建中被移除。若 PathData 的 verbs 与 points 数量不匹配(如文件损坏),ptIndex 将越界读取,导致崩溃或脏数据。建议替换为运行时边界检查 + early return:if (ptIndex >= points.size()) break;。第 216 行同理。
| _indent--; | ||
| writeIndent(); | ||
| } | ||
| assert(!_tags.empty()); |
There was a problem hiding this comment.
[Critical] closeElement()/closeElementSelfClosing() 中 assert(!_tags.empty()) 在 Release 构建中被移除。若 open/close 调用不匹配(逻辑 bug 或异常路径),对空 vector 调用 back()/pop_back() 是 UB,通常导致崩溃。建议在 assert 后补上运行时保护:if (_tags.empty()) return *this;。
|
|
||
| void writeIndent() { | ||
| assert(_indent >= 0); | ||
| _buf.append(static_cast<size_t>(_indent * _indentSpaces), ' '); |
There was a problem hiding this comment.
[Critical] 若 _indent 因 close 调用不平衡而变为负数,_indent * _indentSpaces 为负数,static_cast<size_t>(negative) 回绕为极大正数(~18EB),导致 _buf.append 抛出 std::bad_alloc 或 OOM 崩溃。Release 构建中前面的 assert 无效。建议改为:if (_indent < 0) return;。
| return; | ||
| } | ||
|
|
||
| auto* mutableText = const_cast<Text*>(text); |
There was a problem hiding this comment.
[Critical] const_cast<Text*>(text) 存在未定义行为风险。若 Text 对象是 const 构造的(如来自只读内存映射),TextLayout::Layout 内部的写操作将触发 UB。此处及第 660、690 行同理。根本原因是 TextLayout::Layout 接口要求非 const 指针。建议修改 TextLayout::Layout 接口使其接受 const 输入,或确保调用路径中 Text 始终非 const。
| } | ||
|
|
||
| if (!ok) { | ||
| zipClose(zf, nullptr); |
There was a problem hiding this comment.
[Major] ZIP 写入失败时 zipClose 后直接 return false,但此时文件已被 zipOpen(APPEND_STATUS_CREATE) 创建在磁盘上,遗留一个不完整的损坏 PPTX 文件。建议在 return false 前添加 std::remove(filePath.c_str()) 清理残留文件。
|
|
||
| PPTWriterContext* _ctx = nullptr; | ||
| PAGXDocument* _doc = nullptr; | ||
| bool _convertTextToPath = true; |
There was a problem hiding this comment.
[Design] _convertTextToPath 成员默认值为 true,但 PPTExportOptions::convertTextToPath 默认值为 false。虽然构造函数会正确覆盖,但声明处的默认值容易误导读者。建议改为 bool _convertTextToPath = false; 与 Options 保持一致。
|
|
||
| // ── Transform decomposition ──────────────────────────────────────────────── | ||
|
|
||
| PPTWriter::Xform PPTWriter::decomposeXform(float localX, float localY, float localW, float localH, |
There was a problem hiding this comment.
[Minor] 命名规范:decomposeXform 是 static 类方法(PPTWriter.h:771),按项目规范静态方法应大写开头,建议改为 DecomposeXform。同类中 WriteXfrm、WriteBlip 已正确使用大写。
| // Shared contour-to-custGeom emitter used by writePath and writeTextAsPath | ||
| // for the non-bridged or single-group case (when callers haven't already | ||
| // prepared per-group emission themselves). | ||
| void WriteContourGeom(XMLBuilder& out, std::vector<PathContour>& contours, int64_t pathWidth, |
There was a problem hiding this comment.
[Minor] 命名规范:WriteContourGeom 是非静态成员方法(内部访问 _bridgeContours 成员),按项目规范非静态成员方法应小写开头,建议改为 writeContourGeom。
| * elements when glyph outline data is unavailable. The default value is true. | ||
| * elements when glyph outline data is unavailable. The default value is false. | ||
| */ | ||
| bool convertTextToPath = false; |
There was a problem hiding this comment.
[Minor] convertTextToPath 默认值从 true 改为 false 是用户可感知的 Breaking Change。所有通过 C++ API 调用 SVGExporter::ToFile()/ToSVG() 且未显式设置此选项的下游代码,输出将从 path 元素变为 text 元素。建议在 PR 描述或 CHANGELOG 中明确标注此变更。
| return geom; | ||
| } | ||
|
|
||
| void PPTWriter::emitNativeTextShapeFrame(XMLBuilder& out, const Matrix& m, |
There was a problem hiding this comment.
[Minor] emitNativeTextShapeFrame(此处)与 emitTextBoxShapeFrame(第 520 行)构建的 XML 框架高度相似(p:sp > p:nvSpPr > p:cNvPr + p:cNvSpPr + p:nvPr > p:spPr > a:xfrm + a:prstGeom + a:noFill),差异仅在 a:bodyPr 属性。建议提取公共的 emitTextShapeEnvelope 方法减少重复。
新增 PPT 导出功能,支持将 PAGX 文档导出为 PPTX 格式。
主要变更: