From 916f6836023a23460f5f016138ec5fc8b5fc0850 Mon Sep 17 00:00:00 2001 From: Martin Valigursky <mvaligursky@snapchat.com> Date: Tue, 5 Apr 2022 18:07:50 +0100 Subject: [PATCH 1/8] moving xr, testing {type ClassName} --- src/framework/app-base.js | 6 +++--- src/framework/app-create-options.js | 12 ++++++++++-- src/framework/application.js | 3 +++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/framework/app-base.js b/src/framework/app-base.js index 44363e7bdc0..690810148b4 100644 --- a/src/framework/app-base.js +++ b/src/framework/app-base.js @@ -48,7 +48,6 @@ import { ScriptRegistry } from '../script/script-registry.js'; import { I18n } from '../i18n/i18n.js'; import { VrManager } from '../vr/vr-manager.js'; -import { XrManager } from '../xr/xr-manager.js'; import { ComponentSystemRegistry } from './components/registry.js'; import { script } from './script.js'; @@ -76,8 +75,9 @@ import { /** @typedef {import('../scene/graph-node.js').GraphNode} GraphNode */ /** @typedef {import('../scene/mesh.js').Mesh} Mesh */ /** @typedef {import('../scene/mesh-instance.js').MeshInstance} MeshInstance */ -/** @typedef {import('../scene/lightmapper.js').Lightmapper} Lightmapper */ +/** @typedef {import('../scene/lightmapper/lightmapper.js').Lightmapper} Lightmapper */ /** @typedef {import('../framework/app-create-options.js').AppCreateOptions} AppCreateOptions */ +/** @typedef {import('../xr/xr-manager.js').XrManager} XrManager */ // Mini-object used to measure progress of loading sets class Progress { @@ -522,7 +522,7 @@ class AppBase extends EventHandler { * // VR is available * } */ - this.xr = new XrManager(this); + this.xr = new createOptions.xr(this); if (this.elementInput) this.elementInput.attachSelectEvents(); diff --git a/src/framework/app-create-options.js b/src/framework/app-create-options.js index f84ca715c1c..e8657734d33 100644 --- a/src/framework/app-create-options.js +++ b/src/framework/app-create-options.js @@ -6,8 +6,9 @@ /** @typedef {import('../input/mouse.js').Mouse} Mouse */ /** @typedef {import('../input/touch-device.js').TouchDevice} TouchDevice */ /** @typedef {import('../sound/manager.js').SoundManager} SoundManager */ -/** @typedef {import('../scene/lightmapper.js').Lightmapper} Lightmapper */ +/** @typedef {import('../scene/lightmapper/lightmapper.js').Lightmapper} Lightmapper */ /** @typedef {import('./components/system.js').ComponentSystem} ComponentSystem */ +/** @typedef {import('../xr/xr-manager.js').XrManager} XrManager */ class AppCreateOptions { /** @@ -83,10 +84,17 @@ class AppCreateOptions { /** * The lightmapper. * - * @type {Lightmapper} + * @type {typeof Lightmapper} */ lightmapper; + /** + * The XrManager. + * + * @type {typeof XrManager} + */ + xr; + /** * The component systems the app requires. * diff --git a/src/framework/application.js b/src/framework/application.js index cb0fa5dc77a..94d384c823a 100644 --- a/src/framework/application.js +++ b/src/framework/application.js @@ -59,6 +59,8 @@ import { TextHandler } from '../resources/text.js'; import { TextureAtlasHandler } from '../resources/texture-atlas.js'; import { TextureHandler } from '../resources/texture.js'; +import { XrManager } from '../xr/xr-manager.js'; + /** @typedef {import('../input/element-input.js').ElementInput} ElementInput */ /** @typedef {import('../input/game-pads.js').GamePads} GamePads */ /** @typedef {import('../input/keyboard.js').Keyboard} Keyboard */ @@ -134,6 +136,7 @@ import { TextureHandler } from '../resources/texture.js'; createOptions.soundManager = new SoundManager(options); createOptions.lightmapper = Lightmapper; + createOptions.xr = XrManager; this.init(createOptions); } From 2cbf900c598d9caeeced7c9c9a8ee8d2407443f0 Mon Sep 17 00:00:00 2001 From: Martin Valigursky <mvaligursky@snapchat.com> Date: Tue, 5 Apr 2022 18:20:12 +0100 Subject: [PATCH 2/8] typedef --- src/framework/app-base.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/framework/app-base.js b/src/framework/app-base.js index 690810148b4..e6967e36757 100644 --- a/src/framework/app-base.js +++ b/src/framework/app-base.js @@ -78,6 +78,7 @@ import { /** @typedef {import('../scene/lightmapper/lightmapper.js').Lightmapper} Lightmapper */ /** @typedef {import('../framework/app-create-options.js').AppCreateOptions} AppCreateOptions */ /** @typedef {import('../xr/xr-manager.js').XrManager} XrManager */ +/** @typedef {import('../sound/manager.js').SoundManager} SoundManager */ // Mini-object used to measure progress of loading sets class Progress { From ccb929b207c60420a54a8cde0883d1c55a19a879 Mon Sep 17 00:00:00 2001 From: Martin Valigursky <mvaligursky@snapchat.com> Date: Wed, 6 Apr 2022 09:16:43 +0100 Subject: [PATCH 3/8] deprecate4d vr-manager --- src/deprecated/deprecated.js | 15 +++++++++++++++ src/framework/app-base.js | 27 --------------------------- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/deprecated/deprecated.js b/src/deprecated/deprecated.js index 7bc541d79d8..6809bd94606 100644 --- a/src/deprecated/deprecated.js +++ b/src/deprecated/deprecated.js @@ -115,6 +115,8 @@ import { basisInitialize } from '../resources/basis.js'; import { EventHandler } from '../core/event-handler.js'; import { Asset } from '../asset/asset.js'; +import { VrManager } from '../vr/vr-manager.js'; + // CORE export const log = { @@ -1070,6 +1072,19 @@ Application.prototype.renderLines = function (position, color, options) { this._addLines(position, color, options); }; +Application.prototype.enableVr = function () { + if (!this.vr) { + this.vr = new VrManager(this); + } +}; + +Application.prototype.disableVr = function () { + if (this.vr) { + this.vr.destroy(); + this.vr = null; + } +}; + Object.defineProperty(CameraComponent.prototype, 'node', { get: function () { Debug.deprecated('pc.CameraComponent#node is deprecated. Use pc.CameraComponent#entity instead.'); diff --git a/src/framework/app-base.js b/src/framework/app-base.js index e6967e36757..776f504435a 100644 --- a/src/framework/app-base.js +++ b/src/framework/app-base.js @@ -47,8 +47,6 @@ import { ScriptRegistry } from '../script/script-registry.js'; import { I18n } from '../i18n/i18n.js'; -import { VrManager } from '../vr/vr-manager.js'; - import { ComponentSystemRegistry } from './components/registry.js'; import { script } from './script.js'; import { ApplicationStats } from './stats.js'; @@ -1635,31 +1633,6 @@ class AppBase extends EventHandler { } } - /** - * Create and assign a {@link VrManager} object to allow this application render in VR. - * - * @ignore - * @deprecated - */ - enableVr() { - if (!this.vr) { - this.vr = new VrManager(this); - } - } - - /** - * Destroy the {@link VrManager}. - * - * @ignore - * @deprecated - */ - disableVr() { - if (this.vr) { - this.vr.destroy(); - this.vr = null; - } - } - /** @private */ _firstBake() { if (this.lightmapper) { From 9bf2e43f7565c2674182f450b7b5b0df19036ba2 Mon Sep 17 00:00:00 2001 From: Martin Valigursky <mvaligursky@snapchat.com> Date: Wed, 6 Apr 2022 09:29:01 +0100 Subject: [PATCH 4/8] missing typedef --- src/framework/app-base.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/framework/app-base.js b/src/framework/app-base.js index 776f504435a..acc3d01ddff 100644 --- a/src/framework/app-base.js +++ b/src/framework/app-base.js @@ -76,6 +76,7 @@ import { /** @typedef {import('../scene/lightmapper/lightmapper.js').Lightmapper} Lightmapper */ /** @typedef {import('../framework/app-create-options.js').AppCreateOptions} AppCreateOptions */ /** @typedef {import('../xr/xr-manager.js').XrManager} XrManager */ +/** @typedef {import('../vr/vr-manager.js').VrManager} VrManager */ /** @typedef {import('../sound/manager.js').SoundManager} SoundManager */ // Mini-object used to measure progress of loading sets From 9f2848acd162eb115fe196070c94ea8ceb77eea1 Mon Sep 17 00:00:00 2001 From: Martin Valigursky <mvaligursky@snapchat.com> Date: Wed, 6 Apr 2022 10:19:45 +0100 Subject: [PATCH 5/8] batch manager --- src/framework/app-base.js | 41 ++++++++++++++----- src/framework/app-create-options.js | 8 ++++ src/framework/application.js | 2 + src/framework/components/element/component.js | 8 ++-- src/framework/components/model/component.js | 9 ++-- src/framework/components/render/component.js | 9 ++-- src/framework/components/screen/component.js | 2 +- src/framework/components/sprite/component.js | 8 ++-- src/framework/stats.js | 3 +- src/scene/batching/batch-manager.js | 2 +- 10 files changed, 61 insertions(+), 31 deletions(-) diff --git a/src/framework/app-base.js b/src/framework/app-base.js index acc3d01ddff..1293f246dda 100644 --- a/src/framework/app-base.js +++ b/src/framework/app-base.js @@ -460,13 +460,15 @@ class AppBase extends EventHandler { } /** - * The application's batch manager. The batch manager is used to merge mesh instances in - * the scene, which reduces the overall number of draw calls, thereby boosting performance. + * The application's batch manager. * * @type {BatchManager} */ - this.batcher = new BatchManager(this.graphicsDevice, this.root, this.scene); - this.once('prerender', this._firstBatch, this); + this._batcher = null; + if (createOptions.batchManager) { + this._batcher = new createOptions.batchManager(this.graphicsDevice, this.root, this.scene); + this.once('prerender', this._firstBatch, this); + } /** * The keyboard device. @@ -663,6 +665,17 @@ class AppBase extends EventHandler { return this._soundManager; } + /** + * The application's batch manager. The batch manager is used to merge mesh instances in + * the scene, which reduces the overall number of draw calls, thereby boosting performance. + * + * @type {BatchManager} + */ + get batcher() { + Debug.assert(this._batcher, "BatchManager has not been created and is required for correct functionality."); + return this._batcher; + } + /** * The current fill mode of the canvas. Can be: * @@ -899,9 +912,12 @@ class AppBase extends EventHandler { // add batch groups if (props.batchGroups) { - for (let i = 0, len = props.batchGroups.length; i < len; i++) { - const grp = props.batchGroups[i]; - this.batcher.addGroup(grp.name, grp.dynamic, grp.maxAabbSize, grp.id, grp.layers); + const batcher = this.batcher; + if (batcher) { + for (let i = 0, len = props.batchGroups.length; i < len; i++) { + const grp = props.batchGroups[i]; + batcher.addGroup(grp.name, grp.dynamic, grp.maxAabbSize, grp.id, grp.layers); + } } } @@ -1196,7 +1212,10 @@ class AppBase extends EventHandler { this.fire('prerender'); this.root.syncHierarchy(); - this.batcher.updateAll(); + + if (this._batcher) { + this._batcher.updateAll(); + } // #if _PROFILER ForwardRenderer._skipRenderCounter = 0; @@ -2006,8 +2025,10 @@ class AppBase extends EventHandler { this.lightmapper?.destroy(); this.lightmapper = null; - this.batcher.destroy(); - this.batcher = null; + if (this._batcher) { + this._batcher.destroy(); + this._batcher = null; + } this._entityIndex = {}; diff --git a/src/framework/app-create-options.js b/src/framework/app-create-options.js index e8657734d33..8c35e841aef 100644 --- a/src/framework/app-create-options.js +++ b/src/framework/app-create-options.js @@ -7,6 +7,7 @@ /** @typedef {import('../input/touch-device.js').TouchDevice} TouchDevice */ /** @typedef {import('../sound/manager.js').SoundManager} SoundManager */ /** @typedef {import('../scene/lightmapper/lightmapper.js').Lightmapper} Lightmapper */ +/** @typedef {import('../scene/batching/batch-manager.js').BatchManager} BatchManager */ /** @typedef {import('./components/system.js').ComponentSystem} ComponentSystem */ /** @typedef {import('../xr/xr-manager.js').XrManager} XrManager */ @@ -88,6 +89,13 @@ class AppCreateOptions { */ lightmapper; + /** + * The BatchManager. + * + * @type {typeof BatchManager} + */ + batchManager; + /** * The XrManager. * diff --git a/src/framework/application.js b/src/framework/application.js index 94d384c823a..e9b55516daf 100644 --- a/src/framework/application.js +++ b/src/framework/application.js @@ -5,6 +5,7 @@ import { WebglGraphicsDevice } from '../graphics/webgl/webgl-graphics-device.js' import { SoundManager } from '../sound/manager.js'; import { Lightmapper } from '../scene/lightmapper/lightmapper.js'; +import { BatchManager } from '../scene/batching/batch-manager.js'; import { AppBase } from './app-base.js'; import { AppCreateOptions } from './app-create-options.js'; @@ -136,6 +137,7 @@ import { XrManager } from '../xr/xr-manager.js'; createOptions.soundManager = new SoundManager(options); createOptions.lightmapper = Lightmapper; + createOptions.batchManager = BatchManager; createOptions.xr = XrManager; this.init(createOptions); diff --git a/src/framework/components/element/component.js b/src/framework/components/element/component.js index d5f43bbfa72..4b7c7787a46 100644 --- a/src/framework/components/element/component.js +++ b/src/framework/components/element/component.js @@ -334,11 +334,11 @@ class ElementComponent extends Component { return; if (this.entity.enabled && this._batchGroupId >= 0) { - this.system.app.batcher.remove(BatchGroup.ELEMENT, this.batchGroupId, this.entity); + this.system.app.batcher?.remove(BatchGroup.ELEMENT, this.batchGroupId, this.entity); } if (this.entity.enabled && value >= 0) { - this.system.app.batcher.insert(BatchGroup.ELEMENT, value, this.entity); + this.system.app.batcher?.insert(BatchGroup.ELEMENT, value, this.entity); } if (value < 0 && this._batchGroupId >= 0 && this.enabled && this.entity.enabled) { @@ -1394,7 +1394,7 @@ class ElementComponent extends Component { } if (this._batchGroupId >= 0) { - this.system.app.batcher.insert(BatchGroup.ELEMENT, this.batchGroupId, this.entity); + this.system.app.batcher?.insert(BatchGroup.ELEMENT, this.batchGroupId, this.entity); } this.fire('enableelement'); @@ -1416,7 +1416,7 @@ class ElementComponent extends Component { } if (this._batchGroupId >= 0) { - this.system.app.batcher.remove(BatchGroup.ELEMENT, this.batchGroupId, this.entity); + this.system.app.batcher?.remove(BatchGroup.ELEMENT, this.batchGroupId, this.entity); } this.fire('disableelement'); diff --git a/src/framework/components/model/component.js b/src/framework/components/model/component.js index 5ad0495acd5..25c71a779ab 100644 --- a/src/framework/components/model/component.js +++ b/src/framework/components/model/component.js @@ -547,12 +547,11 @@ class ModelComponent extends Component { set batchGroupId(value) { if (this._batchGroupId === value) return; - const batcher = this.system.app.batcher; if (this.entity.enabled && this._batchGroupId >= 0) { - batcher.remove(BatchGroup.MODEL, this.batchGroupId, this.entity); + this.system.app.batcher?.remove(BatchGroup.MODEL, this.batchGroupId, this.entity); } if (this.entity.enabled && value >= 0) { - batcher.insert(BatchGroup.MODEL, value, this.entity); + this.system.app.batcher?.insert(BatchGroup.MODEL, value, this.entity); } if (value < 0 && this._batchGroupId >= 0 && this.enabled && this.entity.enabled) { @@ -912,7 +911,7 @@ class ModelComponent extends Component { } if (this._batchGroupId >= 0) { - app.batcher.insert(BatchGroup.MODEL, this.batchGroupId, this.entity); + app.batcher?.insert(BatchGroup.MODEL, this.batchGroupId, this.entity); } } @@ -927,7 +926,7 @@ class ModelComponent extends Component { } if (this._batchGroupId >= 0) { - app.batcher.remove(BatchGroup.MODEL, this.batchGroupId, this.entity); + app.batcher?.remove(BatchGroup.MODEL, this.batchGroupId, this.entity); } if (this._model) { diff --git a/src/framework/components/render/component.js b/src/framework/components/render/component.js index 4e8ff38d832..3ee769e318b 100644 --- a/src/framework/components/render/component.js +++ b/src/framework/components/render/component.js @@ -453,12 +453,11 @@ class RenderComponent extends Component { set batchGroupId(value) { if (this._batchGroupId !== value) { - const batcher = this.system.app.batcher; if (this.entity.enabled && this._batchGroupId >= 0) { - batcher.remove(BatchGroup.RENDER, this.batchGroupId, this.entity); + this.system.app.batcher?.remove(BatchGroup.RENDER, this.batchGroupId, this.entity); } if (this.entity.enabled && value >= 0) { - batcher.insert(BatchGroup.RENDER, value, this.entity); + this.system.app.batcher?.insert(BatchGroup.RENDER, value, this.entity); } if (value < 0 && this._batchGroupId >= 0 && this.enabled && this.entity.enabled) { @@ -725,7 +724,7 @@ class RenderComponent extends Component { } if (this._batchGroupId >= 0) { - app.batcher.insert(BatchGroup.RENDER, this.batchGroupId, this.entity); + app.batcher?.insert(BatchGroup.RENDER, this.batchGroupId, this.entity); } } @@ -740,7 +739,7 @@ class RenderComponent extends Component { } if (this._batchGroupId >= 0) { - app.batcher.remove(BatchGroup.RENDER, this.batchGroupId, this.entity); + app.batcher?.remove(BatchGroup.RENDER, this.batchGroupId, this.entity); } this.removeFromLayers(); diff --git a/src/framework/components/screen/component.js b/src/framework/components/screen/component.js index d32e44e31de..d8b2cd862af 100644 --- a/src/framework/components/screen/component.js +++ b/src/framework/components/screen/component.js @@ -71,7 +71,7 @@ class ScreenComponent extends Component { e.element.drawOrder = i++; if (e.element._batchGroupId >= 0 && prevDrawOrder !== e.element.drawOrder) { - this.system.app.batcher.markGroupDirty(e.element._batchGroupId); + this.system.app.batcher?.markGroupDirty(e.element._batchGroupId); } } diff --git a/src/framework/components/sprite/component.js b/src/framework/components/sprite/component.js index f02ea12a8d5..f46b5e3d529 100644 --- a/src/framework/components/sprite/component.js +++ b/src/framework/components/sprite/component.js @@ -419,10 +419,10 @@ class SpriteComponent extends Component { this._batchGroupId = value; if (this.entity.enabled && prev >= 0) { - this.system.app.batcher.remove(BatchGroup.SPRITE, prev, this.entity); + this.system.app.batcher?.remove(BatchGroup.SPRITE, prev, this.entity); } if (this.entity.enabled && value >= 0) { - this.system.app.batcher.insert(BatchGroup.SPRITE, value, this.entity); + this.system.app.batcher?.insert(BatchGroup.SPRITE, value, this.entity); } else { // re-add model to scene in case it was removed by batching if (prev >= 0) { @@ -518,7 +518,7 @@ class SpriteComponent extends Component { this._tryAutoPlay(); if (this._batchGroupId >= 0) { - app.batcher.insert(BatchGroup.SPRITE, this._batchGroupId, this.entity); + app.batcher?.insert(BatchGroup.SPRITE, this._batchGroupId, this.entity); } } @@ -537,7 +537,7 @@ class SpriteComponent extends Component { if (this._batchGroupId >= 0) { - app.batcher.remove(BatchGroup.SPRITE, this._batchGroupId, this.entity); + app.batcher?.remove(BatchGroup.SPRITE, this._batchGroupId, this.entity); } } diff --git a/src/framework/stats.js b/src/framework/stats.js index ab609cef5f5..caa288dcd2b 100644 --- a/src/framework/stats.js +++ b/src/framework/stats.js @@ -99,7 +99,8 @@ class ApplicationStats { } get batcher() { - return getApplication().batcher._stats; + const batcher = getApplication()._batcher; + return batcher ? batcher._stats : null; } } diff --git a/src/scene/batching/batch-manager.js b/src/scene/batching/batch-manager.js index 07367b4b0f1..f89956c6849 100644 --- a/src/scene/batching/batch-manager.js +++ b/src/scene/batching/batch-manager.js @@ -890,7 +890,7 @@ class BatchManager { /** * Updates bounding boxes for all dynamic batches. Called automatically. * - * @private + * @ignore */ updateAll() { // TODO: only call when needed. Applies to skinning matrices as well From 23a0fea6547d509f12e1caed14d12180aa29b1cb Mon Sep 17 00:00:00 2001 From: Martin Valigursky <mvaligursky@snapchat.com> Date: Wed, 6 Apr 2022 10:20:32 +0100 Subject: [PATCH 6/8] removed import --- src/framework/app-base.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/framework/app-base.js b/src/framework/app-base.js index 1293f246dda..be967bfa034 100644 --- a/src/framework/app-base.js +++ b/src/framework/app-base.js @@ -24,7 +24,6 @@ import { LAYERID_DEPTH, LAYERID_IMMEDIATE, LAYERID_SKYBOX, LAYERID_UI, LAYERID_WORLD, SORTMODE_NONE, SORTMODE_MANUAL, SPECULAR_BLINN } from '../scene/constants.js'; -import { BatchManager } from '../scene/batching/batch-manager.js'; import { ForwardRenderer } from '../scene/renderer/forward-renderer.js'; import { AreaLightLuts } from '../scene/area-light-luts.js'; import { Layer } from '../scene/layer.js'; From 5678dc9cfb71ce03f36e34f3c8e405441731c2c1 Mon Sep 17 00:00:00 2001 From: Martin Valigursky <mvaligursky@snapchat.com> Date: Wed, 6 Apr 2022 10:24:34 +0100 Subject: [PATCH 7/8] types --- src/framework/app-base.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/framework/app-base.js b/src/framework/app-base.js index be967bfa034..e3e6d522714 100644 --- a/src/framework/app-base.js +++ b/src/framework/app-base.js @@ -73,6 +73,7 @@ import { /** @typedef {import('../scene/mesh.js').Mesh} Mesh */ /** @typedef {import('../scene/mesh-instance.js').MeshInstance} MeshInstance */ /** @typedef {import('../scene/lightmapper/lightmapper.js').Lightmapper} Lightmapper */ +/** @typedef {import('../scene/batching/batch-manager.js').BatchManager} BatchManager */ /** @typedef {import('../framework/app-create-options.js').AppCreateOptions} AppCreateOptions */ /** @typedef {import('../xr/xr-manager.js').XrManager} XrManager */ /** @typedef {import('../vr/vr-manager.js').VrManager} VrManager */ From 641763131a93abcadc87370b4f530bfdac5cb52b Mon Sep 17 00:00:00 2001 From: Martin Valigursky <mvaligursky@snapchat.com> Date: Wed, 6 Apr 2022 14:29:48 +0100 Subject: [PATCH 8/8] small improvements --- src/framework/app-base.js | 2 +- src/resources/loader.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/framework/app-base.js b/src/framework/app-base.js index e3e6d522714..fe7d25b7ef2 100644 --- a/src/framework/app-base.js +++ b/src/framework/app-base.js @@ -146,7 +146,7 @@ class AppBase extends EventHandler { * // Engine-only example: create the application manually * var createOptions = new AppCreateOptions(); * var app = new pc.AppBase(canvas); - * app.init(options, createOptions); + * app.init(createOptions); * * // Start the application's main loop * app.start(); diff --git a/src/resources/loader.js b/src/resources/loader.js index e2305e0be3e..153d67ffaf4 100644 --- a/src/resources/loader.js +++ b/src/resources/loader.js @@ -99,7 +99,7 @@ class ResourceLoader { load(url, type, callback, asset) { const handler = this._handlers[type]; if (!handler) { - const err = 'No handler for asset type: ' + type; + const err = `No handler for asset type: '${type}' when loading [${url}]`; callback(err); return; }