Unify texture loading and map property fixes #5453
Conversation
| @@ -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); } | |||
There was a problem hiding this comment.
The guards prevented these textures from being removed. Similar to how the main (diffuse) map is handled, the underlying implementation of updateMapMaterialFromData checks if the texture is added, changed or removed, or if only the texture properties are updated.
| texture.repeat.set(repeat.x, repeat.y); | ||
| } | ||
| // Don't bother setting offset if it is 0/0. | ||
| if (offset.x !== 0 || offset.y !== 0) { |
There was a problem hiding this comment.
The guard actually prevented offset from being set back to 0/0 after being set to something else. On top of that, changing the offset doesn't require a texture update, so there's nothing gained by skipping this.
| }, | ||
|
|
||
| update: function (data) { | ||
| this.updateMaterial(data); | ||
| utils.material.updateMap(this, data); | ||
| if (data.normalMap) { utils.material.updateDistortionMap('normal', this, data); } |
There was a problem hiding this comment.
Same as for standard.js
| getMaterialData(data, this.materialData); | ||
| this.material = new THREE.MeshPhongMaterial(this.materialData); | ||
| utils.material.updateMap(this, data); |
There was a problem hiding this comment.
Indeed, this is in the init method, which is followed by update which also calls updateMap. Both flat and standard let the update handle this and phong was the odd one out.
mrxz
force-pushed
the
unify-texture-loading
branch
from
8e9129d to
bf92363
Compare
|
Thanks! |
Description:
Preparation work for #5449. Both
utils.materials.updateMapandutils.materials.updateDistortionMapare used for setting texture maps. These methods had similar, but slightly different implementations. This PR unifies them by lettingupdateDistortionMapcallupdateMapMaterialFromData.Removing these maps or changing texture properties are now also correctly handle. Before only the diffuse map could be removed when changing the map properties of the
standardandphongshader. In case texture properties are changed that require a texture update, theneedsUpdateflag is now also set. Note: textures are still shared based on their initial properties, this will be addressed in upcoming PRs.Changes proposed:
updateMapMaterialFromDatainupdateDistortionMapaligning it withupdateMapstandardandphongshaders)setTexturePropertiessetneedsUpdateon the texture when a property changes that requires itneedsUpdate