[BREAKING] Remove StandardMaterial.ambientTint (#6805)

* [BREAKING] Remove StandardMaterial.ambientTint * test --------- Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com>
playcanvas · Jul 11, 2024 · 1f083b6 · 1f083b6
1 parent ede3f70
commit 1f083b6
Show file tree

Hide file tree

Showing 11 changed files with 7 additions and 24 deletions.
diff --git a/src/deprecated/deprecated.js b/src/deprecated/deprecated.js
@@ -690,6 +690,7 @@ function _deprecateTint(name) {
 
 _deprecateTint('diffuseTint');
 _deprecateTint('emissiveTint');
+_deprecateTint('ambientTint');
 
 _defineAlias('specularTint', 'specularMapTint');
 _defineAlias('aoVertexColor', 'aoMapVertexColor');

diff --git a/src/framework/lightmapper/lightmapper.js b/src/framework/lightmapper/lightmapper.js
@@ -253,7 +253,6 @@ class Lightmapper {
                 ` + bakeLmEndChunk;
             } else {
                 material.ambient = new Color(0, 0, 0);    // don't bake ambient
-                material.ambientTint = true;
             }
             material.chunks.basePS = shaderChunks.basePS + (scene.lightmapPixelFormat === PIXELFORMAT_RGBA8 ? '\n#define LIGHTMAP_RGBM\n' : '');
             material.chunks.endPS = bakeLmEndChunk;

diff --git a/src/scene/materials/lit-material-options-builder.js b/src/scene/materials/lit-material-options-builder.js
@@ -46,7 +46,6 @@ class LitMaterialOptionsBuilder {
     }
 
     static updateMaterialOptions(litOptions, material) {
-        litOptions.useAmbientTint = false;
         litOptions.separateAmbient = false;    // store ambient light color in separate variable, instead of adding it to diffuse directly
         litOptions.customFragmentShader = null;
         litOptions.pixelSnap = material.pixelSnap;

diff --git a/src/scene/materials/standard-material-options-builder.js b/src/scene/materials/standard-material-options-builder.js
@@ -210,7 +210,6 @@ class StandardMaterialOptionsBuilder {
         const isPackedNormalMap = stdMat.normalMap ? (stdMat.normalMap.format === PIXELFORMAT_DXT5 || stdMat.normalMap.type === TEXTURETYPE_SWIZZLEGGGR) : false;
 
         options.opacityTint = (stdMat.blendType !== BLEND_NONE || stdMat.alphaTest > 0 || stdMat.opacityDither !== DITHER_NONE) ? 1 : 0;
-        options.ambientTint = stdMat.ambientTint;
         options.specularTint = specularTint ? 2 : 0;
         options.specularityFactorTint = specularityFactorTint ? 1 : 0;
         options.metalnessTint = (stdMat.useMetalness && stdMat.metalness < 1) ? 1 : 0;
@@ -249,7 +248,6 @@ class StandardMaterialOptionsBuilder {
 
         // LIT OPTIONS
         options.litOptions.separateAmbient = false;    // store ambient light color in separate variable, instead of adding it to diffuse directly
-        options.litOptions.useAmbientTint = stdMat.ambientTint;
         options.litOptions.customFragmentShader = stdMat.customFragmentShader;
         options.litOptions.pixelSnap = stdMat.pixelSnap;
 

diff --git a/src/scene/materials/standard-material-options.js b/src/scene/materials/standard-material-options.js
@@ -15,13 +15,6 @@ class StandardMaterialOptions {
      */
     forceUv1 = false;
 
-    /**
-     * The value of {@link StandardMaterial#ambientTint}.
-     *
-     * @type {boolean}
-     */
-    ambientTint = 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
@@ -23,7 +23,6 @@ const standardMaterialParameterTypes = {
     _engine: 'boolean', // internal param for engine-only loading
 
     ambient: 'rgb',
-    ambientTint: 'boolean',
     ..._textureParameter('ao'),
     ..._textureParameter('aoDetail', true, false),
     aoDetailMode: 'string',

diff --git a/src/scene/materials/standard-material.js b/src/scene/materials/standard-material.js
@@ -457,7 +457,6 @@ const _tempColor = new Color();
  * multiplied by vertex colors.
  * @property {string} lightVertexColorChannel Vertex color channels to use for baked lighting. Can
  * be "r", "g", "b", "a", "rgb" or any swizzled combination.
- * @property {boolean} ambientTint Enables scene ambient multiplication by material ambient color.
  * @property {Texture|null} aoMap The main (primary) baked ambient occlusion (AO) map (default is
  * null). Modulates ambient color.
  * @property {number} aoMapUv Main (primary) AO map UV channel
@@ -1118,7 +1117,7 @@ function _defineFlag(name, defaultValue) {
 }
 
 function _defineMaterialProps() {
-    _defineColor('ambient', new Color(0.7, 0.7, 0.7));
+    _defineColor('ambient', new Color(1, 1, 1));
     _defineColor('diffuse', new Color(1, 1, 1));
     _defineColor('specular', new Color(0, 0, 0));
     _defineColor('emissive', new Color(0, 0, 0));
@@ -1184,7 +1183,6 @@ function _defineMaterialProps() {
         return uniform;
     });
 
-    _defineFlag('ambientTint', false);
     _defineFlag('sheenTint', false);
     _defineFlag('specularTint', false);
     _defineFlag('specularityFactorTint', false);

diff --git a/src/scene/shader-lib/programs/lit-shader-options.js b/src/scene/shader-lib/programs/lit-shader-options.js
@@ -91,8 +91,6 @@ class LitShaderOptions {
 
     diffuseMapEnabled = false;
 
-    useAmbientTint = false;
-
     /**
      * Replaced the whole fragment shader with this string.
      *

diff --git a/src/scene/shader-lib/programs/lit-shader.js b/src/scene/shader-lib/programs/lit-shader.js
@@ -853,7 +853,7 @@ class LitShader {
             }
         }
 
-        if (options.useAmbientTint && !useOldAmbient) {
+        if (!useOldAmbient) {
             decl.append("uniform vec3 material_ambient;");
         }
 
@@ -972,7 +972,7 @@ class LitShader {
             }
         }
 
-        if (options.useAmbientTint && !useOldAmbient) {
+        if (!useOldAmbient) {
             backend.append("    dDiffuseLight *= material_ambient;");
         }
 

diff --git a/test/scene/materials/standard-material.test.mjs b/test/scene/materials/standard-material.test.mjs
@@ -13,10 +13,9 @@ describe('StandardMaterial', function () {
         expect(material).to.be.an.instanceof(Material);
         expect(material.alphaFade).to.equal(1);
         expect(material.ambient).to.be.an.instanceof(Color);
-        expect(material.ambient.r).to.equal(0.7);
-        expect(material.ambient.g).to.equal(0.7);
-        expect(material.ambient.b).to.equal(0.7);
-        expect(material.ambientTint).to.equal(false);
+        expect(material.ambient.r).to.equal(1);
+        expect(material.ambient.g).to.equal(1);
+        expect(material.ambient.b).to.equal(1);
         expect(material.anisotropy).to.equal(0);
 
         expect(material.aoDetailMap).to.be.null;

diff --git a/utils/plugins/rollup-types-fixup.mjs b/utils/plugins/rollup-types-fixup.mjs
@@ -9,7 +9,6 @@ const TYPES_PATH = './build/playcanvas/src';
 const STANDARD_MAT_PROPS = [
     ['alphaFade', 'boolean'],
     ['ambient', 'Color'],
-    ['ambientTint', 'boolean'],
     ['anisotropy', 'number'],
     ['aoMap', 'Texture|null'],
     ['aoMapChannel', 'string'],
-Original file line number
+Diff line change
@@ Expand Up / @@ -91,8 +91,6 @@ class LitShaderOptions { @@
         diffuseMapEnabled = false;
-        useAmbientTint = false;
         /**
          * Replaced the whole fragment shader with this string.
          *
@@ Expand Down @@