From 580d94a9584eb86e83b83228892f3f3645abcc94 Mon Sep 17 00:00:00 2001 From: Noeri Huisman <8823461+mrxz@users.noreply.github.com> Date: Fri, 9 Feb 2024 17:33:07 +0100 Subject: [PATCH] Unify texture loading and map property fixes (#5453) * Use updateMapMaterialFromData in updateDistortionMap * Allow the various maps on standard and phong to be unset * Flag texture as needsUpdate when texture properties change --------- Co-authored-by: Noeri Huisman --- src/shaders/flat.js | 1 - src/shaders/phong.js | 10 ++--- src/shaders/standard.js | 10 ++--- src/utils/material.js | 74 ++++++++++++------------------- tests/components/material.test.js | 15 +++++++ tests/shaders/standard.test.js | 21 +++++++++ 6 files changed, 73 insertions(+), 58 deletions(-) diff --git a/src/shaders/flat.js b/src/shaders/flat.js index 73df91aeaac..5adc38ff317 100755 --- a/src/shaders/flat.js +++ b/src/shaders/flat.js @@ -25,7 +25,6 @@ module.exports.Shader = registerShader('flat', { */ init: function (data) { this.materialData = {color: new THREE.Color()}; - this.textureSrc = null; getMaterialData(data, this.materialData); this.material = new THREE.MeshBasicMaterial(this.materialData); }, diff --git a/src/shaders/phong.js b/src/shaders/phong.js index e0e60e66567..b279ec281e7 100644 --- a/src/shaders/phong.js +++ b/src/shaders/phong.js @@ -53,19 +53,17 @@ module.exports.Shader = registerShader('phong', { */ init: function (data) { this.materialData = { color: new THREE.Color(), specular: new THREE.Color(), emissive: new THREE.Color() }; - this.textureSrc = null; getMaterialData(data, this.materialData); this.material = new THREE.MeshPhongMaterial(this.materialData); - utils.material.updateMap(this, data); }, update: function (data) { this.updateMaterial(data); utils.material.updateMap(this, data); - if (data.normalMap) { utils.material.updateDistortionMap('normal', this, data); } - if (data.displacementMap) { utils.material.updateDistortionMap('displacement', this, data); } - if (data.ambientOcclusionMap) { utils.material.updateDistortionMap('ambientOcclusion', this, data); } - if (data.bump) { utils.material.updateDistortionMap('bump', this, data); } + utils.material.updateDistortionMap('normal', this, data); + utils.material.updateDistortionMap('displacement', this, data); + utils.material.updateDistortionMap('ambientOcclusion', this, data); + utils.material.updateDistortionMap('bump', this, data); this.updateEnvMap(data); }, diff --git a/src/shaders/standard.js b/src/shaders/standard.js index 1b4cb61d278..2db6798ab3e 100755 --- a/src/shaders/standard.js +++ b/src/shaders/standard.js @@ -69,11 +69,11 @@ module.exports.Shader = registerShader('standard', { update: function (data) { this.updateMaterial(data); utils.material.updateMap(this, data); - if (data.normalMap) { utils.material.updateDistortionMap('normal', this, data); } - if (data.displacementMap) { utils.material.updateDistortionMap('displacement', this, data); } - if (data.ambientOcclusionMap) { utils.material.updateDistortionMap('ambientOcclusion', this, data); } - if (data.metalnessMap) { utils.material.updateDistortionMap('metalness', this, data); } - if (data.roughnessMap) { utils.material.updateDistortionMap('roughness', this, data); } + utils.material.updateDistortionMap('normal', this, data); + utils.material.updateDistortionMap('displacement', this, data); + utils.material.updateDistortionMap('ambientOcclusion', this, data); + utils.material.updateDistortionMap('metalness', this, data); + utils.material.updateDistortionMap('roughness', this, data); this.updateEnvMap(data); }, diff --git a/src/utils/material.js b/src/utils/material.js index 1d4244b8ea2..2e76e9ae892 100644 --- a/src/utils/material.js +++ b/src/utils/material.js @@ -16,30 +16,40 @@ function setTextureProperties (texture, data) { var offset = data.offset || {x: 0, y: 0}; var repeat = data.repeat || {x: 1, y: 1}; var npot = data.npot || false; - var anisotropy = data.anisotropy || 0; + var anisotropy = data.anisotropy || THREE.Texture.DEFAULT_ANISOTROPY; + var wrapS = texture.wrapS; + var wrapT = texture.wrapT; + var magFilter = texture.magFilter; + var minFilter = texture.minFilter; + // To support NPOT textures, wrap must be ClampToEdge (not Repeat), // and filters must not use mipmaps (i.e. Nearest or Linear). if (npot) { - texture.wrapS = THREE.ClampToEdgeWrapping; - texture.wrapT = THREE.ClampToEdgeWrapping; - texture.magFilter = THREE.LinearFilter; - texture.minFilter = THREE.LinearFilter; + wrapS = THREE.ClampToEdgeWrapping; + wrapT = THREE.ClampToEdgeWrapping; + magFilter = THREE.LinearFilter; + minFilter = THREE.LinearFilter; } - // Don't bother setting repeat if it is 1/1. Power-of-two is required to repeat. + // Set wrap mode to repeat only if repeat isn't 1/1. Power-of-two is required to repeat. if (repeat.x !== 1 || repeat.y !== 1) { - texture.wrapS = THREE.RepeatWrapping; - texture.wrapT = THREE.RepeatWrapping; - texture.repeat.set(repeat.x, repeat.y); - } - // Don't bother setting offset if it is 0/0. - if (offset.x !== 0 || offset.y !== 0) { - texture.offset.set(offset.x, offset.y); + wrapS = THREE.RepeatWrapping; + wrapT = THREE.RepeatWrapping; } - // Only set anisotropy if it isn't 0, which indicates that the default value should be used. - if (anisotropy !== 0) { + // Apply texture properties + texture.offset.set(offset.x, offset.y); + texture.repeat.set(repeat.x, repeat.y); + + if (texture.wrapS !== wrapS || texture.wrapT !== wrapT || + texture.magFilter !== magFilter || texture.minFilter !== minFilter || + texture.anisotropy !== anisotropy) { + texture.wrapS = wrapS; + texture.wrapT = wrapT; + texture.magFilter = magFilter; + texture.minFilter = minFilter; texture.anisotropy = anisotropy; + texture.needsUpdate = true; } } module.exports.setTextureProperties = setTextureProperties; @@ -127,43 +137,15 @@ module.exports.updateMap = function (shader, data) { module.exports.updateDistortionMap = function (longType, shader, data) { var shortType = longType; if (longType === 'ambientOcclusion') { shortType = 'ao'; } - var el = shader.el; - var material = shader.material; - var rendererSystem = el.sceneEl.systems.renderer; - var src = data[longType + 'Map']; + var info = {}; - info.src = src; + info.src = data[longType + 'Map']; // Pass through the repeat and offset to be handled by the material loader. info.offset = data[longType + 'TextureOffset']; info.repeat = data[longType + 'TextureRepeat']; info.wrap = data[longType + 'TextureWrap']; - - if (src) { - if (src === shader[longType + 'TextureSrc']) { return; } - - // Texture added or changed. - shader[longType + 'TextureSrc'] = src; - el.sceneEl.systems.material.loadTexture(src, info, setMap); - return; - } - - // Texture removed. - if (!material.map) { return; } - setMap(null); - - function setMap (texture) { - var slot = shortType + 'Map'; - material[slot] = texture; - if (texture && COLOR_MAPS.has(slot)) { - rendererSystem.applyColorCorrection(texture); - } - if (texture) { - setTextureProperties(texture, data); - } - material.needsUpdate = true; - handleTextureEvents(el, texture); - } + return module.exports.updateMapMaterialFromData(shortType + 'Map', 'src', shader, info); }; /** diff --git a/tests/components/material.test.js b/tests/components/material.test.js index 7fa055ad4ca..487a2394d4b 100644 --- a/tests/components/material.test.js +++ b/tests/components/material.test.js @@ -93,6 +93,21 @@ suite('material', function () { }); }); + test('updates texture when repeat changes', function (done) { + var imageUrl = 'base/tests/assets/test.png'; + el.setAttribute('material', ''); + assert.notOk(el.components.material.material.texture); + el.setAttribute('material', 'src: url(' + imageUrl + ')'); + el.addEventListener('materialtextureloaded', function (evt) { + var loadedTexture = evt.detail.texture; + assert.ok(el.components.material.material.map === loadedTexture); + var version = loadedTexture.version; + el.setAttribute('material', 'repeat', '2 2'); + assert.ok(el.components.material.material.map.version > version); + done(); + }); + }); + test('removes texture when src attribute removed', function (done) { var imageUrl = 'base/tests/assets/test.png'; el.setAttribute('material', ''); diff --git a/tests/shaders/standard.test.js b/tests/shaders/standard.test.js index 45c9990495f..45d270e244d 100644 --- a/tests/shaders/standard.test.js +++ b/tests/shaders/standard.test.js @@ -78,6 +78,27 @@ suite('standard material', function () { }); }); + [ + { dataName: 'normalMap', materialName: 'normalMap' }, + { dataName: 'displacementMap', materialName: 'displacementMap' }, + { dataName: 'ambientOcclusionMap', materialName: 'aoMap' }, + { dataName: 'metalnessMap', materialName: 'metalnessMap' }, + { dataName: 'roughnessMap', materialName: 'roughnessMap' } + ].forEach(function (names) { + test(`can unset ${names.dataName}`, function (done) { + var el = this.el; + var imageUrl = 'base/tests/assets/test.png'; + assert.isNull(el.getObject3D('mesh').material[names.materialName]); + el.setAttribute('material', names.dataName, `url(${imageUrl})`); + el.addEventListener('materialtextureloaded', function (evt) { + assert.equal(el.getObject3D('mesh').material[names.materialName], evt.detail.texture); + el.setAttribute('material', names.dataName, ''); + assert.isNull(el.getObject3D('mesh').material[names.materialName]); + done(); + }); + }); + }); + test('can use spherical env maps', function (done) { var el = this.el; var imageUrl = 'base/tests/assets/test.png';