Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BREAKING] Remove StandardMaterial.diffuseTint and simply always apply it #6759

Merged
merged 1 commit into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
14 changes: 13 additions & 1 deletion src/deprecated/deprecated.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
5 changes: 1 addition & 4 deletions src/framework/parsers/glb-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
};
Expand Down Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/framework/parsers/material/json-standard-material.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ class JsonStandardMaterialParser {
['glossMapVertexColor', 'glossVertexColor'],
['lightMapVertexColor', 'lightVertexColor'],

['diffuseMapTint', 'diffuseTint'],
['specularMapTint', 'specularTint'],
['emissiveMapTint', 'emissiveTint'],
['metalnessMapTint', 'metalnessTint'],
Expand Down
3 changes: 0 additions & 3 deletions src/scene/materials/standard-material-options-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand All @@ -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;
Expand Down
7 changes: 0 additions & 7 deletions src/scene/materials/standard-material-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
1 change: 0 additions & 1 deletion src/scene/materials/standard-material-parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const standardMaterialParameterTypes = {
aoDetailMode: 'string',

diffuse: 'rgb',
diffuseTint: 'boolean',
..._textureParameter('diffuse'),
..._textureParameter('diffuseDetail', true, false),
diffuseDetailMode: 'string',
Expand Down
11 changes: 2 additions & 9 deletions src/scene/materials/standard-material.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1200,7 +1194,6 @@ function _defineMaterialProps() {
});

_defineFlag('ambientTint', false);
_defineFlag('diffuseTint', false);
_defineFlag('sheenTint', false);
_defineFlag('specularTint', false);
_defineFlag('specularityFactorTint', false);
Expand Down
8 changes: 1 addition & 7 deletions src/scene/shader-lib/chunks/standard/frag/diffuse.js
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
1 change: 0 additions & 1 deletion test/scene/materials/standard-material.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
1 change: 0 additions & 1 deletion utils/plugins/rollup-types-fixup.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const STANDARD_MAT_PROPS = [
['diffuseMapRotation', 'number'],
['diffuseMapTiling', 'Vec2'],
['diffuseMapUv', 'number'],
['diffuseTint', 'boolean'],
['diffuseVertexColor', 'boolean'],
['diffuseVertexColorChannel', 'string'],
['emissive', 'Color'],
Expand Down