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

PBR shader support Iridescence #2425

Merged
merged 44 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
5a26f3d
fix(shader-lab): compatible with empty macro
Sway007 Sep 20, 2023
5ecc318
Merge branch 'main' of https://github.com/galacean/engine
Sway007 Sep 20, 2023
cafc24f
Merge branch 'main' of https://github.com/galacean/engine
Sway007 Sep 20, 2023
dc69489
Merge branch 'main' of https://github.com/galacean/engine
Sway007 Sep 21, 2023
221b7b6
Merge branch 'main' of https://github.com/galacean/engine
Sway007 Sep 21, 2023
0d45d9c
fix(shader-lab): add break and continue syntax
Sway007 Sep 21, 2023
0f14c3f
Merge branch 'main' of https://github.com/galacean/engine
Sway007 Sep 27, 2023
8871d9b
fix: typo
Sway007 Oct 11, 2023
3b4ffd7
Merge branch 'main' of https://github.com/galacean/engine
Sway007 Nov 1, 2023
f649a58
fix(shader-lab): Make usepass compatible with buitin shader
Sway007 Nov 1, 2023
598fc56
Merge branch 'main' of https://github.com/galacean/engine
Sway007 Nov 2, 2023
e33a66f
fix(shader-lab): compatible with no varying variable
Sway007 Nov 2, 2023
41ef06f
feat(shader-lab): detect mismatch return type
Sway007 Nov 6, 2023
b5214fc
fix(shader-lab): renderState assignment
Sway007 Nov 7, 2023
f36ce02
feat: extend material loader data type
Sway007 Nov 13, 2023
ff8b7c2
Merge branch 'main' of https://github.com/galacean/engine
Sway007 Nov 13, 2023
634236f
fix(shader-lab): glsl type pattern
Sway007 Nov 13, 2023
91e6fa4
fix(shader-lab): glsl type pattern
Sway007 Nov 13, 2023
3932448
fix: switch case break
Sway007 Nov 15, 2023
671cace
fix: array index loss
Sway007 Jun 11, 2024
92b972e
feat: merge
Sway007 Jun 11, 2024
9226d38
fix: test-case
Sway007 Jun 11, 2024
ff6a69a
fix: test-case
Sway007 Jun 12, 2024
0838968
feat: support GLTF_Iridescence
hhhhkrx Nov 6, 2024
82d5a8f
Merge branch 'main' of github.com:galacean/engine into gltf_iridescence
hhhhkrx Nov 6, 2024
39043a2
fix: merge
hhhhkrx Nov 6, 2024
ad61eed
Merge branch 'dev/1.4' of github.com:galacean/engine into gltf_irides…
hhhhkrx Nov 6, 2024
2b308ec
fix: vector3
hhhhkrx Nov 6, 2024
2c1604f
fix: vector3
hhhhkrx Nov 6, 2024
0be9f2a
fix: texture name
hhhhkrx Nov 6, 2024
88f89f2
Merge branch 'dev/1.4' into gltf_iridescence
hhhhkrx Nov 6, 2024
b5b9a0a
fix: material uniform
hhhhkrx Nov 7, 2024
d7a82de
fix: enable iridescence
hhhhkrx Nov 7, 2024
d1547d4
Merge branch 'dev/1.4' of github.com:galacean/engine into gltf_irides…
hhhhkrx Nov 11, 2024
76c5423
fix: iridescence name
hhhhkrx Nov 11, 2024
aa41aaf
fix: add }
hhhhkrx Nov 11, 2024
f4b16fd
ci: prettier code
hhhhkrx Nov 12, 2024
9fb543f
fix: iridescence name
hhhhkrx Nov 18, 2024
647b8a9
fix: irisdescence value
hhhhkrx Nov 19, 2024
e70ee42
fix: iridescenceRange value
hhhhkrx Nov 19, 2024
8f41f62
fix: iridescenceRange value
hhhhkrx Nov 19, 2024
499bd70
fix: issue
hhhhkrx Nov 19, 2024
e644dff
fix: iridescence
hhhhkrx Nov 19, 2024
6f418fa
Merge branch 'dev/1.4' of github.com:galacean/engine into gltf_irides…
hhhhkrx Nov 19, 2024
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
101 changes: 100 additions & 1 deletion packages/core/src/material/PBRMaterial.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { MathUtil, Vector3 } from "@galacean/engine-math";
import { MathUtil, Vector2, Vector3, Vector4 } from "@galacean/engine-math";
import { Engine } from "../Engine";
import { ShaderProperty } from "../shader";
import { Shader } from "../shader/Shader";
Expand All @@ -20,6 +20,11 @@

private _anisotropyRotation: number = 0;

private static _iridescenceInfoProp = ShaderProperty.getByName("material_IridescenceInfo");
private static _iridescenceThicknessTextureProp = ShaderProperty.getByName("material_IridescenceThicknessTexture");
private static _iridescenceTextureProp = ShaderProperty.getByName("material_IridescenceTexture");
private _iridescenceRange = new Vector2(100, 400);

/**
* Index Of Refraction.
* @defaultValue `1.5`
Expand Down Expand Up @@ -132,6 +137,91 @@
}
}

/**
* The iridescence intensity factor, from 0.0 to 1.0.
* @defaultValue `0.0`
*/
get iridescence(): number {
return this.shaderData.getVector4(PBRMaterial._iridescenceInfoProp).x;
}

Check warning on line 146 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L145-L146

Added lines #L145 - L146 were not covered by tests

set iridescence(value: number) {
value = Math.max(0, Math.min(1, value));
const iridescenceInfo = this.shaderData.getVector4(PBRMaterial._iridescenceInfoProp);
if (!!iridescenceInfo.x !== !!value) {
if (value === 0) {
this.shaderData.disableMacro("MATERIAL_ENABLE_IRIDESCENCE");
} else {
this.shaderData.enableMacro("MATERIAL_ENABLE_IRIDESCENCE");
}
}
iridescenceInfo.x = value;
}

Check warning on line 159 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L149-L159

Added lines #L149 - L159 were not covered by tests

/**
* The iridescence intensity texture, sampling red channel, and multiply 'iridescence'.
*/
get iridescenceTexture(): Texture2D {
return <Texture2D>this.shaderData.getTexture(PBRMaterial._iridescenceTextureProp);
}

Check warning on line 166 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L165-L166

Added lines #L165 - L166 were not covered by tests

set iridescenceTexture(value: Texture2D) {
this.shaderData.setTexture(PBRMaterial._iridescenceTextureProp, value);

if (value) {
this.shaderData.enableMacro("MATERIAL_HAS_IRIDESCENCE_TEXTURE");
} else {
this.shaderData.disableMacro("MATERIAL_HAS_IRIDESCENCE_TEXTURE");
}
}

Check warning on line 176 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L169-L176

Added lines #L169 - L176 were not covered by tests

/**
* The index of refraction of the dielectric thin-film layer, greater than or equal to 1.0.
* @defaultValue `1.3`
*/
get iridescenceIOR(): number {
return this.shaderData.getVector4(PBRMaterial._iridescenceInfoProp).y;
}

Check warning on line 184 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L183-L184

Added lines #L183 - L184 were not covered by tests

set iridescenceIOR(value: number) {
const iridescenceInfo = this.shaderData.getVector4(PBRMaterial._iridescenceInfoProp);
iridescenceInfo.y = Math.max(value, 1.0);
}

Check warning on line 189 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L187-L189

Added lines #L187 - L189 were not covered by tests

/**
* The range of iridescence thickness, x is minimum, y is maximum.
* @defaultValue `[100, 400]`.
*/
get iridescenceThicknessRange(): Vector2 {
return this._iridescenceRange;
}

Check warning on line 197 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L196-L197

Added lines #L196 - L197 were not covered by tests

set iridescenceThicknessRange(value: Vector2) {
if (this._iridescenceRange !== value) {
this._iridescenceRange.copyFrom(value);
}
}

Check warning on line 203 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L200-L203

