Skip to content

补充 jsb中 spine.updateAnimation() 遗漏的 markForUpdateRenderData() #18548

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

Open
wants to merge 1 commit into
base: v3.8.7
Choose a base branch
from

Conversation

finscn
Copy link
Contributor

@finscn finscn commented Apr 2, 2025

ts 版本的 updateAnimation() 中有 调用 markForUpdateRenderData()

但是 jsb版本中缺失了这条语句

Re: #

Changelog


Continuous Integration

This pull request:

  • needs automatic test cases check.

    Manual trigger with @cocos-robot run test cases afterward.

  • does not change any runtime related code or build configuration

    If any reviewer thinks the CI checks are needed, please uncheck this option, then close and reopen the issue.


Compatibility Check

This pull request:

  • changes public API, and have ensured backward compatibility with deprecated features.
  • affects platform compatibility, e.g. system version, browser version, platform sdk version, platform toolchain, language version, hardware compatibility etc.
  • affects file structure of the build package or build configuration which requires user project upgrade.
  • introduces breaking changes, please list all changes, affected features and the scope of violation.

ts 版本的 updateAnimation()  中有 调用 markForUpdateRenderData()

但是 jsb版本中缺失了这条语句
Copy link

github-actions bot commented Apr 2, 2025

Code Size Check Report

Wechat (WASM) Before After Diff
2D Empty (legacy pipeline) 1002433 bytes 1002433 bytes ✅ 0 bytes
2D All (legacy pipeline) 2664474 bytes 2664474 bytes ✅ 0 bytes
2D All (new pipeline) 2752682 bytes 2752682 bytes ✅ 0 bytes
(2D + 3D) All 10005962 bytes 10005962 bytes ✅ 0 bytes
Web (WASM + ASMJS) Before After Diff
(2D + 3D) All 16919743 bytes 16919743 bytes ✅ 0 bytes

Interface Check Report

This pull request does not change any public interfaces !

@dumganhar dumganhar requested a review from bofeng-song April 2, 2025 06:25
@@ -426,6 +426,8 @@ const cacheManager = require('./jsb-cache-manager');
const node = this.node;
if (!node) return;

this.markForUpdateRenderData();
Copy link
Contributor

@bofeng-song bofeng-song Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个如果没加上会有什么问题吗? 因为 middleWare.render 每帧都执行;理论上可以正常渲染

Copy link
Contributor Author

@finscn finscn Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个如果没加上会有什么问题吗? 因为 middleWare.render 每帧都执行;理论上可以正常渲染

如果你觉得 这个是多余的 , 那建议把 ts 版本的 updateAnimation() 中 的这行代码也移除.

我很看重代码编写思路的一致性. 否则别人在维护你的代码时 会困惑. 为啥做类似的事 , 这里是思路 A, 那里却是思路 B.

我提交的这个 PR #18549 其实也类似.

  • 统一删除 markForUpdateRenderData() (不管它有没有用, 都不应该在 setAnimation/addAnimation 时执行.
  • 统一了判断 animation 是否有效的逻辑, 先判断 , 无效直接返回.
  • 统一通过 this._state 来调用原生方法.

要说这几个不改有什么严重问题吗? 其实也没有. 但是在看代码时 就很困惑.
可能你们很忙, 没时间在乎这些细枝末节. 所以我提 PR, 帮你们做. 希望你们能采纳.

@finscn
Copy link
Contributor Author

finscn commented Apr 7, 2025

又测试了一下,updateAnimation 中的 this.markForUpdateRenderData 还必须要加。
否则 spine动画开始批量模式时,无法正确更新spine的坐标。

@bofeng-song
Copy link
Contributor

又测试了一下,updateAnimation 中的 this.markForUpdateRenderData 还必须要加。 否则 spine动画开始批量模式时,无法正确更新spine的坐标。

这个有具体的demo吗

@finscn
Copy link
Contributor Author

finscn commented Apr 8, 2025

又测试了一下,updateAnimation 中的 this.markForUpdateRenderData 还必须要加。 否则 spine动画开始批量模式时,无法正确更新spine的坐标。

这个有具体的demo吗

在实际项目里. 比较复杂.
是一个scrollview 里 嵌套的UI元素上有开启了批量的 spine动画, 滚动scrollview 时, 如果没有 markForUpdateRenderData() , scrollview 里的其他元素可以正常滚动, 但是 spine动画滚动会出现延迟

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.

2 participants