Skip to content

Fix HTML importer SVG layer hierarchy and radial-gradient rendering fidelity. - #3634

Open
OnionsYu wants to merge 6 commits into
Tencent:mainfrom
OnionsYu:feature/onionsyu_import_html6
Open

Fix HTML importer SVG layer hierarchy and radial-gradient rendering fidelity.#3634
OnionsYu wants to merge 6 commits into
Tencent:mainfrom
OnionsYu:feature/onionsyu_import_html6

Conversation

@OnionsYu

Copy link
Copy Markdown
Collaborator

本 PR 包含两处 HTML 导入保真度修复。

  1. 保持内联 SVG 兄弟形状的层级对称。resolve 内联 SVG 时,之前会把第一个形状层的内容直接摊平进父 Layer,其余兄弟各自包一个 Group,导致同级形状层级不一致(如波浪的两条 path 一条裸露、一条被 Group 包裹)。现改为对所有兄弟形状层统一包成平级 Group(含 skew 的仍保留为 Layer),与 PAGXOptimizer::DowngradeShellChildren 的处理方式对齐,保证源层级得以保留。

  2. 修正 HTML 径向渐变的圆形 extent 尺寸计算,以及导出时透明色标透过黑色渐隐的问题。

新增了对应的 PAGXHTMLImporterTest 测试用例。

…extent-keyword radii, and transparent-stop RGB borrowing.
@codecov-commenter

codecov-commenter commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.85520% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.06%. Comparing base (d578f92) to head (4a207be).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/cli/CommandResolve.cpp 70.00% 3 Missing and 6 partials ⚠️
src/pagx/html/importer/HTMLValueParser.cpp 86.36% 1 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3634      +/-   ##
==========================================
+ Coverage   82.99%   83.06%   +0.07%     
==========================================
  Files         712      712              
  Lines       94876    95021     +145     
  Branches    26597    26610      +13     
==========================================
+ Hits        78742    78930     +188     
+ Misses      10426    10402      -24     
+ Partials     5708     5689      -19     

☔ 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.

@shlzxjp shlzxjp left a comment

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.

深度代码评审(团队评审汇总)

对本 PR 的两处修复做了根因级审查、并对渲染器插值空间做了源码级验证,编译(PAGFullTest)通过。整体质量较高、修复是从源头改而非打补丁。以下为逐行发现的问题,按严重级别标注。

MediumCircleExtentRadiusPx 的 closest-side 在圆心位于盒外时返回负半径;resolve 的 skew 分支「部分降级」会破坏 paint order。
Low/Nit:hasSkew 判据尺度相关、空 Group、若干注释/语义。
测试:skew→children 分支未被任何用例执行。

已核实无误、排除误报的点:删除 LayoutNode.h include 安全;删除 unpackFirst 渲染等价;未处理 group->alpha 安全(canDowngradeAll 已排除 alpha≠1);半径公式数学正确;透明色标 donor 选 earlier 正确。详见各行评论。

Comment thread src/pagx/html/importer/HTMLValueParser.cpp
Comment thread src/cli/CommandResolve.cpp Outdated
Comment thread src/cli/CommandResolve.cpp Outdated
Comment thread src/cli/CommandResolve.cpp
Comment thread src/cli/CommandResolve.cpp Outdated
Comment thread src/pagx/html/importer/HTMLValueParser.cpp Outdated
Comment thread src/pagx/html/importer/HTMLValueParser.cpp Outdated
Comment thread test/src/PAGXCliTest.cpp
OnionsYu added 3 commits July 29, 2026 14:54
…faux bold so imported headings and bold runs resolve the authored heavy face and preserve the SemiBold Bold Black distinction.
…pendent skew detection and fix radial-gradient circle extent radii for centers outside the box.

@shlzxjp shlzxjp left a comment

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.

