From e6a58e5d8d9514b257adaa47cc0a1145608cf764 Mon Sep 17 00:00:00 2001 From: mrmaxm Date: Fri, 13 Dec 2024 15:16:17 +0200 Subject: [PATCH 1/7] optimize render component rootBone property --- src/framework/components/render/component.js | 80 ++++++++++---------- src/framework/components/render/data.js | 1 - src/framework/components/render/system.js | 4 +- 3 files changed, 42 insertions(+), 43 deletions(-) diff --git a/src/framework/components/render/component.js b/src/framework/components/render/component.js index d0b47aca093..aa45faf44ee 100644 --- a/src/framework/components/render/component.js +++ b/src/framework/components/render/component.js @@ -12,8 +12,6 @@ import { AssetReference } from '../../asset/asset-reference.js'; import { Component } from '../component.js'; -import { EntityReference } from '../../utils/entity-reference.js'; - /** * The RenderComponent enables an {@link Entity} to render 3D meshes. The {@link RenderComponent#type} * property can be set to one of several predefined shape types (such as `box`, `sphere`, `cone` @@ -54,9 +52,6 @@ import { EntityReference } from '../../utils/entity-reference.js'; * - [Primitive Shapes](https://playcanvas.github.io/#/graphics/shapes) * - [Loading Render Assets](https://playcanvas.github.io/#/graphics/render-asset) * - * @property {import('../../entity.js').Entity} rootBone A reference to the entity to be used as - * the root bone for any skinned meshes that are rendered by this component. - * * @category Graphics */ class RenderComponent extends Component { @@ -136,10 +131,12 @@ class RenderComponent extends Component { _material; /** - * @type {EntityReference} - * @private + * A reference to the entity to be used as the root bone for any skinned meshes that + * are rendered by this component. + * + * @type {import('../../entity.js').Entity|null} */ - _rootBone; + _rootBone = null; /** * @type {import('../../../core/event-handle.js').EventHandle|null} @@ -177,8 +174,6 @@ class RenderComponent extends Component { super(system, entity); // the entity that represents the root bone if this render component has skinned meshes - this._rootBone = new EntityReference(this, 'rootBone'); - this._rootBone.on('set:entity', this._onSetRootBone, this); // render asset reference this._assetReference = new AssetReference( @@ -275,7 +270,6 @@ class RenderComponent extends Component { * @type {string} */ set type(value) { - if (this._type !== value) { this._area = null; this._type = value; @@ -312,7 +306,6 @@ class RenderComponent extends Component { * @type {MeshInstance[]} */ set meshInstances(value) { - Debug.assert(Array.isArray(value), 'MeshInstances set to a Render component must be an array.'); this.destroyMeshInstances(); @@ -702,28 +695,37 @@ class RenderComponent extends Component { this._assetReference.id = id; } - /** - * @param {import('../../entity.js').Entity} entity - The entity set as the root bone. - * @private - */ - _onSetRootBone(entity) { - if (entity) { - this._onRootBoneChanged(); + set rootBone(value) { + if (this._rootBone !== value) { + const isString = typeof (value) === 'string'; + if (this._rootBone && isString && this._rootBone.getGuid() === value) { + return; + } + + if (this._rootBone) { + this._clearSkinInstances(); + } + + if (value instanceof GraphNode) { + this._rootBone = value; + } else if (isString) { + this._rootBone = this.system.app.getEntityFromIndex(value) || null; + } else { + this._rootBone = null; + } + + if (this._rootBone) { + this._cloneSkinInstances(); + } } } - /** @private */ - _onRootBoneChanged() { - // remove existing skin instances and create new ones, connected to new root bone - this._clearSkinInstances(); - if (this.enabled && this.entity.enabled) { - this._cloneSkinInstances(); - } + get rootBone() { + return this._rootBone; } /** @private */ destroyMeshInstances() { - const meshInstances = this._meshInstances; if (meshInstances) { this.removeFromLayers(); @@ -814,9 +816,9 @@ class RenderComponent extends Component { const scene = app.scene; const layers = scene.layers; - this._rootBone.onParentComponentEnable(); - - this._cloneSkinInstances(); + if (this._rootBone) { + this._cloneSkinInstances(); + } this._evtLayersChanged = scene.on('set:layers', this.onLayersChanged, this); @@ -852,6 +854,10 @@ class RenderComponent extends Component { this._evtLayersChanged?.off(); this._evtLayersChanged = null; + if (this._rootBone) { + this._clearSkinInstances(); + } + if (layers) { this._evtLayerAdded?.off(); this._evtLayerAdded = null; @@ -904,7 +910,6 @@ class RenderComponent extends Component { } _onRenderAssetLoad() { - // remove existing instances this.destroyMeshInstances(); @@ -923,7 +928,6 @@ class RenderComponent extends Component { } _clearSkinInstances() { - for (let i = 0; i < this._meshInstances.length; i++) { const meshInstance = this._meshInstances[i]; @@ -934,23 +938,20 @@ class RenderComponent extends Component { } _cloneSkinInstances() { - - if (this._meshInstances.length && this._rootBone.entity instanceof GraphNode) { - + if (this._meshInstances.length && this._rootBone instanceof GraphNode) { for (let i = 0; i < this._meshInstances.length; i++) { const meshInstance = this._meshInstances[i]; const mesh = meshInstance.mesh; // if skinned but does not have instance created yet if (mesh.skin && !meshInstance.skinInstance) { - meshInstance.skinInstance = SkinInstanceCache.createCachedSkinInstance(mesh.skin, this._rootBone.entity, this.entity); + meshInstance.skinInstance = SkinInstanceCache.createCachedSkinInstance(mesh.skin, this._rootBone, this.entity); } } } } _cloneMeshes(meshes) { - if (meshes && meshes.length) { // cloned mesh instances @@ -1031,10 +1032,9 @@ class RenderComponent extends Component { } resolveDuplicatedEntityReferenceProperties(oldRender, duplicatedIdsMap) { - if (oldRender.rootBone && duplicatedIdsMap[oldRender.rootBone]) { - this.rootBone = duplicatedIdsMap[oldRender.rootBone]; + if (oldRender.rootBone) { + this.rootBone = duplicatedIdsMap[oldRender.rootBone.getGuid()]; } - this._clearSkinInstances(); } } diff --git a/src/framework/components/render/data.js b/src/framework/components/render/data.js index 5ff36ded89e..53ac9f97c13 100644 --- a/src/framework/components/render/data.js +++ b/src/framework/components/render/data.js @@ -1,7 +1,6 @@ class RenderComponentData { constructor() { this.enabled = true; - this.rootBone = null; } } diff --git a/src/framework/components/render/system.js b/src/framework/components/render/system.js index 12688a94cb1..df3d4484cc6 100644 --- a/src/framework/components/render/system.js +++ b/src/framework/components/render/system.js @@ -11,7 +11,6 @@ import { RenderComponent } from './component.js'; import { RenderComponentData } from './data.js'; const _schema = [ - { name: 'rootBone', type: 'entity' }, 'enabled' ]; @@ -30,7 +29,8 @@ const _properties = [ 'type', 'layers', 'isStatic', - 'batchGroupId' + 'batchGroupId', + 'rootBone' ]; /** From f5ba0673aed43bd1cd800ab5b243365e0cfabb71 Mon Sep 17 00:00:00 2001 From: mrmaxm Date: Mon, 16 Dec 2024 12:47:09 +0200 Subject: [PATCH 2/7] expose rootBone property --- src/framework/components/render/component.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/framework/components/render/component.js b/src/framework/components/render/component.js index aa45faf44ee..e04b35c0a26 100644 --- a/src/framework/components/render/component.js +++ b/src/framework/components/render/component.js @@ -135,6 +135,7 @@ class RenderComponent extends Component { * are rendered by this component. * * @type {import('../../entity.js').Entity|null} + * @private */ _rootBone = null; @@ -695,6 +696,11 @@ class RenderComponent extends Component { this._assetReference.id = id; } + /** + * Sets the root bone entity (or entity guid) for the render component. + * + * @type {import('../../entity.js').Entity|string|null} + */ set rootBone(value) { if (this._rootBone !== value) { const isString = typeof (value) === 'string'; @@ -720,6 +726,11 @@ class RenderComponent extends Component { } } + /** + * Gets the root bone entity for the render component. + * + * @type {import('../../entity.js').Entity|null} + */ get rootBone() { return this._rootBone; } From f396776221137efa1ab0f3923f0390c73364b095 Mon Sep 17 00:00:00 2001 From: mrmaxm Date: Mon, 16 Dec 2024 12:54:14 +0200 Subject: [PATCH 3/7] Update src/framework/components/render/component.js Co-authored-by: Will Eastcott --- src/framework/components/render/component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/framework/components/render/component.js b/src/framework/components/render/component.js index e04b35c0a26..b36d1ef5952 100644 --- a/src/framework/components/render/component.js +++ b/src/framework/components/render/component.js @@ -703,7 +703,7 @@ class RenderComponent extends Component { */ set rootBone(value) { if (this._rootBone !== value) { - const isString = typeof (value) === 'string'; + const isString = typeof value === 'string'; if (this._rootBone && isString && this._rootBone.getGuid() === value) { return; } From 26a267d92b38b27e011030cf438b7b72134853ca Mon Sep 17 00:00:00 2001 From: mrmaxm Date: Mon, 16 Dec 2024 13:00:45 +0200 Subject: [PATCH 4/7] use jsdo import --- src/framework/components/render/component.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/framework/components/render/component.js b/src/framework/components/render/component.js index e04b35c0a26..401c7848cec 100644 --- a/src/framework/components/render/component.js +++ b/src/framework/components/render/component.js @@ -12,6 +12,10 @@ import { AssetReference } from '../../asset/asset-reference.js'; import { Component } from '../component.js'; +/** + * @import { Entity } from '../../entity.js' + */ + /** * The RenderComponent enables an {@link Entity} to render 3D meshes. The {@link RenderComponent#type} * property can be set to one of several predefined shape types (such as `box`, `sphere`, `cone` @@ -134,7 +138,7 @@ class RenderComponent extends Component { * A reference to the entity to be used as the root bone for any skinned meshes that * are rendered by this component. * - * @type {import('../../entity.js').Entity|null} + * @type {Entity|null} * @private */ _rootBone = null; @@ -168,7 +172,7 @@ class RenderComponent extends Component { * * @param {import('./system.js').RenderComponentSystem} system - The ComponentSystem that * created this Component. - * @param {import('../../entity.js').Entity} entity - The Entity that this Component is + * @param {Entity} entity - The Entity that this Component is * attached to. */ constructor(system, entity) { @@ -699,7 +703,7 @@ class RenderComponent extends Component { /** * Sets the root bone entity (or entity guid) for the render component. * - * @type {import('../../entity.js').Entity|string|null} + * @type {Entity|string|null} */ set rootBone(value) { if (this._rootBone !== value) { @@ -729,7 +733,7 @@ class RenderComponent extends Component { /** * Gets the root bone entity for the render component. * - * @type {import('../../entity.js').Entity|null} + * @type {Entity|null} */ get rootBone() { return this._rootBone; From dbfcf30b56771e914dee654c8224601859e77707 Mon Sep 17 00:00:00 2001 From: mrmaxm Date: Mon, 16 Dec 2024 13:12:33 +0200 Subject: [PATCH 5/7] . --- src/framework/components/render/component.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/framework/components/render/component.js b/src/framework/components/render/component.js index 745e74204e1..67de4df25bd 100644 --- a/src/framework/components/render/component.js +++ b/src/framework/components/render/component.js @@ -172,8 +172,7 @@ class RenderComponent extends Component { * * @param {import('./system.js').RenderComponentSystem} system - The ComponentSystem that * created this Component. - * @param {Entity} entity - The Entity that this Component is - * attached to. + * @param {Entity} entity - The Entity that this Component is attached to. */ constructor(system, entity) { super(system, entity); From 21ff4c45d1c4823f3e2b3d79d1aebd2c26f4a611 Mon Sep 17 00:00:00 2001 From: mrmaxm Date: Thu, 19 Dec 2024 15:53:57 +0200 Subject: [PATCH 6/7] pr comment --- src/framework/components/render/component.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/framework/components/render/component.js b/src/framework/components/render/component.js index 67de4df25bd..0d5b34ee89a 100644 --- a/src/framework/components/render/component.js +++ b/src/framework/components/render/component.js @@ -719,6 +719,7 @@ class RenderComponent extends Component { this._rootBone = value; } else if (isString) { this._rootBone = this.system.app.getEntityFromIndex(value) || null; + Debug.assert(this._rootBone, 'Failed to find rootBone Entity by GUID'); } else { this._rootBone = null; } From e81a610897be9c22aad5ee8ff33ef5359515d934 Mon Sep 17 00:00:00 2001 From: mrmaxm Date: Thu, 19 Dec 2024 15:54:51 +0200 Subject: [PATCH 7/7] pr comment --- src/framework/components/render/component.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/framework/components/render/component.js b/src/framework/components/render/component.js index 0d5b34ee89a..53dc3adbf4d 100644 --- a/src/framework/components/render/component.js +++ b/src/framework/components/render/component.js @@ -719,7 +719,9 @@ class RenderComponent extends Component { this._rootBone = value; } else if (isString) { this._rootBone = this.system.app.getEntityFromIndex(value) || null; - Debug.assert(this._rootBone, 'Failed to find rootBone Entity by GUID'); + if (!this._rootBone) { + Debug.warn('Failed to find rootBone Entity by GUID'); + } } else { this._rootBone = null; }