Add NoiseStyle and NoiseFilter support with animation binding and SVG export#3483
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3483 +/- ##
==========================================
+ Coverage 80.57% 80.76% +0.18%
==========================================
Files 620 622 +2
Lines 67021 68220 +1199
Branches 19885 20234 +349
==========================================
+ Hits 54005 55099 +1094
- Misses 9079 9152 +73
- Partials 3937 3969 +32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0fb22b5 to
275aabc
Compare
| void bindNoiseStyleChannels(const pagx::NoiseStyle* node) { | ||
| _result.binding.setWriter(node, "size", WriteNoiseStyleSize); | ||
| _result.binding.setWriter(node, "density", WriteNoiseStyleDensity); | ||
| _result.binding.setWriter(node, "seed", WriteNoiseStyleSeed); | ||
| _result.binding.setWriter(node, "color", WriteNoiseStyleColor); | ||
| _result.binding.setWriter(node, "firstColor", WriteNoiseStyleFirstColor); | ||
| _result.binding.setWriter(node, "secondColor", WriteNoiseStyleSecondColor); | ||
| _result.binding.setWriter(node, "opacity", WriteNoiseStyleOpacity); | ||
| } |
There was a problem hiding this comment.
[健壮性 / UB 风险] 这里无视 node->mode 无条件注册了全部 7 个 writer,但 WriteNoiseStyleColor 内部 static_cast<tgfx::MonoNoiseStyle*>(object)、WriteNoiseStyleFirstColor/SecondColor 内部 static_cast<tgfx::DuoNoiseStyle*>(object)、WriteNoiseStyleOpacity 内部 static_cast<tgfx::MultiNoiseStyle*>(object)。
这三个 tgfx 类是 NoiseStyle 的兄弟子类,互相之间没有继承关系。如果用户对 Mono 模式的节点附加了 firstColor channel(或对 Duo 模式附加 opacity channel 等),动画 apply 时会执行兄弟类间的 static_cast——这是未定义行为。
建议根据 node->mode 选择性注册 writer:
_result.binding.setWriter(node, "size", WriteNoiseStyleSize);
_result.binding.setWriter(node, "density", WriteNoiseStyleDensity);
_result.binding.setWriter(node, "seed", WriteNoiseStyleSeed);
switch (node->mode) {
case NoiseMode::Mono:
_result.binding.setWriter(node, "color", WriteNoiseStyleColor);
break;
case NoiseMode::Duo:
_result.binding.setWriter(node, "firstColor", WriteNoiseStyleFirstColor);
_result.binding.setWriter(node, "secondColor", WriteNoiseStyleSecondColor);
break;
case NoiseMode::Multi:
_result.binding.setWriter(node, "opacity", WriteNoiseStyleOpacity);
break;
}bindNoiseFilterChannels 同样问题。
There was a problem hiding this comment.
已修复。改为按 node->mode 选择性注册 writer:
- 通用字段(size/density/seed)始终注册
- Mono → 仅注册 color
- Duo → 仅注册 firstColor + secondColor
- Multi → 仅注册 opacity
这样错误 mode 的 channel 天然无 writer 可调用,避免兄弟子类间的 static_cast UB。测试中增加了 EXPECT_FALSE 断言覆盖。
| void bindNoiseFilterChannels(const pagx::NoiseFilter* node) { | ||
| _result.binding.setWriter(node, "size", WriteNoiseFilterSize); | ||
| _result.binding.setWriter(node, "density", WriteNoiseFilterDensity); | ||
| _result.binding.setWriter(node, "seed", WriteNoiseFilterSeed); | ||
| _result.binding.setWriter(node, "color", WriteNoiseFilterColor); | ||
| _result.binding.setWriter(node, "firstColor", WriteNoiseFilterFirstColor); | ||
| _result.binding.setWriter(node, "secondColor", WriteNoiseFilterSecondColor); | ||
| _result.binding.setWriter(node, "opacity", WriteNoiseFilterOpacity); | ||
| } |
There was a problem hiding this comment.
[健壮性 / UB 风险] 同 bindNoiseStyleChannels 的问题。WriteNoiseFilterColor 直接 static_cast<tgfx::MonoNoiseFilter*>,WriteNoiseFilterFirstColor/SecondColor static_cast<tgfx::DuoNoiseFilter*>,WriteNoiseFilterOpacity static_cast<tgfx::MultiNoiseFilter*>——这三者是 tgfx::NoiseFilter 的兄弟子类,对错误 mode 的 filter 触发是 UB。
建议同样按 node->mode 选择性注册 writer。
| std::string SVGWriter::writeNoiseTurbulence(const NoiseStyle* noise, | ||
| const std::string& resultName) { | ||
| auto freq = noise->size > 0.0f ? 1.0f / noise->size : 0.25f; | ||
| _defs->openElement("feTurbulence"); | ||
| _defs->addAttribute("type", "fractalNoise"); | ||
| _defs->addAttribute("baseFrequency", FloatToString(freq)); | ||
| _defs->addAttribute("stitchTiles", "stitch"); | ||
| _defs->addAttribute("numOctaves", "3"); | ||
| _defs->addAttribute("seed", FloatToString(noise->seed)); | ||
| _defs->addAttribute("result", resultName); | ||
| _defs->closeElementSelfClosing(); | ||
| return resultName; | ||
| } | ||
|
|
||
| std::string SVGWriter::writeNoiseBand(const NoiseStyle* noise, bool isDark, | ||
| const std::string& label) { | ||
| auto turbResult = writeNoiseTurbulence(noise, "turb" + label); | ||
|
|
||
| _defs->openElement("feColorMatrix"); | ||
| _defs->addAttribute("in", turbResult); | ||
| _defs->addAttribute("type", "luminanceToAlpha"); | ||
| _defs->addAttribute("result", "luma" + label); | ||
| _defs->closeElementSelfClosing(); | ||
|
|
||
| auto d = std::clamp(noise->density, 0.0f, 1.0f); | ||
| int lower = 0; | ||
| int upper = 0; | ||
| if (isDark) { | ||
| lower = std::clamp(static_cast<int>(std::lround(-25.0f * d + 25.0f)), 0, 99); | ||
| upper = std::clamp(static_cast<int>(std::lround(24.0f * d + 25.0f)), 0, 99); | ||
| } else { | ||
| lower = std::clamp(static_cast<int>(std::lround(-24.0f * d + 74.0f)), 0, 99); | ||
| upper = std::clamp(static_cast<int>(std::lround(25.0f * d + 74.0f)), 0, 99); | ||
| } | ||
| std::string table; | ||
| table.reserve(300); | ||
| for (int i = 0; i < 100; i++) { | ||
| table += (i >= lower && i <= upper) ? "1 " : "0 "; | ||
| } | ||
| table.pop_back(); | ||
|
|
||
| _defs->openElement("feComponentTransfer"); | ||
| _defs->addAttribute("in", "luma" + label); | ||
| _defs->addAttribute("result", "band" + label); | ||
| _defs->closeElementStart(); | ||
| _defs->openElement("feFuncA"); | ||
| _defs->addAttribute("type", "discrete"); | ||
| _defs->addAttribute("tableValues", table); | ||
| _defs->closeElementSelfClosing(); | ||
| _defs->closeElement(); | ||
| return "band" + label; |
There was a problem hiding this comment.
[可维护性 / 代码重复] 这两个 NoiseStyle 重载(writeNoiseTurbulence 和 writeNoiseBand)与上方 NoiseFilter 版本(991–1041 行)函数体完全相同,只有参数类型不同(都只读 noise->size、noise->density、noise->seed)。再加上后面 writeNoiseStyle Multi 分支(1390+)与 writeNoiseFilter Multi 分支(1211+)也大段重复——总共约 200 行重复代码。
建议用模板抽取共用实现:
template <typename T>
std::string writeNoiseTurbulenceImpl(const T* noise, const std::string& resultName) {
// ... shared body using noise->size / noise->seed ...
}T 同时支持 NoiseFilter 和 NoiseStyle(依赖鸭子类型即可)。writeNoiseBand 同理。
There was a problem hiding this comment.
已修复。抽取了三个参数级公共 helper:
- writeNoiseTurbulence(float size, float seed, ...)
- writeNoiseBand(float size, float density, float seed, bool isDark, ...)
- writeNoiseMultiCore(float size, float density, float seed, float opacity, ...)
没有使用模板方案,而是直接传标量参数,这样 helper 不依赖具体节点类型,实现完全在 .cpp 内部。
| } | ||
|
|
||
| protected: | ||
| protected: |
There was a problem hiding this comment.
[代码规范] 这一处 protected: 前导空格被去掉,与 PR 主题(noise)无关,是 stray 改动。且与项目其他兄弟类不一致——例如 include/pagx/nodes/Group.h:91 使用 protected:(1 空格)。建议还原此行,或在本地运行 ./codeformat.sh 重新格式化。
There was a problem hiding this comment.
已还原为 protected:(前导一空格),与项目其他类一致。
| std::string SVGWriter::writeNoiseStyle(const NoiseStyle* noise, int& noiseStyleIndex) { | ||
| std::string styleId = "noiseStyle" + std::to_string(noiseStyleIndex++); | ||
|
|
||
| if (noise->mode == NoiseMode::Mono) { | ||
| auto band = writeNoiseBand(noise, true, "Dark" + styleId); | ||
| _defs->openElement("feFlood"); | ||
| _defs->addAttribute("flood-color", ColorToSVGString(noise->color)); | ||
| if (noise->color.alpha < 1.0f) { | ||
| _defs->addAttribute("flood-opacity", FloatToString(noise->color.alpha)); | ||
| } | ||
| _defs->addAttribute("result", "flood" + styleId); | ||
| _defs->closeElementSelfClosing(); | ||
|
|
||
| _defs->openElement("feComposite"); | ||
| _defs->addAttribute("in", "flood" + styleId); | ||
| _defs->addAttribute("in2", band); | ||
| _defs->addAttribute("operator", "in"); | ||
| _defs->addAttribute("result", "colored" + styleId); | ||
| _defs->closeElementSelfClosing(); | ||
|
|
||
| auto resultName = "noiseStyleOut" + styleId; | ||
| _defs->openElement("feComposite"); | ||
| _defs->addAttribute("in", "colored" + styleId); | ||
| _defs->addAttribute("in2", "SourceGraphic"); | ||
| _defs->addAttribute("operator", "in"); | ||
| _defs->addAttribute("result", resultName); | ||
| _defs->closeElementSelfClosing(); | ||
| return resultName; |
There was a problem hiding this comment.
[API 一致性 / SVG 导出] NoiseStyle 继承自 LayerStyle,其 blendMode 字段在运行时会被 LayerBuilderContext 调 tgfxStyle->setBlendMode(...) 应用,但本函数完全忽略了 noise->blendMode,最终通过 agg.aboveResults→feMergeNode 合成(feMerge 等价 SrcOver)。
如果用户给 NoiseStyle 设置 Multiply/Screen 等 blendMode,运行时(tgfx)渲染会生效,SVG 导出会静默丢失该效果——与 writeNoiseFilter 末尾通过 feBlend 显式应用 blendMode 不一致(参考本文件 1188 行附近)。
建议二选一:
- 在
writeNoiseStyle输出末尾增加一个 feBlend 节点应用 blendMode,与 NoiseFilter 行为对齐; - 或在
NoiseStyle头文件 / SVG 导出文档中明确标注"SVG 导出场景下 blendMode 不生效"。
注:DropShadowStyle 在 SVG 导出中也忽略了 blendMode,所以这不是新增 regression,但 NoiseStyle 与 NoiseFilter 不一致这一点是新引入的。
There was a problem hiding this comment.
已修复。当 noise->blendMode != Normal 时,在 writeStyleList 中生成 <feBlend in="noiseResult" in2="SourceGraphic" mode="...">,通过已有的 BlendModeToFEBlendString() 映射模式名称。
支持 16/18 种标准 CSS blend mode;PlusLighter 和 PlusDarker 在 SVG feBlend 中无对应值,优雅降级为 Normal(不生成 feBlend)。
新增 NoiseStyleBlendModeOnImage 测试用例覆盖 Multiply 模式。
| Color color = {}; | ||
|
|
||
| /** | ||
| * The first noise color for Duo mode. The alpha component controls its opacity. | ||
| */ | ||
| Color firstColor = {}; | ||
|
|
||
| /** | ||
| * The second noise color for Duo mode. The alpha component controls its opacity. | ||
| */ | ||
| Color secondColor = {}; |
There was a problem hiding this comment.
[API 易用性] pagx::Color 默认 alpha = 1(见 pagx/types/Color.h:47),所以这三处 Color color = {} / Color firstColor = {} / Color secondColor = {} 实际值都是不透明黑 (0,0,0,1)。
这意味着用户构造 Duo 模式但忘记同时设置 firstColor 和 secondColor 时,得到的是两个不透明黑——视觉上等同于禁用了 Duo 双色效果。
对应的 tgfx 端默认值更合理:MonoNoiseStyle._color = Color::Black()、DuoNoiseStyle._firstColor = Black()、_secondColor = White()(见 third_party/tgfx/include/tgfx/layers/layerstyles/NoiseStyle.h:154/201/202)。
建议让 pagx 默认值与 tgfx 一致,至少 secondColor 默认为白色:
Color secondColor = {1.0f, 1.0f, 1.0f, 1.0f};NoiseFilter.h 同样问题。
There was a problem hiding this comment.
已修复。NoiseFilter.h 和 NoiseStyle.h 中 secondColor 默认值改为 {1, 1, 1, 1}(白色),与 tgfx 运行时一致。
| * A noise layer style that overlays procedural Perlin noise above the layer content. Three noise | ||
| * modes are available: Mono (single color), Duo (two complementary colors), and Multi (preserving | ||
| * original noise RGB with enhanced contrast). | ||
| */ | ||
| class NoiseStyle : public LayerStyle { |
There was a problem hiding this comment.
[API 设计 / 文档] NoiseStyle 同时暴露了 color(Mono 用)、firstColor/secondColor(Duo 用)、opacity(Multi 用)四个字段,但任意时刻只有一组生效。这种"扁平字段 + mode 切换"的设计是相对 tgfx 的子类层次(MonoNoiseStyle/DuoNoiseStyle/MultiNoiseStyle)的折衷,便于用户构造,但带来副作用:
- 用户从代码层面看不出哪些字段必须设置(Issue 请问下 ios有没有可以监听动画播放实时进度的API #5 默认值问题就是这个的副作用);
- 序列化/反序列化无法决定要不要写出未使用的字段;
- 引发
bindNoiseStyleChannels中的 UB 风险(见 LayerBuilder.cpp 行级评论)。
建议在 class doc(这段顶部注释)中明确写出:"非当前 mode 对应的字段会被忽略;切换 mode 后请重新设置对应字段。"目前的字段级注释"used in X mode"较容易被忽略。
NoiseFilter.h 同样建议。
There was a problem hiding this comment.
已修复。补充了:
- 类顶部注释:"Mode-specific fields for inactive modes are ignored; set the fields for the new mode after changing mode."
- 每个 mode 专属字段注释标注被哪些 mode 忽略,如 color → "Ignored by Duo and Multi modes."
- NoiseFilter.h 同步处理。
| std::string SVGWriter::writeNoiseTurbulence(const NoiseFilter* noise, | ||
| const std::string& resultName) { | ||
| auto freq = noise->size > 0.0f ? 1.0f / noise->size : 0.25f; | ||
| _defs->openElement("feTurbulence"); | ||
| _defs->addAttribute("type", "fractalNoise"); | ||
| _defs->addAttribute("baseFrequency", FloatToString(freq)); | ||
| _defs->addAttribute("stitchTiles", "stitch"); | ||
| _defs->addAttribute("numOctaves", "3"); | ||
| _defs->addAttribute("seed", FloatToString(noise->seed)); | ||
| _defs->addAttribute("result", resultName); | ||
| _defs->closeElementSelfClosing(); | ||
| return resultName; | ||
| } |
There was a problem hiding this comment.
[视觉一致性 / 文档] freq = 1.0f / size 没有考虑 contentScale。tgfx 端 MakeNoiseShader 计算的是 freq = 1.0f / (size * scale)(见 third_party/tgfx/src/layers/filters/NoiseFilter.cpp:32)。
feTurbulence 在 SVG filter region 像素空间生成,浏览器以 viewBox 缩放渲染时不会自动缩放频率——因此非 1.0 contentScale 下,SVG 导出与 tgfx GPU 渲染的颗粒大小会有偏差。
这不是 bug,是 SVG vs GPU shader 的固有差异。建议在此处加一行注释说明这一点,避免后续维护者误以为是计算错误。
There was a problem hiding this comment.
已修复。在 freq 计算处添加注释说明:
// SVG filter primitives operate in document units and do not receive the runtime content scale
// used by GPU rasterization, so exported noise frequency intentionally derives from authoring size.
| pagx::FontConfig fontConfig; | ||
| fontConfig.addFallbackTypefaces(GetFallbackTypefaces()); | ||
|
|
||
| auto typeface = Typeface::MakeFromPath("/System/Library/Fonts/Helvetica.ttc"); |
There was a problem hiding this comment.
[测试质量 / 跨平台] 这里硬编码了 macOS 系统字体路径 /System/Library/Fonts/Helvetica.ttc,仅在 macOS 可用。Linux/Windows CI 上 typeface 为 null,会跳过 fontConfig.registerTypeface,文本节点退回 fallback——但 NoiseFilterAllElements 的截图基线包含 Text/TextBox 字形,跨 OS 跑测试时基线 hash 会无法对齐。
建议使用项目中现有的 fallback typeface 机制(如 GetFallbackTypefaces()),或参照其他测试用例使用项目自带的测试字体资源(如 test/resources/font/...)。
There was a problem hiding this comment.
已修复。改用项目内 resources/font/NotoSerifSC-Regular.otf,通过 Typeface::MakeFromPath() 加载并注册到 FontConfig,不再依赖 macOS 系统字体路径。基线已更新。
…er position displacement.
…th tgfx ink bounds
…ts test for consistent font rendering
…nt tgfx and SVG rendering
…ch to align with Figma.
…fer for contrast and density banding.
…ve debug log output.
ae79c84 to
126228e
Compare
… blend mode support.
9b2e7cc to
fdb3cb9
Compare
| return resultName; | ||
| } | ||
|
|
||
| auto final = |
There was a problem hiding this comment.
[代码规范] 变量名 final 是 C++11+ 的上下文标识符(用于 class X final {} 和 void f() final;),用作普通变量名虽合法,但容易与类继承语义混淆,常见 C++ 编码规范(包括 LLVM Coding Standards)明确禁止此模式。
建议改名为 finalResult 或 coreResult:
auto finalResult =
writeNoiseMultiCore(noise->size, noise->density, noise->seed, noise->opacity, filterId);
_defs->openElement("feComposite");
_defs->addAttribute("in", finalResult);writeNoiseStyle 第 1341 行同样问题,建议一并修改。
| std::string opacityValues = "1 0 0 0 0 0 1 0 0 0 0 0 1 0 0 0 0 0 "; | ||
| opacityValues += FloatToString(opacity); | ||
| opacityValues += " 0"; | ||
| _defs->addAttribute("values", opacityValues); | ||
| _defs->addAttribute("result", "final" + id); | ||
| _defs->closeElementSelfClosing(); |
There was a problem hiding this comment.
[一致性 / 健壮性] 这里直接把 opacity 写入 feColorMatrix 的 alpha multiplier,未做范围裁剪。但 tgfx 的 MultiNoiseFilter 构造函数对 opacity 做了 clamp(见 third_party/tgfx/src/layers/filters/NoiseFilter.cpp MultiNoiseFilter 构造体:_opacity(std::max(0.0f, std::min(1.0f, opacity))))。
如果用户传入 opacity = 2.0 创建一个 NoiseFilter:
- 运行时(tgfx):实际
_opacity = 1.0 - SVG 导出:matrix 写入
2.0
两端结果不一致。
建议在 writeNoiseMultiCore 顶部对 opacity 做同样的 clamp:
opacity = std::clamp(opacity, 0.0f, 1.0f);注:tgfx MultiNoiseStyle 构造函数没有 clamp(只有 setOpacity clamp),所以严格来说 NoiseStyle 路径 SVG 与运行时是一致的(都不 clamp)。但 SVG 端做兜底 clamp 不会破坏 NoiseStyle 行为,可同时覆盖两个路径。
| std::string SVGWriter::writeNoiseTurbulence(float size, float seed, const std::string& resultName) { | ||
| // SVG filter primitives operate in document units and do not receive the runtime content scale | ||
| // used by GPU rasterization, so exported noise frequency intentionally derives from authoring size. | ||
| auto freq = size > 0.0f ? 1.0f / size : 0.25f; |
There was a problem hiding this comment.
[一致性] size <= 0 时 SVG 退回到 freq = 0.25f 默认,但 tgfx 端 NoiseFilter::MakeMono/Duo/Multi 和 NoiseStyle::MakeMono/Duo/Multi 在 size <= 0 时都返回 nullptr(见 third_party/tgfx/src/layers/filters/NoiseFilter.cpp:62/73/82 与 third_party/tgfx/src/layers/layerstyles/NoiseStyle.cpp:36/45/54)。
这意味着用户传入 size = 0 或负值(例如错误数据 / 反序列化的边界情况)时:
- 运行时:noise 被静默丢弃(LayerBuilder 拿到 nullptr),不渲染任何东西
- SVG 导出:仍然以 freq=0.25 渲染出一个可见 noise
两端表现不一致。
建议二选一:
- 在
writeNoiseFilter/writeNoiseStyle入口处检查noise->size <= 0,直接return ""跳过整段 noise 的导出(writeFilterList / writeStyleList 不要把空结果加入 agg); - 或在 doc 中明确 SVG 与运行时对无效 size 处理不同。
推荐方案 1,与运行时静默丢弃对齐。
| if (noise->mode == NoiseMode::Mono) { | ||
| auto band = writeNoiseBand(noise->size, noise->density, noise->seed, true, "Dark" + filterId); | ||
| _defs->openElement("feFlood"); | ||
| _defs->addAttribute("flood-color", ColorToSVGString(noise->color)); |
There was a problem hiding this comment.
[API 一致性 / Display P3 支持] 这里直接用 ColorToSVGString(noise->color) 输出 flood-color,没有判断 colorSpace,强制按 sRGB 输出 #RRGGBB。
但同文件其他位置都对 colorSpace 做了分支处理:
- 第 151 行 SolidColor 输出:
if (solid->color.colorSpace != ColorSpace::DisplayP3) { ColorToSVGString } else { ColorToDisplayP3String } - 第 487 行 gradient stops:同样判断
如果用户给 noise 颜色设置了 color.colorSpace = ColorSpace::DisplayP3:
- 运行时(tgfx):按 P3 渲染
- SVG 导出:当作 sRGB 渲染
两端色调差异明显(尤其在饱和色上)。
建议封装一个统一的辅助函数 / 在所有 noise flood-color 处补上 colorSpace 判断:
auto floodColorStr = noise->color.colorSpace == ColorSpace::DisplayP3
? ColorToDisplayP3String(noise->color)
: ColorToSVGString(noise->color);
_defs->addAttribute("flood-color", floodColorStr);本文件中受影响的 6 处:1123(NoiseFilter Mono color)、1171(NoiseFilter Duo firstColor)、1186(NoiseFilter Duo secondColor)、1258(NoiseStyle Mono color)、1295(NoiseStyle Duo firstColor)、1310(NoiseStyle Duo secondColor)。
| /** | ||
| * Test NoiseStyle with blendMode applied to an image layer, verifying both rendering and SVG | ||
| * export. The blendMode is set to Multiply so the noise composites differently from Normal. | ||
| * Currently SVG export ignores blendMode, so the SVG output will differ from the GPU rendering. | ||
| */ | ||
| PAGX_TEST(PAGXTest, NoiseStyleBlendModeOnImage) { | ||
| constexpr int canvasW = 200; | ||
| constexpr int canvasH = 200; | ||
| auto doc = pagx::PAGXDocument::Make(canvasW, canvasH); | ||
|
|
||
| auto* layer = doc->makeNode<pagx::Layer>(); | ||
|
|
||
| auto* rect = doc->makeNode<pagx::Rectangle>(); | ||
| rect->position = {100, 100}; | ||
| rect->size = {200, 200}; | ||
|
|
||
| auto* image = doc->makeNode<pagx::Image>(); | ||
| auto imageData = | ||
| tgfx::Data::MakeFromFile(ProjectPath::Absolute("resources/apitest/imageReplacement.png")); | ||
| ASSERT_TRUE(imageData != nullptr); | ||
| image->data = pagx::Data::MakeWithCopy(imageData->bytes(), imageData->size()); | ||
|
|
||
| auto* pattern = doc->makeNode<pagx::ImagePattern>(); | ||
| pattern->image = image; | ||
| pattern->matrix = {1, 0, 0, 1, 0, 0}; | ||
|
|
||
| auto* fill = doc->makeNode<pagx::Fill>(); | ||
| fill->color = pattern; | ||
|
|
||
| auto* noise = doc->makeNode<pagx::NoiseStyle>(); | ||
| noise->mode = pagx::NoiseMode::Mono; | ||
| noise->size = 8; | ||
| noise->density = 1.0f; | ||
| noise->seed = 7; | ||
| noise->color = {0.5f, 0.5f, 0.5f, 1.0f}; | ||
| noise->blendMode = pagx::BlendMode::Multiply; | ||
|
|
||
| layer->contents.push_back(rect); | ||
| layer->contents.push_back(fill); | ||
| layer->styles.push_back(noise); | ||
| doc->layers.push_back(layer); | ||
|
|
||
| doc->applyLayout(); | ||
| auto tgfxLayer = pagx::LayerBuilder::Build(doc.get()); | ||
| ASSERT_TRUE(tgfxLayer != nullptr); | ||
|
|
||
| auto surface = Surface::Make(context, canvasW, canvasH); | ||
| ASSERT_TRUE(surface != nullptr); | ||
| DisplayList displayList; | ||
| displayList.root()->addChild(tgfxLayer); | ||
| displayList.render(surface.get(), false); | ||
|
|
||
| EXPECT_TRUE(Baseline::Compare(surface, "PAGXTest/NoiseStyleBlendModeOnImage")); | ||
|
|
||
| pagx::SVGExportOptions svgOpts; | ||
| auto svg = pagx::SVGExporter::ToSVG(*doc, svgOpts); | ||
| EXPECT_FALSE(svg.empty()); | ||
| EXPECT_NE(svg.find("feTurbulence"), std::string::npos); | ||
| EXPECT_NE(svg.find("<image"), std::string::npos); |
There was a problem hiding this comment.
[测试质量 / 注释] 两点问题:
-
注释过时:第 7536 行注释说 "Currently SVG export ignores blendMode, so the SVG output will differ from the GPU rendering." 但本 PR 同一次 commit 已经在
src/pagx/svg/SVGExporter.cpp:1439-1455的writeStyleList中给 NoiseStyle 增加了 feBlend 输出,让 SVG 也支持 blendMode 了——这个测试本身就是为了验证这个修复。注释和实现自相矛盾,建议删除 "Currently SVG export ignores blendMode" 这句。 -
断言不够强:第 7590-7591 行只断言
feTurbulence和<image>存在,但没有断言mode="multiply"真的写入了 SVG——也就是说,如果未来writeStyleList的 blendMode 分支被改坏(例如又退回到 "忽略 blendMode"),这个测试仍然会通过,无法兜底。
建议增加:
EXPECT_NE(svg.find("feBlend"), std::string::npos);
EXPECT_NE(svg.find("multiply"), std::string::npos);或更精确:
EXPECT_NE(svg.find("mode=\"multiply\""), std::string::npos);There was a problem hiding this comment.
感谢修复了注释(R5.1)。但本评论中提到的第 2 点(断言强度不足)尚未处理。
当前测试(PAGXTest.cpp:7600-7606)只断言:
EXPECT_NE(svg.find("feTurbulence"), std::string::npos);
EXPECT_NE(svg.find("<image"), std::string::npos);但本测试的核心目的是验证 writeStyleList 中 NoiseStyle 的 blendMode 真的被输出到 SVG(这正是 round-1 评论 #4 的修复点)。如果未来重构不慎让 writeStyleList 退回到不输出 feBlend,这两个断言仍会通过——回归保护缺失。
建议追加:
EXPECT_NE(svg.find("feBlend"), std::string::npos);
EXPECT_NE(svg.find("multiply"), std::string::npos);或更精确的 svg.find("mode=\"multiply\"")。
| std::string blendResult = result + "Blended"; | ||
| _defs->openElement("feBlend"); | ||
| _defs->addAttribute("in", result); | ||
| _defs->addAttribute("in2", "SourceGraphic"); | ||
| _defs->addAttribute("mode", modeStr); | ||
| _defs->addAttribute("result", blendResult); | ||
| _defs->closeElementSelfClosing(); |
There was a problem hiding this comment.
[设计 / NoiseFilter vs NoiseStyle 不一致] 这里给 NoiseStyle 的 blendMode 输出 feBlend 时,用 in2="SourceGraphic"(原始未经滤镜处理的内容)。但本文件中 writeNoiseFilter 的三个 mode 分支(行 1147、1217、1241)都用 in2=currentSource(filter chain 当前的输出)。
这导致一个微妙但真实的差异:
当 layer 同时有 LayerFilter(例如 BlurFilter)+ NoiseStyle(带 blendMode)时:
writeFilterList先跑,currentSource从"SourceGraphic"变成"blurred0"writeStyleList处理 NoiseStyle,feBlend 用in2="SourceGraphic"—— 与原图做 blendwriteShadowMerge把 blendResult 与currentSource("blurred0")一起 feMerge
语义上:noise 的 blend 是针对原图的,但最终覆盖在模糊后的图上。这与 NoiseFilter 的语义(noise 与 filter chain 当前结果做 blend)不一致,也可能与运行时 tgfx 把 styles 应用到 filter 输出后的内容不一致。
建议确认设计意图:
- 如果有意让 NoiseStyle 始终对 raw source 做 blend(与 LayerStyle = "独立于 filter chain 的图层装饰" 的语义一致),请在注释中明确,并考虑在测试中加入 NoiseStyle + 上游 filter(Blur 等)的组合用例覆盖这个边界;
- 如果应该与 NoiseFilter 行为一致,则改为
_defs->addAttribute("in2", currentSource);(需要将currentSource传入writeStyleList)。
There was a problem hiding this comment.
Round-3 验证:本评论尚未在代码或回复中处理。最新 commit bf644e591 中 writeStyleList 仍是 _defs->addAttribute("in2", "SourceGraphic");(行 1421),且未补充设计意图注释。
请作者澄清下面两个语义中哪一个是预期行为:
方案 A(当前实现): NoiseStyle 的 blend 始终对原始 SourceGraphic 计算
- 语义:LayerStyle 是"独立于 filter chain 的图层装饰",blend 不受上游 filter 影响
- 与运行时 tgfx:tgfx 的 LayerStyle 实际上是叠加到 layer 内容(filter chain 输出)上,方案 A 的 SVG 行为可能与运行时不一致
- 优点:实现简单,blend 结果稳定
方案 B(与 NoiseFilter 一致): NoiseStyle 的 blend 对 currentSource(filter chain 当前输出)计算
- 语义:noise 与上游 filter 的产物做 blend,与 NoiseFilter 的 1147/1217/1241 行行为一致
- 与运行时 tgfx:更可能匹配 tgfx 的实际渲染顺序
- 实现:把
currentSource传入writeStyleList,feBlendin2=currentSource
这两种解释都合理,但 SVG 与 tgfx 应该选定一种并保持一致。如选方案 A,请在 writeStyleList 处加注释说明这一选择;如选方案 B,需要改代码并补一个 "NoiseStyle + 上游 BlurFilter" 组合用例验证。
…ip size<=0, add Display P3 flood-color support, remove stale comment.
… size is non-positive.
… code in noise tests.
…ngthen SVG assertions.
概述
新增 NoiseStyle 和 NoiseFilter 两种噪声节点的定义、动画绑定及 SVG 导出支持。
主要变更
新增文件
include/pagx/types/NoiseMode.h— NoiseMode 枚举,定义 Mono / Duo / Multi 三种噪声模式include/pagx/nodes/NoiseStyle.h— NoiseStyle 节点定义(LayerStyle,覆盖 SourceGraphic)include/pagx/nodes/NoiseFilter.h— NoiseFilter 节点定义(LayerFilter,覆盖全部合成结果)核心修改
include/pagx/nodes/Node.h— 添加 NoiseStyle、NoiseFilter 枚举值src/pagx/utils/StringParser.cpp— 添加 NoiseStyle / NoiseFilter 的字符串解析src/renderer/LayerBuilder.cpp:src/pagx/svg/SVGExporter.cpp— 实现三种噪声模式的 SVG 滤镜导出测试
test/src/PAGXTest.cpp— 新增测试用例:test/baseline/version.json— 新增基线版本