Skip to content
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 the performance of zIndex when there are a large number of nodes. #5701

Closed
wants to merge 4 commits into from

Conversation

caryliu1999
Copy link
Contributor

@caryliu1999 caryliu1999 commented Nov 7, 2019

相关issue Re: cocos-creator/2d-tasks#1186

Changes:

1.延迟localZOrder的更新到sortAllChildren之前。
2.删除之前的childArrivalZOrder的字段,改为使用节点在children里的索引。

问题说明:

测试例:
testdemo 3.zip

在子节点数量较多时,每帧对所有子节点的zIndex更新时会性能消耗较高,这种极端情况的使用场景比较少,所以这里的问题正常情况下不是很明显。测试1600个节点每帧更新zIndex的性能在Web端,耗时140ms左右,帧率只有7帧。

image

耗时主要是在zIndex更新时,会立即同步更新所有子节点的localZOrder,这样在同一时间对所有子节点进行zIndex修改时,这部分的消耗就比较明显。

image

优化之后,在web端的表现基本没有性能消耗,Logic的耗时只有2ms左右,帧率也能达到57-60帧。

image

cocos2d/core/CCNode.js Show resolved Hide resolved
cocos2d/core/CCNode.js Outdated Show resolved Hide resolved
var _children = this._children, child;
for (let i = 0, len = _children.length; i < len; i++) {
child = _children[i];
child._updateOrderOfArrival(i + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

我记得以前 _childArrivalOrder 是用来标记节点的激活顺序的,确保节点之间的排序是稳定的。不过看了下现有代码似乎不是这么回事。如果这样的话,这个变量似乎之前就不应该存在呀 @pandamicro
我能看到的一个问题是,这样一来只要一个节点改变平级顺序,所有同级节点的 SIBLING_ORDER_CHANGED 事件都会触发,这应该是不需要的。

Copy link
Contributor

Choose a reason for hiding this comment

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

我记得以前 _childArrivalOrder 是用来标记节点的激活顺序的,确保节点之间的排序是稳定的。不过看了下现有代码似乎不是这么回事。如果这样的话,这个变量似乎之前就不应该存在呀

这个 childArrivalOrder 是我加的,事情是这样的,最开始的时候是没有childArrivalOrder的,当时有一个globalArrivalOrder,全局使用这个变量,而且只增不减,后面斗地主他们那边反馈,他们的节点太多了,随随便便这个globalArrivalOrder就超过 ffff 爆了,后面就增加了一个childArrivalOrder 由父节点维护,就没那么容易溢出,并且加了重新分配的机制,即使溢出了也没事。当时就和刘航讨论过用 children 数组里的 index 来代替这个数值的可能性,但当时不熟,就没敢这么改。然后后面刘航又调整过几次这个arrivalOrder的事,这个arrivalOrder就越来越没用了,现在就想着干脆移除好了,直接用index来代替。

我能看到的一个问题是,这样一来只要一个节点改变平级顺序,所有同级节点的 SIBLING_ORDER_CHANGED 事件都会触发,这应该是不需要的。

这个结果和现在的行为是一样的,现在也是一个节点的zIndex变了,会导致全部兄弟节点触发事件。感觉没有必要,但是不太确定优化之后会不会有隐藏问题。

Copy link
Contributor

Choose a reason for hiding this comment

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

当时的pr,#3720

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我能看到的一个问题是,这样一来只要一个节点改变平级顺序,所有同级节点的 SIBLING_ORDER_CHANGED 事件都会触发,这应该是不需要的。

一个节点修改了zIndex之后,其他平级节点在children中的顺序就不一样了,需要同步更新一下最新的index次序也就是arrivalOrder,现在延迟之后跟排序一样只会执行一次,也还好。

Copy link
Contributor

Choose a reason for hiding this comment

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

我说一下原始的逻辑

在任何节点新增、插入的过程中都会对 arrivalOrder 做更新,这样就可以保障节点按照用户加入的顺序是稳定的,而不会导致每次排序后可能调换位置的可能。js sort 对相同的 value 是不稳定排序

现在的逻辑变成了保障当前顺序不变,结合 insertChild,addChild,setSiblingIndex 的使用,看上去没有问题

_updateOrderOfArrival () {
var arrivalOrder = this._parent ? ++this._parent._childArrivalOrder : 0;
_updateOrderOfArrival (index) {
var arrivalOrder = index || (this._parent ? this._parent.children.length + 1 : 1);
Copy link
Contributor

@holycanvas holycanvas Nov 8, 2019

Choose a reason for hiding this comment

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

这里貌似有点隐患,当 index 传入 0 的时候,arrivalOrder会是 length + 1

Copy link
Contributor

Choose a reason for hiding this comment

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

哦,没有传0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里貌似有点隐患,当 index 传入 0 的时候,arrivalOrder会是 length + 1

这个次序都是从1开始,传进来的最小值是1

Copy link
Contributor

Choose a reason for hiding this comment

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

如果重复调用这个方法(不传参),可能会导致结果错误吧,因为大家都是使用 length+1 作为 arrival order,那就没意义了

Copy link
Contributor

Choose a reason for hiding this comment

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

目前引擎中没有这样的调用,不过这算是一个隐患

Copy link
Contributor

Choose a reason for hiding this comment

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

目前在 _onBatchCreated 调用的时候,由于 parent.children 长度是固定的,所以这里会导致 arrivalOrder 的值都相等。这是不是就有问题了?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

目前在 _onBatchCreated 调用的时候,由于 parent.children 长度是固定的,所以这里会导致 arrivalOrder 的值都相等。这是不是就有问题了?

_onBatchCreated 里触发节点的 _updateOrderOfArrival 最终产生的zOrder是全部相等的,不过在节点的node-active里会去重置并更新为最新的顺序,但是在父节点是未激活的情况下就不正确了,跟之前的行为不一致。这里为了消除这个隐患,保证版本行为一致,还是保留了 _childArrivalOrder 字段。

修改说明:

  1. 保留 _childArrivalOrder ,但是还是删除了 _childArrivalOrder 溢出时的判定,在sortAllChildren里面,进行更新之前重置 _childArrivalOrder 为 1,这样不再传递index,也不需要在每个 _updateOrderOfArrival 里面去做是否溢出的判定。

补充:

现在的ArrivalOrder更新行为其实就跟之前保持一致了,只不过改变了重置的时机,保证不会溢出。从最早的globalArrivalOrder到现在的 _childArrivalOrder都没有保证所有状态下的次序稳定,按道理这个 ArrivalOrder 应该是在添加到某个节点下时,设置 1 次,记录添加到其父节点的时机。但是一直以来这个值都会在重排所有子节点时_updateOrderOfArrival里进行全局ArrivalOrder的自增并更新为最新的children顺序,所以目前这个并不是绝对稳定。

举个例子:比如场景创建时子节点 A01, B02, C03, D04 (01,代表zIndex为0,arrival为1),设置C的zIndex为1之后,所有子节点的arrivalOrder重排,次序变为 A01, B02, D03, C14,再重新设置C的zIndex为0之后,最终的顺序就是 A01, B02, D03, C04。虽然这种情况没想到有什么具体的使用场景,但是目前的 ArrivalOrder 只是保证了大部分状态下的次序稳定。

_updateOrderOfArrival () {
var arrivalOrder = this._parent ? ++this._parent._childArrivalOrder : 0;
_updateOrderOfArrival (index) {
var arrivalOrder = index || (this._parent ? this._parent.children.length + 1 : 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

哦,没有传0


this._parent._childArrivalOrder = siblings.length;
}

this.emit(EventType.SIBLING_ORDER_CHANGED);
Copy link
Contributor

Choose a reason for hiding this comment

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

目前调用到 _updateOrderOfArrival 的地方好像都不需要触发这个事件了。主要是 sortAllChildren、_onHierarchyChanged(也会调用 sortAllChildren)、_onBatchCreated(场景加载过程中不需要触发事件)。所以建议在 sortAllChildren 的过程中再触发 SIBLING_ORDER_CHANGED

Copy link
Contributor

Choose a reason for hiding this comment

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

赞同

cocos2d/core/CCNode.js Show resolved Hide resolved
@holycanvas holycanvas changed the base branch from v2.2.1-release to v2.2.2 November 15, 2019 09:20
@caryliu1999
Copy link
Contributor Author

@holycanvas 这个PR里面是合并了其他需要在2.2.2更新的几个PR修改吗?

@holycanvas
Copy link
Contributor

不是,rebase的时候带上的,你清理一下

…ndex

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@cocos-robot cocos-robot changed the base branch from v2.2.2 to v2.3.0 November 28, 2019 06:28
Copy link
Contributor

@cocos-robot cocos-robot left a comment

Choose a reason for hiding this comment

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

@caryliu1999, v2.2.2 branch will be deleted, so we edited the base branch to v2.3.0, or this PR will be killed by GitHub.
Please review the commits history to ensure that the PR does not polluted by unneeded commits from your origin branch.
If you need to merge to other branch, you can first click the Edit button on the right side of the PR title, then switch the base branch.
If necessary, welcome to resubmit a new PR. Thanks!

@holycanvas
Copy link
Contributor

应该是合了material的pr冲突了。。。

@caryliu1999
Copy link
Contributor Author

caryliu1999 commented Dec 13, 2019

已重新提交 #5899 关闭该PR

@caryliu1999 caryliu1999 deleted the v2.2.1-release-zIndex branch June 22, 2021 03:08
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.

None yet

5 participants