第二轮复审补充(font-weight 提交 9cd196a

上一轮评审的 SVG 层级 / 径向渐变问题已全部修复、验证通过,mask visible 清理也确认安全。本轮针对 PR 后续新增的 font-weight 字重轴改动(commit 9cd196a,与本 PR 标题的 SVG/渐变主题无关)复审,发现一个 Critical 功能回归和一个 High 测试缺口,均经多轮二次确认并逐条核实证据链、排除误报。详见下方两条行级评论。

此外提醒:该 font-weight 改动属于 PR 范围膨胀(独立功能),建议评估是否拆分为单独 PR。

// not synthesised: this lets the renderer resolve the authored heavy face when it is installed or
// embedded, and only fall back to faux emboldening (applied by the render host on a missing face)
// without collapsing SemiBold / Bold / Black into one indistinguishable step.
out.fauxBold = 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.

[Critical] font-weight 改为真实字面 label 后,缺失重字面时字重丢失,退化为 Regular 细体。

本行把 fauxBold 恒置为 false,weight 只以真实字面 label(Bold/SemiBold/Black)携带。上方注释(:137-140)声称「缺失字面时由 render host 做 faux 加粗回退」——但这个宿主兜底机制在 libpag/tgfx 中并不存在

证据链(已独立核实)

  1. TextLayout.cpp:394 primaryFont.setFauxBold(glyph.fauxBold)——渲染是否加粗完全由 fauxBold 驱动,恒 false 即永不加粗。
  2. LayoutContext.cpp:62-80 Stage 2:注册字体缺 Bold 面时,StylePriority 确定性优先返回 Regular。
  3. 全 tgfx 源码 faux bold 唯一入口是 isFauxBold(),无任何按 weight 自动合成加粗的逻辑。

触发范围(分平台/分路径精确界定)

  • 注册字体 Stage 2/3(importer 主路径,CSS family 被注册为 fallback):确定性退化为 Regular 细体——在任何 CoreText 兜底之前就返回。
  • Webfont-family.ts:46 缺面回退裸 family,CSS 串未带 weight → 浏览器渲染 Regular。
  • FreeType(Android/Linux/OHOS)走到 Stage 5SystemFont.cpp:261-268 非 Windows 直接 return nullptr,字面/字重全丢,比退化更糟。
  • (仅 Apple 桌面、且 family 未注册、落到 Stage 5 时,CoreText 近似匹配有条件兜底;但 importer 会注册 CSS family 为 fallback,此路径边缘。)

为何 Critical:HTML importer 主战场是「离线导入 web 内容 → 移动端/嵌入式 host 播放」,host 通过 RegisterFont 走 Stage 2/3,且 web 字体(Noto Sans SC Black 等)几乎不随 host 安装。旧逻辑用 fauxBold=true 至少保证 faux 加粗成粗体,本改动删除了这层保护,主路径落在确定退化区。

根本修复(方案 B,推荐):在渲染层(TextLayout::shapeText)解析到 typeface 后,用已有的 ParseFontStyleName(本文件 :152,可从字面 style 反解 weight)比对实际字面 weight 与请求 weight,不足时运行时补 fauxBold。这才是注释承诺机制的真正实现,能统一覆盖全平台全 Stage,且不牺牲精确命中真面(Stage 1)时区分 SemiBold/Bold/Black 的收益。同时需订正 :137-140 描述不存在机制的注释。

auto out = pagx::ResolveFontStyleSynthesis("900", "italic");
EXPECT_EQ(out.fontStyleName, "");
EXPECT_TRUE(out.fauxBold);
EXPECT_EQ(out.fontStyleName, "Black");

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.

[High] 测试只固化了新的 CSS→label 映射,完全未覆盖关键回退路径。

本用例及相邻的 Synthesis_* 断言("900"→"Black"/fauxBold:false"700"→"Bold" 等)都只是跟着实现改期望值,测的是 ResolveFontStyleSynthesis 这一纯函数映射层。

没有任何测试验证上一条 Critical 的核心契约——「当 render host 缺失重字面时,Bold/Black 文本是否仍渲染成粗体」。于是单测全绿,但线上主路径会退化为 Regular 细体,给了虚假信心。

建议:配合 Critical 的修复,补一个渲染层/集成测试——构造一个只注册 Regular 字面的 FontConfig,导入 weight:700 的文本,断言解析到的字面 + 渲染结果仍为粗体,作为回归守卫。

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