Merge pull request #9606 from breakin/shareable-instance-geometry

Make Geometry shareable by sourceMeshes

dist/preview release/what's new.md

@@ -14,6 +14,9 @@
 - Moving button to shared uI folder.([msDestiny14](https://github.com/msDestiny14))
 - Moving additional components to shared UI folder.([msDestiny14](https://github.com/msDestiny14))
 
+### Engine
+- Moved all instance data from Geometry to Mesh such that the same Geometry objects can be used by many meshes with instancing. Reduces memory consumption on CPU/GPU. ([breakin](https://github.com/breakin)
+
 ### Loaders
 
 - Added support for EXT_meshopt_compression for glTF loader. ([zeux](https://github.com/zeux))
@@ -83,3 +86,4 @@
     - [Shader support differences](https://doc.babylonjs.com/advanced_topics/webGPU/webGPUBreakingChanges#shader-code-differences)
 - Use both `mesh.visibility` and `material.alpha` values to compute the global alpha value used by the soft transparent shadow rendering code. Formerly was only using `mesh.visibility` ([Popov72](https://github.com/Popov72))
 - Depth renderer: don't render mesh if `infiniteDistance = true` or if `material.disableDepthWrite = true` ([Popov72](https://github.com/Popov72))
+- Mesh.createInstance no longer make a unique Geometry for the Mesh so updating one Geometry can affect more meshes than before. Use Mesh.makeUniqueGeometry for old behaviour. ([breakin](https://github.com/breakin))

src/Engines/thinEngine.ts

@@ -1766,7 +1766,7 @@ export class ThinEngine {
         }
     }
 
-    private _bindVertexBuffersAttributes(vertexBuffers: { [key: string]: Nullable<VertexBuffer> }, effect: Effect): void {
+    private _bindVertexBuffersAttributes(vertexBuffers: { [key: string]: Nullable<VertexBuffer> }, effect: Effect, overrideVertexBuffers?: { [kind: string]: Nullable<VertexBuffer>}): void {
         var attributes = effect.getAttributesNames();
 
         if (!this._vaoRecordInProgress) {
@@ -1779,7 +1779,16 @@ export class ThinEngine {
             var order = effect.getAttributeLocation(index);
 
             if (order >= 0) {
-                var vertexBuffer = vertexBuffers[attributes[index]];
+                var ai = attributes[index];
+                var vertexBuffer: Nullable<VertexBuffer> = null;
+
+                if (overrideVertexBuffers) {
+                    vertexBuffer = overrideVertexBuffers[ai];
+                }
+
+                if (!vertexBuffer) {
+                    vertexBuffer = vertexBuffers[ai];
+                }
 
                 if (!vertexBuffer) {
                     continue;
@@ -1812,9 +1821,10 @@ export class ThinEngine {
      * @param vertexBuffers defines the list of vertex buffers to store
      * @param indexBuffer defines the index buffer to store
      * @param effect defines the effect to store
+     * @param overrideVertexBuffers defines optional list of avertex buffers that overrides the entries in vertexBuffers
      * @returns the new vertex array object
      */
-    public recordVertexArrayObject(vertexBuffers: { [key: string]: VertexBuffer; }, indexBuffer: Nullable<DataBuffer>, effect: Effect): WebGLVertexArrayObject {
+    public recordVertexArrayObject(vertexBuffers: { [key: string]: VertexBuffer; }, indexBuffer: Nullable<DataBuffer>, effect: Effect, overrideVertexBuffers?: { [kind: string]: Nullable<VertexBuffer>}): WebGLVertexArrayObject {
         var vao = this._gl.createVertexArray();
 
         this._vaoRecordInProgress = true;
@@ -1822,7 +1832,7 @@ export class ThinEngine {
         this._gl.bindVertexArray(vao);
 
         this._mustWipeVertexAttributes = true;
-        this._bindVertexBuffersAttributes(vertexBuffers, effect);
+        this._bindVertexBuffersAttributes(vertexBuffers, effect, overrideVertexBuffers);
 
         this.bindIndexBuffer(indexBuffer);
 
@@ -1904,13 +1914,14 @@ export class ThinEngine {
      * @param vertexBuffers defines the list of vertex buffers to bind
      * @param indexBuffer defines the index buffer to bind
      * @param effect defines the effect associated with the vertex buffers
+     * @param overrideVertexBuffers defines optional list of avertex buffers that overrides the entries in vertexBuffers
      */
-    public bindBuffers(vertexBuffers: { [key: string]: Nullable<VertexBuffer> }, indexBuffer: Nullable<DataBuffer>, effect: Effect): void {
+    public bindBuffers(vertexBuffers: { [key: string]: Nullable<VertexBuffer> }, indexBuffer: Nullable<DataBuffer>, effect: Effect, overrideVertexBuffers?: {[kind: string]: Nullable<VertexBuffer>}): void {
         if (this._cachedVertexBuffers !== vertexBuffers || this._cachedEffectForVertexBuffers !== effect) {
             this._cachedVertexBuffers = vertexBuffers;
             this._cachedEffectForVertexBuffers = effect;
 
-            this._bindVertexBuffersAttributes(vertexBuffers, effect);
+            this._bindVertexBuffersAttributes(vertexBuffers, effect, overrideVertexBuffers);
         }
 
         this._bindIndexBufferWithCache(indexBuffer);

src/Meshes/geometry.ts

@@ -253,6 +253,10 @@ export class Geometry implements IGetSetVerticesData {
             this._vertexBuffers[kind].dispose();
             delete this._vertexBuffers[kind];
         }
+
+        if (this._vertexArrayObjects) {
+            this._disposeVertexArrayObjects();
+        }
     }
 
     /**
@@ -267,6 +271,8 @@ export class Geometry implements IGetSetVerticesData {
         }
 
         this._vertexBuffers[kind] = buffer;
+        var meshes = this._meshes;
+        var numOfMeshes = meshes.length;
 
         if (kind === VertexBuffer.PositionKind) {
             var data = <FloatArray>buffer.getData();
@@ -281,9 +287,6 @@ export class Geometry implements IGetSetVerticesData {
             this._updateExtend(data);
             this._resetPointsArrayCache();
 
-            var meshes = this._meshes;
-            var numOfMeshes = meshes.length;
-
             for (var index = 0; index < numOfMeshes; index++) {
                 var mesh = meshes[index];
                 mesh._boundingInfo = new BoundingInfo(this._extend.minimum, this._extend.maximum);
@@ -296,7 +299,6 @@ export class Geometry implements IGetSetVerticesData {
 
         if (this._vertexArrayObjects) {
             this._disposeVertexArrayObjects();
-            this._vertexArrayObjects = {}; // Will trigger a rebuild of the VAO if supported
         }
     }
 
@@ -367,7 +369,7 @@ export class Geometry implements IGetSetVerticesData {
     }
 
     /** @hidden */
-    public _bind(effect: Nullable<Effect>, indexToBind?: Nullable<DataBuffer>): void {
+    public _bind(effect: Nullable<Effect>, indexToBind?: Nullable<DataBuffer>, overrideVertexBuffers?: { [kind: string]: Nullable<VertexBuffer>}, overrideVertexArrayObjects?: {[key: string]: WebGLVertexArrayObject}): void {
         if (!effect) {
             return;
         }
@@ -381,17 +383,19 @@ export class Geometry implements IGetSetVerticesData {
             return;
         }
 
-        if (indexToBind != this._indexBuffer || !this._vertexArrayObjects) {
-            this._engine.bindBuffers(vbs, indexToBind, effect);
+        if (indexToBind != this._indexBuffer || (!this._vertexArrayObjects && !overrideVertexArrayObjects)) {
+            this._engine.bindBuffers(vbs, indexToBind, effect, overrideVertexBuffers);
             return;
         }
 
+        var vaos = overrideVertexArrayObjects ? overrideVertexArrayObjects : this._vertexArrayObjects;
+
         // Using VAO
-        if (!this._vertexArrayObjects[effect.key]) {
-            this._vertexArrayObjects[effect.key] = this._engine.recordVertexArrayObject(vbs, indexToBind, effect);
+        if (!vaos[effect.key]) {
+            vaos[effect.key] = this._engine.recordVertexArrayObject(vbs, indexToBind, effect, overrideVertexBuffers);
         }
 
-        this._engine.bindVertexArrayObject(this._vertexArrayObjects[effect.key], indexToBind);
+        this._engine.bindVertexArrayObject(vaos[effect.key], indexToBind);
     }
 
     /**
@@ -580,8 +584,6 @@ export class Geometry implements IGetSetVerticesData {
             this._engine._releaseBuffer(this._indexBuffer);
         }
 
-        this._disposeVertexArrayObjects();
-
         this._indices = indices;
         this._indexBufferIsUpdatable = updatable;
         if (this._meshes.length !== 0 && this._indices) {
@@ -672,6 +674,10 @@ export class Geometry implements IGetSetVerticesData {
 
         meshes.splice(index, 1);
 
+        if (this._vertexArrayObjects) {
+            mesh._invalidateInstanceVertexArrayObject();
+        }
+
         mesh._geometry = null;
 
         if (meshes.length === 0 && shouldDispose) {
@@ -693,6 +699,10 @@ export class Geometry implements IGetSetVerticesData {
             previousGeometry.releaseForMesh(mesh);
         }
 
+        if (this._vertexArrayObjects) {
+            mesh._invalidateInstanceVertexArrayObject();
+        }
+
         var meshes = this._meshes;
 
         // must be done before setting vertexBuffers because of mesh._createGlobalSubMesh()
@@ -913,7 +923,13 @@ export class Geometry implements IGetSetVerticesData {
             for (var kind in this._vertexArrayObjects) {
                 this._engine.releaseVertexArrayObject(this._vertexArrayObjects[kind]);
             }
-            this._vertexArrayObjects = {};
+            this._vertexArrayObjects = {}; // Will trigger a rebuild of the VAO if supported
+
+            var meshes = this._meshes;
+            var numOfMeshes = meshes.length;
+            for (var index = 0; index < numOfMeshes; index++) {
+                meshes[index]._invalidateInstanceVertexArrayObject();
+            }
         }
     }
 

src/Meshes/instancedMesh.ts

@@ -491,6 +491,11 @@ declare module "./mesh" {
         registerInstancedBuffer(kind: string, stride: number): void;
 
         /**
+         * Invalidate VertexArrayObjects belonging to the mesh (but not to the Geometry of the mesh).
+         */
+        _invalidateInstanceVertexArrayObject(): void;
+
+        /**
          * true to use the edge renderer for all instances of this mesh
          */
         edgesShareWithInstances: boolean;
@@ -500,7 +505,8 @@ declare module "./mesh" {
             data: {[key: string]: Float32Array},
             sizes: {[key: string]: number},
             vertexBuffers: {[key: string]: Nullable<VertexBuffer>},
-            strides: {[key: string]: number}
+            strides: {[key: string]: number},
+            vertexArrayObjects?: {[key: string]: WebGLVertexArrayObject}
         };
     }
 }
@@ -518,9 +524,6 @@ declare module "./abstractMesh" {
 Mesh.prototype.edgesShareWithInstances = false;
 
 Mesh.prototype.registerInstancedBuffer = function(kind: string, stride: number): void {
-    // Remove existing one
-    this.removeVerticesData(kind);
-
     // Creates the instancedBuffer field if not present
     if (!this.instancedBuffers) {
         this.instancedBuffers = {};
@@ -533,7 +536,8 @@ Mesh.prototype.registerInstancedBuffer = function(kind: string, stride: number):
             data: {},
             vertexBuffers: {},
             strides: {},
-            sizes: {}
+            sizes: {},
+            vertexArrayObjects: (this.getEngine().getCaps().vertexArrayObject) ? {} : undefined
         };
     }
 
@@ -544,11 +548,12 @@ Mesh.prototype.registerInstancedBuffer = function(kind: string, stride: number):
     this._userInstancedBuffersStorage.sizes[kind] = stride * 32; // Initial size
     this._userInstancedBuffersStorage.data[kind] = new Float32Array(this._userInstancedBuffersStorage.sizes[kind]);
     this._userInstancedBuffersStorage.vertexBuffers[kind] = new VertexBuffer(this.getEngine(), this._userInstancedBuffersStorage.data[kind], kind, true, false, stride, true);
-    this.setVerticesBuffer(this._userInstancedBuffersStorage.vertexBuffers[kind]!);
 
     for (var instance of this.instances) {
         instance.instancedBuffers[kind] = null;
     }
+
+    this._invalidateInstanceVertexArrayObject();
 };
 
 Mesh.prototype._processInstancedBuffers = function(visibleInstances: InstancedMesh[], renderSelf: boolean) {
@@ -607,13 +612,25 @@ Mesh.prototype._processInstancedBuffers = function(visibleInstances: InstancedMe
         // Update vertex buffer
         if (!this._userInstancedBuffersStorage.vertexBuffers[kind]) {
             this._userInstancedBuffersStorage.vertexBuffers[kind] = new VertexBuffer(this.getEngine(), this._userInstancedBuffersStorage.data[kind], kind, true, false, stride, true);
-            this.setVerticesBuffer(this._userInstancedBuffersStorage.vertexBuffers[kind]!);
+            this._invalidateInstanceVertexArrayObject();
         } else {
             this._userInstancedBuffersStorage.vertexBuffers[kind]!.updateDirectly(data, 0);
         }
     }
 };
 
+Mesh.prototype._invalidateInstanceVertexArrayObject = function() {
+    if (!this._userInstancedBuffersStorage || this._userInstancedBuffersStorage.vertexArrayObjects === undefined) {
+        return;
+    }
+
+    for (var kind in this._userInstancedBuffersStorage.vertexArrayObjects) {
+        this.getEngine().releaseVertexArrayObject(this._userInstancedBuffersStorage.vertexArrayObjects[kind]);
+    }
+
+    this._userInstancedBuffersStorage.vertexArrayObjects = {};
+};
+
 Mesh.prototype._disposeInstanceSpecificData = function() {
     if (this._instanceDataStorage.instancesBuffer) {
         this._instanceDataStorage.instancesBuffer.dispose();
@@ -630,5 +647,7 @@ Mesh.prototype._disposeInstanceSpecificData = function() {
         }
     }
 
+    this._invalidateInstanceVertexArrayObject();
+
     this.instancedBuffers = {};
 };

src/Meshes/mesh.ts

@@ -1521,7 +1521,11 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
         }
 
         // VBOs
-        this._geometry._bind(effect, indexToBind);
+        if (!this._userInstancedBuffersStorage) {
+            this._geometry._bind(effect, indexToBind);
+        } else {
+            this._geometry._bind(effect, indexToBind, this._userInstancedBuffersStorage.vertexBuffers, this._userInstancedBuffersStorage.vertexArrayObjects);
+        }
         return this;
     }
 
@@ -1678,11 +1682,20 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
 
             instancesBuffer = new Buffer(engine, instanceStorage.instancesData, true, 16, false, true);
             instanceStorage.instancesBuffer = instancesBuffer;
+            if (!this._userInstancedBuffersStorage) {
+                this._userInstancedBuffersStorage = {
+                    data: {},
+                    vertexBuffers: {},
+                    strides: {},
+                    sizes: {},
+                    vertexArrayObjects: (this.getEngine().getCaps().vertexArrayObject) ? {} : undefined
+                };
+            }
 
-            this.setVerticesBuffer(instancesBuffer.createVertexBuffer("world0", 0, 4));
-            this.setVerticesBuffer(instancesBuffer.createVertexBuffer("world1", 4, 4));
-            this.setVerticesBuffer(instancesBuffer.createVertexBuffer("world2", 8, 4));
-            this.setVerticesBuffer(instancesBuffer.createVertexBuffer("world3", 12, 4));
+            this._userInstancedBuffersStorage.vertexBuffers["world0"] = instancesBuffer.createVertexBuffer("world0", 0, 4);
+            this._userInstancedBuffersStorage.vertexBuffers["world1"] = instancesBuffer.createVertexBuffer("world0", 4, 4);
+            this._userInstancedBuffersStorage.vertexBuffers["world2"] = instancesBuffer.createVertexBuffer("world0", 8, 4);
+            this._userInstancedBuffersStorage.vertexBuffers["world3"] = instancesBuffer.createVertexBuffer("world0", 12, 4);
         } else {
             if (!this._instanceDataStorage.isFrozen) {
                 instancesBuffer!.updateDirectly(instanceStorage.instancesData, 0, instancesCount);
@@ -1779,6 +1792,19 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
             this._instanceDataStorage.instancesBuffer.dispose();
             this._instanceDataStorage.instancesBuffer = null;
         }
+        if (this._userInstancedBuffersStorage) {
+            for (var kind in this._userInstancedBuffersStorage.vertexBuffers) {
+                var buffer = this._userInstancedBuffersStorage.vertexBuffers[kind];
+                if (buffer) {
+                    // Dispose instance buffer to be recreated in _renderWithInstances when rendered
+                    buffer.dispose();
+                    this._userInstancedBuffersStorage.vertexBuffers[kind] = null;
+                }
+            }
+            if (this._userInstancedBuffersStorage.vertexArrayObjects) {
+                this._userInstancedBuffersStorage.vertexArrayObjects = {};
+            }
+        }
         super._rebuild();
     }
 
@@ -2409,6 +2435,11 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
         // Do nothing
     }
 
+    /** @hidden */
+    public _invalidateInstanceVertexArrayObject() {
+        // Do nothing
+    }
+
     /**
      * Modifies the mesh geometry according to a displacement map.
      * A displacement map is a colored image. Each pixel color value (actually a gradient computed from red, green, blue values) will give the displacement to apply to each mesh vertex.
@@ -2949,18 +2980,6 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
      * @returns a new InstancedMesh
      */
     public createInstance(name: string): InstancedMesh {
-        let geometry = this.geometry;
-
-        if (geometry && geometry.meshes.length > 1) {
-            let others = geometry.meshes.slice(0);
-            for (var other of others) {
-                if (other === this) {
-                    continue;
-                }
-                other.makeGeometryUnique();
-            }
-        }
-
         return Mesh._instancedMeshFactory(name, this);
     }