Skip to content

Commit d94bf47

Browse files
mrxzmrxz
authored
Remove attributes array and cleanup Shader (#5310)
* Remove attributes array and cleanup Shader * Update msdf and sdf shader to calll initUniforms instead fo initVariables --------- Co-authored-by: Noeri Huisman <mrxz@users.noreply.github.com>
1 parent af4396d commit d94bf47

File tree

4 files changed

+21
-55
lines changed

4 files changed

+21
-55
lines changed

‎src/core/shader.js‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,8 @@ Shader.prototype = {
5050
* Called during shader initialization and is only run once.
5151
*/
5252
init: function (data) {
53-
this.attributes = this.initVariables(data, 'attribute');
54-
this.uniforms = this.initVariables(data, 'uniform');
53+
this.uniforms = this.initUniforms();
5554
this.material = new (this.raw ? THREE.RawShaderMaterial : THREE.ShaderMaterial)({
56-
// attributes: this.attributes,
5755
uniforms: this.uniforms,
5856
glslVersion: this.raw || this.glsl3 ? THREE.GLSL3 : null,
5957
vertexShader: this.vertexShader,
@@ -62,18 +60,18 @@ Shader.prototype = {
6260
return this.material;
6361
},
6462

65-
initVariables: function (data, type) {
63+
initUniforms: function () {
6664
var key;
6765
var schema = this.schema;
6866
var variables = {};
6967
var varType;
7068

7169
for (key in schema) {
72-
if (schema[key].is !== type) { continue; }
70+
if (schema[key].is !== 'uniform') { continue; }
7371
varType = propertyToThreeMapping[schema[key].type];
7472
variables[key] = {
7573
type: varType,
76-
value: undefined // Let updateVariables handle setting these.
74+
value: undefined // Let update handle setting these.
7775
};
7876
}
7977
return variables;
@@ -86,36 +84,30 @@ Shader.prototype = {
8684
* @param {object} data - New material data.
8785
*/
8886
update: function (data) {
89-
this.updateVariables(data, 'attribute');
90-
this.updateVariables(data, 'uniform');
91-
},
92-
93-
updateVariables: function (data, type) {
9487
var key;
9588
var materialKey;
9689
var schema = this.schema;
97-
var variables;
90+
var uniforms = this.uniforms;
9891

99-
variables = type === 'uniform' ? this.uniforms : this.attributes;
10092
for (key in data) {
101-
if (!schema[key] || schema[key].is !== type) { continue; }
93+
if (!schema[key] || schema[key].is !== 'uniform') { continue; }
10294

10395
if (schema[key].type === 'map') {
10496
// If data unchanged, get out early.
105-
if (!variables[key] || variables[key].value === data[key]) { continue; }
97+
if (!uniforms[key] || uniforms[key].value === data[key]) { continue; }
10698

10799
// Special handling is needed for textures.
108100
materialKey = '_texture_' + key;
109101

110102
// We can't actually set the variable correctly until we've loaded the texture.
111-
this.setMapOnTextureLoad(variables, key, materialKey);
103+
this.setMapOnTextureLoad(uniforms, key, materialKey);
112104

113105
// Kick off the texture update now that handler is added.
114106
utils.material.updateMapMaterialFromData(materialKey, key, this, data);
115107
continue;
116108
}
117-
variables[key].value = this.parseValue(schema[key].type, data[key]);
118-
variables[key].needsUpdate = true;
109+
uniforms[key].value = this.parseValue(schema[key].type, data[key]);
110+
uniforms[key].needsUpdate = true;
119111
}
120112
},
121113

@@ -141,11 +133,11 @@ Shader.prototype = {
141133
}
142134
},
143135

144-
setMapOnTextureLoad: function (variables, key, materialKey) {
136+
setMapOnTextureLoad: function (uniforms, key, materialKey) {
145137
var self = this;
146138
this.el.addEventListener('materialtextureloaded', function () {
147-
variables[key].value = self.material[materialKey];
148-
variables[key].needsUpdate = true;
139+
uniforms[key].value = self.material[materialKey];
140+
uniforms[key].needsUpdate = true;
149141
});
150142
}
151143
};
@@ -170,7 +162,7 @@ module.exports.registerShader = function (name, definition) {
170162
});
171163

172164
if (shaders[name]) {
173-
throw new Error('The shader ' + name + ' has been already registered');
165+
throw new Error('The shader ' + name + ' has already been registered');
174166
}
175167
NewShader = function () { Shader.call(this); };
176168
NewShader.prototype = Object.create(Shader.prototype, proto);

‎src/shaders/msdf.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ module.exports.Shader = registerShader('msdf', {
8383

8484
fragmentShader: FRAGMENT_SHADER,
8585

86-
init: function (data) {
86+
init: function () {
8787
this.uniforms = THREE.UniformsUtils.merge([
8888
THREE.UniformsLib.fog,
89-
this.initVariables(data, 'uniform')
89+
this.initUniforms()
9090
]);
9191
this.material = new THREE.ShaderMaterial({
9292
uniforms: this.uniforms,

‎src/shaders/sdf.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ module.exports.Shader = registerShader('sdf', {
9393

9494
fragmentShader: FRAGMENT_SHADER,
9595

96-
init: function (data) {
96+
init: function () {
9797
this.uniforms = THREE.UniformsUtils.merge([
9898
THREE.UniformsLib.fog,
99-
this.initVariables(data, 'uniform')
99+
this.initUniforms()
100100
]);
101101
this.material = new THREE.ShaderMaterial({
102102
uniforms: this.uniforms,

‎tests/core/shader.test.js‎

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ suite('shader', function () {
3333
assert.ok(shader.prototype.vertexShader);
3434
assert.ok(shader.prototype.fragmentShader);
3535
assert.notOk(shader.prototype.uniforms);
36-
assert.notOk(shader.prototype.attributes);
3736
});
3837

3938
test('shader instance receives methods and properties', function () {
@@ -49,7 +48,6 @@ suite('shader', function () {
4948
assert.equal(instance.vertexShader, shader.prototype.vertexShader);
5049
assert.equal(instance.fragmentShader, shader.prototype.fragmentShader);
5150
assert.equal(Object.keys(instance.uniforms).length, 0);
52-
assert.equal(Object.keys(instance.attributes).length, 0);
5351
assert.ok(instance.material);
5452
});
5553

@@ -94,8 +92,7 @@ suite('shader data binding', function () {
9492
src: {type: 'map', is: 'uniform'},
9593
otherMap: {type: 'map', is: 'uniform'},
9694
vec2Uniform: {type: 'vec2', default: {x: 1, y: 2}, is: 'uniform'},
97-
vec2Attribute: {type: 'vec2', default: {x: 3, y: 4}, is: 'attribute'},
98-
vec2Neither: {type: 'vec2', default: {x: 5, y: 6}}
95+
vec2NotUniform: {type: 'vec2', default: {x: 5, y: 6}}
9996
}
10097
});
10198

@@ -126,8 +123,6 @@ suite('shader data binding', function () {
126123
assert.ok(updateSpy.calledOnce);
127124
// The value won't be assigned until the texture loads.
128125
assert.ok(instance.uniforms['src']);
129-
assert.notOk(instance.attributes && (instance.attributes['map'] ||
130-
instance.attributes['src']));
131126
});
132127

133128
test('src loads inline video', function (done) {
@@ -152,8 +147,6 @@ suite('shader data binding', function () {
152147
assert.ok(updateSpy.calledOnce);
153148
// The value won't be assigned until the texture loads.
154149
assert.ok(instance.uniforms['src']);
155-
assert.notOk(instance.attributes && (instance.attributes['map'] ||
156-
instance.attributes['src']));
157150
});
158151

159152
test('otherMap loads inline video', function (done) {
@@ -178,8 +171,6 @@ suite('shader data binding', function () {
178171
assert.ok(initSpy.calledOnce);
179172
assert.ok(updateSpy.calledOnce);
180173
assert.ok(instance.uniforms['otherMap']);
181-
// The value won't be assigned until the texture loads.
182-
assert.notOk(instance.attributes && instance.attributes['otherMap']);
183174
});
184175

185176
test('vec2Uniform parameter is uniform', function () {
@@ -194,25 +185,9 @@ suite('shader data binding', function () {
194185
assert.ok(instance.uniforms['vec2Uniform']);
195186
assert.equal(instance.uniforms['vec2Uniform'].value.x, 1);
196187
assert.equal(instance.uniforms['vec2Uniform'].value.y, 2);
197-
assert.notOk(instance.attributes['vec2Uniform']);
198-
});
199-
200-
test('vec2Attribute parameter is attribute', function () {
201-
assert.notOk(initSpy.called);
202-
assert.notOk(updateSpy.called);
203-
el.setAttribute('material', 'shader: testShader');
204-
const material = el.components.material;
205-
const instance = material.shader;
206-
assert.ok(instance);
207-
assert.ok(initSpy.calledOnce);
208-
assert.ok(updateSpy.calledOnce);
209-
assert.ok(instance.attributes['vec2Attribute']);
210-
assert.equal(instance.attributes['vec2Attribute'].value.x, 3);
211-
assert.equal(instance.attributes['vec2Attribute'].value.y, 4);
212-
assert.notOk(instance.uniforms['vec2Attribute']);
213188
});
214189

215-
test('vec2Neither parameter is neither uniform nor attribute', function () {
190+
test('vec2NotUniform parameter is not a uniform', function () {
216191
assert.notOk(initSpy.called);
217192
assert.notOk(updateSpy.called);
218193
el.setAttribute('material', 'shader: testShader');
@@ -221,7 +196,6 @@ suite('shader data binding', function () {
221196
assert.ok(instance);
222197
assert.ok(initSpy.calledOnce);
223198
assert.ok(updateSpy.calledOnce);
224-
assert.notOk(instance.attributes['vec2Neither']);
225-
assert.notOk(instance.uniforms['vec2Neither']);
199+
assert.notOk(instance.uniforms['vec2NotUniform']);
226200
});
227201
});

0 commit comments

Comments
 (0)