Optimization: use EventHandle in Components for scene and layers (#7138)

* implement more optimized EventHandle.off * use EventHandle for scene and layers events for improved performance * more EventHandle.off in critical places * asset-reference event handlers * remove uncecessary off's --------- Co-authored-by: Will Eastcott <[email protected]>
playcanvas · Dec 3, 2024 · 955d2f0 · 955d2f0
1 parent 0e0698e
commit 955d2f0
Show file tree

Hide file tree

Showing 11 changed files with 383 additions and 96 deletions.
diff --git a/src/framework/asset/asset-reference.js b/src/framework/asset/asset-reference.js
@@ -11,6 +11,48 @@
  * @category Asset
  */
 class AssetReference {
+    /**
+     * @type {import('../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLoadById = null;
+
+    /**
+     * @type {import('../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtUnloadById = null;
+
+    /**
+     * @type {import('../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtAddById = null;
+
+    /**
+     * @type {import('../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtRemoveById = null;
+
+    /**
+     * @type {import('../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLoadByUrl = null;
+
+    /**
+     * @type {import('../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtAddByUrl = null;
+
+    /**
+     * @type {import('../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtRemoveByUrl = null;
+
     /**
      * Create a new AssetReference instance.
      *
@@ -109,30 +151,37 @@ class AssetReference {
 
     _bind() {
         if (this.id) {
-            if (this._onAssetLoad) this._registry.on(`load:${this.id}`, this._onLoad, this);
-            if (this._onAssetAdd) this._registry.once(`add:${this.id}`, this._onAdd, this);
-            if (this._onAssetRemove) this._registry.on(`remove:${this.id}`, this._onRemove, this);
-            if (this._onAssetUnload) this._registry.on(`unload:${this.id}`, this._onUnload, this);
+            if (this._onAssetLoad) this._evtLoadById = this._registry.on(`load:${this.id}`, this._onLoad, this);
+            if (this._onAssetAdd) this._evtAddById = this._registry.once(`add:${this.id}`, this._onAdd, this);
+            if (this._onAssetRemove) this._evtRemoveById = this._registry.on(`remove:${this.id}`, this._onRemove, this);
+            if (this._onAssetUnload) this._evtUnloadById = this._registry.on(`unload:${this.id}`, this._onUnload, this);
         }
 
         if (this.url) {
-            if (this._onAssetLoad) this._registry.on(`load:url:${this.url}`, this._onLoad, this);
-            if (this._onAssetAdd) this._registry.once(`add:url:${this.url}`, this._onAdd, this);
-            if (this._onAssetRemove) this._registry.on(`remove:url:${this.url}`, this._onRemove, this);
+            if (this._onAssetLoad) this._evtLoadByUrl = this._registry.on(`load:url:${this.url}`, this._onLoad, this);
+            if (this._onAssetAdd) this._evtAddByUrl = this._registry.once(`add:url:${this.url}`, this._onAdd, this);
+            if (this._onAssetRemove) this._evtRemoveByUrl = this._registry.on(`remove:url:${this.url}`, this._onRemove, this);
         }
     }
 
     _unbind() {
         if (this.id) {
-            if (this._onAssetLoad) this._registry.off(`load:${this.id}`, this._onLoad, this);
-            if (this._onAssetAdd) this._registry.off(`add:${this.id}`, this._onAdd, this);
-            if (this._onAssetRemove) this._registry.off(`remove:${this.id}`, this._onRemove, this);
-            if (this._onAssetUnload) this._registry.off(`unload:${this.id}`, this._onUnload, this);
+            this._evtLoadById?.off();
+            this._evtLoadById = null;
+            this._evtAddById?.off();
+            this._evtAddById = null;
+            this._evtRemoveById?.off();
+            this._evtRemoveById = null;
+            this._evtUnloadById?.off();
+            this._evtUnloadById = null;
         }
         if (this.url) {
-            if (this._onAssetLoad) this._registry.off(`load:${this.url}`, this._onLoad, this);
-            if (this._onAssetAdd) this._registry.off(`add:${this.url}`, this._onAdd, this);
-            if (this._onAssetRemove) this._registry.off(`remove:${this.url}`, this._onRemove, this);
+            this._evtLoadByUrl?.off();
+            this._evtLoadByUrl = null;
+            this._evtAddByUrl?.off();
+            this._evtAddByUrl = null;
+            this._evtRemoveByUrl?.off();
+            this._evtRemoveByUrl = null;
         }
     }
 

diff --git a/src/framework/components/camera/component.js b/src/framework/components/camera/component.js
@@ -97,6 +97,24 @@ class CameraComponent extends Component {
     /** @private */
     _camera = new Camera();
 
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayersChanged = null;
+
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayerAdded = null;
+
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayerRemoved = null;
+
     /**
      * Create a new CameraComponent instance.
      *
@@ -1099,16 +1117,20 @@ class CameraComponent extends Component {
     }
 
     onEnable() {
-        const system = this.system;
-        const scene = system.app.scene;
+        const scene = this.system.app.scene;
         const layers = scene.layers;
 
-        system.addCamera(this);
+        this.system.addCamera(this);
+
+        this._evtLayersChanged?.off();
+        this._evtLayersChanged = scene.on('set:layers', this.onLayersChanged, this);
 
-        scene.on('set:layers', this.onLayersChanged, this);
         if (layers) {
-            layers.on('add', this.onLayerAdded, this);
-            layers.on('remove', this.onLayerRemoved, this);
+            this._evtLayerAdded?.off();
+            this._evtLayerAdded = layers.on('add', this.onLayerAdded, this);
+
+            this._evtLayerRemoved?.off();
+            this._evtLayerRemoved = layers.on('remove', this.onLayerRemoved, this);
         }
 
         if (this.enabled && this.entity.enabled) {
@@ -1119,21 +1141,24 @@ class CameraComponent extends Component {
     }
 
     onDisable() {
-        const system = this.system;
-        const scene = system.app.scene;
+        const scene = this.system.app.scene;
         const layers = scene.layers;
 
         this.postEffects.disable();
 
         this.removeCameraFromLayers();
 
-        scene.off('set:layers', this.onLayersChanged, this);
+        this._evtLayersChanged?.off();
+        this._evtLayersChanged = null;
+
         if (layers) {
-            layers.off('add', this.onLayerAdded, this);
-            layers.off('remove', this.onLayerRemoved, this);
+            this._evtLayerAdded?.off();
+            this._evtLayerAdded = null;
+            this._evtLayerRemoved?.off();
+            this._evtLayerRemoved = null;
         }
 
-        system.removeCamera(this);
+        this.system.removeCamera(this);
     }
 
     onRemove() {

diff --git a/src/framework/components/element/component.js b/src/framework/components/element/component.js
@@ -218,6 +218,24 @@ class ElementComponent extends Component {
      */
     static EVENT_TOUCHCANCEL = 'touchcancel';
 
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayersChanged = null;
+
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayerAdded = null;
+
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayerRemoved = null;
+
     /**
      * Create a new ElementComponent instance.
      *
@@ -2530,6 +2548,9 @@ class ElementComponent extends Component {
     }
 
     onEnable() {
+        const scene = this.system.app.scene;
+        const layers = scene.layers;
+
         if (this._image) {
             this._image.onEnable();
         }
@@ -2544,10 +2565,11 @@ class ElementComponent extends Component {
             this.system.app.elementInput.addElement(this);
         }
 
-        this.system.app.scene.on('set:layers', this.onLayersChanged, this);
-        if (this.system.app.scene.layers) {
-            this.system.app.scene.layers.on('add', this.onLayerAdded, this);
-            this.system.app.scene.layers.on('remove', this.onLayerRemoved, this);
+        this._evtLayersChanged = scene.on('set:layers', this.onLayersChanged, this);
+
+        if (layers) {
+            this._evtLayerAdded = layers.on('add', this.onLayerAdded, this);
+            this._evtLayerRemoved = layers.on('remove', this.onLayerRemoved, this);
         }
 
         if (this._batchGroupId >= 0) {
@@ -2558,10 +2580,17 @@ class ElementComponent extends Component {
     }
 
     onDisable() {
-        this.system.app.scene.off('set:layers', this.onLayersChanged, this);
-        if (this.system.app.scene.layers) {
-            this.system.app.scene.layers.off('add', this.onLayerAdded, this);
-            this.system.app.scene.layers.off('remove', this.onLayerRemoved, this);
+        const scene = this.system.app.scene;
+        const layers = scene.layers;
+
+        this._evtLayersChanged?.off();
+        this._evtLayersChanged = null;
+
+        if (layers) {
+            this._evtLayerAdded?.off();
+            this._evtLayerAdded = null;
+            this._evtLayerRemoved?.off();
+            this._evtLayerRemoved = null;
         }
 
         if (this._image) this._image.onDisable();

diff --git a/src/framework/components/element/image-element.js b/src/framework/components/element/image-element.js
@@ -263,6 +263,12 @@ class ImageRenderable {
 }
 
 class ImageElement {
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtSetMeshes = null;
+
     constructor(element) {
         this._element = element;
         this._entity = element.entity;
@@ -794,7 +800,7 @@ class ImageElement {
 
     // Hook up event handlers on sprite asset
     _bindSprite(sprite) {
-        sprite.on('set:meshes', this._onSpriteMeshesChange, this);
+        this._evtSetMeshes = sprite.on('set:meshes', this._onSpriteMeshesChange, this);
         sprite.on('set:pixelsPerUnit', this._onSpritePpuChange, this);
         sprite.on('set:atlas', this._onAtlasTextureChange, this);
         if (sprite.atlas) {
@@ -803,7 +809,8 @@ class ImageElement {
     }
 
     _unbindSprite(sprite) {
-        sprite.off('set:meshes', this._onSpriteMeshesChange, this);
+        this._evtSetMeshes?.off();
+        this._evtSetMeshes = null;
         sprite.off('set:pixelsPerUnit', this._onSpritePpuChange, this);
         sprite.off('set:atlas', this._onAtlasTextureChange, this);
         if (sprite.atlas) {

diff --git a/src/framework/components/gsplat/component.js b/src/framework/components/gsplat/component.js
@@ -52,6 +52,24 @@ class GSplatComponent extends Component {
      */
     _materialOptions = null;
 
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayersChanged = null;
+
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayerAdded = null;
+
+    /**
+     * @type {import('../../../core/event-handle.js').EventHandle|null}
+     * @private
+     */
+    _evtLayerRemoved = null;
+
     /**
      * Create a new GSplatComponent.
      *
@@ -324,10 +342,13 @@ class GSplatComponent extends Component {
 
     onEnable() {
         const scene = this.system.app.scene;
-        scene.on('set:layers', this.onLayersChanged, this);
-        if (scene.layers) {
-            scene.layers.on('add', this.onLayerAdded, this);
-            scene.layers.on('remove', this.onLayerRemoved, this);
+        const layers = scene.layers;
+
+        this._evtLayersChanged = scene.on('set:layers', this.onLayersChanged, this);
+
+        if (layers) {
+            this._evtLayerAdded = layers.on('add', this.onLayerAdded, this);
+            this._evtLayerRemoved = layers.on('remove', this.onLayerRemoved, this);
         }
 
         if (this._instance) {
@@ -339,10 +360,16 @@ class GSplatComponent extends Component {
 
     onDisable() {
         const scene = this.system.app.scene;
-        scene.off('set:layers', this.onLayersChanged, this);
-        if (scene.layers) {
-            scene.layers.off('add', this.onLayerAdded, this);
-            scene.layers.off('remove', this.onLayerRemoved, this);
+        const layers = scene.layers;
+
+        this._evtLayersChanged?.off();
+        this._evtLayersChanged = null;
+
+        if (layers) {
+            this._evtLayerAdded?.off();
+            this._evtLayerAdded = null;
+            this._evtLayerRemoved?.off();
+            this._evtLayerRemoved = null;
         }
 
         this.removeFromLayers();