WebGL shader program management

Question

I'm writing a tiny WebGL/JS framework to improve my knowledge of JavaScript and WebGL. Following is a class that wraps a Shader Program and provides methods for setting the shader parameters (AKA uniform variables):

ShaderProgram class:

"use strict";

/*
===========================================================
ShaderProgram class:
===========================================================
*/
function ShaderProgram(progName) {
    this.detail = {
        gl              : Renderer.getWebGLContext(),
        webGLProgramObj : null, // WebGLProgram.
        vertexAttribs   : null, // Map of { name: string, index: int } for each vertex attribute.
        uniformVars     : null, // Map of uniform var locations, indexed by var name.
        name            : progName || "unnamed" // Optional name/id string used for debugging.
    };
};

/*
 * ---- Methods of ShaderProgram: ----
 */

ShaderProgram.createShaderFromHtmlElement = function (shaderElementId) { // -> WebGLShader ['static' method]
    console.assert(shaderElementId, "Provide a valid HTML element id!");

    // Get shader text source element:
    var shaderScript = document.getElementById(shaderElementId);
    if (!shaderScript) {
        console.error("Unable to find shader element for id '" + shaderElementId + "'!");
        return null;
    }

    // Grab the GLSL source code:
    var shaderSourceText = "";
    var domNode = shaderScript.firstChild;
    while (domNode) {
        // NOTE: nodeType == 3 indicates a TEXT mode, which is what we want.
        // See http://www.w3schools.com/jsref/prop_node_nodetype.asp
        // for a list of all node types.
        if (domNode.nodeType == 3) {
            shaderSourceText += domNode.textContent;
        }
        domNode = domNode.nextSibling;
    }

    var gl = Renderer.getWebGLContext();

    // Created WebGL shader object:
    //
    // (Note that the 'x-shader' type is a user defined value.
    // It could be any arbitrary string that does not conflict
    // with a string expected by the common browsers).
    //
    var shader;
    if (shaderScript.type === "x-shader/x-vertex") {
        shader = gl.createShader(gl.VERTEX_SHADER);
    } else if (shaderScript.type === "x-shader/x-fragment") {
        shader = gl.createShader(gl.FRAGMENT_SHADER);
    } else {
        console.error("Invalid shader type! shaderElementId = '" + shaderElementId + "'");
        return null;
    }

    if (!shader) {
        console.error("Failed to allocate new WebGL shader object! Possibly out of memory...");
        return null;
    }

    gl.shaderSource(shader, shaderSourceText);
    gl.compileShader(shader);

    var infolog = gl.getShaderInfoLog(shader);

    if (!gl.getShaderParameter(shader, gl.COMPILE_STATUS)) {
        console.error("Failed to compile shader '" + shaderElementId +
            "'\n" + "Compiler info log:\n" + infolog);
        return null;
    }

    // If we got an info log but COMPILE_STATUS was OK, there might be
    // some warnings and other issues with the code that we should still print.
    if (infolog) {
        console.warn("Shader compiler info log for '" + shaderElementId + "':\n" + infolog);
        // Allow it to continue.
    }

    // If we get here, the shader should be in a valid state for rendering.
    Renderer.checkErrors();
    return shader;
};

ShaderProgram.disposeShader = function (shaderObj) { // -> void ['static' method]
    if (shaderObj) {
        var gl = Renderer.getWebGLContext();
        gl.deleteShader(shaderObj);
        shaderObj = null;
    }
};

