[BREAKING] Remove StandardMaterial.diffuseTint and simply always appl…

…y it (#6759) Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com>
playcanvas · Jun 27, 2024 · bb4f3f7 · bb4f3f7
1 parent 9f04bae
commit bb4f3f7
Show file tree

Hide file tree

Showing 12 changed files with 17 additions and 38 deletions.
diff --git a/examples/src/examples/graphics/material-clear-coat.example.mjs b/examples/src/examples/graphics/material-clear-coat.example.mjs
@@ -97,7 +97,6 @@ assetListLoader.load(() => {
     material.glossMapChannel = 'g';
     material.normalMap = assets.normal.resource;
     material.diffuse = new pc.Color(0.6, 0.6, 0.9);
-    material.diffuseTint = true;
     material.metalness = 1.0;
     material.gloss = 0.9;
     material.bumpiness = 0.7;
@@ -114,7 +113,6 @@ assetListLoader.load(() => {
     clearCoatMaterial.glossMapChannel = 'g';
     clearCoatMaterial.normalMap = assets.normal.resource;
     clearCoatMaterial.diffuse = new pc.Color(0.6, 0.6, 0.9);
-    clearCoatMaterial.diffuseTint = true;
     clearCoatMaterial.metalness = 1.0;
     clearCoatMaterial.gloss = 0.9;
     clearCoatMaterial.bumpiness = 0.7;

diff --git a/examples/src/examples/graphics/render-to-texture.example.mjs b/examples/src/examples/graphics/render-to-texture.example.mjs
@@ -175,7 +175,6 @@ assetListLoader.load(() => {
 
     // make the texture tiles and use anisotropic filtering to prevent blurring
     planeMaterial.diffuseMap = assets.checkerboard.resource;
-    planeMaterial.diffuseTint = true;
     planeMaterial.diffuseMapTiling.set(10, 10);
     planeMaterial.anisotropy = 16;
 

diff --git a/src/deprecated/deprecated.js b/src/deprecated/deprecated.js
@@ -676,7 +676,19 @@ function _defineAlias(newName, oldName) {
     });
 }
 
-_defineAlias('diffuseTint', 'diffuseMapTint');
+function _deprecateTint(name) {
+    Object.defineProperty(StandardMaterial.prototype, name, {
+        get: function () {
+            Debug.deprecated(`pc.StandardMaterial#${name} is deprecated, and the behaviour is as if ${name} was always true`);
+            return true;
+        },
+        set: function (value) {
+            Debug.deprecated(`pc.StandardMaterial#${name} is deprecated, and the behaviour is as if ${name} was always true`);
+        }
+    });
+}
+
+_deprecateTint('diffuseTint');
 _defineAlias('specularTint', 'specularMapTint');
 _defineAlias('emissiveTint', 'emissiveMapTint');
 _defineAlias('aoVertexColor', 'aoMapVertexColor');

diff --git a/src/framework/parsers/glb-parser.js b/src/framework/parsers/glb-parser.js
@@ -955,7 +955,6 @@ const extensionUnlit = (data, material, textures) => {
 
     // copy diffuse into emissive
     material.emissive.copy(material.diffuse);
-    material.emissiveTint = material.diffuseTint;
     material.emissiveMap = material.diffuseMap;
     material.emissiveMapUv = material.diffuseMapUv;
     material.emissiveMapTiling.copy(material.diffuseMapTiling);
@@ -970,8 +969,7 @@ const extensionUnlit = (data, material, textures) => {
     material.useSkybox = false;
 
     // blank diffuse
-    material.diffuse.set(0, 0, 0);
-    material.diffuseTint = false;
+    material.diffuse.set(1, 1, 1);
     material.diffuseMap = null;
     material.diffuseVertexColor = false;
 };
@@ -1118,7 +1116,6 @@ const createMaterial = (gltfMaterial, textures) => {
     // glTF doesn't define how to occlude specular
     material.occludeSpecular = SPECOCC_AO;
 
-    material.diffuseTint = true;
     material.diffuseVertexColor = true;
 
     material.specularTint = true;

diff --git a/src/framework/parsers/material/json-standard-material.js b/src/framework/parsers/material/json-standard-material.js
@@ -119,7 +119,6 @@ class JsonStandardMaterialParser {
             ['glossMapVertexColor', 'glossVertexColor'],
             ['lightMapVertexColor', 'lightVertexColor'],
 
-            ['diffuseMapTint', 'diffuseTint'],
             ['specularMapTint', 'specularTint'],
             ['emissiveMapTint', 'emissiveTint'],
             ['metalnessMapTint', 'metalnessTint'],

diff --git a/src/scene/materials/standard-material-options-builder.js b/src/scene/materials/standard-material-options-builder.js
@@ -193,8 +193,6 @@ class StandardMaterialOptionsBuilder {
     }
 
     _updateMaterialOptions(options, stdMat) {
-        const diffuseTint = (stdMat.diffuseTint || (!stdMat.diffuseMap && !stdMat.diffuseVertexColor));
-
         const useSpecular = !!(stdMat.useMetalness || stdMat.specularMap || stdMat.sphereMap || stdMat.cubeMap ||
                             notBlack(stdMat.specular) || (stdMat.specularityFactor > 0 && stdMat.useMetalness) ||
                             stdMat.enableGGXSpecular ||
@@ -215,7 +213,6 @@ class StandardMaterialOptionsBuilder {
 
         options.opacityTint = (stdMat.blendType !== BLEND_NONE || stdMat.alphaTest > 0 || stdMat.opacityDither !== DITHER_NONE) ? 1 : 0;
         options.ambientTint = stdMat.ambientTint;
-        options.diffuseTint = diffuseTint ? 2 : 0;
         options.specularTint = specularTint ? 2 : 0;
         options.specularityFactorTint = specularityFactorTint ? 1 : 0;
         options.metalnessTint = (stdMat.useMetalness && stdMat.metalness < 1) ? 1 : 0;

diff --git a/src/scene/materials/standard-material-options.js b/src/scene/materials/standard-material-options.js
@@ -22,13 +22,6 @@ class StandardMaterialOptions {
      */
     ambientTint = false;
 
-    /**
-     * Defines if {@link StandardMaterial#diffuse} constant should affect diffuse color.
-     *
-     * @type {boolean}
-     */
-    diffuseTint = false;
-
     /**
      * Defines if {@link StandardMaterial#specular} constant should affect specular color.
      *

diff --git a/src/scene/materials/standard-material-parameters.js b/src/scene/materials/standard-material-parameters.js
@@ -29,7 +29,6 @@ const standardMaterialParameterTypes = {
     aoDetailMode: 'string',
 
     diffuse: 'rgb',
-    diffuseTint: 'boolean',
     ..._textureParameter('diffuse'),
     ..._textureParameter('diffuseDetail', true, false),
     diffuseDetailMode: 'string',

diff --git a/src/scene/materials/standard-material.js b/src/scene/materials/standard-material.js
@@ -54,8 +54,6 @@ const _tempColor = new Color();
  * (RGB), where each component is between 0 and 1.
  * @property {Color} diffuse The diffuse color of the material. This color value is 3-component
  * (RGB), where each component is between 0 and 1. Defines basic surface color (aka albedo).
- * @property {boolean} diffuseTint Multiply main (primary) diffuse map and/or diffuse vertex color
- * by the constant diffuse value.
  * @property {import('../../platform/graphics/texture.js').Texture|null} diffuseMap The main
  * (primary) diffuse map of the material (default is null).
  * @property {number} diffuseMapUv Main (primary) diffuse map UV channel.
@@ -66,8 +64,7 @@ const _tempColor = new Color();
  * (primary) diffuse map.
  * @property {string} diffuseMapChannel Color channels of the main (primary) diffuse map to use.
  * Can be "r", "g", "b", "a", "rgb" or any swizzled combination.
- * @property {boolean} diffuseVertexColor Use mesh vertex colors for diffuse. If diffuseMap or are
- * diffuseTint are set, they'll be multiplied by vertex colors.
+ * @property {boolean} diffuseVertexColor Multiply diffuse by the mesh vertex colors.
  * @property {string} diffuseVertexColorChannel Vertex color channels to use for diffuse. Can be
  * "r", "g", "b", "a", "rgb" or any swizzled combination.
  * @property {import('../../platform/graphics/texture.js').Texture|null} diffuseDetailMap The
@@ -732,10 +729,7 @@ class StandardMaterial extends Material {
         };
 
         this._setParameter('material_ambient', getUniform('ambient'));
-
-        if (!this.diffuseMap || this.diffuseTint) {
-            this._setParameter('material_diffuse', getUniform('diffuse'));
-        }
+        this._setParameter('material_diffuse', getUniform('diffuse'));
 
         if (this.useMetalness) {
             if (!this.metalnessMap || this.metalness < 1) {
@@ -1200,7 +1194,6 @@ function _defineMaterialProps() {
     });
 
     _defineFlag('ambientTint', false);
-    _defineFlag('diffuseTint', false);
     _defineFlag('sheenTint', false);
     _defineFlag('specularTint', false);
     _defineFlag('specularityFactorTint', false);

diff --git a/src/scene/shader-lib/chunks/standard/frag/diffuse.js b/src/scene/shader-lib/chunks/standard/frag/diffuse.js
@@ -1,14 +1,8 @@
 export default /* glsl */`
-#ifdef MAPCOLOR
 uniform vec3 material_diffuse;
-#endif
 
 void getAlbedo() {
-    dAlbedo = vec3(1.0);
-
-#ifdef MAPCOLOR
-    dAlbedo *= material_diffuse.rgb;
-#endif
+    dAlbedo = material_diffuse.rgb;
 
 #ifdef MAPTEXTURE
     vec3 albedoBase = $DECODE(texture2DBias($SAMPLER, $UV, textureBias)).$CH;

diff --git a/test/scene/materials/standard-material.test.mjs b/test/scene/materials/standard-material.test.mjs
@@ -112,7 +112,6 @@ describe('StandardMaterial', function () {
         expect(material.diffuseMapTiling.x).to.equal(1);
         expect(material.diffuseMapTiling.y).to.equal(1);
         expect(material.diffuseMapUv).to.equal(0);
-        expect(material.diffuseTint).to.equal(false);
         expect(material.diffuseVertexColor).to.equal(false);
         expect(material.diffuseVertexColorChannel).to.equal('rgb');
 

diff --git a/utils/plugins/rollup-types-fixup.mjs b/utils/plugins/rollup-types-fixup.mjs
@@ -69,7 +69,6 @@ const STANDARD_MAT_PROPS = [
     ['diffuseMapRotation', 'number'],
     ['diffuseMapTiling', 'Vec2'],
     ['diffuseMapUv', 'number'],
-    ['diffuseTint', 'boolean'],
     ['diffuseVertexColor', 'boolean'],
     ['diffuseVertexColorChannel', 'string'],
     ['emissive', 'Color'],