chore(webgl): use new shadertools function for shader log parsing (#1516)

modules/engine/src/geometries/cone-geometry.ts

@@ -5,7 +5,7 @@ export type ConeGeometryProps = {
   id?: string;
   radius?: number;
   cap?: boolean;
-}
+};
 
 export class ConeGeometry extends TruncatedConeGeometry {
   constructor(props: ConeGeometryProps = {}) {

modules/engine/src/transform/texture-transform.ts

@@ -1,16 +1,10 @@
 import GL from '@luma.gl/constants';
 
-import {
-  cloneTextureFrom,
-  readPixelsToArray,
-  getShaderVersion,
-  Buffer,
-  Texture2D,
-  Framebuffer
-} from '@luma.gl/webgl';
+import {cloneTextureFrom, readPixelsToArray, Buffer, Texture2D, Framebuffer} from '@luma.gl/webgl';
 
 import {
   _transform as transformModule,
+  getShaderInfo,
   getPassthroughFS,
   typeToChannelCount,
   combineInjects
@@ -336,7 +330,7 @@ export default class TextureTransform {
     const fs =
       props._fs ||
       getPassthroughFS({
-        version: getShaderVersion(vs),
+        version: getShaderInfo(vs).version,
         input: this.targetTextureVarying,
         inputType: targetTextureType,
         output: FS_OUTPUT_VARIABLE

modules/engine/src/transform/transform.ts

@@ -1,8 +1,9 @@
 import GL from '@luma.gl/constants';
-import {getPassthroughFS} from '@luma.gl/shadertools';
+import {getShaderInfo, getPassthroughFS} from '@luma.gl/shadertools';
 import {isWebGL2} from '@luma.gl/gltools';
 import type {Framebuffer, Buffer} from '@luma.gl/webgl';
-import {assert, isObjectEmpty, getShaderVersion} from '@luma.gl/webgl';
+import {assert, isObjectEmpty} from '@luma.gl/webgl';
+
 import Model from '../lib/model';
 import BufferTransform from './buffer-transform';
 import TextureTransform from './texture-transform';
@@ -110,7 +111,7 @@ export default class Transform {
     this.model = new Model(
       gl,
       Object.assign({}, props, {
-        fs: props.fs || getPassthroughFS({version: getShaderVersion(props.vs)}),
+        fs: props.fs || getPassthroughFS({version: getShaderInfo(props.vs).version}),
         id: props.id || 'transform-model',
         drawMode: props.drawMode || GL.POINTS,
         vertexCount: props.elementCount

modules/webgl/src/classes/shader.ts

@@ -1,6 +1,8 @@
+// luma.gl, MIT license
 import GL from '@luma.gl/constants';
 import {assertWebGLContext, log} from '@luma.gl/gltools';
-import {parseGLSLCompilerError, getShaderName} from '../glsl-utils';
+import {getShaderInfo, CompilerMessage, formatCompilerLog} from '@luma.gl/shadertools';
+import {parseShaderCompilerLog} from '../webgl-utils/parse-shader-compiler-log';
 import {uid, assert} from '../utils';
 import Resource, {ResourceProps} from './resource';
 
@@ -16,16 +18,21 @@ export type ShaderProps = ImmutableShaderProps & {
 const ERR_SOURCE = 'Shader: GLSL source code must be a JavaScript string';
 
 export class ImmutableShader extends Resource {
-  private readonly _stage: 'vertex' | 'fragment';
+  readonly stage: 'vertex' | 'fragment';
 
   constructor(gl: WebGLRenderingContext, props: ShaderProps) {
     super(gl, {id: getShaderIdFromProps(props)});
-    this._stage = props.stage;
+    this.stage = props.stage;
     this._compile(props.source);
   }
 
-   // PRIVATE METHODS
-   _compile(source) {
+  async compilationInfo(): Promise<readonly CompilerMessage[]> {
+    const log = this.gl.getShaderInfoLog(this.handle);
+    return parseShaderCompilerLog(log);
+  }
+
+  // PRIVATE METHODS
+  _compile(source) {
     if (!source.startsWith('#version ')) {
       source = `#version 100\n${source}`;
     }
@@ -37,22 +44,19 @@ export class ImmutableShader extends Resource {
     // https://gamedev.stackexchange.com/questions/30429/how-to-detect-glsl-warnings
     const compileStatus = this.getParameter(GL.COMPILE_STATUS);
     if (!compileStatus) {
-      const infoLog = this.gl.getShaderInfoLog(this.handle);
-      const {shaderName, errors, warnings} = parseGLSLCompilerError(
-        infoLog,
-        source,
-        this._stage === 'vertex' ? GL.VERTEX_SHADER : GL.FRAGMENT_SHADER,
-        this.id
-      );
-      log.error(`GLSL compilation errors in ${shaderName}\n${errors}`)();
-      log.warn(`GLSL compilation warnings in ${shaderName}\n${warnings}`)();
+      const shaderLog = this.gl.getShaderInfoLog(this.handle);
+      const messages = parseShaderCompilerLog(shaderLog).filter(message => message.type === 'error');
+      const formattedLog = formatCompilerLog(messages, source);
+      const shaderName: string = getShaderInfo(source).name;
+      const shaderDescription = `${this.stage} shader ${shaderName}`;
+      log.error(`GLSL compilation errors in ${shaderName}\n${formattedLog}`)();
       throw new Error(`GLSL compilation errors in ${shaderName}`);
     }
   }
 
   // PRIVATE METHODS
   _createHandle() {
-    return this.gl.createShader(this._stage === 'vertex' ? GL.VERTEX_SHADER : GL.FRAGMENT_SHADER);
+    return this.gl.createShader(this.stage === 'vertex' ? GL.VERTEX_SHADER : GL.FRAGMENT_SHADER);
   }
 
   _deleteHandle(): void {
@@ -64,7 +68,7 @@ export class ImmutableShader extends Resource {
  * Encapsulates the compiled or linked Shaders that execute portions of the WebGL Pipeline
  * For now this is an internal class
  */
- export class Shader extends ImmutableShader {
+export class Shader extends ImmutableShader {
   shaderType: GL.FRAGMENT_SHADER | GL.VERTEX_SHADER;
   source: string;
 
@@ -89,7 +93,7 @@ export class ImmutableShader extends Resource {
     this.shaderType = props.shaderType;
     this.source = props.source;
 
-    const shaderName = getShaderName(props.source, null);
+    const shaderName = getShaderInfo(props.source).name;
     if (shaderName) {
       this.id = uid(shaderName);
     }
@@ -97,7 +101,7 @@ export class ImmutableShader extends Resource {
 
   initialize(props: ShaderProps): this {
     this._compile(props.source);
-    const shaderName = getShaderName(props.source, null);
+    const shaderName = getShaderInfo(props.source).name;
     if (shaderName) {
       this.id = uid(shaderName);
     }
@@ -115,7 +119,7 @@ export class ImmutableShader extends Resource {
   }
 
   getName(): string {
-    return getShaderName(this.source) || 'unnamed-shader';
+    return getShaderInfo(this.source).name || 'unnamed-shader';
   }
 
   getSource(): string {
@@ -179,7 +183,7 @@ function getShaderProps(props: ShaderProps | string, shaderType: GL.VERTEX_SHADE
 
 /** Deduce an id, from shader source, or supplied id, or shader type */
 function getShaderIdFromProps(props: ShaderProps): string {
-  return getShaderName(props.source, null) ||
+  return getShaderInfo(props.source).name ||
     props.id ||
     uid(`unnamed ${props.stage || Shader.getTypeName(props.shaderType)}`);
 }

modules/webgl/src/glsl-utils/format-glsl-error.d.ts

@@ -1,16 +0,0 @@
-export default function formatGLSLCompilerError(errLog: any, src: any, shaderType: any): string;
-
-/**
- * Parse a GLSL compiler error log into a string showing the source code around each error.
- * Based on https://github.com/wwwtyro/gl-format-compiler-error (public domain)
- */
-export function parseGLSLCompilerError(
-  errLog: any,
-  src: any,
-  shaderType: any,
-  shaderName: any
-): {
-  shaderName: string;
-  errors: string;
-  warnings: string;
-};

modules/webgl/src/glsl-utils/format-glsl-error.js

@@ -1,107 +0,0 @@
-// TODO - formatGLSLCompilerError should not depend on this
-import getShaderName from './get-shader-name';
-import getShaderTypeName from './get-shader-type-name';
-
-// Formats GLSL compiler error log into single string
-export default function formatGLSLCompilerError(errLog, src, shaderType) {
-  const {shaderName, errors, warnings} = parseGLSLCompilerError(errLog, src, shaderType);
-  return `GLSL compilation error in ${shaderName}\n\n${errors}\n${warnings}`;
-}
-
-/**
- * Parse a GLSL compiler error log into a string showing the source code around each error.
- * Based on https://github.com/wwwtyro/gl-format-compiler-error (public domain)
- */
-/* eslint-disable no-continue, max-statements */
-export function parseGLSLCompilerError(errLog, src, shaderType, shaderName) {
-  const errorStrings = errLog.split(/\r?\n/);
-  const errors = {};
-  const warnings = {};
-
-  // Patch the shader name
-  const name = shaderName || getShaderName(src) || '(unnamed)';
-  const shaderDescription = `${getShaderTypeName(shaderType)} shader ${name}`;
-
-  // Parse the error - note: browser and driver dependent
-  for (let i = 0; i < errorStrings.length; i++) {
-    const errorString = errorStrings[i];
-    if (errorString.length <= 1) {
-      continue;
-    }
-    const segments = errorString.split(':');
-    const type = segments[0];
-    const line = parseInt(segments[2], 10);
-    if (isNaN(line)) {
-      throw new Error(`GLSL compilation error in ${shaderDescription}: ${errLog}`);
-    }
-    if (type !== 'WARNING') {
-      errors[line] = errorString;
-    } else {
-      warnings[line] = errorString;
-    }
-  }
-
-  // Format the error inline with the code
-  const lines = addLineNumbers(src);
-
-  return {
-    shaderName: shaderDescription,
-    errors: formatErrors(errors, lines),
-    warnings: formatErrors(warnings, lines)
-  };
-}
-
-// helper function, outputs annotated errors or warnings
-function formatErrors(errors, lines) {
-  let message = '';
-  for (let i = 0; i < lines.length; i++) {
-    const line = lines[i];
-    if (!errors[i + 3] && !errors[i + 2] && !errors[i + 1]) {
-      continue;
-    }
-    message += `${line}\n`;
-    if (errors[i + 1]) {
-      const error = errors[i + 1];
-      const segments = error.split(':', 3);
-      const type = segments[0];
-      const column = parseInt(segments[1], 10) || 0;
-      const err = error.substring(segments.join(':').length + 1).trim();
-      message += padLeft(`^^^ ${type}: ${err}\n\n`, column);
-    }
-  }
-  return message;
-}
-
-/**
- * Prepends line numbers to each line of a string.
- * The line numbers will be left-padded with spaces to ensure an
- * aligned layout when rendered using monospace fonts.
- * @param {String} string - multi-line string to add line numbers to
- * @param {Number} start=1 - number of spaces to add
- * @param {String} delim =': ' - injected between line number and original line
- * @return {String[]} strings - array of string, one per line, with line numbers added
- */
-function addLineNumbers(string, start = 1, delim = ': ') {
-  const lines = string.split(/\r?\n/);
-  const maxDigits = String(lines.length + start - 1).length;
-  return lines.map((line, i) => {
-    const lineNumber = String(i + start);
-    const digits = lineNumber.length;
-    const prefix = padLeft(lineNumber, maxDigits - digits);
-    return prefix + delim + line;
-  });
-}
-
-/**
- * Pads a string with a number of spaces (space characters) to the left
- * @param {String} string - string to pad
- * @param {Number} digits - number of spaces to add
- * @return {String} string - The padded string
- */
-function padLeft(string, digits) {
-  let result = '';
-  for (let i = 0; i < digits; ++i) {
-    result += ' ';
-  }
-  return `${result}${string}`;
-}

modules/webgl/src/glsl-utils/get-shader-name.ts

@@ -1,7 +0,0 @@
-// Supports GLSLIFY style naming of shaders
-// #define SHADER_NAME ...
-export default function getShaderName(shader, defaultName = 'unnamed') {
-  const SHADER_NAME_REGEXP = /#define[\s*]SHADER_NAME[\s*]([A-Za-z0-9_-]+)[\s*]/;
-  const match = shader.match(SHADER_NAME_REGEXP);
-  return match ? match[1] : defaultName;
-}

modules/webgl/src/glsl-utils/get-shader-type-name.ts

@@ -1,13 +0,0 @@
-const GL_FRAGMENT_SHADER = 0x8b30;
-const GL_VERTEX_SHADER = 0x8b31;
-
-export default function getShaderTypeName(type) {
-  switch (type) {
-    case GL_FRAGMENT_SHADER:
-      return 'fragment';
-    case GL_VERTEX_SHADER:
-      return 'vertex';
-    default:
-      return 'unknown type';
-  }
-}

modules/webgl/src/glsl-utils/get-shader-version.ts

@@ -1,12 +0,0 @@
-// returns GLSL shader version of given shader string
-export default function getShaderVersion(source) {
-  let version = 100;
-  const words = source.match(/[^\s]+/g);
-  if (words.length >= 2 && words[0] === '#version') {
-    const v = parseInt(words[1], 10);
-    if (Number.isFinite(v)) {
-      version = v;
-    }
-  }
-  return version;
-}

modules/webgl/src/glsl-utils/index.js

@@ -1,8 +0,0 @@
-// PARSE SHADER ERRORS
-export {default as formatGLSLCompilerError, parseGLSLCompilerError} from './format-glsl-error';
-
-// PARSE SHADER SOURCE
-export {default as getShaderName} from './get-shader-name';
-export {default as getShaderVersion} from './get-shader-version';
-
-export {default as getShaderTypeName} from './get-shader-type-name';

modules/webgl/src/index.ts

@@ -55,10 +55,6 @@ export {default as UniformBufferLayout} from './classes/uniform-buffer-layout';
 
 export {setPathPrefix, loadFile, loadImage} from './utils/load-file';
 
-// PARSE SHADER SOURCE
-export {default as getShaderName} from './glsl-utils/get-shader-name';
-export {default as getShaderVersion} from './glsl-utils/get-shader-version';
-
 // UTILS
 export {log} from '@luma.gl/gltools';
 export {default as assert} from './utils/assert';
@@ -69,3 +65,8 @@ export {parseUniformName, getUniformSetter} from './classes/uniforms';
 export {getDebugTableForUniforms} from './debug/debug-uniforms';
 export {getDebugTableForVertexArray} from './debug/debug-vertex-array';
 export {getDebugTableForProgramConfiguration} from './debug/debug-program-configuration';
+
+// REMOVED in v8.6
+// getShaderInfo,
+// getShaderName
+// getShaderVersion

modules/webgl/src/webgl-utils/parse-shader-compiler-log.ts

@@ -0,0 +1,48 @@
+// luma.gl, MIT license
+import type {CompilerMessage} from '@luma.gl/shadertools';
+
+const MESSAGE_TYPES = ['warning', 'error', 'info'];
+
+/**
+ * Parse a WebGL-format GLSL compilation log into an array of WebGPU style message records.
+ * This follows documented WebGL conventions for compilation logs.
+ * Based on https://github.com/wwwtyro/gl-format-compiler-error (public domain)
+ */
+export function parseShaderCompilerLog(errLog: string) : readonly CompilerMessage[] {
+  // Parse the error - note: browser and driver dependent
+  const lines = errLog.split(/\r?\n/);
+
+  const messages: CompilerMessage[] = [];
+
+  for (const line of lines) {
+    if (line.length <= 1) {
+      continue;
+    }
+
+    const segments: string[] = line.split(':');
+    const [messageType, linePosition, lineNumber, ...rest] = segments;
+
+    let lineNum = parseInt(lineNumber, 10);
+    if (isNaN(lineNum)) {
+      lineNum = 0;
+    }
+
+    let linePos = parseInt(linePosition, 10);
+    if (isNaN(linePos)) {
+      linePos = 0;
+    }
+
+    // Ensure supported type
+    const lowerCaseType = messageType.toLowerCase();
+    const type = (MESSAGE_TYPES.includes(lowerCaseType) ? lowerCaseType : 'info') as 'warning' | 'error' | 'info';
+
+    messages.push({
+      message: rest.join(':').trim(),
+      type,
+      lineNum,
+      linePos // TODO
+    })
+  }
+
+  return messages;
+}

modules/webgl/test/glsl-utils/get-shader-name.spec.js

@@ -1,20 +0,0 @@
-import getShaderName from '@luma.gl/webgl/glsl-utils/get-shader-name';
-import test from 'tape-promise/tape';
-
-const SHADER_1 = `\
-uniform float floaty;
-#define SHADER_NAME name-of-shader
-main() {}
-`;
-
-const SHADER_2 = `\
-uniform float floaty;
-#define SHADER name
-main() {}
-`;
-
-test('WebGL#getShaderName', (t) => {
-  t.equal(getShaderName(SHADER_1), 'name-of-shader', 'getShaderName extracted correct name');
-  t.equal(getShaderName(SHADER_2), 'unnamed', 'getShaderName extracted default name');
-  t.end();
-});

modules/webgl/test/glsl-utils/get-shader-version.spec.js

@@ -1,40 +0,0 @@
-import test from 'tape-promise/tape';
-import getShaderVersion from '@luma.gl/webgl/glsl-utils/get-shader-version';
-
-const SHADER1 = 'void main() {}';
-const SHADER2 = '#version 100 void main() {}';
-const SHADER3 = `\
-void main() {
-
-}`;
-const SHADER4 = `\
-#version 300 es
-void main() {
-
-}`;
-const SHADER5 = `#version 300 es
-void main() {
-
-}`;
-const SHADER6 = `\
-#version 100
-void main() {
-
-}`;
-const SHADER7 = '#version 300 es void main() {}';
-const versionTests = {
-  [SHADER1]: 100,
-  [SHADER2]: 100,
-  [SHADER3]: 100,
-  [SHADER4]: 300,
-  [SHADER5]: 300,
-  [SHADER6]: 100,
-  [SHADER7]: 300
-};
-
-test('Shader-utils#getShaderVersion', (t) => {
-  for (const string in versionTests) {
-    t.equal(getShaderVersion(string), versionTests[string], 'Version should match');
-  }
-  t.end();
-});

modules/webgl/test/glsl-utils/index.js

@@ -1,3 +0,0 @@
-import './format-glsl-error.spec';
-import './get-shader-name.spec';
-import './get-shader-version.spec';

modules/webgl/test/index.js

@@ -1,4 +1,4 @@
 import './utils';
-import './glsl-utils';
+import './webgl-utils/parse-shader-compiler-log.spec';
 import './classes';
 import './features';

modules/webgl/test/webgl-utils/parse-shader-compiler-log.spec.js

@@ -0,0 +1,162 @@
+import test from 'tape-promise/tape';
+import {parseShaderCompilerLog} from '@luma.gl/webgl/webgl-utils/parse-shader-compiler-log';
+
+test('parseShaderCompilerLog', (t) => {
+  const {ERROR_LOG, EXPECTED_MESSAGES} = getFixtures();
+  const messages = parseShaderCompilerLog(ERROR_LOG); // SHADER_SOURCE, SHADER_TYPE;
+  t.deepEqual(messages, EXPECTED_MESSAGES, 'parseShaderCompilerLog generated correct messages');
+  t.end();
+});
+
+const ERROR_LOG = `\
+WARNING: 0:264: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:264: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:294: '/' : Divide by zero during constant folding
+WARNING: 0:294: '/' : Divide by zero during constant folding
+WARNING: 0:344: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:344: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:447: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:447: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:470: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:470: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:557: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:557: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:580: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:580: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:669: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:669: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:681: '/' : Zero divided by zero during constant folding generated NaN
+WARNING: 0:681: '/' : Zero divided by zero during constant folding generated NaN
+ERROR: 0:967: 'project_scale' : no matching overloaded function found
+ERROR: 0:994: 'project_scale' : no matching overloaded function found
+`;
+
+const EXPECTED_MESSAGES = [
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 264,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 264,
+    linePos: 0
+  },
+  {
+    message: "'/' : Divide by zero during constant folding",
+    type: 'warning',
+    lineNum: 294,
+    linePos: 0
+  },
+  {
+    message: "'/' : Divide by zero during constant folding",
+    type: 'warning',
+    lineNum: 294,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 344,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 344,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 447,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 447,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 470,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 470,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 557,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 557,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 580,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 580,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 669,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 669,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 681,
+    linePos: 0
+  },
+  {
+    message: "'/' : Zero divided by zero during constant folding generated NaN",
+    type: 'warning',
+    lineNum: 681,
+    linePos: 0
+  },
+  {
+    message: "'project_scale' : no matching overloaded function found",
+    type: 'error',
+    lineNum: 967,
+    linePos: 0
+  },
+  {
+    message: "'project_scale' : no matching overloaded function found",
+    type: 'error',
+    lineNum: 994,
+    linePos: 0
+  }
+];
+
+function getFixtures() {
+  return {
+    ERROR_LOG,
+    EXPECTED_MESSAGES
+  };
+}