Remove attributes array and cleanup Shader

Author: mrxz

src/core/shader.js

@@ -50,29 +50,27 @@ Shader.prototype = {
    * Called during shader initialization and is only run once.
    */
   init: function (data) {
-    this.attributes = this.initVariables(data, 'attribute');
-    this.uniforms = this.initVariables(data, 'uniform');
+    this.uniforms = this.initUniforms();
     this.material = new (this.raw ? THREE.RawShaderMaterial : THREE.ShaderMaterial)({
-      // attributes: this.attributes,
       uniforms: this.uniforms,
       vertexShader: this.vertexShader,
       fragmentShader: this.fragmentShader
     });
     return this.material;
   },
 
-  initVariables: function (data, type) {
+  initUniforms: function () {
     var key;
     var schema = this.schema;
     var variables = {};
     var varType;
 
     for (key in schema) {
-      if (schema[key].is !== type) { continue; }
+      if (schema[key].is !== 'uniform') { continue; }
       varType = propertyToThreeMapping[schema[key].type];
       variables[key] = {
         type: varType,
-        value: undefined  // Let updateVariables handle setting these.
+        value: undefined  // Let update handle setting these.
       };
     }
     return variables;
@@ -85,36 +83,30 @@ Shader.prototype = {
    * @param {object} data - New material data.
    */
   update: function (data) {
-    this.updateVariables(data, 'attribute');
-    this.updateVariables(data, 'uniform');
-  },
-
-  updateVariables: function (data, type) {
     var key;
     var materialKey;
     var schema = this.schema;
-    var variables;
+    var uniforms = this.uniforms;
 
-    variables = type === 'uniform' ? this.uniforms : this.attributes;
     for (key in data) {
-      if (!schema[key] || schema[key].is !== type) { continue; }
+      if (!schema[key] || schema[key].is !== 'uniform') { continue; }
 
       if (schema[key].type === 'map') {
         // If data unchanged, get out early.
-        if (!variables[key] || variables[key].value === data[key]) { continue; }
+        if (!uniforms[key] || uniforms[key].value === data[key]) { continue; }
 
         // Special handling is needed for textures.
         materialKey = '_texture_' + key;
 
         // We can't actually set the variable correctly until we've loaded the texture.
-        this.setMapOnTextureLoad(variables, key, materialKey);
+        this.setMapOnTextureLoad(uniforms, key, materialKey);
 
         // Kick off the texture update now that handler is added.
         utils.material.updateMapMaterialFromData(materialKey, key, this, data);
         continue;
       }
-      variables[key].value = this.parseValue(schema[key].type, data[key]);
-      variables[key].needsUpdate = true;
+      uniforms[key].value = this.parseValue(schema[key].type, data[key]);
+      uniforms[key].needsUpdate = true;
     }
   },
 
@@ -143,11 +135,11 @@ Shader.prototype = {
     }
   },
 
-  setMapOnTextureLoad: function (variables, key, materialKey) {
+  setMapOnTextureLoad: function (uniforms, key, materialKey) {
     var self = this;
     this.el.addEventListener('materialtextureloaded', function () {
-      variables[key].value = self.material[materialKey];
-      variables[key].needsUpdate = true;
+      uniforms[key].value = self.material[materialKey];
+      uniforms[key].needsUpdate = true;
     });
   }
 };
@@ -172,7 +164,7 @@ module.exports.registerShader = function (name, definition) {
   });
 
   if (shaders[name]) {
-    throw new Error('The shader ' + name + ' has been already registered');
+    throw new Error('The shader ' + name + ' has already been registered');
   }
   NewShader = function () { Shader.call(this); };
   NewShader.prototype = Object.create(Shader.prototype, proto);

tests/core/shader.test.js

@@ -33,7 +33,6 @@ suite('shader', function () {
       assert.ok(shader.prototype.vertexShader);
       assert.ok(shader.prototype.fragmentShader);
       assert.notOk(shader.prototype.uniforms);
-      assert.notOk(shader.prototype.attributes);
     });
 
     test('shader instance receives methods and properties', function () {
@@ -49,7 +48,6 @@ suite('shader', function () {
       assert.equal(instance.vertexShader, shader.prototype.vertexShader);
       assert.equal(instance.fragmentShader, shader.prototype.fragmentShader);
       assert.equal(Object.keys(instance.uniforms).length, 0);
-      assert.equal(Object.keys(instance.attributes).length, 0);
       assert.ok(instance.material);
     });
 
@@ -94,8 +92,7 @@ suite('shader data binding', function () {
         src: {type: 'map', is: 'uniform'},
         otherMap: {type: 'map', is: 'uniform'},
         vec2Uniform: {type: 'vec2', default: {x: 1, y: 2}, is: 'uniform'},
-        vec2Attribute: {type: 'vec2', default: {x: 3, y: 4}, is: 'attribute'},
-        vec2Neither: {type: 'vec2', default: {x: 5, y: 6}}
+        vec2NotUniform: {type: 'vec2', default: {x: 5, y: 6}}
       }
     });
 
@@ -126,8 +123,6 @@ suite('shader data binding', function () {
     assert.ok(updateSpy.calledOnce);
     // The value won't be assigned until the texture loads.
     assert.ok(instance.uniforms['src']);
-    assert.notOk(instance.attributes && (instance.attributes['map'] ||
-                                         instance.attributes['src']));
   });
 
   test('src loads inline video', function (done) {
@@ -152,8 +147,6 @@ suite('shader data binding', function () {
     assert.ok(updateSpy.calledOnce);
     // The value won't be assigned until the texture loads.
     assert.ok(instance.uniforms['src']);
-    assert.notOk(instance.attributes && (instance.attributes['map'] ||
-                                         instance.attributes['src']));
   });
 
   test('otherMap loads inline video', function (done) {
@@ -178,8 +171,6 @@ suite('shader data binding', function () {
     assert.ok(initSpy.calledOnce);
     assert.ok(updateSpy.calledOnce);
     assert.ok(instance.uniforms['otherMap']);
-    // The value won't be assigned until the texture loads.
-    assert.notOk(instance.attributes && instance.attributes['otherMap']);
   });
 
   test('vec2Uniform parameter is uniform', function () {
@@ -194,25 +185,9 @@ suite('shader data binding', function () {
     assert.ok(instance.uniforms['vec2Uniform']);
     assert.equal(instance.uniforms['vec2Uniform'].value.x, 1);
     assert.equal(instance.uniforms['vec2Uniform'].value.y, 2);
-    assert.notOk(instance.attributes['vec2Uniform']);
-  });
-
-  test('vec2Attribute parameter is attribute', function () {
-    assert.notOk(initSpy.called);
-    assert.notOk(updateSpy.called);
-    el.setAttribute('material', 'shader: testShader');
-    const material = el.components.material;
-    const instance = material.shader;
-    assert.ok(instance);
-    assert.ok(initSpy.calledOnce);
-    assert.ok(updateSpy.calledOnce);
-    assert.ok(instance.attributes['vec2Attribute']);
-    assert.equal(instance.attributes['vec2Attribute'].value.x, 3);
-    assert.equal(instance.attributes['vec2Attribute'].value.y, 4);
-    assert.notOk(instance.uniforms['vec2Attribute']);
   });
 
-  test('vec2Neither parameter is neither uniform nor attribute', function () {
+  test('vec2NotUniform parameter is not a uniform', function () {
     assert.notOk(initSpy.called);
     assert.notOk(updateSpy.called);
     el.setAttribute('material', 'shader: testShader');
@@ -221,7 +196,6 @@ suite('shader data binding', function () {
     assert.ok(instance);
     assert.ok(initSpy.calledOnce);
     assert.ok(updateSpy.calledOnce);
-    assert.notOk(instance.attributes['vec2Neither']);
-    assert.notOk(instance.uniforms['vec2Neither']);
+    assert.notOk(instance.uniforms['vec2NotUniform']);
   });
 });