Skip to content

Add SVG SMIL animation import and text/mask fidelity fixes - #3808

Open
fengdyli wants to merge 77 commits into
mainfrom
feature/lifengdy_smil_import
Open

fengdyli wants to merge 77 commits into
mainfrom
feature/lifengdy_smil_import

Conversation

@fengdyli

@fengdyli fengdyli commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

概览

把 SVG 的 SMIL 动画接入 SVG → PAGX 转换链路,并补齐文本、遮罩、变换上的转换保真问题。

Layer.anchor 已随 #3807 合入 main,本分支已 rebase 到 main,不再包含任何锚点实现,只保留 SVG 导入相关改动。

动画:SMIL → PAGX 通道

新增 SMILAnimationParser:

  • 时间轴:keyTimes / keySplines / paced / discrete / additive / accumulate / begin-end(含 syncbase、负数 begin、repeatDur、fractional repeatCount、to / by)
  • animateTransform / animateMotion 统一烘焙为 Layer 的 matrix 通道
  • animateMotion 支持往返 keyPoints(0→1→0,非单调)
  • 旋转关键帧步长上限 20°,避免矩阵线性插值走弦导致轨道半径塌陷
  • 圆 / 椭圆 stroke-dashoffset 的相位与方向修正,关键帧按 pathLength
  • 宽高动画锚定到矩形静态左上角(PAGX 的 Rectangle 以中心定位)

保真修复

  • 文本:tspan 分段、多行拆分、每行独立节点、per-run 填充
  • 遮罩:use 引用层上的 mask、mask 描边动画、clipPath 不透明填充
  • 其它:CSS 类规则分离、SVG 导出、HTML 导出

