fix(webgl): Enable deep sharing of WebGLPrograms

modules/core/src/adapter/device.ts

@@ -361,8 +361,8 @@ export abstract class Device {
     // Experimental
     _reuseDevices: false,
     _requestMaxLimits: true,
-    _cacheShaders: false,
-    _cachePipelines: false,
+    _cacheShaders: true,
+    _cachePipelines: true,
     _cacheDestroyPolicy: 'unused',
     // TODO - Change these after confirming things work as expected
     _initializeFeatures: true,

modules/core/src/adapter/resources/render-pipeline.ts

@@ -3,7 +3,6 @@
 // Copyright (c) vis.gl contributors
 
 import type {Device} from '../device';
-import type {UniformValue} from '../types/uniforms';
 import type {PrimitiveTopology, RenderPipelineParameters} from '../types/parameters';
 import type {ShaderLayout, Binding} from '../types/shader-layout';
 import type {BufferLayout} from '../types/buffer-layout';
@@ -51,12 +50,12 @@ export type RenderPipelineProps = ResourceProps & {
   /** Parameters that are controlled by pipeline */
   parameters?: RenderPipelineParameters;
 
-  // Dynamic bindings (TODO - pipelines should be immutable, move to RenderPass)
+  /** Some applications intentionally supply unused attributes and bindings, and want to disable warnings */
+  disableWarnings?: boolean;
 
+  // Dynamic bindings (TODO - pipelines should be immutable, move to RenderPass)
   /** Buffers, Textures, Samplers for the shader bindings */
   bindings?: Record<string, Binding>;
-  /** @deprecated uniforms (WebGL only) */
-  uniforms?: Record<string, UniformValue>;
 };
 
 /**
@@ -85,12 +84,6 @@ export abstract class RenderPipeline extends Resource<RenderPipelineProps> {
     this.bufferLayout = this.props.bufferLayout || [];
   }
 
-  /** Set bindings (stored on pipeline and set before each call) */
-  abstract setBindings(
-    bindings: Record<string, Binding>,
-    options?: {disableWarnings?: boolean}
-  ): void;
-
   /** Draw call. Returns false if the draw call was aborted (due to resources still initializing) */
   abstract draw(options: {
     /** Render pass to draw into (targeting screen or framebuffer) */
@@ -118,6 +111,10 @@ export abstract class RenderPipeline extends Resource<RenderPipelineProps> {
     baseVertex?: number;
     /** Transform feedback. WebGL only. */
     transformFeedback?: TransformFeedback;
+    /** Bindings applied for this draw (textures, samplers, uniform buffers) */
+    bindings?: Record<string, Binding>;
+    /** WebGL-only uniforms */
+    uniforms?: Record<string, unknown>;
   }): boolean;
 
   static override defaultProps: Required<RenderPipelineProps> = {
@@ -139,8 +136,7 @@ export abstract class RenderPipeline extends Resource<RenderPipelineProps> {
     depthStencilAttachmentFormat: undefined!,
 
     parameters: {},
-
-    bindings: {},
-    uniforms: {}
+    disableWarnings: false,
+    bindings: undefined!
   };
 }

modules/engine/src/factories/pipeline-factory.ts

@@ -200,8 +200,9 @@ export class PipelineFactory {
     const {type} = this.device;
     switch (type) {
       case 'webgl':
-        // WebGL is more dynamic
-        return `${type}/R/${vsHash}/${fsHash}V${varyingHash}BL${bufferLayoutHash}`;
+        // WebGL is more dynamic but still needs to avoid sharing pipelines when render parameters differ
+        const webglParameterHash = this._getHash(JSON.stringify(props.parameters));
+        return `${type}/R/${vsHash}/${fsHash}V${varyingHash}P${webglParameterHash}BL${bufferLayoutHash}`;
 
       case 'webgpu':
       default:

modules/engine/src/model/model.ts

@@ -64,6 +64,8 @@ export type ModelProps = Omit<RenderPipelineProps, 'vs' | 'fs' | 'bindings'> & {
   shaderInputs?: ShaderInputs;
   /** Bindings */
   bindings?: Record<string, Binding | DynamicTexture>;
+  /** WebGL-only uniforms */
+  uniforms?: Record<string, unknown>;
   /** Parameters that are built into the pipeline */
   parameters?: RenderPipelineParameters;
 
@@ -132,6 +134,8 @@ export class Model {
     indexBuffer: null,
     attributes: {},
     constantAttributes: {},
+    bindings: {},
+    uniforms: {},
     varyings: [],
 
     isInstanced: undefined!,
@@ -422,14 +426,7 @@ export class Model {
       // Application can call Model.predraw() to avoid this.
       this.pipeline = this._updatePipeline();
 
-      // Set pipeline state, we may be sharing a pipeline so we need to set all state on every draw
-      // Any caching needs to be done inside the pipeline functions
-      // TODO this is a busy initialized check for all bindings every frame
-
       const syncBindings = this._getBindings();
-      this.pipeline.setBindings(syncBindings, {
-        disableWarnings: this.props.disableWarnings
-      });
 
       const {indexBuffer} = this.vertexArray;
       const indexCount = indexBuffer
@@ -444,6 +441,8 @@ export class Model {
         instanceCount: this.instanceCount,
         indexCount,
         transformFeedback: this.transformFeedback || undefined,
+        bindings: syncBindings,
+        uniforms: this.props.uniforms,
         // 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)

modules/engine/test/lib/model.spec.ts

@@ -265,20 +265,15 @@ test('Model#pipeline caching', async t => {
 
   const renderPass = webglDevice.beginRenderPass({clearColor: [0, 0, 0, 0]});
 
-  const uniforms: Record<string, unknown> = {};
+  t.ok(model1.draw(renderPass), 'First model draw succeeded');
 
-  model1.draw(renderPass);
-  t.deepEqual(uniforms, {x: 0.5}, 'Pipeline uniforms set');
-
-  model2.draw(renderPass);
-  t.deepEqual(uniforms, {x: -0.5}, 'Pipeline uniforms set');
+  t.ok(model2.draw(renderPass), 'Second model draw succeeded');
 
   model2.setBufferLayout([{name: 'a', format: 'float32x3'}]);
   model2.predraw(); // Forces a pipeline update
   t.ok(model1.pipeline !== model2.pipeline, 'Pipeline updated');
 
-  model2.draw(renderPass);
-  t.deepEqual(uniforms, {x: -0.5}, 'Pipeline uniforms set');
+  t.ok(model2.draw(renderPass), 'Pipeline updates still draw');
 
   renderPass.destroy();
 

modules/test-utils/src/null-device/resources/null-render-pipeline.ts

@@ -2,13 +2,7 @@
 // SPDX-License-Identifier: MIT
 // Copyright (c) vis.gl contributors
 
-import type {
-  UniformValue,
-  RenderPipelineProps,
-  Binding,
-  RenderPass,
-  VertexArray
-} from '@luma.gl/core';
+import type {RenderPipelineProps, Binding, RenderPass, VertexArray} from '@luma.gl/core';
 import {RenderPipeline} from '@luma.gl/core';
 
 import type {NullDevice} from '../null-device';
@@ -22,9 +16,6 @@ export class NullRenderPipeline extends RenderPipeline {
   vs: NullShader;
   fs: NullShader;
 
-  uniforms: Record<string, UniformValue> = {};
-  bindings: Record<string, Binding> = {};
-
   constructor(device: NullDevice, props: RenderPipelineProps) {
     super(device, props);
     this.device = device;
@@ -39,15 +30,13 @@ export class NullRenderPipeline extends RenderPipeline {
     };
   }
 
-  setBindings(bindings: Record<string, Binding>): void {
-    Object.assign(this.bindings, bindings);
-  }
-
   draw(options: {
     renderPass: RenderPass;
     vertexArray: VertexArray;
     vertexCount?: number;
     instanceCount?: number;
+    bindings?: Record<string, Binding>;
+    uniforms?: Record<string, unknown>;
   }): boolean {
     const {renderPass, vertexArray} = options;
     vertexArray.bindBeforeRender(renderPass);

modules/webgl/src/adapter/resources/webgl-render-pipeline.ts

@@ -46,10 +46,6 @@ export class WEBGLRenderPipeline extends RenderPipeline {
   /** The layout extracted from shader by WebGL introspection APIs */
   introspectedLayout: ShaderLayout;
 
-  /** Uniforms set on this model */
-  uniforms: Record<string, UniformValue> = {};
-  /** Bindings set on this model */
-  bindings: Record<string, Binding> = {};
   /** WebGL varyings */
   varyings: string[] | null = null;
 
@@ -104,68 +100,6 @@ export class WEBGLRenderPipeline extends RenderPipeline {
     }
   }
 
-  /**
-   * Bindings include: textures, samplers and uniform buffers
-   * @todo needed for portable model
-   */
-  setBindings(bindings: Record<string, Binding>, options?: {disableWarnings?: boolean}): void {
-    // if (log.priority >= 2) {
-    //   checkUniformValues(uniforms, this.id, this._uniformSetters);
-    // }
-
-    for (const [name, value] of Object.entries(bindings)) {
-      // Accept both `xyz` and `xyzUniforms` as valid names for `xyzUniforms` uniform block
-      // This convention allows shaders to name uniform blocks as `uniform appUniforms {} app;`
-      // and reference them as `app` from both GLSL and JS.
-      // TODO - this is rather hacky - we could also remap the name directly in the shader layout.
-      const binding =
-        this.shaderLayout.bindings.find(binding_ => binding_.name === name) ||
-        this.shaderLayout.bindings.find(binding_ => binding_.name === `${name}Uniforms`);
-
-      if (!binding) {
-        const validBindings = this.shaderLayout.bindings
-          .map(binding_ => `"${binding_.name}"`)
-          .join(', ');
-        if (!options?.disableWarnings) {
-          log.warn(
-            `No binding "${name}" in render pipeline "${this.id}", expected one of ${validBindings}`,
-            value
-          )();
-        }
-        continue; // eslint-disable-line no-continue
-      }
-      if (!value) {
-        log.warn(`Unsetting binding "${name}" in render pipeline "${this.id}"`)();
-      }
-      switch (binding.type) {
-        case 'uniform':
-          // @ts-expect-error
-          if (!(value instanceof WEBGLBuffer) && !(value.buffer instanceof WEBGLBuffer)) {
-            throw new Error('buffer value');
-          }
-          break;
-        case 'texture':
-          if (
-            !(
-              value instanceof WEBGLTextureView ||
-              value instanceof WEBGLTexture ||
-              value instanceof WEBGLFramebuffer
-            )
-          ) {
-            throw new Error(`${this} Bad texture binding for ${name}`);
-          }
-          break;
-        case 'sampler':
-          log.warn(`Ignoring sampler ${name}`)();
-          break;
-        default:
-          throw new Error(binding.type);
-      }
-
-      this.bindings[name] = value;
-    }
-  }
-
   /** @todo needed for portable model
    * @note The WebGL API is offers many ways to draw things
    * This function unifies those ways into a single call using common parameters with sane defaults
@@ -184,6 +118,8 @@ export class WEBGLRenderPipeline extends RenderPipeline {
     firstInstance?: number;
     baseVertex?: number;
     transformFeedback?: WEBGLTransformFeedback;
+    bindings?: Record<string, Binding>;
+    uniforms?: Record<string, UniformValue>;
   }): boolean {
     const {
       renderPass,
@@ -198,7 +134,9 @@ export class WEBGLRenderPipeline extends RenderPipeline {
       // firstIndex,
       // firstInstance,
       // baseVertex,
-      transformFeedback
+      transformFeedback,
+      bindings = {},
+      uniforms = {}
     } = options;
 
     const glDrawMode = getGLDrawMode(topology);
@@ -216,7 +154,7 @@ export class WEBGLRenderPipeline extends RenderPipeline {
     // Note: async textures set as uniforms might still be loading.
     // Now that all uniforms have been updated, check if any texture
     // in the uniforms is not yet initialized, then we don't draw
-    if (!this._areTexturesRenderable()) {
+    if (!this._areTexturesRenderable(bindings)) {
       log.info(2, `RenderPipeline:${this.id}.draw() aborted - textures not yet loaded`)();
       //  Note: false means that the app needs to redraw the pipeline again.
       return false;
@@ -239,8 +177,8 @@ export class WEBGLRenderPipeline extends RenderPipeline {
     }
 
     // We have to apply bindings before every draw call since other draw calls will overwrite
-    this._applyBindings();
-    this._applyUniforms();
+    this._applyBindings(bindings, {disableWarnings: this.props.disableWarnings});
+    this._applyUniforms(uniforms);
 
     const webglRenderPass = renderPass as WEBGLRenderPass;
 
@@ -402,14 +340,11 @@ export class WEBGLRenderPipeline extends RenderPipeline {
    * Update a texture if needed (e.g. from video)
    * Note: This is currently done before every draw call
    */
-  _areTexturesRenderable() {
+  _areTexturesRenderable(bindings: Record<string, Binding>) {
     let texturesRenderable = true;
 
     for (const bindingInfo of this.shaderLayout.bindings) {
-      if (
-        !this.bindings[bindingInfo.name] &&
-        !this.bindings[bindingInfo.name.replace(/Uniforms$/, '')]
-      ) {
+      if (!bindings[bindingInfo.name] && !bindings[bindingInfo.name.replace(/Uniforms$/, '')]) {
         log.warn(`Binding ${bindingInfo.name} not found in ${this.id}`)();
         texturesRenderable = false;
       }
@@ -426,7 +361,7 @@ export class WEBGLRenderPipeline extends RenderPipeline {
   }
 
   /** Apply any bindings (before each draw call) */
-  _applyBindings() {
+  _applyBindings(bindings: Record<string, Binding>, _options?: {disableWarnings?: boolean}) {
     // If we are using async linking, we need to wait until linking completes
     if (this.linkStatus !== 'success') {
       return;
@@ -439,8 +374,7 @@ export class WEBGLRenderPipeline extends RenderPipeline {
     let uniformBufferIndex = 0;
     for (const binding of this.shaderLayout.bindings) {
       // Accept both `xyz` and `xyzUniforms` as valid names for `xyzUniforms` uniform block
-      const value =
-        this.bindings[binding.name] || this.bindings[binding.name.replace(/Uniforms$/, '')];
+      const value = bindings[binding.name] || bindings[binding.name.replace(/Uniforms$/, '')];
       if (!value) {
         throw new Error(`No value for binding ${binding.name} in ${this.id}`);
       }
@@ -519,10 +453,10 @@ export class WEBGLRenderPipeline extends RenderPipeline {
    * Due to program sharing, uniforms need to be reset before every draw call
    * (though caching will avoid redundant WebGL calls)
    */
-  _applyUniforms() {
+  _applyUniforms(uniforms: Record<string, UniformValue>) {
     for (const uniformLayout of this.shaderLayout.uniforms || []) {
       const {name, location, type, textureUnit} = uniformLayout;
-      const value = this.uniforms[name] ?? textureUnit;
+      const value = uniforms[name] ?? textureUnit;
       if (value !== undefined) {
         setUniform(this.device.gl, location, type, value);
       }

modules/webgpu/src/adapter/resources/webgpu-render-pass.ts

@@ -19,6 +19,9 @@ export class WebGPURenderPass extends RenderPass {
   /** Active pipeline */
   pipeline: WebGPURenderPipeline | null = null;
 
+  /** Latest bindings applied to this pass */
+  bindings: Record<string, Binding> = {};
+
   constructor(device: WebGPUDevice, props: RenderPassProps = {}) {
     super(device, props);
     this.device = device;
@@ -78,8 +81,8 @@ export class WebGPURenderPass extends RenderPass {
 
   /** Sets an array of bindings (uniform buffers, samplers, textures, ...) */
   setBindings(bindings: Record<string, Binding>): void {
-    this.pipeline?.setBindings(bindings);
-    const bindGroup = this.pipeline?._getBindGroup();
+    this.bindings = bindings;
+    const bindGroup = this.pipeline?._getBindGroup(bindings);
     if (bindGroup) {
       this.handle.setBindGroup(0, bindGroup);
     }