ShaderProgram.prototype.initWithShaders = function (webGLVertexShaderObj, webGLFragmentShaderObj,
                                                    vertexAttributes, uniformVarNames) { // -> bool

    if (!webGLVertexShaderObj) {
        console.error("Null vertex shader for ShaderProgram.initWithShaders('" + this.detail.name + "')!");
        return false;
    }

    if (!webGLFragmentShaderObj) {
        console.error("Null fragment shader for ShaderProgram.initWithShaders('" + this.detail.name + "')!");
        return false;
    }

    if (this.detail.webGLProgramObj) {
        console.warn("Dispose the current program before initializing it again!");
        return true;
    }

    this.detail.webGLProgramObj = this.detail.gl.createProgram();
    if (!this.detail.webGLProgramObj) {
        console.error("Failed to allocate new WebGL program object! Possibly out of memory...");
        return false;
    }

    this.detail.gl.attachShader(this.detail.webGLProgramObj, webGLVertexShaderObj);
    this.detail.gl.attachShader(this.detail.webGLProgramObj, webGLFragmentShaderObj);

    // Bind the vertex attributes, if any.
    // This must happen BEFORE linking the program.
    if (vertexAttributes) {
        this.detail.vertexAttribs = {};
        for (var i = 0; i < vertexAttributes.length; ++i) {
            this.detail.gl.bindAttribLocation(this.detail.webGLProgramObj,
                    vertexAttributes[i].index, vertexAttributes[i].name);

            // Save the attribute indexes inside the this object for use with `gl.vertexAttribPointer()`.
            this.detail.vertexAttribs[vertexAttributes[i].name] = vertexAttributes[i].index;
        }
    }

    // Link the program into a GPU "executable" and check link status:
    this.detail.gl.linkProgram(this.detail.webGLProgramObj);
    if (!this.detail.gl.getProgramParameter(this.detail.webGLProgramObj, this.detail.gl.LINK_STATUS)) {
        console.error("Could not link GLSL shader program for '" + this.detail.name + "'!");
        return false;
    }

    // Query uniform variable locations and store their handles inside this object:
    if (uniformVarNames) {
        this.detail.gl.useProgram(this.detail.webGLProgramObj);

        this.detail.uniformVars = {};
        for (var i = 0; i < uniformVarNames.length; ++i) {
            var varHandle = this.detail.gl.getUniformLocation(this.detail.webGLProgramObj, uniformVarNames[i]);
            if (!varHandle) {
                console.warn("Unable to get uniform var '" + uniformVarNames[i] +
                    "' location for ShaderProgram '" + this.detail.name + "'!");
                continue;
            }
            this.detail.uniformVars[uniformVarNames[i]] = varHandle;
        }

        this.detail.gl.useProgram(null);
    }

    Renderer.checkErrors();
    console.log("New ShaderProgram '" + this.detail.name + "' initialized.");
    return true;
};

ShaderProgram.prototype.initFromHtmlElements = function (vertexShaderElemId, fragmentShaderElemId,
                                                         vertexAttributes, uniformVarNames) { // -> bool

    if (this.detail.webGLProgramObj) {
        console.warn("Dispose the current program before initializing it again!");
        return true;
    }

    var vertexShaderObj = ShaderProgram.createShaderFromHtmlElement(vertexShaderElemId);
    if (!vertexShaderObj) {
        return false;
    }

    var fragmentShaderObj = ShaderProgram.createShaderFromHtmlElement(fragmentShaderElemId);
    if (!fragmentShaderObj) {
        return false;
    }

    var result = this.initWithShaders(
            vertexShaderObj, fragmentShaderObj,
            vertexAttributes, uniformVarNames);

    // Once the program is created, shader objects may be disposed.
    ShaderProgram.disposeShader(vertexShaderObj);
    ShaderProgram.disposeShader(fragmentShaderObj);

    return result;
};

ShaderProgram.prototype.dispose = function () { // -> void
    if (this.detail.webGLProgramObj) {
        // Unbind first if needed.
        if (this.detail.gl.getParameter(this.detail.gl.CURRENT_PROGRAM) === this.detail.webGLProgramObj) {
            this.detail.gl.useProgram(null);
        }

        // Delete WebGL handle and reset this object:
        this.detail.gl.deleteProgram(this.detail.webGLProgramObj);
        this.detail.webGLProgramObj = null;
        this.detail.vertexAttribs   = null;
        this.detail.uniformVars     = null;
        // Leave `this.detail.name` intact.
    }
};

ShaderProgram.prototype.bind = function () { // -> void
    if (this.detail.webGLProgramObj) {
        this.detail.gl.useProgram(this.detail.webGLProgramObj);
    }
};

ShaderProgram.bindNull = function () { // -> void ['static' method]
    var gl = Renderer.getWebGLContext();
    gl.useProgram(null);
};

ShaderProgram.prototype.isBound = function () { // -> bool
    if (!this.detail.webGLProgramObj) {
        return false;
    }
    return this.detail.gl.getParameter(this.detail.gl.CURRENT_PROGRAM) === this.detail.webGLProgramObj;
};

ShaderProgram.prototype.getName = function () { // -> String
    return this.detail.name;
};

ShaderProgram.prototype.setName = function (newName) { // -> void
    this.detail.name = newName;
};

ShaderProgram.prototype.getVertexAttribIndex = function (name) { // -> GLenum
    console.assert(name, "Invalid vertex attribute name!");

    if (!this.detail.vertexAttribs) {
        console.warn("No vertex attributes available for shader program '" + this.detail.name + "'!");
        return -1;
    }
    if (!this.detail.vertexAttribs.hasOwnProperty(name)) {
        console.warn("Undefined vertex attribute '" + name + "' for shader program '" + this.detail.name + "'!");
        return -1;
    }

    return this.detail.vertexAttribs[name];
};

