8000 Optimize the logic of remove and add entity. by cptbtptpbcptdtptp · Pull Request #1930 · galacean/engine · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Optimize the logic of remove and add entity. #1930

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

Merged
merged 4 commits into from
Feb 18, 2025
Merged

Optimize the logic of remove and add entity. #1930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
82cd7bb
feat: update code
cptbtptpbcptdtptp Dec 27, 2023
25e7678
Merge remote-tracking branch 'origin/main' into opt/removeAdd
cptbtptpbcptdtptp Feb 18, 2025
9ba1e15
feat: update code
cptbtptpbcptdtptp Feb 18, 2025
21747ad
feat: update code
cptbtptpbcptdtptp Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 41 additions & 24 deletions packages/core/src/Entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,43 @@
return entity;
}

/**
* @internal
*/
static _removeFormChildren(children: Entity[], entity: Entity): void {
const count = children.length - 1;
for (let i = entity._siblingIndex; i < count; i++) {
const child = children[i + 1];
children[i] = child;
child._siblingIndex = i;
}
children.length = count;
entity._siblingIndex = -1;
}
Comment on lines +84 to +93
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in method name.

The method name has a typo: _removeFormChildren should be _removeFromChildren.

Apply this diff to fix the typo:

-  static _removeFormChildren(children: Entity[], entity: Entity): void {
+  static _removeFromChildren(children: Entity[], entity: Entity): void {

Note: This change will require updating all references to this method.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentat 10000 ion. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
static _removeFormChildren(children: Entity[], entity: Entity): void {
const count = children.length - 1;
for (let i = entity._siblingIndex; i < count; i++) {
const child = children[i + 1];
children[i] = child;
child._siblingIndex = i;
}
children.length = count;
entity._siblingIndex = -1;
}
static _removeFromChildren(children: Entity[], entity: Entity): void {
const count = children.length - 1;
for (let i = entity._siblingIndex; i < count; i++) {
const child = children[i + 1];
children[i] = child;
child._siblingIndex = i;
}
children.length = count;
entity._siblingIndex = -1;
}


/**
* @internal
*/
static _addToChildren(children: Entity[], entity: Entity, index?: number): void {
const childCount = children.length;
children.length = childCount + 1;
if (index === undefined) {
children[childCount] = entity;
entity._siblingIndex = childCount;
} else {
if (index < 0 || index > childCount) {
throw `The index ${index} is out of child list bounds ${childCount}`;
}
for (let i = childCount; i > index; i--) {
const swapChild = children[i - 1];
swapChild._siblingIndex = i;
children[i] = swapChild;
}
entity._siblingIndex = index;
children[index] = entity;
}

Check warning on line 115 in packages/core/src/Entity.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/Entity.ts#L105-L115 

Added lines #L105 - L115 were not covered by tests
}

/** The name of entity. */
name: string;
/** The layer the entity belongs to. */
Expand Down Expand Up @@ -315,13 +352,13 @@
}

if (child._isRoot) {
child._scene._removeFromEntityList(child);
const oldScene = child._scene;
Entity._removeFormChildren(oldScene._rootEntities, child);
child._isRoot = false;

this._addToChildrenList(index, child);
child._parent = this;

const oldScene = child._scene;
const newScene = this._scene;

let inActiveChangeFlag = ActiveChangeFlag.None;
Expand Down Expand Up @@ -582,14 +619,8 @@
_removeFromParent(): void {
const oldParent = this._parent;
if (oldParent != null) {
const oldSibling = oldParent._children;
let index = this._siblingIndex;
oldSibling.splice(index, 1);
for (let n = oldSibling.length; index < n; index++) {
oldSibling[index]._siblingIndex--;
}
Entity._removeFormChildren(oldParent._children, this);

Check warning on line 622 in packages/core/src/Entity.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/Entity.ts#L622 

Added line #L622 was not covered by tests
this._parent = null;
this._siblingIndex = -1;
this._dispatchModify(EntityModifyFlags.Child, oldParent);
}
}
Expand Down Expand Up @@ -645,21 +676,7 @@
}

private _addToChildrenList(index: number, child: Entity): void {
const children = this._children;
const childCount = children.length;
if (index === undefined) {
child._siblingIndex = childCount;
children.push(child);
} else {
if (index < 0 || index > childCount) {
throw `The index ${index} is out of child list bounds ${childCount}`;
}
child._siblingIndex = index;
children.splice(index, 0, child);
for (let i = index + 1, n = childCount + 1; i < n; i++) {
children[i]._siblingIndex++;
}
}
Entity._addToChildren(this._children, child, index);
this._dispatchModify(EntityModifyFlags.Child, this);
}

Expand Down
40 changes: 4 additions & 36 deletions packages/core/src/Scene.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,11 @@
const oldScene = enti 8000 ty._scene;
if (oldScene !== this) {
if (oldScene && isRoot) {
oldScene._removeFromEntityList(entity);
Entity._removeFormChildren(oldScene._rootEntities, entity);

Check warning on line 340 in packages/core/src/Scene.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/Scene.ts#L340 

Added line #L340 was not covered by tests
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update method name to match the fix in Entity class.

Once the typo in Entity._removeFormChildren is fixed, update the references here to use the corrected name.

Apply this diff to fix the references:

-      Entity._removeFormChildren(oldScene._rootEntities, child);
+      Entity._removeFromChildren(oldScene._rootEntities, child);

Also applies to: 378-378

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 340-340: packages/core/src/Scene.ts#L340
Added line #L340 was not covered by tests

}
this._addToRootEntityList(index, entity);
Entity._addToChildren(this._rootEntities, entity, index);
} else if (!isRoot) {
this._addToRootEntityList(index, entity);
Entity._addToChildren(this._rootEntities, entity, index);

Check warning on line 344 in packages/core/src/Scene.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/Scene.ts#L344 

Added line #L344 was not covered by tests
}

// Process entity active/inActive
Expand Down Expand Up @@ -375,9 +375,8 @@
*/
removeRootEntity(entity: Entity): void {
if (entity._isRoot && entity._scene == this) {
this._removeFromEntityList(entity);
Entity._removeFormChildren(this._rootEntities, entity);
entity._isRoot = false;

let inActiveChangeFlag = ActiveChangeFlag.None;
this._isActiveInEngine && entity._isActiveInHierarchy && (inActiveChangeFlag |= ActiveChangeFlag.Hierarchy);
entity._isActiveInScene && (inActiveChangeFlag |= ActiveChangeFlag.Scene);
Expand Down Expand Up @@ -494,19 +493,6 @@
);
}

/**
* @internal
*/
_removeFromEntityList(entity: Entity): void {
const rootEntities = this._rootEntities;
let index = entity._siblingIndex;
rootEntities.splice(index, 1);
for (let n = rootEntities.length; index < n; index++) {
rootEntities[index]._siblingIndex--;
}
entity._siblingIndex = -1;
}

/**
* @internal
*/
Expand All @@ -530,24 +516,6 @@
allCreatedScenes.splice(allCreatedScenes.indexOf(this), 1);
}

private _addToRootEntityList(index: number, rootEntity: Entity): void {
const rootEntities = this._rootEntities;
const rootEntityCount = rootEntities.length;
if (index === undefined) {
rootEntity._siblingIndex = rootEntityCount;
rootEntities.push(rootEntity);
} else {
if (index < 0 || index > rootEntityCount) {
throw `The index ${index} is out of child list bounds ${rootEntityCount}`;
}
rootEntity._siblingIndex = index;
rootEntities.splice(index, 0, rootEntity);
for (let i = index + 1, n = rootEntityCount + 1; i < n; i++) {
rootEntities[i]._siblingIndex++;
}
}
}

private _computeLinearFogParams(fogStart: number, fogEnd: number): void {
const fogRange = fogEnd - fogStart;
const fogParams = this._fogParams;
Expand Down
0