Skip to content

Commit

Permalink
Remove attributes array and cleanup Shader (#5310)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@mrxz
mrxz and mrxz committed Mar 12, 2024
1 parent af4396d commit d94bf47
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 55 deletions.
36 changes: 14 additions & 22 deletions src/core/shader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@ 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,
glslVersion: this.raw || this.glsl3 ? THREE.GLSL3 : null,
vertexShader: this.vertexShader,
Expand All @@ -62,18 +60,18 @@ Shader.prototype = {
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;
Expand All @@ -86,36 +84,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;
}
},

Expand All @@ -141,11 +133,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;
});
}
};
Expand All @@ -170,7 +162,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);
Expand Down
4 changes: 2 additions & 2 deletions src/shaders/msdf.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ module.exports.Shader = registerShader('msdf', {

fragmentShader: FRAGMENT_SHADER,

init: function (data) {
init: function () {
this.uniforms = THREE.UniformsUtils.merge([
THREE.UniformsLib.fog,
this.initVariables(data, 'uniform')
this.initUniforms()
]);
this.material = new THREE.ShaderMaterial({
uniforms: this.uniforms,
Expand Down
4 changes: 2 additions & 2 deletions src/shaders/sdf.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ module.exports.Shader = registerShader('sdf', {

fragmentShader: FRAGMENT_SHADER,

init: function (data) {
init: function () {
this.uniforms = THREE.UniformsUtils.merge([
THREE.UniformsLib.fog,
this.initVariables(data, 'uniform')
this.initUniforms()
]);
this.material = new THREE.ShaderMaterial({
uniforms: this.uniforms,
Expand Down
32 changes: 3 additions & 29 deletions tests/core/shader.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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);
});

Expand Down Expand Up @@ -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}}
}
});

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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 () {
Expand All @@ -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');
Expand All @@ -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']);
});
});

0 comments on commit d94bf47

Please sign in to comment.