fix: Support sharing cached Pipelines with different parameters/topol…

…ogy in WebGL (#2034)
visgl · Mar 14, 2024 · ce87eac · ce87eac
1 parent 54af690
commit ce87eac
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 41 deletions.
diff --git a/modules/core/src/adapter/resources/render-pipeline.ts b/modules/core/src/adapter/resources/render-pipeline.ts
@@ -110,6 +110,10 @@ export abstract class RenderPipeline extends Resource<RenderPipelineProps> {
   abstract draw(options: {
     /** Render pass to draw into (targeting screen or framebuffer) */
     renderPass?: RenderPass;
+    /** Parameters to be set during draw call. Note that most parameters can only be overridden in WebGL. */
+    parameters?: RenderPipelineParameters;
+    /** Topology. Note can only be overridden in WebGL. */
+    topology?: PrimitiveTopology;
     /** vertex attributes */
     vertexArray: VertexArray;
     /** Number of "rows" in index buffer */

diff --git a/modules/engine/src/lib/pipeline-factory.ts b/modules/engine/src/lib/pipeline-factory.ts
@@ -104,9 +104,10 @@ export class PipelineFactory {
     const bufferLayoutHash = this._getHash(JSON.stringify(props.bufferLayout));
 
     switch (this.device.type) {
-      // case 'webgl':
-      // WebGL is more dynamic
-      // return `${vsHash}/${fsHash}V${varyingHash}BL${bufferLayoutHash}`;
+      case 'webgl':
+        // WebGL is more dynamic
+        return `${vsHash}/${fsHash}V${varyingHash}BL${bufferLayoutHash}`;
+
       default:
         // On WebGPU we need to rebuild the pipeline if topology, parameters or bufferLayout change
         const parameterHash = this._getHash(JSON.stringify(props.parameters));

diff --git a/modules/engine/src/model/model.ts b/modules/engine/src/model/model.ts
@@ -384,7 +384,12 @@ export class Model {
         vertexCount: this.vertexCount,
         instanceCount: this.instanceCount,
         indexCount,
-        transformFeedback: this.transformFeedback || undefined
+        transformFeedback: this.transformFeedback || undefined,
+        // WebGL shares underlying cached pipelines even for models that have different parameters and topology,
+        // so we must provide our unique parameters to each draw
+        // (In WebGPU most parameters are encoded in the pipeline and cannot be changed per draw call)
+        parameters: this.parameters,
+        topology: this.topology
       });
     } finally {
       this._logDrawCallEnd();

diff --git a/modules/engine/test/lib/model.spec.ts b/modules/engine/test/lib/model.spec.ts
@@ -172,14 +172,26 @@ test('Model#topology', async t => {
       fragmentEntryPoint: 'fragmentMain'
     });
 
-    t.equal(model.pipeline.props.topology, 'triangle-list', 'Pipeline has triangle-list topology');
+    t.equal(model.topology, 'triangle-list', 'Pipeline has triangle-list topology');
+    if (device.type === 'webgpu') {
+      // Cached model in WebGL can have a different topology
+      t.equal(
+        model.pipeline.props.topology,
+        'triangle-list',
+        'Pipeline has triangle-list topology'
+      );
+    }
 
     model.setTopology('line-strip');
 
     const renderPass = device.beginRenderPass({clearColor: [0, 0, 0, 0]});
     model.draw(renderPass);
 
-    t.equal(model.pipeline.props.topology, 'line-strip', 'Pipeline has line-strip topology');
+    t.equal(model.topology, 'line-strip', 'Pipeline has line-strip topology');
+    if (device.type === 'webgpu') {
+      // Cached model in WebGL can have a different topology
+      t.equal(model.pipeline.props.topology, 'line-strip', 'Pipeline has triangle-list topology');
+    }
 
     renderPass.end();
     device.submit();

diff --git a/modules/webgl/src/adapter/resources/webgl-render-pipeline.ts b/modules/webgl/src/adapter/resources/webgl-render-pipeline.ts
@@ -2,8 +2,8 @@
 // SPDX-License-Identifier: MIT
 // Copyright (c) vis.gl contributors
 
-import type {UniformValue, RenderPipelineProps, Binding} from '@luma.gl/core';
-import type {ShaderLayout} from '@luma.gl/core';
+import type {RenderPipelineProps, RenderPipelineParameters, PrimitiveTopology} from '@luma.gl/core';
+import type {ShaderLayout, UniformValue, Binding} from '@luma.gl/core';
 import type {RenderPass, VertexArray} from '@luma.gl/core';
 import {RenderPipeline, log} from '@luma.gl/core';
 // import {getAttributeInfosFromLayouts} from '@luma.gl/core';
@@ -155,7 +155,8 @@ export class WEBGLRenderPipeline extends RenderPipeline {
    */
   draw(options: {
     renderPass: RenderPass;
-    /** vertex attributes */
+    parameters?: RenderPipelineParameters;
+    topology?: PrimitiveTopology;
     vertexArray: VertexArray;
     vertexCount?: number;
     indexCount?: number;
@@ -168,6 +169,8 @@ export class WEBGLRenderPipeline extends RenderPipeline {
   }): boolean {
     const {
       renderPass,
+      parameters = this.props.parameters,
+      topology = this.props.topology,
       vertexArray,
       vertexCount,
       // indexCount,
@@ -179,7 +182,7 @@ export class WEBGLRenderPipeline extends RenderPipeline {
       transformFeedback
     } = options;
 
-    const glDrawMode = getGLDrawMode(this.props.topology);
+    const glDrawMode = getGLDrawMode(topology);
     const isIndexed: boolean = Boolean(vertexArray.indexBuffer);
     const glIndexType = (vertexArray.indexBuffer as WEBGLBuffer)?.glIndexType;
     const isInstanced: boolean = Number(instanceCount) > 0;
@@ -222,39 +225,34 @@ export class WEBGLRenderPipeline extends RenderPipeline {
 
     const webglRenderPass = renderPass as WEBGLRenderPass;
 
-    withDeviceAndGLParameters(
-      this.device,
-      this.props.parameters,
-      webglRenderPass.glParameters,
-      () => {
-        if (isIndexed && isInstanced) {
-          this.device.gl.drawElementsInstanced(
-            glDrawMode,
-            vertexCount || 0, // indexCount?
-            glIndexType,
-            firstVertex,
-            instanceCount || 0
-          );
-          // } else if (isIndexed && this.device.isWebGL2 && !isNaN(start) && !isNaN(end)) {
-          //   this.device.gldrawRangeElements(glDrawMode, start, end, vertexCount, glIndexType, offset);
-        } else if (isIndexed) {
-          this.device.gl.drawElements(glDrawMode, vertexCount || 0, glIndexType, firstVertex); // indexCount?
-        } else if (isInstanced) {
-          this.device.gl.drawArraysInstanced(
-            glDrawMode,
-            firstVertex,
-            vertexCount || 0,
-            instanceCount || 0
-          );
-        } else {
-          this.device.gl.drawArrays(glDrawMode, firstVertex, vertexCount || 0);
-        }
+    withDeviceAndGLParameters(this.device, parameters, webglRenderPass.glParameters, () => {
+      if (isIndexed && isInstanced) {
+        this.device.gl.drawElementsInstanced(
+          glDrawMode,
+          vertexCount || 0, // indexCount?
+          glIndexType,
+          firstVertex,
+          instanceCount || 0
+        );
+        // } else if (isIndexed && this.device.isWebGL2 && !isNaN(start) && !isNaN(end)) {
+        //   this.device.gldrawRangeElements(glDrawMode, start, end, vertexCount, glIndexType, offset);
+      } else if (isIndexed) {
+        this.device.gl.drawElements(glDrawMode, vertexCount || 0, glIndexType, firstVertex); // indexCount?
+      } else if (isInstanced) {
+        this.device.gl.drawArraysInstanced(
+          glDrawMode,
+          firstVertex,
+          vertexCount || 0,
+          instanceCount || 0
+        );
+      } else {
+        this.device.gl.drawArrays(glDrawMode, firstVertex, vertexCount || 0);
+      }
 
-        if (transformFeedback) {
-          transformFeedback.end();
-        }
+      if (transformFeedback) {
+        transformFeedback.end();
       }
-    );
+    });
 
     vertexArray.unbindAfterRender(renderPass);