测试

  • test/src/PAGXSVGTest.cpp(2000+ 行)
  • resources/svg/animation/** 与 test/fixtures 的 SVG 夹具
  • CLI:render-frames 命令(SVG 与 PAGX 逐帧对比)

已知缺口

  • 旋转中心在关键帧间变化时,暂未抽取为单个 Layer.anchor,仍烘焙在关键帧平移里

@fengdyli
fengdyli force-pushed the feature/lifengdy_smil_import branch from 557782e to 5ac17d5 Compare November 3, 2026 08:53
@fengdyli fengdyli changed the title Add SVG SMIL animation import, text/mask fidelity fixes and frame-compare tooling Add SVG SMIL animation import and text/mask fidelity fixes Nov 3, 2026
@fengdyli

fengdyli commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Branch rebased onto main after #3807 landed: all anchor code is gone (Layer.h, LayerTransformUtils, HTMLWriterUtils, PAGXAnchorTest unchanged vs main), and the local frame-compare scripts were dropped from the tree (kept out of the repo).

CI status: 9 passed, 1 pending (web/webgpu), and both mac backends fail on PAGXTest.SVGToPAGXAll for the baseline key svg_complex4. That SVG has no SMIL animation, so the difference comes from the SVG fidelity work in this PR (text runs / clipPath fill / CSS class handling). Local OpenGL passes against the existing baseline, so this looks like an expected baseline update rather than a functional regression — please confirm whether the new baseline should be regenerated for both mac backends.

@fengdyli
fengdyli force-pushed the feature/lifengdy_smil_import branch from cdc78e4 to c707d25 Compare November 4, 2026 07:17
@fengdyli
fengdyli marked this pull request as ready for review November 4, 2026 09:05
@fengdyli
fengdyli force-pushed the feature/lifengdy_smil_import branch from c9f1a80 to f6bc541 Compare November 4, 2026 11:04
@codecov-commenter

codecov-commenter commented Nov 4, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.64516% with 182 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.19%. Comparing base (2b7ac3f) to head (3181319).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/pagx/svg/SVGImporter.cpp 66.37% 103 Missing and 53 partials ⚠️
src/pagx/svg/SVGExporter.cpp 62.50% 5 Missing and 4 partials ⚠️
src/pagx/runtime/KeyframeEvaluatorImpl.h 88.00% 0 Missing and 6 partials ⚠️
src/pagx/utils/ExporterUtils.cpp 78.94% 1 Missing and 3 partials ⚠️
src/pagx/PAGXOptimizer.cpp 0.00% 1 Missing and 1 partial ⚠️
src/pagx/html/HTMLExporter.cpp 50.00% 1 Missing and 1 partial ⚠️
src/pagx/svg/SVGParserContext.h 90.00% 2 Missing ⚠️
src/pagx/ppt/PPTExporter.cpp 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3808      +/-   ##
==========================================
- Coverage   84.28%   83.19%   -1.10%     
==========================================
  Files         748      750       +2     
  Lines      112014   109762    -2252     
  Branches    29940    30882     +942     
==========================================
- Hits        94415    91317    -3098     
- Misses      10798    11354     +556     
- Partials     6801     7091     +290     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

double progress = normalizedTime + phaseShift;
double pathProgress = phaseShift > 0.0 ? progress - std::floor(progress) : progress;
if (!keyPoints.empty()) {
pathProgress = keyPoints[i];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P0】keyPoints 单值越界读

keyPoints.size()==1 时,sampleCount 被 2518-2520 行抬到 2,denseCount 直接取 sampleCount(2547),循环到 i==1 时 keyPoints[1] 越界。ParseKeyPointList 会正常接受 keyPoints="0.5",属于外部 SVG 可触发的未定义行为。

建议:keyPoints.size() < 2 时清空列表回退均匀采样,并让 denseCount 单独取 keyPoints.size(),不要复用被兜底修改过的 sampleCount。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。keyPoints 长度 < 2 时清空并回退均匀采样,不再出现 denseCount=2 却读 keyPoints[1] 的越界。回归:animate_motion_invalid_key_points.svg + SVGImport_AnimateMotionSingleKeyPointFallsBack。

Comment thread src/pagx/svg/SVGImporter.cpp Outdated
}

addFillStroke(element, group->elements, inheritedStyle);
addFillStroke(element, group->elements, inheritedStyle, uniformFill);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】uniformFill 时 tspan 的 fill 被丢弃

uniformFill==true 时这里回退到 addFillStroke(element, ...),而它只读取 <text>/继承样式的 fill(2230-2263)。firstFill 只参与比较、从未参与绘制,因此只写在 tspan 上的 fill(或所有 run 同色覆盖)会丢失并回退黑色。

复现:<text><tspan fill="red">A</tspan></text>。

建议:uniform 时用 firstFill 创建组级 Fill,并复用 addFillStroke 的构造逻辑(含 alpha/fillRule)。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。uniformFill 为真时用首个 run 的 fill 建组级 Fill,并把 addFillStroke 的 includeFill 改为 uniformFill && groupFill.empty(),避免重复填充。回归:tspan_fill_only.svg + SVGImport_TspanFillOnly。

auto* currentLine = opensLine ? nullptr : &state->lines[state->currentLineIndex];
bool fillChanged = currentLine != nullptr && currentLine->fill != fill;
if (opensLine || fillChanged) {
state->lines.push_back(TextLine{state->currentX, state->currentY, "", fill});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】同一行内 fill 变化导致文字重叠

fillChanged 新开 run 时直接使用 state->currentX,但裸文本追加不推进 pen(currentX 只在 tspan 有 x/dx 时更新,1316-1325),第二个 run 与第一个 run 同 x 坐标,渲染叠字。

复现:<text x="10" fill="red">Hello <tspan fill="blue">world</tspan></text>。

建议:为 run 记录实际起始 x(按前序 run 排版宽度推进),或同位置拆分时改为单 Text + 多 Fill 的等价结构。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修,但换了实现路径:导入期拿不到字形 advance(Text::glyphRuns 要 layout 之后才有,也没有公开测量接口),所以在没有显式 x 时,同一行内的 fill 变化不再拆 run,整行保留首个 run 的 paint,避免两个 run 同 x 叠字。新增 TextWalkState::positioned 与 SVGImport_TextFillChangeWithoutPosition。代价是该场景颜色取首段;补上测量接口后可恢复按 run 着色。

const auto& staticMatrix = nodeInfo.targetLayer->matrix;
auto* ch = CreateChannelForType(doc, ChannelValueType::Matrix, "matrix");
for (size_t i = 0; i < paramSets.size(); ++i) {
Matrix m = TransformParamsToMatrix(type, paramSets[i]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】viewBox 的 contentMatrix 被 matrix 通道覆盖

单顶层图层且 needsContentTransform 时,SVGImporter.cpp:274 会把 contentMatrix 预乘到该层 matrix;这里非 additive 关键帧只烘焙元素自身 transform(2353-2363)。运行时 LayerBuilder::applyMatrix 在 mix==1 时用关键帧矩阵替换整个 pose,动画一开始就丢失 viewBox 的缩放/偏移。

复现:<svg width="24" height="24" viewBox="0 0 1024 1024"><g><animateTransform .../></g></svg>。现有夹具 viewBox 均为 1:1,未覆盖。

建议:用独立包装层承载 content 变换,或生成关键帧时统一预乘 contentMatrix。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。SVGParserContext 记录 _contentMatrix 与是否已预乘到单顶层图层;parseAnimateTransform 对替换式(非 additive)关键帧预乘 viewBox 映射,动画开始不再丢缩放。回归:viewbox_transform_animation.svg + SVGImport_ViewBoxTransformAnimation(48→24 的 viewBox 下 translate 24 落在 12)。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

复核最新 head:transform 分支已修(2580-2584),但 animateMotion 分支(2832-2835)仍只有 additive 合成,没有 contentMatrix 预乘;单顶层图层 + 缩放 viewBox + animateMotion 仍会丢 viewBox。补充:hoist 场景还会双重应用 contentMatrix(另见本轮新评论)。

// downstream consumers once merging is done.
for (auto* layer : convertedLayers) {
RestoreMaskLayerVisibility(layer);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】恢复遮罩可见性后,HTML/PPT 导出把遮罩当内容绘制

RestoreMaskLayerVisibility 把遮罩层置为 visible=true,但只有 SVGExporter 新增了 isMaskReferenceLayer 跳过;PPTExporter.cpp:712 与 HTMLWriterLayer.cpp:2890 只跳过"作为 layer->mask 子节点"的遮罩,SVG 导入的顶层遮罩兄弟层会被当普通可见内容输出。clip-path 还带 EnsureOpaqueClipFill 的不透明白色 Fill,会直接盖住画面。

建议:给三个导出器统一加 mask 引用集合,或把遮罩挂回 owner children 并保持不可见(这也更符合加载器预期)。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。CollectMaskReferenceLayers 提到 ExporterUtils 共用,PPTExporter::writeDocument 与 HTMLExporter 顶层遍历跳过被 Layer::mask 引用的层,SVGExporter 改用同一实现。依据是运行时语义:LayerBuilder::resolvePendingMasks 强制置 visible 并 setMask,tgfx 对 maskOwner 层不绘制,所以它只是遮罩定义、不是内容。

Comment thread src/pagx/PAGXOptimizer.cpp Outdated
layer->height = h;
// Detaching the reference orphans the mask layer, and its geometry already lives in the
// scrollRect above, so hide it to keep it out of the SVG output.
m->visible = false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】新增赋值不可达,且 SVG 矩形遮罩不再折叠为 scrollRect

621 行已有 if (m->visible) return false;,所以 660 行永远执行不到。同时本 PR 在 SVGImporter.cpp:264 把 SVG 遮罩恢复为 visible,所有 SVG 矩形遮罩都会被 621 拦下,无法再转 scrollRect(pagx import 在 CommandImport.cpp:652 会调用优化器)。

建议:明确策略——若保留优化,需在转换前隐藏或区分"导入期临时可见";否则删除这行死代码,并补测试说明该优化对 SVG 不再生效。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修一半。不可达的 m->visible = false 已删除(621 行 if (m->visible) return false; 会先返回)。矩形遮罩不再折叠为 scrollRect 是恢复遮罩可见性的必然结果,用 rect_mask_optimization.svg + SVGImport_RectMaskIsNotFoldedIntoScrollRect 锁定现状,避免后续误判。

bool fillFreeze = (fillStr == "freeze") || IsIndefiniteRepeat(animElement);
KeyValue baseValue = ReadBaseValue(target);
for (auto* ch : channels) {
AddBaseValueKeyframes(ch, target.valueType, baseValue, beginFrames, outEndFrame, fillFreeze);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】additive="sum" 的 float/color 语义错误

这里对所有通道(包括 additive 通道)都补基值关键帧;MergeAdditiveFloat/Color(969 起)在时间并集上做 base + additive,begin 之前会得到 base + base(如 opacity 变成 2)。另一个问题是只有一个 sum 通道时(1227 附近)没有 base 参与,会被当作绝对值输出,静态基值丢失。

建议:sum 通道 begin 前写单位元(0/全 0 颜色),单通道先与静态基值合成。矩阵路径已在 2354-2358 做 staticMatrix * m,float/color 缺少对等逻辑。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。MergeAdditiveFloat/Color 在 additive 轨道 begin 之前按单位元(0 / 全 0 色)处理,不再把基值重复叠加;Matrix / Bool 维持原规则。相关 SVGImport_AdditiveSum* 全绿。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

复核最新 head(3181319e):仍未修复。AddBaseValueKeyframes 对所有通道(含 additive)在 begin>0 时写入 frame 0 基值 key(1571-1576),因此 additiveCh->keyframes.front().time 恒为 0,MergeAdditiveFloat/Color 中“begin 前单位元”的判断(1118-1124)永远不会命中,base+base 仍在;单条 sum 通道也仍未与静态基值合成。现有 additive_sum*.svg 都是 begin=0,覆盖不到。建议补 begin>0 的用例后再确认修复。

Comment thread src/pagx/svg/SMILAnimationParser.cpp Outdated
if (!obj->channels.empty()) {
animation->objects.push_back(obj);
if (elementIndefinite && layerEndFrame > 0) {
infiniteObjects.push_back({obj, layerEndFrame});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】indefinite 平铺周期取错,每圈多一段静止

infiniteObjects 的 period 用的是 layerEndFrame(含 begin),不是 durFrames。dur="1s" begin="1s" repeatCount="indefinite" 时 duration=90 帧、keyframes 只在 30-90,循环回 0 后前 30 帧静止;即使存在更长动画,平铺也会在每圈之间插入 begin 帧的静止。

建议:period 取该 SMIL 元素自身的 durFrames,并确保 duration 至少覆盖一个完整周期。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。infiniteObjects 的 period 改为元素自身 dur(indefinitePeriod),animate / animateTransform / animateMotion 三个分支都记录,不再用含 begin 的 layerEndFrame,每圈不会多一段静止。

float ax = 0.0f, ay = 0.0f, bx = 0.0f, by = 0.0f;
const bool aHasPivot = SolveMatrixFixedPoint(a, &ax, &ay);
const bool bHasPivot = SolveMatrixFixedPoint(b, &bx, &by);
if (aHasPivot && bHasPivot && std::abs(ax - bx) < 0.5f && std::abs(ay - by) < 0.5f) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】全局运行时矩阵插值行为变更(影响所有 PAGX 文档)

这是播放器共享路径,不是 SVG 专用。bHasPivot && IsIdentityTransform(a) 分支会把"identity→translate+rotate"的线性平移改成绕 b 的固定点旋转;端点不变,但中间帧偏离。HTML 导入的 from { transform:none } to { translate(...) rotate(...) }(origin 已烘焙进矩阵)会命中该路径。

PR 没有为非 SVG 来源的 matrix 动画补回归测试,也没有在描述中单列此运行时改动。

建议:去掉 identity 单边推断,或给该逻辑加数据来源标记(仅 SVG 导入的通道启用),并补 PAGX 手写 matrix / HTML CSS 动画回归用例。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

采纳「加开关」的方案,已按通道粒度把两种语义分开:

  • Channel 新增 bool pivotInterpolation(默认 false),只有 SVG 的 animateTransform / animateMotion 在解析时置 true;
  • 运行时拆成两个入口:LerpKeyframeValueWithMode<T> 对非 Matrix 类型忽略该标志;Matrix 特化在 pivotInterpolation == false 时走原来的分解线性插值(CSS 语义),为 true 时才用固定点展开;
  • 该标志参与 PAGX 序列化(pivotInterpolation="true",spec/pagx.xsd 同步),否则存盘再打开会退回线性模式。

验证用的两端数值:

场景 线性(默认) 固定点(SVG)
none -> rotate(90, 50, 50),t=0.5 ty = 0(走弦 20.7px) ty = -20.71(原地转)
none -> translate(50,50) rotate(90),t=0.5 (25, 25)(CSS 语义) (35.36, 14.64)

用例:PAGXRuntimeTest.MatrixInterpolationRespectsPivotFlag 覆盖两种模式与两条曲线;PAGXSVGTest.SVGImport_MatrixPivotFlagRoundTrip 锁定 SVG 导入置 true + PAGX 往返保真。全量 1301 passed。

也就是说:非 SVG 来源(HTML 导入、手写 PAGX)恢复成改动前的线性插值,行为不再被这条修复影响。

if (original.empty() || repeatCount <= 1.0) {
return;
}
int fullRepeats = static_cast<int>(std::floor(repeatCount));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】无界 repeat / 角度导致内存爆炸与整数溢出

fullRepeats 没有上限,repeatCount="1000000" 或超长 values 会按 original.size() 成倍复制关键帧;steps = ceil(|delta|/20)(2313)对 rotate 角度 1e9 会生成 5e7 个关键帧;dur="1e999" 时 round(inf) 转 int64 是 UB。SVG 可能来自不可信输入。

建议:入口校验有限值,并对单通道关键帧数、重复次数、总时长、旋转切分段数设上限,超限降级并输出 warning。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。新增 kMaxDurationSeconds(24h)、kMaxRepeatCount(1000)、kMaxRotationSteps(10000);dur 解析补 isfinite 校验,ExpandRepeatCount 入口截断,旋转切分段数封顶,避免超长时长/超大角度导致的内存爆炸与整数溢出。

Comment thread src/pagx/svg/SMILAnimationParser.cpp Outdated
}
} else {
// additive="replace": this channel becomes the new base.
base = (*channels)[idx].channel;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】同属性多个 replace 通道只保留最后一个

同属性两段不重叠的动画(如 opacity 0-1s 与 1-2s)会在这里丢弃前一个 base,result 只输出最后一条;第一段动画及其 freeze 区间完全消失,begin 前还会补静态基值。这是手写 SVG 常见的分段时间轴写法。

建议:按时间区间把两段拼进同一 channel(前段在 end 处 Hold 截断,后段 begin 接管)。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。additive="replace" 时若新段起点晚于旧段,先把旧段按新段起点 Hold 截断再拼接(ApplyEndTimeLimit + AppendTypedKeyframes);同起点仍保持替换语义,避免 morph_entrance 那类同帧冲突(该用例仍绿)。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

复核最新 head:仍未修复。ChannelStartFrame(next)(503-526)返回 keyframes.front().time;第二段 begin>0 时该通道同样有 frame 0 基值 hold(1571-1576),所以 handover 恒为 0,handover <= ChannelStartFrame(base) 成立,仍走“同起点替换”,第一段被丢弃。且 AppendTypedKeyframes(496-501)直接 append、不排序。没有分段 replace 的回归测试。

Comment thread src/pagx/svg/SMILAnimationParser.cpp Outdated
double repeatDurSeconds = SMILAnimationParser::parseSMILClockValue(repeatDurStr);
if (repeatDurSeconds > 0.0 && durSeconds > 0.0) {
double fromDur = repeatDurSeconds / durSeconds;
if (fromDur < 1.0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】repeatDur 语义不完整

repeatDur < dur 时 fromDur 被抬回 1,未按 repeatDur 截断;repeatDur="indefinite" 解析为 -1 被忽略,而 IsIndefiniteRepeat 已把它当作 indefinite(说明本意是支持),结果播放一遍后冻结。

建议:repeatDur < dur 直接换算成 end 截断;repeatDur="indefinite" 与 repeatCount="indefinite" 等价处理。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。① repeatDur="indefinite" 现在等价于 repeatCount="indefinite";② 去掉 fromDur 被抬回 1.0 的逻辑,repeatDur 短于一次迭代时按分数截断,属性/变换/运动三个 parse 分支都加了 ApplyEndTimeLimit。

Comment thread src/pagx/svg/SMILAnimationParser.cpp Outdated
}
break;
}
default: {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】Bool 通道不展开 repeatCount

Bool/Int/String 落到 default 分支,但 outEndFrame 仍按 repeatCount 拉长(2062),TileChannelKeyframes 也没有 Bool 分支。display/visibility 的重复闪烁只播一遍,后半段静止。

建议:为 Bool 等类型补复制式展开/tile,或不拉长 outEndFrame,避免时间轴与内容不一致。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。ExpandRepeatCount 补 Bool 分支(按周期复制关键帧 + 小数尾帧),TileChannelKeyframes 也补 Bool 平铺,时间轴长度与实际关键帧一致。

if (denseCount > 512) denseCount = 512;
}
for (int i = 0; i < denseCount; ++i) {
double normalizedTime = static_cast<double>(i) / static_cast<double>(denseCount - 1);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】animateMotion 忽略 keyTimes

整个 parseAnimateMotion 没有读取 keyTimes(对比 parseAnimate:1862、parseAnimateTransform:2252),keyPoints 的到达时刻被强制均匀分布,显式 keyTimes 以及 calcMode/keySplines 全部丢失,速度曲线会与浏览器不一致。

建议:读取 keyTimes 并在 keyPoints 分支用它计算 time(长度不匹配时回退均匀分布)。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。parseAnimateMotion 读取 keyTimes,与 keyPoints 长度一致时按它分布采样时间,否则回退均匀分布。回归:animate_motion_key_times.svg + SVGImport_AnimateMotionKeyTimes(0/24/120 三个时间点)。

auto* anchor_channel = CreateChannelForType(doc, ChannelValueType::Float,
horizontal ? "position.x" : "position.y");
for (const auto& key : size_channel->keyframes) {
AppendKeyframe(anchor_channel, ChannelValueType::Float, key.time, anchor + key.value * 0.5f,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】width/height 的锚定通道丢失 bezier 句柄

这里用 AppendKeyframe(..., key.interpolation, {}, {}) 复制 size 关键帧,但丢弃了 key.bezierOut/bezierIn。calcMode="spline" 时尺寸走缓动、位置走线性,矩形"固定边"会在动画过程中漂移。

建议:一并复制 bezier 句柄。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。width/height 锚定到 position 通道时复制 key.bezierOut / key.bezierIn,缓动不再让固定边漂移。

}
outEndFrame = beginFrames + static_cast<Frame>(durFrames * repeatCount);
} else {
outEndFrame = beginFrames + durFrames;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】负 begin + indefinite 的 float/color 动画未折叠相位

transform/motion 在 2247-2250、2484-2486 都做了 phaseShift 折叠,parseAnimate 没有。begin="-1.3s" dur="4s" repeatCount="indefinite" 时 timeline 只有 0.7 个周期,循环回绕会跳变。

建议:抽公共函数,float/color 同样折叠相位后从 frame 0 开始铺满整个周期。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。新增 ShiftChannelPhase<T>:float/color 在负 begin + indefinite 时按相位重采样、从 0 帧铺满一个周期,与 transform/motion 的现有处理一致。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

复核最新 head:ShiftChannelPhase(533-552)仍会把首尾 key 映射到同一相位。以 begin=-1s、dur=2s 为例:beginFrames 已置 0,keyTimes 0;1 的两个 key 时间为 0 和 120,phaseShift=0.5,二者都映射到 time=60;且 value 用的是 evaluateAt(新时间),不是相位重采样。轨道会变成常量。negative_begin.svg 不是 indefinite,覆盖不到。

Comment thread src/pagx/svg/SMILAnimationParser.cpp Outdated
return sets.back();
}

static void ApplyEndTimeLimit(Channel* channel, ChannelValueType valueType, Frame endFrame) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】ApplyEndTimeLimit 的类型覆盖与重复时间戳

只处理 Float/Color/Matrix,Bool/Int/String 的 end 截断失效;另外当 endFrame 已有 keyframe 时,这里会再 push 一个同时间戳的 Hold key(按 47 行注释,编辑器轨道按时间戳去重会丢掉 key)。

建议:补类型分支;先查找同时间戳 key 并覆盖,而不是追加。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。四份重复实现合并为模板 ApplyEndTimeLimitTo<T>;补 Bool 分支;末尾同时间戳改为覆盖而非追加,避免以 time 为键的轨道丢帧。

Comment thread src/pagx/svg/SMILAnimationParser.cpp Outdated
for (const auto& entry : layerEntries) {
groupedByTarget[entry.targetId].push_back({entry.channel, entry.additive});
}
for (auto& [targetId, channels] : groupedByTarget) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】AnimationObject 生成顺序不确定

_animatedNodeMap(2726 行)与这里的 groupedByTarget 都是 unordered_map,AnimationObject 的生成顺序不稳定,输出的 PAGX/SVG 不可复现,测试对 objects[0] 的断言也可能随平台/实现漂移。

建议:按 targetId 或文档顺序排序后再生成对象。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。groupedByTarget 遍历前按 targetId 排序,生成顺序不再依赖 unordered_map 迭代顺序,导出可复现。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

复核最新 head:targetId 已排序(3115),但外层 for (const auto& pair : animatedNodeMap)(2996)仍是无序 map,跨元素的 AnimationObject 顺序仍依赖指针哈希;导出器也没有排序。建议把外层也按稳定 key 排序。

Comment thread src/pagx/svg/SVGImporter.cpp Outdated
if (layer->id.empty()) {
layer->id = generateUniqueId("anim_layer");
}
_document->registerNode(layer, layer->id);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】 重复实例只保留最后一个动画

convertUseContainer 会对每个 <use> 重新 convertToLayer 同一 DOM 子树(1531-1540),registerAnimatedElement 用 element.get() 覆盖 _animatedNodeMap(3746 行),只有最后一个实例有动画;若元素带 id,registerNode 还会重复注册并报 Duplicate node id。

建议:同一 DOM 元素维护实例列表,或为每个实例生成唯一 id 并分别建 AnimationObject。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已修。_animatedNodeMap 改为 map<DOMNode*, vector<AnimatedNodeInfo>>,一个 DOM 元素对应多个实例;并补上 <use> 引用形状时的注册(原先只走叶子转换,从不调用 registerAnimatedElement,动画整体丢失)。回归:use_instance_animations.svg + SVGImport_UseInstanceAnimations。

@fengdyli
fengdyli force-pushed the feature/lifengdy_smil_import branch from f2ea0a5 to 3181319 Compare November 6, 2026 09:14
// SMIL additive="sum": the animated transform post-multiplies the base value, so each
// keyframe composes as static * animated.
m = staticMatrix * m;
} else if (ctx.isContentMatrixBakedIntoLayer()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】viewBox + rotate-about-point 会重复应用 contentMatrix

单顶层图层且 needsContentTransform 时,SVGImporter.cpp:273-277 会把 contentMatrix 预乘到该层并置 _contentMatrixBakedIntoLayer=true;但 rotate 中心动画会在 SVGImporter.cpp:549 把真正的动画目标 hoist 到 inner layer(contentMatrix 留在 pivot 父层)。这里又给 inner layer 的关键帧乘了一次 contentMatrix,再经 NormalizePivotKeyframes(SVGImporter.cpp:3828-3861)共轭后,合成结果为 contentMatrix² · M,viewBox 缩放/偏移被平方;静态 BakeFirstMatrixKeyframe 也会固化同样的双重矩阵。

现有 viewbox_transform_animation.svg 只用 translate、animate_transform_rotate_with_center.svg 的 viewBox 为 1:1,均覆盖不到。

建议:预乘 contentMatrix 仅在动画目标就是承载它的那一层时进行;hoist 场景应跳过,或把 contentMatrix 放到 inner layer;补“缩放 viewBox + 旋转中心”用例。

if (IsClockToken(token)) {
double t = parseSMILClockValue(token);
if (!info.hasClockBegin || t < info.clockBegin) {
info.clockBegin = t;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P1】begin/end 仍未做有限值与上限校验

ResolveAllTimings 的 clockBegin/clockEnd(659-675)直接使用 parseSMILClockValue;四个解析分支的 beginFrames(1982/2351/2458/2716)和 endLimit(2277-2279)直接强转 Frame。新增的 kMaxDurationSeconds/kMaxRepeatCount/kMaxRotationSteps 只覆盖 dur/repeat/角度,begin/end 没有检查。

可触发:

  • begin="1e999s" 或 end="1e999s" → atof 得到 inf,round(inf) 转 Frame 是 UB;
  • begin="1000000000s" dur="1s" repeatCount="indefinite" → maxEnd≈6e10、period=60,TileTypedChannel(2901-2927)会循环约 1e9 次并生成约 2e9 个 key,直接 OOM。

建议:对 begin/end 增加 isfinite 与 ±kMaxDurationSeconds(或相对文档长度)校验,并给平铺 key 数加上限。

// referenced element's SMIL animations belong to this instance as well. Registering them on
// the <use> element itself would find nothing: the animations live on the referenced node.
if (refIt != _elementsById.end() && refIt->second != element) {
registerAnimatedElement(refIt->second, layer);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】 引用带动画的容器时,容器自身动画仍丢失

convertUseContainer(1537-1548)只遍历 ref 的 children,没有对 ref 自身调用 registerAnimatedElement;本轮补的 leaf 分支(574-579)只覆盖形状引用。因此 <defs><g id="g"><animateTransform .../></g></defs><use href="#g"/> 或 symbol 上的 animate/animateTransform 不会进入 _animatedNodeMap,buildAnimation 直接跳过。

use_instance_animations.svg 使用的是 <rect> leaf,覆盖不到容器引用。建议在 convertUseContainer 返回前对 refElement 调用 registerAnimatedElement(refElement, group),并补 <use> + <g> 用例。

// to continue the previous run's advance, which the importer cannot measure, and the two runs
// would overlap. In that case the line keeps the first run's paint.
auto* currentLine = opensLine ? nullptr : &state->lines[state->currentLineIndex];
bool fillChanged = currentLine != nullptr && currentLine->fill != fill && state->positioned;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】positioned 是整棵 text 的全局标志,混合显式 x 后仍会叠字

fillChanged 依赖 state->positioned(1335),但任意 tspan 有 x 就把 positioned 置 true 且之后不再重置(1350-1352,声明在 SVGParserContext.h:149-151)。

复现:<text x="10">A<tspan x="30" fill="blue">B</tspan><tspan fill="green">C</tspan></text>。C 没有 x,currentX 仍是 30,但 positioned 已为 true,新 run 从 x=30 开始与 B 重叠。

现有 text_fill_change_inline.svg 完全没有 tspan x,覆盖不到。建议按 run 记录实际起点(或在追加文本后清除 positioned),并补混合定位用例。

}
text->textAnchor = textAnchorValue;
auto fill = run.fill.empty() ? elementFill : run.fill;
if (!uniformFill && fill != "none") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P2】新的 per-run/uniform 文本 fill 只支持纯色,fill-opacity 与渐变仍丢失

per-run Fill(1259-1272)只创建 SolidColor,没有设置 Fill::alpha;uniform 的 groupFill(1291-1312)同样只 parseColor,而 parseColor 不支持 url(#...)。TextLine 也只保存 fill 字符串(SVGParserContext.h:134-141),没有 fill-opacity/ColorSource。

因此 <tspan fill="red" fill-opacity="0.5"> 会按不透明红渲染,fill="url(#grad)" 会退化为默认色;另外所有 run 都是 fill="none" 时 firstFill 为空、uniformFill 仍为 true,addFillStroke 会给整组补上 <text> 的 fill。

建议在 TextLine 携带 opacity/ColorSource,或对带 opacity/gradient 的 tspan 不拆分、复用 addFillStroke 的构色逻辑,并修正 all-none 判定。

auto* ch = CreateChannelForType(doc, ChannelValueType::Matrix, "matrix");
// Motion samples are baked around the layer origin; keep the same fixed-point spelling as
// transform animations so the path is followed without sliding.
ch->pivotInterpolation = true;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P3】parseAnimateTransform 重复设置 pivotInterpolation,注释写的是 motion

2567-2573 连续两次 ch->pivotInterpolation = true;,第一段注释 "Motion samples are baked..." 却位于 parseAnimateTransform;parseAnimateMotion 实际没有设置该标志。不是功能错误,但属于明显的复制残留,请清理并确认 motion 是否确实不需要 pivot 模式。


// Internal bridge for importers that register addressable nodes while building a document. Kept
// out of PAGXDocument's public surface so public headers do not name the parser classes.
struct DocumentAccess {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P3】公共头暴露了 detail::DocumentAccess::RegisterNode

include/pagx/PAGXDocument.h:49-58 在安装的公共头中定义了 detail::DocumentAccess,RegisterNode 是 public static;任何包含该头的代码都能调用 pagx::detail::DocumentAccess::RegisterNode(doc, node, id),绕过 PAGXDocument 的公开 API。detail 命名空间不是访问控制。

建议公共头只前向声明 DocumentAccess,把定义放到内部头;或改为内部 friend 函数。


} // namespace

void CollectMaskReferenceLayers(const PAGXDocument& document,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P3】遮罩引用层跳过只覆盖顶层,嵌套 sibling mask 仍会被导出

CollectMaskReferenceLayers(224-236)扫描整棵树,但 PPT/HTML 只在 doc->layers 顶层跳过(PPTExporter.cpp:903、HTMLExporter.cpp:270);嵌套写入时只判断 child == layer->mask(PPTExporter.cpp:870、HTMLWriterLayer.cpp:2889),不查全局 mask 集合。PAGX 允许 mask 作为 owner 的 sibling(optimizer 的 sharesParent 分支),这类嵌套 sibling mask 仍会被当内容绘制。

对 SVG 导入的顶层遮罩已修好;建议在 writeLayer 的 child 循环里也查询 mask 集合,或统一把 mask 挂到 owner 下。

Comment thread test/src/PAGXSVGTest.cpp
* Splitting there would need the previous run's advance width, which the importer cannot measure,
* and the two runs would overlap.
*/
PAGX_TEST(PAGXSVGTest, SVGImport_TextFillChangeWithoutPosition) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

【P3】新增测试覆盖不到本轮仍未修好的路径

缺少以下用例:

  • begin>0 的 additive="sum"(现有 additive_sum*.svg 全部 begin=0)
  • 第二段 begin>0 的分段 replace
  • negative begin + repeatCount/repeatDur="indefinite"(negative_begin.svg 不是 indefinite)
  • animateMotion + viewBox 缩放(viewport_transform_animation.svg 只测 transform)
  • pivot-hoist + viewBox(本轮重复 contentMatrix 的场景)
  • 输出对象顺序

另外 SVGImport_TextFillChangeWithoutPosition 只断言文本数量/内容,不校验 paint;SVGImport_UseInstanceAnimations 只校验 channel 值,不校验两个实例的位置。

建议为上述每条补最小夹具,至少把“已修/未修”的边界用测试锁定。

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