Added lines #L200 - L203 were not covered by tests

/**
* The thickness texture of the thin-film layer, sampling green channel.
* @remarks
* If iridescenceThicknessTexture is defined, iridescence thickness between the 'iridescenceThicknessRange'.
* If iridescenceThicknessTexture is not defined, iridescence thickness will use only 'iridescenceThicknessRange.y'.
*/
get iridescenceThicknessTexture(): Texture2D {
return <Texture2D>this.shaderData.getTexture(PBRMaterial._iridescenceThicknessTextureProp);
}

Check warning on line 213 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L212-L213

Added lines #L212 - L213 were not covered by tests

set iridescenceThicknessTexture(value: Texture2D) {
this.shaderData.setTexture(PBRMaterial._iridescenceThicknessTextureProp, value);

if (value) {
this.shaderData.enableMacro("MATERIAL_HAS_IRIDESCENCE_THICKNESS_TEXTURE");
} else {
this.shaderData.disableMacro("MATERIAL_HAS_IRIDESCENCE_THICKNESS_TEXTURE");
}
}

Check warning on line 223 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L216-L223

Added lines #L216 - L223 were not covered by tests

/**
* Create a pbr metallic-roughness workflow material instance.
* @param engine - Engine to which the material belongs
Expand All @@ -144,6 +234,15 @@
shaderData.setFloat(PBRMaterial._roughnessProp, 1);
shaderData.setFloat(PBRMaterial._iorProp, 1.5);
shaderData.setVector3(PBRMaterial._anisotropyInfoProp, new Vector3(1, 0, 0));
shaderData.setVector4(PBRMaterial._iridescenceInfoProp, new Vector4(0, 1.3, 100, 400));
// @ts-ignore
this._iridescenceRange._onValueChanged = this._onIridescenceRangeChanged.bind(this);
}

private _onIridescenceRangeChanged(): void {
const iridescenceInfo = this.shaderData.getVector4(PBRMaterial._iridescenceInfoProp);
iridescenceInfo.z = this._iridescenceRange.x;
iridescenceInfo.w = this._iridescenceRange.y;

Check warning on line 245 in packages/core/src/material/PBRMaterial.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/material/PBRMaterial.ts#L243-L245

Added lines #L243 - L245 were not covered by tests
}

/**
Expand Down
23 changes: 18 additions & 5 deletions packages/loader/src/gltf/extensions/GLTFExtensionSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ export interface IKHRLightsPunctual {
* Interfaces from the KHR_materials_clearcoat extension
*/
export interface IKHRMaterialsClearcoat {
clearcoatFactor: number;
clearcoatTexture: ITextureInfo;
clearcoatRoughnessFactor: number;
clearcoatRoughnessTexture: ITextureInfo;
clearcoatNormalTexture: IMaterialNormalTextureInfo;
clearcoatFactor?: number;
clearcoatTexture?: ITextureInfo;
clearcoatRoughnessFactor?: number;
clearcoatRoughnessTexture?: ITextureInfo;
clearcoatNormalTexture?: IMaterialNormalTextureInfo;
}

/**
Expand Down Expand Up @@ -176,6 +176,18 @@ export interface IGalaceanAnimation {
}[];
}

/**
* Interfaces from the KHR_materials_iridescence extension
*/
export interface IKHRMaterialsIridescence {
iridescenceFactor?: number;
iridescenceTexture?: ITextureInfo;
iridescenceIor?: number;
iridescenceThicknessMinimum?: number;
iridescenceThicknessMaximum?: number;
iridescenceThicknessTexture?: ITextureInfo;
}

