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

fixed the findCamera problem #3642

Merged
merged 1 commit into from Dec 18, 2018
Merged

Conversation

ColinCollins
Copy link
Contributor

issue: https://github.com/cocos-creator/2d-tasks/issues/878

fixed the findCamera can't get the correct Camera target when node prop still not init.

  1. 首先保证了父节点的 cullingMask 会优先于 default group 状态下的子节点 cullingMask.
  2. 不去修改 Node 是为了保证只有调用到这个 API 时才会进行遍历父节点。

_getGroupIndex (node) {
let groupIndex = node.groupIndex;
if (groupIndex === 0 && node.parent) {
groupIndex = this._getGroupIndex(node.parent);
Copy link
Contributor

Choose a reason for hiding this comment

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

这样就是 node.hitTest 里性能消耗比较大,@pandamicro 有其他建议吗

Copy link
Contributor

Choose a reason for hiding this comment

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

group 设置如果是一个非常低频的操作,建议直接在 group setter 中刷新子节点树

Copy link
Contributor

@2youyou2 2youyou2 Dec 18, 2018

Choose a reason for hiding this comment

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

还有修改 parent 的时候也要再计算,这样 render flow 中 culling mask 的计算可以去掉了

Copy link
Contributor

Choose a reason for hiding this comment

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

可以啊,RenderFlow 的调用频率比 group setter, parent setter 高太多了

@ColinCollins
Copy link
Contributor Author

ColinCollins commented Dec 17, 2018

二改,增加了 cullingMask 判断避免非 default 分组的多次重复遍历。
但若是 default 仍然会有性能问题。

@ColinCollins
Copy link
Contributor Author

  1. 在 _onBatchCreatred 进行序列化后的及时更新,之后用户在修改 group 的时候回同步的检测到 groupIndex.
  2. 去除了 render-flow 中对 cullingMask 的遍历同步。

this.groupIndex = cc.game.groupList.indexOf(value);
this.groupIndex = this._getParentCullingMask(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

这个获取的包括父节点的信息,不要修改到当前节点。。

@@ -3053,6 +3059,15 @@ var Node = cc.Class({
eventManager.pauseTarget(this);
}
},

// traversal the node tree, child cullingMask must keep the same with the parent.
_getParentCullingMask (node) {
Copy link
Contributor

@2youyou2 2youyou2 Dec 18, 2018

Choose a reason for hiding this comment

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

这个函数获取的是 groupIndex,不是 cullingmask,叫 _getParentGroupIndex 吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

有道理,石乐志

@@ -1308,6 +1311,9 @@ var Node = cc.Class({

this._updateOrderOfArrival();

// synchronize _cullingMask
this._cullingMask = 1 << this._getParentGroupIndex(this);;
Copy link
Contributor

Choose a reason for hiding this comment

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

onBatchRestored 里面也需要做同样的逻辑

@@ -3053,6 +3059,15 @@ var Node = cc.Class({
eventManager.pauseTarget(this);
}
},

// traversal the node tree, child cullingMask must keep the same with the parent.
_getParentGroupIndex (node) {
Copy link
Contributor

Choose a reason for hiding this comment

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

_getActualGroupIndex

@pandamicro pandamicro merged commit d625b4b into cocos:v2.0-release Dec 18, 2018
_getActualGroupIndex (node) {
let groupIndex = node.groupIndex;
if (groupIndex === 0 && node.parent) {
groupIndex = this._getActualGroupIndex(node.parent);
Copy link
Contributor

Choose a reason for hiding this comment

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

干嘛要传入参数啊,直接用 this 不就完事了吗?你都不用 this 的话,这个函数就改成局部函数呗

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

4 participants