-
Notifications
You must be signed in to change notification settings - Fork 505
Optimize NativeDisplayLink to execute VSync callback on UI thread for OHOS platform. #3178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize NativeDisplayLink to execute VSync callback on UI thread for OHOS platform. #3178
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3178 +/- ##
=======================================
Coverage 77.25% 77.25%
=======================================
Files 413 413
Lines 21999 21999
Branches 6283 6283
=======================================
Hits 16996 16996
- Misses 3797 3802 +5
+ Partials 1206 1201 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
另外是不是可以替换为 NativeDisplaySoloist 这样不需要通过每帧都重新request https://developer.huawei.com/consumer/cn/doc/harmonyos-references/capi-native-display-soloist-h |
| namespace pag { | ||
|
|
||
| static std::unordered_map<uint32_t, std::weak_ptr<NativeDisplayLink>> VSyncCallbacks = {}; | ||
| static napi_threadsafe_function js_threadsafe_function = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不需要添加 VSyncCallbacks, 全局只会存在一个 NativeDisplayLink 实例, 多个 PAG 播放时共享同一个 VSync 回调,具体可以看下 PAGAnimator 的实现,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好的
| playing = true; | ||
| OH_NativeVSync_RequestFrame(vSync, &PAGVSyncCallback, this); | ||
| VSyncCallbacks.insert_or_assign(id, weak_from_this()); | ||
| OH_NativeVSync_RequestFrame(vSync, &PAGVSyncCallback, &id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如你所说,这里 OH_NativeVSync_RequestFrame 可以切换为 NativeDisplaySoloist,避免每帧重新调用
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌晚点我再改改
a0a02d0 to
e75ca47
Compare
|
@kevingpqi123 已完成修改。 |
src/platform/ohos/JPAG.cpp
Outdated
| pag::JPAGDiskCache::Init(env, exports) && | ||
| pag::XComponentHandler::Init(env, exports); | ||
| pag::XComponentHandler::Init(env, exports) && | ||
| pag::NativeDisplayLink::Init(env, exports); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
需要明确这里增加定义的原因,这个 Init 函数是 NAPI 模块的入口点,用于向 HarmonyOS/OpenHarmony 的 JavaScript 运行时注册 C++ 类和方法。所有需要暴露给 ArkTS/JS 层调用的 C++ 类都需要在这里初始化, 如所有对外公开的 API 类,以及接收系统事件的入口。
NativeDisplayLink 就是一个系统时钟控制器, 属于 C++ 内部类,不涉及上述内容
| displayLink->callback(); | ||
| if (displayLink->playing) { | ||
| OH_NativeVSync_RequestFrame(displayLink->vSync, &PAGVSyncCallback, displayLink); | ||
| static std::weak_ptr<NativeDisplayLink> weakNativeDisplayLink; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
关于 NativeDisplayLink 的实现,感觉你考虑的太复杂了,可以参考下:https://github.com/Tencent/libpag/blob/main/src/platform/ios/private/NativeDisplayLink.mm
1、关于之前提到的全局只会存在一个 NativeDisplayLink 实例, 多个 PAG 播放时共享同一个 VSync 回调,这个地方是在 AnimationTicker 这个类中实现的,通过单例的方式,对于 NativeDisplayLink 就是一个正常的对象,不需要做额外的处理,正常的生命周期管理即可,AnimationTicker 这个类是在 C++ 层,不属于平台层,很方便的实现全局只有一个 VSync 回调,做到高内聚,而不是把这个功能分散到各平台层
2、相对比之前,这里的修改应该仅仅是切换主线程执行和替换接口实现,不涉及其它内容
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
确实。已按照建议全部进行了修改
2251eaa to
d993f82
Compare
src/platform/ohos/JPAG.cpp
Outdated
| pag::JPAGDiskCache::Init(env, exports) && | ||
| pag::XComponentHandler::Init(env, exports); | ||
| pag::XComponentHandler::Init(env, exports) && | ||
| pag::NativeDisplayLink::InitializeThreadSafeFunction(env); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里也修改成 Init 吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修正
d993f82 to
7743e7f
Compare
|
|
||
| NativeDisplayLink::~NativeDisplayLink() { | ||
| OH_NativeVSync_Destroy(vSync); | ||
| OH_DisplaySoloist_Destroy(displaySoloist); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
析构的时候需要添加上 js_threadsafe_function 的释放吧,不然应用退出后会存在内存泄漏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不好意思,这一点刚处于 pending 状态,没有评论成功
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
应该不行。threadsafe_function 是和 env 绑定的。如果是在析构中释放的话,那需要 NativeDisplayLink 持有 env
然后在创建 NativeDisplayLink 的时候再创建 threadsafe_function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我再找找文档看看,有没有像 Android JNI 那样的机制可以通过 JavaVM 来自动获取当前的 env
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里要结合 https://github.com/Tencent/libpag/blob/main/src/rendering/PAGAnimator.cpp
的代码来看,NativeDisplayLink 在应用启动后只会创建一次,如果中间过程中发生异常被销毁,将不会再触发二次创建,后续也将不会再播放动画
std::shared_ptr PAGAnimator::MakeFrom(std::weak_ptr listener) {
if (listener.expired() || !AnimationTicker::GetInstance()->available()) {
return nullptr;
}
auto animator = std::shared_ptr(new PAGAnimator(std::move(listener)));
animator->weakThis = animator;
return animator;
}
NativeDisplayLink 是由 AnimationTicker 的单例持有的,AnimationTicker 的实例不会创建多个,其生命周期是和整个应用保持一致的,只有当应用销毁的时候,AnimationTicker 才会销毁,进而销毁 NativeDisplayLink
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
看了下 AnimationTicker 确实这里需要加上释放。已更新代码
7743e7f to
be8816b
Compare
优化 OHOS 平台的 NativeDisplayLink 实现,将 VSync 的回调从独立线程切换到 UI 线程(ArkTS 线程)执行。
同时使用 native_display_soloist 替换 native_vsync