Skip to content

[Code Review] Possible improvement of webgl-util.js, why not use the data type from shader when setting attributes #444

Open
@GuichiZhao

Description

@GuichiZhao

When I read the chapter "WebGL - Less Code, More Fun", I read the code in webgl-util.js to figure out how it is possible to "write less code" by simply specifying the uniform/attribute something like

{
      u_color: [0.2, 1, 0.2, 1],
      u_textureSize: [width, height],
      u_reverseLightDirection: [1, 1, -1],
 }

The key takeaway from the source code is that the type of uniform can be parsed out from the shader code by gl.getActiveUniform so the type of uniform can be set using gl.uniformMatrix4fv,gl.uniformMatrix2fv ... correctly. I highly appreciate the idea because it exhibits the "single source of truth", the only source of data type is the shader code!

However, when writing less code the set attributes, we should write something like

      a_position: {
        buffer,
        numComponents: 3,
        type: gl.FLOAT,
        stride,
      },
      a_color: {
        buffer,
        numComponents: 3,
        type: gl.UNSIGNED_BYTE,
        normalize: true,
        stride,
        offset: 3 * 4,
      },

We should specify the data type explicitly! The source code of webgl-util.js does not make good use of the data type from shader at all, rather, it did a lot of guesswork on the data type like:

        attribs[attribName] = {
          buffer:        createBufferFromTypedArray(gl, array),
          numComponents: origArray.numComponents || array.numComponents || guessNumComponentsFromName(bufferName),
          type:          getGLTypeForTypedArray(gl, array),
          normalize:     getNormalizationForTypedArray(array),
        };
  function getGLTypeForTypedArray(gl, typedArray) {
    if (typedArray instanceof Int8Array)    { return gl.BYTE; }            // eslint-disable-line
    if (typedArray instanceof Uint8Array)   { return gl.UNSIGNED_BYTE; }   // eslint-disable-line
    if (typedArray instanceof Int16Array)   { return gl.SHORT; }           // eslint-disable-line
    if (typedArray instanceof Uint16Array)  { return gl.UNSIGNED_SHORT; }  // eslint-disable-line
    if (typedArray instanceof Int32Array)   { return gl.INT; }             // eslint-disable-line
    if (typedArray instanceof Uint32Array)  { return gl.UNSIGNED_INT; }    // eslint-disable-line
    if (typedArray instanceof Float32Array) { return gl.FLOAT; }           // eslint-disable-line
    throw 'unsupported typed array type';
  }

The type of array relies heavily on guesswork as well

My question is why not get the attributes data type directly from the shader code like what we did for uniforms. Is there any good reason I fail to understand? If not, I am glad to send a PR to change the code to what I see appropriate

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions