Merge pull request #11886 from tschaub/throw-on-gl-error

Throw an error if shaders fail to compile or program fails to link

src/ol/renderer/webgl/Layer.js

@@ -80,15 +80,6 @@ class WebGLLayerRenderer extends LayerRenderer {
     super.disposeInternal();
   }
 
-  /**
-   * Will return the last shader compilation errors. If no error happened, will return null;
-   * @return {string|null} Errors, or null if last compilation was successful
-   * @api
-   */
-  getShaderCompileErrors() {
-    return this.helper.getShaderCompileErrors();
-  }
-
   /**
    * @param {import("../../render/EventType.js").default} type Event type.
    * @param {import("../../PluggableMap.js").FrameState} frameState Frame state.

src/ol/webgl/Helper.js

@@ -709,8 +709,7 @@ class WebGLHelper extends Disposable {
   }
 
   /**
-   * Create a program for a vertex and fragment shader. The shaders compilation may have failed:
-   * use `WebGLHelper.getShaderCompileErrors()`to have details if any.
+   * Create a program for a vertex and fragment shader.  Throws if shader compilation fails.
    * @param {string} fragmentShaderSource Fragment shader source.
    * @param {string} vertexShaderSource Vertex shader source.
    * @return {WebGLProgram} Program
@@ -723,39 +722,41 @@ class WebGLHelper extends Disposable {
       fragmentShaderSource,
       gl.FRAGMENT_SHADER
     );
+
     const vertexShader = this.compileShader(
       vertexShaderSource,
       gl.VERTEX_SHADER
     );
-    this.shaderCompileErrors_ = null;
-
-    if (gl.getShaderInfoLog(fragmentShader)) {
-      this.shaderCompileErrors_ = `Fragment shader compilation failed:\n${gl.getShaderInfoLog(
-        fragmentShader
-      )}`;
-    }
-    if (gl.getShaderInfoLog(vertexShader)) {
-      this.shaderCompileErrors_ =
-        (this.shaderCompileErrors_ || '') +
-        `Vertex shader compilation failed:\n${gl.getShaderInfoLog(
-          vertexShader
-        )}`;
-    }
 
     const program = gl.createProgram();
     gl.attachShader(program, fragmentShader);
     gl.attachShader(program, vertexShader);
     gl.linkProgram(program);
-    return program;
-  }
 
-  /**
-   * Will return the last shader compilation errors. If no error happened, will return null;
-   * @return {string|null} Errors description, or null if last compilation was successful
-   * @api
-   */
-  getShaderCompileErrors() {
-    return this.shaderCompileErrors_;
+    if (!gl.getShaderParameter(fragmentShader, gl.COMPILE_STATUS)) {
+      const message = `Fragment shader compliation failed: ${gl.getShaderInfoLog(
+        fragmentShader
+      )}`;
+      throw new Error(message);
+    }
+    gl.deleteShader(fragmentShader);
+
+    if (!gl.getShaderParameter(vertexShader, gl.COMPILE_STATUS)) {
+      const message = `Vertex shader compilation failed: ${gl.getShaderInfoLog(
+        vertexShader
+      )}`;
+      throw new Error(message);
+    }
+    gl.deleteShader(vertexShader);
+
+    if (!gl.getProgramParameter(program, gl.LINK_STATUS)) {
+      const message = `GL program linking failed: ${gl.getShaderInfoLog(
+        vertexShader
+      )}`;
+      throw new Error(message);
+    }
+
+    return program;
   }
 
   /**

test/spec/ol/webgl/helper.test.js

@@ -46,7 +46,14 @@ const FRAGMENT_SHADER = `
     gl_FragColor = vec4(1.0, 1.0, 1.0, 1.0);
   }`;
 
-describe('ol.webgl.WebGLHelper', function () {
+const INVALID_FRAGMENT_SHADER = `
+  precision mediump float;
+
+  void main(void) {
+    gl_FragColor = vec4(oops, 1.0, 1.0, 1.0);
+  }`;
+
+describe('ol/webgl/WebGLHelper', function () {
   describe('constructor', function () {
     describe('without an argument', function () {
       let h;
@@ -177,25 +184,22 @@ describe('ol.webgl.WebGLHelper', function () {
     });
 
     describe('invalid shader compiling', function () {
-      let h;
-      let p;
-      beforeEach(function () {
-        h = new WebGLHelper();
-
-        p = h.getProgram(FRAGMENT_SHADER, INVALID_VERTEX_SHADER);
-        h.useProgram(p);
+      it('throws for an invalid vertex shader', function () {
+        const helper = new WebGLHelper();
+        expect(() =>
+          helper.getProgram(FRAGMENT_SHADER, INVALID_VERTEX_SHADER)
+        ).to.throwException(
+          /Vertex shader compilation failed: ERROR: 0:10: 'bla' : syntax error/
+        );
       });
 
-      it('has saved the program', function () {
-        expect(h.currentProgram_).to.eql(p);
-      });
-
-      it('has shader compilation errors', function () {
-        expect(h.shaderCompileErrors_).to.not.eql(null);
-      });
-
-      it('cannot find the uniform location', function () {
-        expect(h.getUniformLocation('u_test')).to.eql(null);
+      it('throws for an invalid fragment shader', function () {
+        const helper = new WebGLHelper();
+        expect(() =>
+          helper.getProgram(INVALID_FRAGMENT_SHADER, VERTEX_SHADER)
+        ).to.throwException(
+          /Fragment shader compliation failed: ERROR: 0:5: 'oops' : undeclared identifier/
+        );
       });
     });
 
@@ -354,7 +358,7 @@ describe('ol.webgl.WebGLHelper', function () {
         uniform float u_test;
 
         void main(void) {
-          gl_Position = vec4(u_test, a_test, 0.0, 1.0);
+          gl_Position = vec4(u_test, attr3, 0.0, 1.0);
         }`
         )
       );