ShaderProgram.prototype.validateSetUniformParams = function (varName, varValue) { // -> bool
    // This internal helper is used to validate the inputs of the `setUniformX()` methods.

    if (this.detail.webGLProgramObj === null || this.detail.uniformVars === null) {
        console.warn("Shader program '" + this.detail.name + "' is invalid or has no uniform vars!");
        return false;
    }

    console.assert(varName  !== undefined && varName  !== null, "Invalid uniform var name/id!");
    console.assert(varValue !== undefined && varValue !== null, "Invalid uniform var value!");
    console.assert(this.detail.gl.getParameter(this.detail.gl.CURRENT_PROGRAM) === this.detail.webGLProgramObj,
            "Bind the shader program '" + this.detail.name + "' first!");

    if (!this.detail.uniformVars.hasOwnProperty(varName)) {
        console.warn("Uniform var '" + varName + "' not found on program '" + this.detail.name + "'!");
        return false;
    }

    return true;
};

ShaderProgram.prototype.setUniform1i = function (varName, ival) { // -> bool
    if (!this.validateSetUniformParams(varName, ival)) {
        return false;
    }
    this.detail.gl.uniform1i(this.detail.uniformVars[varName], ival);
    return true;
};

ShaderProgram.prototype.setUniform1f = function (varName, fval) { // -> bool
    if (!this.validateSetUniformParams(varName, fval)) {
        return false;
    }
    this.detail.gl.uniform1f(this.detail.uniformVars[varName], fval);
    return true;
};

ShaderProgram.prototype.setUniformVec2 = function (varName, v2) { // -> bool
    if (!this.validateSetUniformParams(varName, v2)) {
        return false;
    }
    this.detail.gl.uniform2fv(this.detail.uniformVars[varName], v2);
    return true;
};

ShaderProgram.prototype.setUniformVec3 = function (varName, v3) { // -> bool
    if (!this.validateSetUniformParams(varName, v3)) {
        return false;
    }
    this.detail.gl.uniform3fv(this.detail.uniformVars[varName], v3);
    return true;
};

ShaderProgram.prototype.setUniformVec4 = function (varName, v4) { // -> bool
    if (!this.validateSetUniformParams(varName, v4)) {
        return false;
    }
    this.detail.gl.uniform4fv(this.detail.uniformVars[varName], v4);
    return true;
};

ShaderProgram.prototype.setUniformMatrix4x4 = function (varName, m4x4) { // -> bool
    if (!this.validateSetUniformParams(varName, m4x4)) {
        return false;
    }
    this.detail.gl.uniformMatrix4fv(this.detail.uniformVars[varName], /* transpose = */ false, m4x4);
    return true;
};

Usage example:

// Assume a 'debug_line' shader is defined inside some `<script>` element.
var shdr = new ShaderProgram("debug_line");

var vertexAttribs = [
    { name: "vertex_position", index: 0 },
    { name: "vertex_color",    index: 1 }
];
var shaderUniforms = [ "mvp_matrix" ];

shdr.initFromHtmlElements(
    "vs_debug_line", "fs_debug_line", 
    vertexAttribs, shaderUniforms);

shdr.setUniformMatrix4x4("mvp_matrix", Matrix4x4.identity());

...

Please review any and all aspects of the code and let me know if I'm following good and up-to-date JavaScript programming practices. I'm fairly new to JS, so tips on how to improve my coding style will be appreciated.

In particular, I had this idea of placing the "private" data of ShaderProgram inside a sub-object literal conveniently named detail, to emphasize that the data is not meant for direct external access. Is this okay? Are there better ways to define private class data in JavaScript?

Answer 1

Skimming through the code, a few items jump out:

1. Why assume shader sourcecode is stored in a HTML container? Split the creation and reading logic into two objects or at least methods.
2. Consider using gl.getAttribLocation and gl.getUniformLocation instead of prebinding locations (if anything, your setupcode is not longer required).

3. Consider using type deduction when uploading uniforms, e.g., (in slightly pseudo code)

   Shader.prototype.set(uniformName, data) {
       var handle = this.lookup(uniformName);
   
       if(data instanceof Vec2) {
           // use gl.uniform2fv(uniformName...)
       } else if(data instanceof Vec3) {
           // use gl.uniform3fv(uniformName...)
       }
   }
   

   This will ease development significantly.

