Add PAGXOptimizer for PAGX structure simplification#3393
Add PAGXOptimizer for PAGX structure simplification#3393OnionsYu wants to merge 29 commits intoTencent:mainfrom
Conversation
…ons and preserving small float values.
…ature and merge-vector allocation.
OnionsYu
requested review from
Hparty,
domchen,
kevingpqi123 and
shlzxjp
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3393 +/- ##
==========================================
+ Coverage 78.41% 78.85% +0.43%
==========================================
Files 532 540 +8
Lines 41448 43109 +1661
Branches 12486 12860 +374
==========================================
+ Hits 32502 33993 +1491
- Misses 6391 6454 +63
- Partials 2555 2662 +107 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
OnionsYu
force-pushed
the
feature/onionsyu_pagx_verify2
branch
from
55a1c0d to
8df6c42
Compare
OnionsYu
force-pushed
the
feature/onionsyu_pagx_verify2
branch
from
d04ef06 to
7055b85
Compare
…_verify2 # Conflicts: # test/baseline/version.json
OnionsYu
force-pushed
the
feature/onionsyu_pagx_verify2
branch
from
702f412 to
66de2e5
Compare
OnionsYu
changed the title
shlzxjp
left a comment
shlzxjp
left a comment
There was a problem hiding this comment.
初次评审反馈：新增 PAGXOptimizer 模块整体思路清晰，保守性原则和幂等保证都到位。以下是按严重度梳理的 Blocker / Major 问题行级评论，建议合入前优先处理。Minor/Nit 级别的问题后续单独提。
…sk canonicalization, and expand option coverage tests.
…_verify2 # Conflicts: # src/cli/CliUtils.cpp resolved by feature/onionsyu_pagx_verify2 version
|
[Nit] 新 N4 — commit
两类变更的回归风险不同（前者是数值精度级别、后者是结构级别），混在一个 version bump 里 bisect 或回滚都不便。 建议（下次类似变更时）： 把"精度修复"与"结构优化"的 baseline 更新拆成独立 commit，方便未来 |
OnionsYu
force-pushed
the
feature/onionsyu_pagx_verify2
branch
3 times, most recently
from
578e89f to
52effa1
Compare
# Conflicts: # test/baseline/version.json resolved by main version
…ingle-line control flow statements.
…round-trip across the full rule set.
…optimizer convergence warnings.
| return false; | ||
| } | ||
|
|
||
| bool LayoutNodeHasConstraints(const LayoutNode* node) { |
There was a problem hiding this comment.
LayoutNodeHasConstraints 遗漏了 percentWidth/percentHeight 检查。Path 和 Rectangle 都继承自 LayoutNode，该基类含 float percentWidth = NAN 和 float percentHeight = NAN 字段。当一个 Path 设了 percentWidth=50 但 6 个约束字段都是 NaN 时，此函数返回 false，导致 TryCanonicalizePath 将其错误改写为固定尺寸 Rectangle，丢失 percent 布局语义。同理影响 TryConvertRectMaskToScrollRect。
LayoutNode 自身已有 hasConstraints() 方法（src/pagx/LayoutNode.cpp:35-38）正确包含了 percent 检查，建议直接复用 node->hasConstraints() 替代此手写版本。
| if (group->skewAxis != 0) { | ||
| return false; | ||
| } | ||
| if (!std::isnan(group->width) || !std::isnan(group->height)) { |
There was a problem hiding this comment.
IsDefaultTransformGroup 同样遗漏了 percentWidth/percentHeight 检查。Group 继承自 LayoutNode，如果 Group 设了 percentWidth=50 但 width 为 NAN，此函数会误判为 default-transform group，导致 UnwrapRedundantFirstGroup 或 MergeAdjacentGroups 错误处理（展开或合并携带 percent 布局语义的 Group）。
建议补充：
if (!std::isnan(group->percentWidth) || !std::isnan(group->percentHeight)) return false;| if (layer->mask != nullptr) { | ||
| return true; | ||
| } | ||
| if (layer->maskType != MaskType::Alpha) { |
There was a problem hiding this comment.
maskType 检查未绑定 mask 指针存在性。当 mask == nullptr && maskType != Alpha 时（可能由构造器默认值异常或未来代码路径解除 mask 后残留），此条件仍返回 true，永久阻止该 Layer 被识别为 shell。
建议改为：
if (layer->mask != nullptr && layer->maskType != MaskType::Alpha) return true;即把 maskType 的语义绑定到 mask != nullptr 时才生效。
|
|
||
| // ============================================================================ | ||
| // Local helpers. `HasLayerOnlyFeatures`, `IsLayerShell`, `IsPainter`, and | ||
| // `HasPainter` are the shared definitions from pagx/utils/VerifyUtils.h |
There was a problem hiding this comment.
注释表达方向有误。当前措辞像是 optimizer 是 VerifyUtils 的使用方、verify/resolve 是"源头"。实际这些谓词是共享定义，optimizer 也是定义方之一。
建议改为：// Shared layer/element classification predicates — single source-of-truth in pagx/utils/VerifyUtils.h, used by verify, resolve, and optimizer.
| // Layer -> Group conversion | ||
| // ---------------------------------------------------------------------------- | ||
|
|
||
| // Wraps a downgradable shell Layer's contents in a new Group, transferring customData. The |
There was a problem hiding this comment.
注释 "caller is responsible for releasing the old Layer reference" 不准确。doc->nodes 仍通过 unique_ptr 持有该 Layer，调用方不能也不应该释放它。实际语义是调用方须将 Layer 从拥有它的 layers/children 向量中移除。
建议改为：// Wraps a downgradable shell Layer's contents in a new Group, transferring customData. Caller must have verified the layer is downgradable and is responsible for removing it from the owning list.
| * Number of rule iterations actually executed (both on the root layer list and on each | ||
| * Composition's layer list). Useful as a monotonic signal for regression / telemetry. | ||
| */ | ||
| int iterationsUsed = 0; |
There was a problem hiding this comment.
iterationsUsed 是所有 layer list（root + 每个 Composition）的累加值，但 maxIterations 是每段独立上限。建议注释中明确此为累加值，不可直接与单段阈值比较：
// Cumulative iteration count across all layer lists (root + each Composition).
// Not directly comparable to any single-segment cap; use `converged` for the
// convergence signal instead.
3 participants
新增 PAGXOptimizer 模块，用于自动简化 PAGX 文件结构（解包冗余 Group、合并可合并节点等）。
主要变更：