Merge pull request #9650 from Popov72/fix-instancedmesh

Various fixes after changes to instanced mesh vertex buffer management

src/Engines/WebGPU/webgpuCacheRenderPipeline.ts

@@ -168,6 +168,7 @@ export class WebGPUCacheRenderPipeline {
     private _stencilWriteMask: number;
     private _depthStencilState: number;
     private _vertexBuffers: Nullable<{ [key: string]: Nullable<VertexBuffer> }>;
+    private _overrideVertexBuffers: Nullable<{ [key: string]: Nullable<VertexBuffer> }>;
     private _indexBuffer: Nullable<DataBuffer>;
 
     constructor(device: GPUDevice, emptyVertexBuffer: VertexBuffer) {
@@ -195,7 +196,7 @@ export class WebGPUCacheRenderPipeline {
         this.setDepthStencilFormat(WebGPUConstants.TextureFormat.Depth24PlusStencil8);
         this.setStencilEnabled(false);
         this.resetStencilState();
-        this.setBuffers(null, null);
+        this.setBuffers(null, null, null);
     }
 
     public getRenderPipeline(fillMode: number, effect: Effect, sampleCount: number): GPURenderPipeline {
@@ -425,8 +426,9 @@ export class WebGPUCacheRenderPipeline {
         this.setStencilWriteMask(writeMask);
     }
 
-    public setBuffers(vertexBuffers: Nullable<{ [key: string]: Nullable<VertexBuffer> }>, indexBuffer: Nullable<DataBuffer>): void {
+    public setBuffers(vertexBuffers: Nullable<{ [key: string]: Nullable<VertexBuffer> }>, indexBuffer: Nullable<DataBuffer>, overrideVertexBuffers: Nullable<{ [key: string]: Nullable<VertexBuffer> }>): void {
         this._vertexBuffers = vertexBuffers;
+        this._overrideVertexBuffers = overrideVertexBuffers;
         this._indexBuffer = indexBuffer;
     }
 
@@ -745,7 +747,7 @@ export class WebGPUCacheRenderPipeline {
             const location = effect.getAttributeLocation(index);
 
             if (location >= 0) {
-                let  vertexBuffer = this._vertexBuffers![attributes[index]];
+                let vertexBuffer = (this._overrideVertexBuffers && this._overrideVertexBuffers[attributes[index]]) ?? this._vertexBuffers![attributes[index]];
                 if (!vertexBuffer) {
                     // In WebGL it's valid to not bind a vertex buffer to an attribute, but it's not valid in WebGPU
                     // So we must bind a dummy buffer when we are not given one for a specific attribute
@@ -851,7 +853,7 @@ export class WebGPUCacheRenderPipeline {
             const location = effect.getAttributeLocation(index);
 
             if (location >= 0) {
-                let  vertexBuffer = this._vertexBuffers![attributes[index]];
+                let vertexBuffer = (this._overrideVertexBuffers && this._overrideVertexBuffers[attributes[index]]) ?? this._vertexBuffers![attributes[index]];
                 if (!vertexBuffer) {
                     // In WebGL it's valid to not bind a vertex buffer to an attribute, but it's not valid in WebGPU
                     // So we must bind a dummy buffer when we are not given one for a specific attribute

src/Engines/webgpuEngine.ts

@@ -212,6 +212,7 @@ export class WebGPUEngine extends Engine {
     // Effect is on the parent class
     // protected _currentEffect: Nullable<Effect> = null;
     private _currentVertexBuffers: Nullable<{ [key: string]: Nullable<VertexBuffer> }> = null;
+    private _currentOverrideVertexBuffers: Nullable<{ [key: string]: Nullable<VertexBuffer> }> = null;
     private _currentIndexBuffer: Nullable<DataBuffer> = null;
     private __colorWrite = true;
     private _uniformsBuffers: { [name: string]: WebGPUDataBuffer } = {};
@@ -707,7 +708,8 @@ export class WebGPUEngine extends Engine {
         this._forceEnableEffect = true;
         this._currentIndexBuffer = null;
         this._currentVertexBuffers = null;
-        this._cacheRenderPipeline.setBuffers(null, null);
+        this._currentOverrideVertexBuffers = null;
+        this._cacheRenderPipeline.setBuffers(null, null, null);
 
         if (bruteForce) {
             this._currentProgram = null;
@@ -1067,11 +1069,13 @@ export class WebGPUEngine extends Engine {
      * @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 {
         this._currentIndexBuffer = indexBuffer;
         this._currentVertexBuffers = vertexBuffers;
-        this._cacheRenderPipeline.setBuffers(vertexBuffers, indexBuffer);
+        this._currentOverrideVertexBuffers = overrideVertexBuffers ?? null;
+        this._cacheRenderPipeline.setBuffers(vertexBuffers, indexBuffer, this._currentOverrideVertexBuffers);
     }
 
     /** @hidden */
@@ -3582,7 +3586,7 @@ export class WebGPUEngine extends Engine {
             const order = effect.getAttributeLocation(index);
 
             if (order >= 0) {
-                let vertexBuffer = this._currentVertexBuffers![attributes[index]];
+                let vertexBuffer = (this._currentOverrideVertexBuffers && this._currentOverrideVertexBuffers[attributes[index]]) ?? this._currentVertexBuffers![attributes[index]];
                 if (!vertexBuffer) {
                     // In WebGL it's valid to not bind a vertex buffer to an attribute, but it's not valid in WebGPU
                     // So we must bind a dummy buffer when we are not given one for a specific attribute

src/Meshes/buffer.ts

@@ -1,6 +1,7 @@
-import { Nullable, DataArray } from "../types";
+import { Nullable, DataArray, FloatArray } from "../types";
 import { ThinEngine } from "../Engines/thinEngine";
-import { DataBuffer } from './dataBuffer';
+import { DataBuffer } from "./dataBuffer";
+import { SliceTools } from "../Misc/sliceTools";
 
 /**
  * Class used to store data that will be store in GPU memory
@@ -390,6 +391,62 @@ export class VertexBuffer {
     }
 
     /**
+     * Gets current buffer's data as a float array. Float data is constructed if the vertex buffer data cannot be returned directly.
+     * @param totalVertices number of vertices in the buffer to take into account
+     * @param forceCopy defines a boolean indicating that the returned array must be cloned upon returning it
+     * @returns a float array containing vertex data
+     */
+    public getFloatData(totalVertices: number, forceCopy?: boolean): Nullable<FloatArray> {
+        let data = this.getData();
+        if (!data) {
+            return null;
+        }
+
+        const tightlyPackedByteStride = this.getSize() * VertexBuffer.GetTypeByteLength(this.type);
+        const count = totalVertices * this.getSize();
+
+        if (this.type !== VertexBuffer.FLOAT || this.byteStride !== tightlyPackedByteStride) {
+            const copy: number[] = [];
+            this.forEach(count, (value) => copy.push(value));
+            return copy;
+        }
+
+        if (!(data instanceof Array || data instanceof Float32Array) || this.byteOffset !== 0 || data.length !== count) {
+            if (data instanceof Array) {
+                const offset = this.byteOffset / 4;
+                return SliceTools.Slice(data, offset, offset + count);
+            } else if (data instanceof ArrayBuffer) {
+                return new Float32Array(data, this.byteOffset, count);
+            } else {
+                let offset = data.byteOffset + this.byteOffset;
+                if (forceCopy) {
+                    let result = new Float32Array(count);
+                    let source = new Float32Array(data.buffer, offset, count);
+
+                    result.set(source);
+
+                    return result;
+                }
+
+                // Portect against bad data
+                let remainder = offset % 4;
+
+                if (remainder) {
+                    offset = Math.max(0, offset - remainder);
+                }
+
+                return new Float32Array(data.buffer, offset, count);
+            }
+        }
+
+        if (forceCopy) {
+            return SliceTools.Slice(data);
+        }
+
+        return data;
+    }
+
+    /**
      * Gets underlying native buffer
      * @returns underlying native buffer
      */

src/Meshes/geometry.ts

@@ -423,53 +423,7 @@ export class Geometry implements IGetSetVerticesData {
             return null;
         }
 
-        let data = vertexBuffer.getData();
-        if (!data) {
-            return null;
-        }
-
-        const tightlyPackedByteStride = vertexBuffer.getSize() * VertexBuffer.GetTypeByteLength(vertexBuffer.type);
-        const count = this._totalVertices * vertexBuffer.getSize();
-
-        if (vertexBuffer.type !== VertexBuffer.FLOAT || vertexBuffer.byteStride !== tightlyPackedByteStride) {
-            const copy: number[] = [];
-            vertexBuffer.forEach(count, (value) => copy.push(value));
-            return copy;
-        }
-
-        if (!(data instanceof Array || data instanceof Float32Array) || vertexBuffer.byteOffset !== 0 || data.length !== count) {
-            if (data instanceof Array) {
-                const offset = vertexBuffer.byteOffset / 4;
-                return Tools.Slice(data, offset, offset + count);
-            } else if (data instanceof ArrayBuffer) {
-                return new Float32Array(data, vertexBuffer.byteOffset, count);
-            } else {
-                let offset = data.byteOffset + vertexBuffer.byteOffset;
-                if (forceCopy || (copyWhenShared && this._meshes.length !== 1)) {
-                    let result = new Float32Array(count);
-                    let source = new Float32Array(data.buffer, offset, count);
-
-                    result.set(source);
-
-                    return result;
-                }
-
-                // Portect against bad data
-                let remainder = offset % 4;
-
-                if (remainder) {
-                    offset = Math.max(0, offset - remainder);
-                }
-
-                return new Float32Array(data.buffer, offset, count);
-            }
-        }
-
-        if (forceCopy || (copyWhenShared && this._meshes.length !== 1)) {
-            return Tools.Slice(data);
-        }
-
-        return data;
+        return vertexBuffer.getFloatData(this._totalVertices, forceCopy || (copyWhenShared && this._meshes.length !== 1));
     }
 
     /**

src/Meshes/instancedMesh.ts

@@ -524,6 +524,9 @@ declare module "./abstractMesh" {
 Mesh.prototype.edgesShareWithInstances = false;
 
 Mesh.prototype.registerInstancedBuffer = function(kind: string, stride: number): void {
+    // Remove existing one
+    this._userInstancedBuffersStorage?.vertexBuffers[kind]?.dispose();
+
     // Creates the instancedBuffer field if not present
     if (!this.instancedBuffers) {
         this.instancedBuffers = {};

src/Meshes/mesh.ts

@@ -852,7 +852,11 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
         if (!this._geometry) {
             return null;
         }
-        return this._geometry.getVerticesData(kind, copyWhenShared, forceCopy);
+        let data = this._userInstancedBuffersStorage?.vertexBuffers[kind]?.getFloatData(this._geometry.getTotalVertices(), forceCopy || (copyWhenShared && this._geometry.meshes.length !== 1));
+        if (!data) {
+            data = this._geometry.getVerticesData(kind, copyWhenShared, forceCopy);
+        }
+        return data;
     }
 
     /**
@@ -877,7 +881,8 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
         if (!this._geometry) {
             return null;
         }
-        return this._geometry.getVertexBuffer(kind);
+
+        return this._userInstancedBuffersStorage?.vertexBuffers[kind] ?? this._geometry.getVertexBuffer(kind);
     }
 
     /**
@@ -905,7 +910,7 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
             }
             return false;
         }
-        return this._geometry.isVerticesDataPresent(kind);
+        return this._userInstancedBuffersStorage?.vertexBuffers[kind] !== undefined || this._geometry.isVerticesDataPresent(kind);
     }
 
     /**
@@ -932,7 +937,7 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
             }
             return false;
         }
-        return this._geometry.isVertexBufferUpdatable(kind);
+        return this._userInstancedBuffersStorage?.vertexBuffers[kind]?.isUpdatable() || this._geometry.isVertexBufferUpdatable(kind);
     }
 
     /**
@@ -963,7 +968,13 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
             }
             return result;
         }
-        return this._geometry.getVerticesDataKinds();
+        const kinds = this._geometry.getVerticesDataKinds();
+        if (this._userInstancedBuffersStorage) {
+            for (const kind in this._userInstancedBuffersStorage.vertexBuffers) {
+                kinds.push(kind);
+            }
+        }
+        return kinds;
     }
 
     /**
@@ -1693,9 +1704,9 @@ export class Mesh extends AbstractMesh implements IGetSetVerticesData {
             }
 
             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);
+            this._userInstancedBuffersStorage.vertexBuffers["world1"] = instancesBuffer.createVertexBuffer("world1", 4, 4);
+            this._userInstancedBuffersStorage.vertexBuffers["world2"] = instancesBuffer.createVertexBuffer("world2", 8, 4);
+            this._userInstancedBuffersStorage.vertexBuffers["world3"] = instancesBuffer.createVertexBuffer("world3", 12, 4);
         } else {
             if (!this._instanceDataStorage.isFrozen) {
                 instancesBuffer!.updateDirectly(instanceStorage.instancesData, 0, instancesCount);

src/Misc/sliceTools.ts

@@ -0,0 +1,35 @@
+/**
+ * Class used to provide helpers for slicing
+ */
+export class SliceTools {
+    /**
+     * Provides a slice function that will work even on IE
+     * @param data defines the array to slice
+     * @param start defines the start of the data (optional)
+     * @param end defines the end of the data (optional)
+     * @returns the new sliced array
+     */
+    public static Slice<T>(data: T, start?: number, end?: number): T {
+        if ((data as any).slice) {
+            return (data as any).slice(start, end);
+        }
+
+        return Array.prototype.slice.call(data, start, end);
+    }
+
+    /**
+     * Provides a slice function that will work even on IE
+     * The difference between this and Slice is that this will force-convert to array
+     * @param data defines the array to slice
+     * @param start defines the start of the data (optional)
+     * @param end defines the end of the data (optional)
+     * @returns the new sliced array
+     */
+    public static SliceToArray<T, P>(data: T, start?: number, end?: number): Array<P> {
+        if (Array.isArray(data)) {
+            return (data as Array<P>).slice(start, end);
+        }
+
+        return Array.prototype.slice.call(data, start, end);
+    }
+}

src/Misc/tools.ts

@@ -16,6 +16,7 @@ import { TimingTools } from "./timingTools";
 import { InstantiationTools } from "./instantiationTools";
 import { GUID } from "./guid";
 import { IScreenshotSize } from "./interfaces/screenshotSize";
+import { SliceTools } from "./sliceTools";
 
 declare type Camera = import("../Cameras/camera").Camera;
 declare type Engine = import("../Engines/engine").Engine;
@@ -163,11 +164,7 @@ export class Tools {
      * @returns the new sliced array
      */
     public static Slice<T>(data: T, start?: number, end?: number): T {
-        if ((data as any).slice) {
-            return (data as any).slice(start, end);
-        }
-
-        return Array.prototype.slice.call(data, start, end);
+        return SliceTools.Slice(data, start, end);
     }
 
     /**
@@ -179,11 +176,7 @@ export class Tools {
      * @returns the new sliced array
      */
     public static SliceToArray<T, P>(data: T, start?: number, end?: number): Array<P> {
-        if (Array.isArray(data)) {
-            return (data as Array<P>).slice(start, end);
-        }
-
-        return Array.prototype.slice.call(data, start, end);
+        return SliceTools.SliceToArray(data, start, end);
     }
 
     /**

tests/validation/config.json

@@ -1017,6 +1017,11 @@
             "renderCount": 10,
             "playgroundId": "#MSAHKR#13",
             "referenceImage": "shadowsinstancesright.png"
+        },
+        {
+            "title": "Instances with color buffer",
+            "playgroundId": "#YPABS1#91",
+            "referenceImage": "instancecolors.png"
         }
     ]
 }

tests/validation/validation.js

@@ -104,7 +104,7 @@ async function evaluate(test, resultCanvas, result, renderImage, waitRing, done)
     var testRes = true;
 
     // gl check
-    var gl = engine._gl, glError = gl.getError();
+    var gl = engine._gl, glError = gl ? gl.getError() : 0;
     if (gl && glError !== 0) {
         result.classList.add("failed");
         result.innerHTML = "×";