export type GLTFExtensionSchema =
| IKHRLightsPunctual_Light
| IKHRMaterialsClearcoat
Expand All @@ -194,4 +206,5 @@ export type GLTFExtensionSchema =
| IKHRXmp
| IKHRXmp_Node
| IGalaceanAnimation
| IKHRMaterialsIridescence
| Object;
39 changes: 39 additions & 0 deletions packages/loader/src/gltf/extensions/KHR_materials_iridescence.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { PBRMaterial, Texture2D } from "@galacean/engine-core";
import { GLTFMaterialParser } from "../parser/GLTFMaterialParser";
import { registerGLTFExtension } from "../parser/GLTFParser";
import { GLTFParserContext, GLTFParserType } from "../parser/GLTFParserContext";
import { GLTFExtensionMode, GLTFExtensionParser } from "./GLTFExtensionParser";
import { IKHRMaterialsIridescence } from "./GLTFExtensionSchema";

@registerGLTFExtension("KHR_materials_iridescence", GLTFExtensionMode.AdditiveParse)
class KHR_materials_iridescence extends GLTFExtensionParser {
override additiveParse(context: GLTFParserContext, material: PBRMaterial, schema: IKHRMaterialsIridescence): void {
const {
iridescenceFactor = 0,
iridescenceTexture,
iridescenceIor = 1.3,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation for iridescenceIor

The IOR (Index of Refraction) value should be validated as it typically should be greater than 1.0 for physical accuracy.

-      iridescenceIor = 1.3,
+      iridescenceIor = Math.max(1.0, schema.iridescenceIor ?? 1.3),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
iridescenceIor = 1.3,
iridescenceIor = Math.max(1.0, schema.iridescenceIor ?? 1.3),

iridescenceThicknessMinimum = 100,
iridescenceThicknessMaximum = 400,
Comment on lines +15 to +16
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Validate thickness range values

The thickness range values should be validated to ensure minimum is less than maximum and both are positive.

-      iridescenceThicknessMinimum = 100,
-      iridescenceThicknessMaximum = 400,
+      iridescenceThicknessMinimum = Math.max(0, schema.iridescenceThicknessMinimum ?? 100),
+      iridescenceThicknessMaximum = Math.max(
+        schema.iridescenceThicknessMinimum ?? 100,
+        schema.iridescenceThicknessMaximum ?? 400
+      ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
iridescenceThicknessMinimum = 100,
iridescenceThicknessMaximum = 400,
iridescenceThicknessMinimum = Math.max(0, schema.iridescenceThicknessMinimum ?? 100),
iridescenceThicknessMaximum = Math.max(
schema.iridescenceThicknessMinimum ?? 100,
schema.iridescenceThicknessMaximum ?? 400
),

iridescenceThicknessTexture
} = schema;

material.iridescence = iridescenceFactor;
material.iridescenceIOR = iridescenceIor;
material.iridescenceThicknessRange.set(iridescenceThicknessMinimum, iridescenceThicknessMaximum);

if (iridescenceTexture) {
GLTFMaterialParser._checkOtherTextureTransform(iridescenceTexture, "Iridescence texture");

context.get<Texture2D>(GLTFParserType.Texture, iridescenceTexture.index).then((texture) => {
material.iridescenceTexture = texture;
});
}
Comment on lines +27 to +30
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve texture loading implementation

The current texture loading implementation has several issues:

  1. No error handling for failed texture loads
  2. Promises are not being tracked, which could lead to race conditions
  3. No cleanup handling if the material is disposed before texture loads complete

Consider implementing this pattern:

     if (iridescenceTexture) {
       GLTFMaterialParser._checkOtherTextureTransform(iridescenceTexture, "Iridescence texture");
 
-      context.get<Texture2D>(GLTFParserType.Texture, iridescenceTexture.index).then((texture) => {
-        material.iridescenceTexture = texture;
-      });
+      const texturePromise = context.get<Texture2D>(GLTFParserType.Texture, iridescenceTexture.index)
+        .then((texture) => {
+          if (!material.destroyed) {
+            material.iridescenceTexture = texture;
+          }
+        })
+        .catch((error) => {
+          console.warn(`Failed to load iridescence texture: ${error.message}`);
+        });
+      context.addPendingTask(texturePromise);
     }

Apply similar changes to the iridescenceThicknessTexture loading block.

Also applies to: 34-37

if (iridescenceThicknessTexture) {
GLTFMaterialParser._checkOtherTextureTransform(iridescenceThicknessTexture, "IridescenceThickness texture");

context.get<Texture2D>(GLTFParserType.Texture, iridescenceThicknessTexture.index).then((texture) => {
material.iridescenceThicknessTexture = texture;
});
}
}

Check warning on line 38 in packages/loader/src/gltf/extensions/KHR_materials_iridescence.ts

View check run for this annotation

Codecov / codecov/patch

packages/loader/src/gltf/extensions/KHR_materials_iridescence.ts#L11-L38

Added lines #L11 - L38 were not covered by tests
Comment on lines +11 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the gathered information, I can now provide the final response:

Test coverage needed for KHR_materials_iridescence extension

The codebase shows that GLTF material extensions are primarily tested through integration tests in GLTFLoader.test.ts, but KHR_materials_iridescence is missing from these tests. A dedicated test suite should be added to cover:

  • Default value initialization (iridescenceFactor=0, iridescenceIor=1.3, etc.)
  • Texture loading scenarios for both iridescence and thickness textures
  • Material property assignments
  • Integration with the GLTFLoader

Reference the existing test patterns from other material extensions in tests/src/loader/GLTFLoader.test.ts which already cover KHR_materials_clearcoat, KHR_materials_unlit, and others.

🔗 Analysis chain

Add comprehensive test coverage

The static analysis indicates this extension lacks test coverage. This is critical for a parser implementation that handles material properties and async texture loading.

Let's verify the current test coverage:

Would you like me to help generate a test suite that covers:

  1. Default value handling
  2. Texture loading success/failure scenarios
  3. Input validation
  4. Edge cases for thickness values and IOR

Let me check the test coverage more thoroughly by examining the test files for material extensions and the implementation details.


Let me try a different approach to check for GLTF extension tests and material-related test patterns.


Let me check specifically for tests of other KHR material extensions to understand the testing patterns used.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for existing tests
fd -e test.ts -e spec.ts . | grep -i "iridescence"

# Check if similar extensions have tests we can reference
fd -e test.ts -e spec.ts . | grep -i "material.*test"

Length of output: 480


Script:

#!/bin/bash
# Check for GLTF material extension tests
fd -e test.ts -e spec.ts . | grep -i "gltf.*material"

# Look at the implementation of other material extension tests
rg -l "describe.*material.*extension" -g "*.test.ts" -g "*.spec.ts"

# Check the structure of KHR_materials_iridescence implementation
ast-grep --pattern 'export class KHR_materials_iridescence {
  $$$
}'

Length of output: 197


Script:

#!/bin/bash
# Check for any GLTF extension tests
rg -g "*.test.ts" -g "*.spec.ts" "KHR_materials" -A 5

# Look for test patterns in material-related tests
rg -g "*.test.ts" "describe\(.*Material" -A 2 tests/src/core/material/

# Check the actual implementation directory for other extension tests
fd . packages/loader/src/gltf/extensions/ -t f

Length of output: 5399


Script:

#!/bin/bash
# Check for test files of other KHR material extensions
rg -g "*.test.ts" -g "*.spec.ts" "KHR_materials_(clearcoat|unlit|transmission)" -B 2 -A 10

# Look at the implementation of a similar extension test
rg "class KHR_materials_clearcoat" -A 10 packages/loader/src/gltf/extensions/KHR_materials_clearcoat.ts

# Check if there's a test directory for GLTF extensions
fd . tests/src/loader/gltf/extensions/ -t d

Length of output: 3854

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 11-38: packages/loader/src/gltf/extensions/KHR_materials_iridescence.ts#L11-L38
Added lines #L11 - L38 were not covered by tests

}
1 change: 1 addition & 0 deletions packages/loader/src/gltf/extensions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import "./GALACEAN_materials_remap";
import "./GALACEAN_animation_event";
import "./EXT_meshopt_compression";
import "./KHR_materials_anisotropy";
import "./KHR_materials_iridescence";
import "./EXT_texture_webp";

export { GLTFExtensionParser, GLTFExtensionMode } from "./GLTFExtensionParser";
Expand Down
Loading