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.

Sign up for GitHub

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

Jump to bottom

Optimize the performance of zIndex when there are a large number of nodes. #5701

Closed
wants to merge 4 commits into from
Closed

Optimize the performance of zIndex when there are a large number of nodes. #5701

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4d971f1
Optimize the performance of zIndex when there are a large number of n…
caryliu1999 Nov 7, 2019
f2ed989
refine code
caryliu1999 Nov 8, 2019
e9eb565
refine code
caryliu1999 Nov 22, 2019
df7a8ed
Merge remote-tracking branch 'upstream/v2.2.2' into v2.2.1-release-zI…
caryliu1999 Nov 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 16 additions & 29 deletions cocos2d/core/CCNode.js
View file
Open in desktop
Expand Up @@ -1251,10 +1251,6 @@ let NodeDefines = {
this._onSiblingIndexChanged();
}
}

if (CC_JSB && CC_NATIVERENDERER) {
this._proxy.updateZOrder();
holycanvas marked this conversation as resolved.
Show resolved Hide resolved
}
}
},
},
Expand Down Expand Up @@ -1282,7 +1278,6 @@ let NodeDefines = {

this._eventMask = 0;
this._cullingMask = 1;
this._childArrivalOrder = 1;

// Proxy
if (CC_JSB && CC_NATIVERENDERER) {
Expand All @@ -1308,14 +1303,7 @@ let NodeDefines = {

_onSiblingIndexChanged () {
// update rendering scene graph, sort them by arrivalOrder
var parent = this._parent;
var siblings = parent._children;
var i = 0, len = siblings.length, sibling;
for (; i < len; i++) {
sibling = siblings[i];
sibling._updateOrderOfArrival();
}
parent._delaySort();
this._parent._delaySort();
holycanvas marked this conversation as resolved.
Show resolved Hide resolved
},

_onPreDestroy () {
Expand Down Expand Up @@ -3295,19 +3283,10 @@ let NodeDefines = {
return rect;
},

_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 •
edited

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 只是保证了大部分状态下的次序稳定。

this._localZOrder = (this._localZOrder & 0xffff0000) | arrivalOrder;
// redistribute
if (arrivalOrder === 0x0000ffff) {
var siblings = this._parent._children;

siblings.forEach(function (node, index) {
node._localZOrder = (node._localZOrder & 0xffff0000) | (index + 1);
});

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.

赞同

},

Expand Down Expand Up @@ -3372,16 +3351,24 @@ let NodeDefines = {
*/
sortAllChildren () {
if (this._reorderChildDirty) {

this._reorderChildDirty = false;

// delay update arrivalOrder before sort children
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 的使用，看上去没有问题

}

// Optimize reordering event code to fix problems with setting zindex
// https://github.com/cocos-creator/2d-tasks/issues/1186
eventManager._setDirtyForNode(this);

this._reorderChildDirty = false;
var _children = this._children;
if (_children.length > 1) {
// insertion sort
var len = _children.length, i, j, child;
for (i = 1; i < len; i++) {
var j, child;
for (let i = 1, len = _children.length; i < len; i++) {
child = _children[i];
j = i - 1;

Expand Down
2 changes: 1 addition & 1 deletion cocos2d/core/node-activator.js
View file
Open in desktop
Expand Up @@ -187,7 +187,7 @@ var NodeActivator = cc.Class({
--originCount;
}
}
node._childArrivalOrder = node._children.length;

// activate children recursively
for (let i = 0, len = node._children.length; i < len; ++i) {
let child = node._children[i];
Expand Down