4. Names like bindNull and getName are vague. Are we binding a null texture? Is name synonymous for a shader program?
5. Perhaps create a method that checks gl.VALIDATE_STATUS

6. If you go for some fancy documentation as Ethan suggests, consider using three slashes instead of multi-line comments, e.g.,

   /// A simple AND gate.
   /// @param {bool} port1 - The first input port.
   /// @param {bool} port2 - The second input port.
   /// @returns {bool}
   ///
   function ANDGate(port1, port2) {
       return port1 && port2;
   }
   

   This way you can still use multi-line comments to disable whole functions. The syntax is well supported.

7. Future work: Consider implementing your own #include directive for GLSL and a way to define GLSL macro values from your code.
8. var gl = Renderer.getWebGLContext(); seems highly hardcoded. My own last project had about 4 renderers. Perhaps inject this dependency via a constructor.
9. Your code is too friendly, e.g., your ShaderProgram.prototype.bind function silently progresses if the shader isn't even created. Clearly, calling bind on a shader that does not exist is a throwable error. Read about defensive programming.

10. There are some design patterns to create truly private variables, e.g.,

    function Person(name) {
        var _name = name;
    
        this.getName = function() {
            return _name;
        };
    
        this.setName = function(name) {
            _name = name;
        }
    }
    
    var p = new Person("Gerard");
    console.log(p.getName()); // As expected
    console.log(p._name);      // Yields undefined
    

Answer 2

Since I'm not a JS guru or anything, I'm going to give more of a style review, rather than a review of the code, but if there is anything I see, I'll try to point it out.

• First off, you have some odd comments, which I assume are to describe the return values of certain functions, e.g, // -> bool. While there are many different commenting styles, the one I usually use the most is JSDoc. Here's an example usage of JSDoc.

---

/**
 * A simple AND gate.
 * @param {bool} port1 - The first input port.
 * @param {bool} port2 - The second input port.
 * @returns {bool}
 */
function ANDGate(port1, port2) {
    return port1 && port2;
}

---

• This comment may or may not have a strange structure, but I'll break it down for you. At the top of the comment, is a small description of what the function or constructor does, then, if there are any parameters, use @param {type} name - Description. If you're documenting a constructor, then just add @constructor underneath it's primary description. Here's a template that you can reference if you forget. Do take note though, use the style of comment with the two leading asterisks, /** ... */, not /* ... /*.

---

/**
 * Description of method here
 * ...
 * @param {type} name - Description here ...
 * ...
 * @returns {type}
 */
function myMethod(params) {
    ...
}


/**
 * Description of constructor here
 * ...
 * @constructor
 * @param {type} name - Description here ...
 * ...
 */
function MyConstructor(params) {
    ...
}

---

• Another thing, while it's not much of an issue in this code, is just to give variables slightly better names, as @skiwi mentioned in chat.. It's not necessary to drop vowels from names as short as shader.

• Also, for the sake of some users, I'd also use the alert() function, in addition to using console.something to warn of an error or such. This is simply for users who might not have access to the console, don't know how to get it, or don't know what it does.

• Finally, I'd just add some more empty lines in there, just to improve the readability. While this isn't necessarily part of an "official style guide" or anything, I just find that it's generally easier to read. I usually just have two empty lines in between methods and constructors, and then inside of those methods, I separate different sections by one line.

Anyways, I hope this helped, even if I couldn't give a full review on the WebGL stuff itself! :)

Answer 3

Disclaimer: I'm no JS guru, just a casual JS user.

Please review any and all aspects of the code and let me know if I'm following good and up-to-date JavaScript programming practices. I'm fairly new to JS, so tips on how to improve my coding style will be appreciated.

Your implementation seems good. It's easy to read, the comments are helpful, and I usually hate comments. It seems a bit long, and at times tedious, but I see why it is this way, I think it's necessary, and I couldn't write it better.

As for style, one teeny-tiny thing I could add over @Ethan's review is the unnecessary extra semicolon at the end of the function ShaderProgram declaration.

In particular, I had this idea of placing the "private" data of ShaderProgram inside a sub-object literal conveniently named detail, to emphasize that the data is not meant for direct external access. Is this okay? Are there better ways to define private class data in JavaScript?

I think that's well done. There's no access control in JavaScript as in other languages like C++ and Java. What you did seems good encapsulation and information hiding.

A related concept is programming "into" a language, as explained in Code Complete by Steve McConnell. I should also point out a counter-point to that, explained on Jon Skeet's blog. But I don't think that applies in this case, in this particular example, programming into the language is definitely a good thing.