Add WeChat MiniProgram support for PAGX viewer with rendering fixes and performance optimizations.#3507
Conversation
…GXDocument fields.
…ers and add comment overlay demo.
…es regardless of XML order.
| * Low-resolution preview tgfx::Image used as a fallback when decodedImage is unavailable | ||
| * (initial load not yet complete, or the full-resolution texture has been evicted under memory | ||
| * pressure). When non-null, the renderer falls back to this so the affected fill area shows a | ||
| * blurry preview rather than a blank rectangle. Populated via PAGXDocument::loadDecodedImage() |
There was a problem hiding this comment.
文档错误:注释里写 Populated via PAGXDocument::loadDecodedImage() with a thumbnail-quality image,但实际写入 thumbnailImage 字段的方法是 loadDecodedImageAsThumbnail()(见 src/pagx/PAGXDocument.cpp:606)。loadDecodedImage() 写入的是 decodedImage 字段,与本字段无关。
接入方按文档调用 loadDecodedImage 不会写到 thumbnailImage,会被误导。
建议把第 60 行 loadDecodedImage() 改为 loadDecodedImageAsThumbnail()。
| * @param decodedImage the pre-decoded tgfx::Image to attach | ||
| * @return the first matching Image node, or nullptr if no match was found. | ||
| */ | ||
| Image* loadDecodedImage(const std::string& filePath, std::shared_ptr<tgfx::Image> decodedImage); |
There was a problem hiding this comment.
返回值语义不清:loadDecodedImage / loadDecodedImageAsThumbnail / clearDecodedImage / clearThumbnailImage 这 4 个公开 API 文档都说 "Image nodes matching the given file path"(plural — 多个节点都会被更新),但 @return 只说 "the first matching Image node",没有明确"其他匹配节点也会被同样更新,只是返回值反映第一个"。
实际行为(src/pagx/PAGXDocument.cpp:574-595 的 setImageFieldByFilePath):循环遍历所有匹配 filePath 的 Image 节点全部更新,仅返回首个匹配项指针。
调用方拿到返回值后可能误以为只更新了这一个节点,进而手动遍历其他节点重复操作,或对未返回的节点做错误假设。
建议把 4 处 @return 改为类似:
@return the first matching Image node (all matching nodes are updated regardless), or nullptr if no match was found.
| * @param imageFilePath the Image node filePath to match against | ||
| * @return the list of Layers referencing the given image filePath, empty when no match | ||
| */ | ||
| const std::vector<const Layer*>& findLayersByImageFilePath(const std::string& imageFilePath); |
There was a problem hiding this comment.
返回引用有效期未文档化:本方法返回 const std::vector<const Layer*>&,引用指向内部缓存 layersByImageFilePath。当前文档说明了"懒构建"和"notifyChange() 自动失效",但没有明确说明返回引用的有效期。
下游代码如果保存了引用,再调用 notifyChange() 或下次 findLayersByImageFilePath()(不同 filePath 时虽不重建,但同一 filePath 走另一分支也会读 map),引用可能失效(map rehash / 条目被替换)。
建议在 @return 后补一行:
* Returned reference is invalidated by the next notifyChange() call; copy if you need to retain it.
| } | ||
| return tgfx::Color::FromRGBA(r, g, b); | ||
| } | ||
| return DefaultBackgroundColor(); |
There was a problem hiding this comment.
缺失 #RGBA(4 字符)短格式支持:ParseHexColor 当前仅识别:
#RGB(4 chars)→ 扩展为#RRGGBB#RRGGBB(7 chars)#RRGGBBAA(9 chars)
而 CSS Color Level 4 标准还定义了 #RGBA(5 chars,等价于 #RRGGBBAA 的短格式)。当前实现遇到 #ABC5 这类 5 字符输入会落到 return DefaultBackgroundColor()(line 79),用户体验是文档背景色出了一个意料之外的灰色。
实际场景中 PAGX 内部产物用 #RRGGBB 居多,触发概率较低,故定为 minor。
建议在 hex.size() == 4 之后加一个 hex.size() == 5 分支:
if (hex.size() == 5) {
int r = ParseHexDigit(hex[1]);
int g = ParseHexDigit(hex[2]);
int b = ParseHexDigit(hex[3]);
int a = ParseHexDigit(hex[4]);
if (r < 0 || g < 0 || b < 0 || a < 0) {
return DefaultBackgroundColor();
}
return tgfx::Color::FromRGBA(r * 17, g * 17, b * 17, a * 17);
}… reference lifetime, and RGBA short format.
… into feature/billyjin_pagx_new
| * populated by platforms where synchronous codec decoding is expensive and an async native | ||
| * decoder is preferred. Platforms that do not use this flow leave it null. | ||
| */ | ||
| std::shared_ptr<tgfx::Image> decodedImage = nullptr; |
There was a problem hiding this comment.
架构建议:把 decodedImage / thumbnailImage 字段下沉到 ImageResourceProvider,让 pagx 核心层与 tgfx 解耦
这是一条架构层面的建议,不阻塞当前 PR 合入,可作为后续重构方向。但当前 PR 还在 review 阶段、未发版,是动这块的最佳时机。
当前设计的问题
这两个 public 字段把 tgfx 运行时图像资源 放进了 include/pagx/ 的文档模型层:
- 抽象层级错配:
pagx::Image节点在 PAGX 文档模型中代表"是什么"——一张通过 base64 或外部路径引用的图片。但decodedImage/thumbnailImage是渲染时的 GPU/CPU 资源缓存,属于"渲染时怎么用"。这把文档描述和渲染状态混在了同一个类里。 - 平台特定特性侵入跨平台核心层:这两个字段实际上只服务于 WeChat 小程序的渐进式加载流程(注释也提到 "Typically populated by platforms where synchronous codec decoding is expensive"),其他平台永远是 nullptr,但每个
Image节点固定多付出 ~32 字节,且include/pagx/多了对tgfx::Image的(前向)依赖。 - 两种索引并存:当前同时存在三套缓存——
PAGXDocument::layersByImageFilePath(按 filePath)、LayerBuilder::_imageCache(按const Image*)、Image::decodedImage/thumbnailImage(按Image*找到节点后按 filePath 批量写入)。这套混合索引是上几轮评审多个问题的根源:- 第一轮
_imageCacheraw 指针 key 不稳的问题 - 第一轮
layersByImageFilePath的失效机制 - 第二轮 4 个 load/clear API 的
@return语义 - 第二轮 thumbnailImage 注释方法名错指
- 第一轮
- 半遮半掩的封装:字段是 public 但通过
setImageFieldByFilePath的 member pointer 模板访问,用户既能绕过 4 个 API 直接image->decodedImage = ...,又被 friend + private 构造暗示"别这么做"。
建议方案:Resource Provider 注入
把"按 filePath 解析 tgfx::Image"这件事抽象成一个 provider 接口,从节点字段里彻底搬走。
include/pagx/PAGXDocument.h(不引入 tgfx 依赖;返回类型用 shared_ptr<void> 做类型擦除,由 src/renderer/ 端转回 tgfx::Image):
namespace pagx {
class ImageResourceProvider {
public:
virtual ~ImageResourceProvider() = default;
virtual std::shared_ptr<void> resolveImage(const std::string& filePath) = 0;
virtual bool has(const std::string& filePath) const = 0;
};
class PAGXDocument {
public:
void setImageResourceProvider(std::shared_ptr<ImageResourceProvider> provider);
const std::shared_ptr<ImageResourceProvider>& imageResourceProvider() const;
};
}include/pagx/nodes/Image.h 恢复干净:
class Image : public Node {
public:
std::shared_ptr<Data> data = nullptr;
std::string filePath = {};
// 没有 decodedImage / thumbnailImage / tgfx 依赖
};src/renderer/LayerBuilder.cpp::getOrCreateImage 在 fallback chain 前先查 provider:
if (auto provider = document->imageResourceProvider()) {
if (auto resolved = provider->resolveImage(imageNode->filePath)) {
return std::static_pointer_cast<tgfx::Image>(resolved);
}
}
// 然后是原有的 data / dataURI / filePath 解码路径WeChat 端在 pagx/wechat/src/ 实现具体的 provider,自己维护 decoded / thumbnail 双层缓存:
class PAGXImageResourceProvider : public pagx::ImageResourceProvider {
std::shared_ptr<void> resolveImage(const std::string& filePath) override {
if (auto it = decoded.find(filePath); it != decoded.end()) return it->second;
if (auto it = thumbnails.find(filePath); it != thumbnails.end()) return it->second;
return nullptr;
}
void putDecoded(...); void putThumbnail(...); void clearDecoded(...); ...
};PAGXView::attachNativeImage 原来调 document->loadDecodedImage(...),改调 provider->putDecoded(...)。
能消除掉的现有代码
Image::decodedImage/Image::thumbnailImage字段(含相关注释和 tgfx 前向声明)PAGXDocument::loadDecodedImage/loadDecodedImageAsThumbnail/clearDecodedImage/clearThumbnailImage共 4 个公开 APIPAGXDocument::setImageFieldByFilePath(member pointer 模板那段)LayerBuilderSession::invalidateImagesByFilePath/invalidateAllImages- 大概率也能简化
LayerBuilder::_imageCache(provider 本身就是 cache,per-build cache 仅作为优化保留与否都可以) getExternalFilePaths的过滤条件从 4 字段检查简化为 1 字段 + provider 查询
预计净减少代码 ≈ 100-150 行,比当前实现更少。
收益
include/pagx/不再依赖 tgfx,跨平台核心层重新干净- 其他平台零开销:不创建 provider 即可,没有 32 字节常驻代价
- 统一索引语义:所有 image 资源都按 filePath 索引,消除上述 4 个评审问题的根源
- 宿主侧策略自由:小程序可以放 wx storage,原生可以走 disk cache,测试可以 mock provider
PAGXView中的externalTextures/thumbnailTextures与Image节点字段的双重簿记也能统一到 provider 里
关于性能
provider->resolveImage(filePath) = 1 次虚函数派发 + 1 次 unordered_map 查找;当前是 2 次 shared_ptr 判空。差异在纳秒级,相对于 mipmap 生成、纹理上传等动辄毫秒级的工作完全可忽略。
当前是最佳时机
PR 还未合入、还未发布 npm 包,4 个公开 API 还没有外部业务方依赖。一旦合入并发版,再改 PAGXDocument 的公开 API 就是 breaking change,只能等下一个 major version。
如果接受这个方向,建议作为下一个 PR 处理(不阻塞本 PR 合入);如果不接受,也欢迎讨论替代方案。
There was a problem hiding this comment.
补充说明:采用方案 B 后,PAGXDocument 上新增的 4 个公开 API 会自然消失
把上一条架构评论再推一步——这条建议其实顺带解决了 PAGXDocument 上新增的另一组 API 异味。
include/pagx/PAGXDocument.h 这次为 decodedImage / thumbnailImage 字段配套引入了 4 个公开 API:
Image* loadDecodedImage(const std::string& filePath, std::shared_ptr<tgfx::Image> decodedImage);
Image* loadDecodedImageAsThumbnail(const std::string& filePath, std::shared_ptr<tgfx::Image> thumbnailImage);
Image* clearDecodedImage(const std::string& filePath);
Image* clearThumbnailImage(const std::string& filePath);它们本身也存在几个独立的设计问题:
- 笛卡尔积展开成 4 个具名方法:这 4 个方法是
(操作: set/clear) × (品质: full/thumbnail)的 2×2 展开。未来加一个品质 tier(例如 Medium)就要新增 2 个方法。更合理的做法是把品质参数化为enum class ImageQuality。 - 命名风格不一致:
loadDecodedImage/loadDecodedImageAsThumbnail/clearDecodedImage/clearThumbnailImage三种命名风格混杂。 - 与
loadFileData共用load前缀却语义冲突:loadFileData调用后 filePath 会被清空(节点改用 data),而loadDecodedImage故意保留 filePath。两个方法第一个参数都叫filePath、前缀都是load,但调用后命运截然相反,文档(line 161-162)已经在专门提示这点差异,说明设计者也意识到了这是个坑。 - 返回
Image*但只返回首个匹配,实际却更新所有匹配节点——上一轮已经为此发过评论让你补(all matching nodes are updated regardless)。返回Image*既不准确(隐藏了多匹配)又泄漏了节点指针。返回size_t(匹配数)或bool更诚实。 - 职责堆砌:PAGXDocument 原本是"文档节点容器 + 布局执行 + 字体配置";这次又被加上"按 filePath 索引的 image cache"和"按 filePath 索引的 layer 查询"两个新职责,类越来越胖。
采用方案 B(ImageResourceProvider)后这 4 个 API 全部自然消失:
| 当前 API | 方案 B 下的等价调用 |
|---|---|
document->loadDecodedImage(filePath, img) |
provider->putDecoded(filePath, img) |
document->loadDecodedImageAsThumbnail(filePath, img) |
provider->putThumbnail(filePath, img) |
document->clearDecodedImage(filePath) |
provider->clearDecoded(filePath) |
document->clearThumbnailImage(filePath) |
provider->clearThumbnail(filePath) |
进而 PAGXDocument 的公开接口表面积净减 4 个,再加上一条评论列出的 setImageFieldByFilePath 私有 helper、LayerBuilderSession::invalidateImagesByFilePath / invalidateAllImages、Image 节点的 2 个字段等,PR 整体的代码净减少进一步扩大。
宿主端(WeChat 的 PAGXImageResourceProvider)还能把品质参数化:
provider->put(filePath, ImageQuality::Full, img);
provider->put(filePath, ImageQuality::Thumbnail, img);
provider->clear(filePath, ImageQuality::Full);未来加 quality tier 时改 enum 即可,不再需要新增公开 API。
如果不采纳方案 B 的退路(方案 A):
即使保留"4 个字段长在 Image 节点上"的现状,仍建议把 PAGXDocument 上的 4 个 API 收口成 2 个:
size_t attachDecodedImage(const std::string& filePath, ImageQuality quality,
std::shared_ptr<tgfx::Image> image);
size_t detachDecodedImage(const std::string& filePath, ImageQuality quality);- 公开 API 数量 4 → 2
- 命名一致(
attach/detach),避免与loadFileData的load混淆 - 返回
size_t(匹配数)替代Image*,更诚实 - 加 quality tier 只扩 enum,不增 API
方案 A 改动很小、纯粹 API 收口(内部实现可不变),是不动节点字段前提下能做的最优解。
总之,方案 B 是一揽子解法,能同时清理:
include/pagx/nodes/Image.h的 2 个字段 + tgfx 依赖include/pagx/PAGXDocument.h的 4 个公开 APIsetImageFieldByFilePath+ 两个 invalidate 方法- 上几轮评审遗留下来的几个 API 文档/语义问题的根源
如果作为后续 PR 推进,建议把"删除这 4 个 API"和"引入 provider"放进同一次重构,让 API 演进一步到位,避免 deprecated 期带来更多兼容包袱。
| * Returned reference is invalidated by the next notifyChange() call; copy if you need to retain | ||
| * it. | ||
| */ | ||
| const std::vector<const Layer*>& findLayersByImageFilePath(const std::string& imageFilePath); |
There was a problem hiding this comment.
API 边界建议:imageResourceProvider() getter 和 findLayersByImageFilePath() 是否该出现在公开头 include/pagx/ 里?
架构层面的建议,不阻塞合入。结论:这两个方法都更适合作为内部接口,不该进入对外公开面。
1. imageResourceProvider() getter(line 166)
setImageResourceProvider() 是依赖注入入口,必须公开,没问题。但配套的 getter 看调用点:
src/pagx/PAGXDocument.cpp:60(getExternalFilePaths内部 —— PAGXDocument 自己调自己,直接用成员_imageResourceProvider即可,不需要 public getter)src/renderer/LayerBuilder.cpp:1630(渲染层内部)
宿主侧(PAGXView)自己持有 shared_ptr<PAGXImageResourceProvider> imageProvider,从不需要从 document 把它取回来。也就是说没有任何真正的"外部使用者"调用这个 getter,它的受众只有 PAGXDocument 自身 + 同库的 LayerBuilder。
→ 建议降为内部:PAGXDocument 内部直接访问成员;LayerBuilder 通过 friend 拿到(目前 LayerBuilder 还不是 PAGXDocument 的 friend,需要加一行)。
这顺带也化解了上面讨论过的 "setter 收 shared_ptr、getter 返回裸指针不对称 + getter 是 const 方法却返回非 const 指针" 的问题 —— 一旦 getter 不再对外,它的返回类型就纯属内部约定,对称性/const 正确性都退化为实现细节。
2. findLayersByImageFilePath()(line 187)
这个更应该移出公开头,理由更强:
a. 返回内部节点类型 const Layer*。 Layer 是 pagx 文档模型的内部节点。把它通过公开 API 暴露给外部,外部其实什么也做不了 —— 必须再配合同样内部的 LayerBuilderSession::getTgfxLayers() 才有意义。
b. 它与 LayerBuilderSession::getTgfxLayers() 强耦合且必须配对使用。 看两个真实调用点 PAGXView.cpp:815 和 :976,模式完全一样:
const auto& affectedLayers = document->findLayersByImageFilePath(path);
for (const auto* pagxLayer : affectedLayers) {
auto tgfxLayers = builderSession->getTgfxLayers(pagxLayer); // 必须再过一手
for (const auto& tgfxLayer : tgfxLayers) {
auto bounds = tgfxLayer->getBounds(rootLayer);
// ...
}
}findLayersByImageFilePath 返回的 const Layer* 对调用方本身无用,只是喂给 getTgfxLayers 的中转 key。一个方法在公开头 include/pagx/PAGXDocument.h、与之必须配对的另一个在内部 src/renderer/LayerBuilder.h,这种割裂本身就说明 public 的那个放错了层。
c. 更优雅的做法:把完整能力收口到 LayerBuilderSession(它本就同时掌握 document 和 tgfx 层映射)。 例如:
// src/renderer/LayerBuilder.h —— 这些逻辑本来就该归属渲染内部层
class LayerBuilderSession {
public:
// 返回某 filePath 关联的所有 tgfx 层(已解析 Composition 实例化)
std::vector<std::shared_ptr<tgfx::Layer>> getTgfxLayersByImageFilePath(
const std::string& filePath);
};这样:
findLayersByImageFilePath从 PAGXDocument 的 public 降为 private +friend class LayerBuilder(或整体移入 LayerBuilderSession),不再出现在公开头- PAGXView 两处双层循环可简化为一步
builderSession->getTgfxLayersByImageFilePath(path) - 内部节点类型
const Layer*不再泄漏到公开 API
建议的层次划分
| 方法 | 当前位置 | 建议 |
|---|---|---|
setImageResourceProvider() |
public | ✅ 保留 public(注入入口) |
imageResourceProvider() getter |
public | → 内部(friend LayerBuilder + PAGXDocument 内部用成员) |
findLayersByImageFilePath() |
public | → 内部(private+friend,或整体移入 LayerBuilderSession) |
整体目标:include/pagx/ 的公开面恢复到"纯文档模型 + setImageResourceProvider 这一个对外注入点",不泄漏任何渲染层 / 内部节点类型(Layer*)。当前 PR 还未发版,是收紧公开面的最佳时机 —— 一旦发布,这两个方法就成了需要维护兼容性的对外契约。
如作为后续 PR 处理可不阻塞本 PR;若不采纳也欢迎说明这两个方法的预期外部使用场景。
…lic to private API.
…ate imageProvider caching.
… into feature/billyjin_pagx_new
本 PR 为 PAGX viewer 增加微信小程序(WeChat MiniProgram)端的完整支持,并包含一系列渲染修复与性能优化。
主要内容:
基线更新说明:
本次提交更新了 PAGX 测试的基线截图(version.json),原因如下:
变更列表: