Skip to content

Commit

Permalink
Fix: destroy and removeChild performance (#10505)
Browse files Browse the repository at this point in the history 
* - optimise remove child
- change the way render group is stored
- added parentRenderGroup
- renderGroup now the owned render group

* remove dead code

* optimise destroy remove children

* optimise the rendergroup

* fix test import

* finak clean up

* fix parent issue

* final tweaks
update tests tio be more readable

* lint

* address feedback
  • Loading branch information
@GoodBoyDigital
GoodBoyDigital committed May 14, 2024
1 parent af5db9e commit da12bce
Show file tree
Hide file tree
Showing 21 changed files with 315 additions and 358 deletions.
106 changes: 41 additions & 65 deletions src/scene/container/Container.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,13 +362,15 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
/** @private */
public _updateFlags = 0b1111;

// is this container the root of a renderGroup?
// TODO implement this in a few more places
/** @private */
public isRenderGroupRoot = false;
// the render group this container belongs to OR owns
// the render group this container owns
/** @private */
public renderGroup: RenderGroup = null;
// the render group this container belongs to
/** @private */
public parentRenderGroup: RenderGroup = null;
// the index of the container in the render group
/** @private */
public parentRenderGroupIndex: number = 0;

// set to true if the container has changed. It is reset once the changes have been applied
// by the transform system
Expand Down Expand Up @@ -626,9 +628,9 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
this.children.splice(this.children.indexOf(child), 1);
this.children.push(child);

if (this.renderGroup && !this.isRenderGroupRoot)
if (this.parentRenderGroup)
{
this.renderGroup.structureDidChange = true;
this.parentRenderGroup.structureDidChange = true;
}

return child;
Expand All @@ -652,9 +654,11 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
// TODO - OPtimise this? could check what the parent has set?
child._updateFlags = 0b1111;

if (this.renderGroup)
const renderGroup = this.renderGroup || this.parentRenderGroup;

if (renderGroup)
{
this.renderGroup.addChild(child);
renderGroup.addChild(child);
}

this.emit('childAdded', child, this, this.children.length - 1);
Expand Down Expand Up @@ -703,6 +707,10 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
{
this.renderGroup.removeChild(child);
}
else if (this.parentRenderGroup)
{
this.parentRenderGroup.removeChild(child);
}

child.parent = null;
this.emit('childRemoved', child, this, index);
Expand Down Expand Up @@ -730,25 +738,15 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
if (this.didChange) return;
this.didChange = true;

if (this.isRenderGroupRoot)
if (this.parentRenderGroup)
{
const renderGroupParent = this.renderGroup.renderGroupParent;
// lets update its parent..

if (renderGroupParent)
{
renderGroupParent.onChildUpdate(this);
}
}
else if (this.renderGroup)
{
this.renderGroup.onChildUpdate(this);
this.parentRenderGroup.onChildUpdate(this);
}
}

set isRenderGroup(value: boolean)
{
if (this.isRenderGroupRoot && value === false)
if (this.renderGroup && value === false)
{
throw new Error('[Pixi] cannot undo a render group just yet');
}
Expand All @@ -765,18 +763,16 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
*/
get isRenderGroup(): boolean
{
return this.isRenderGroupRoot;
return !!this.renderGroup;
}

/** This enables the container to be rendered as a render group. */
public enableRenderGroup()
{
// does it OWN the render group..
if (this.renderGroup && this.renderGroup.root === this) return;

this.isRenderGroupRoot = true;
if (this.renderGroup) return;

const parentRenderGroup = this.renderGroup;
const parentRenderGroup = this.parentRenderGroup;

if (parentRenderGroup)
{
Expand All @@ -785,27 +781,9 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE

this.renderGroup = new RenderGroup(this);

// find children render groups and move them out..
if (parentRenderGroup)
{
for (let i = 0; i < parentRenderGroup.renderGroupChildren.length; i++)
{
const childRenderGroup = parentRenderGroup.renderGroupChildren[i];
let parent = childRenderGroup.root;

while (parent)
{
if (parent === this)
{
this.renderGroup.addRenderGroupChild(childRenderGroup);

break;
}
parent = parent.parent;
}
}

parentRenderGroup.addRenderGroupChild(this.renderGroup);
parentRenderGroup.addChild(this);
}

this._updateIsSimple();
Expand All @@ -818,7 +796,7 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
/** @ignore */
public _updateIsSimple()
{
this.isSimple = !(this.isRenderGroupRoot) && (this.effects.length === 0);
this.isSimple = !(this.renderGroup) && (this.effects.length === 0);
}

/**
Expand All @@ -831,14 +809,11 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE

if (this.renderGroup)
{
if (this.isRenderGroupRoot)
{
this._worldTransform.copyFrom(this.renderGroup.worldTransform);
}
else
{
this._worldTransform.appendFrom(this.relativeGroupTransform, this.renderGroup.worldTransform);
}
this._worldTransform.copyFrom(this.renderGroup.worldTransform);
}
else if (this.parentRenderGroup)
{
this._worldTransform.appendFrom(this.relativeGroupTransform, this.parentRenderGroup.worldTransform);
}

return this._worldTransform;
Expand Down Expand Up @@ -1224,9 +1199,9 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
set blendMode(value: BLEND_MODES)
{
if (this.localBlendMode === value) return;
if (this.renderGroup && !this.isRenderGroupRoot)
if (this.parentRenderGroup)
{
this.renderGroup.structureDidChange = true;
this.parentRenderGroup.structureDidChange = true;
}

this._updateFlags |= UPDATE_BLEND;
Expand Down Expand Up @@ -1259,9 +1234,9 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE

if ((this.localDisplayStatus & 0b010) >> 1 === valueNumber) return;

if (this.renderGroup && !this.isRenderGroupRoot)
if (this.parentRenderGroup)
{
this.renderGroup.structureDidChange = true;
this.parentRenderGroup.structureDidChange = true;
}

this._updateFlags |= UPDATE_VISIBLE;
Expand All @@ -1284,9 +1259,9 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE

if ((this.localDisplayStatus & 0b100) >> 2 === valueNumber) return;

if (this.renderGroup && !this.isRenderGroupRoot)
if (this.parentRenderGroup)
{
this.renderGroup.structureDidChange = true;
this.parentRenderGroup.structureDidChange = true;
}

this._updateFlags |= UPDATE_VISIBLE;
Expand All @@ -1310,9 +1285,9 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
this._updateFlags |= UPDATE_VISIBLE;
this.localDisplayStatus ^= 0b001;

if (this.renderGroup && !this.isRenderGroupRoot)
if (this.parentRenderGroup)
{
this.renderGroup.structureDidChange = true;
this.parentRenderGroup.structureDidChange = true;
}

this._onUpdate();
Expand Down Expand Up @@ -1343,6 +1318,9 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE
if (this.destroyed) return;
this.destroyed = true;

// remove children is faster than removeChild..
const oldChildren = this.removeChildren(0, this.children.length);

this.removeFromParent();
this.parent = null;
this._maskEffect = null;
Expand All @@ -1359,8 +1337,6 @@ export class Container<C extends ContainerChild = ContainerChild> extends EventE

const destroyChildren = typeof options === 'boolean' ? options : options?.children;

const oldChildren = this.removeChildren(0, this.children.length);

if (destroyChildren)
{
for (let i = 0; i < oldChildren.length; ++i)
Expand Down

0 comments on commit da12bce

Please sign in to comment.