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

Advanced Shader #280

Merged
merged 27 commits into from
Jul 19, 2024
Merged

Advanced Shader #280

merged 27 commits into from
Jul 19, 2024

Conversation

hhhhkrx
Copy link
Contributor

@hhhhkrx hhhhkrx commented Jun 19, 2024

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

Other information:

image

Summary by CodeRabbit

  • New Features

    • Introduced advanced shaders for realistic eye, hair, and subsurface scattering rendering, allowing for customizable properties.
    • Added functions for calculating eye color, hair specular effects, and iridescence, enhancing realism in 3D graphics.
  • Improvements

    • Enhanced visual fidelity through new shader properties and functions, providing detailed control over material characteristics.
    • Improved flexibility with conditional texture options and parameters for eye, hair, and subsurface scattering effects.

@GuoLei1990 GuoLei1990 requested a review from JujieX June 20, 2024 09:40
@GuoLei1990
Copy link
Member

  • Include file relative path problem: @Sway007

@GuoLei1990
Copy link
Member

@coderabbitai review

Copy link

coderabbitai bot commented Jun 28, 2024

Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

coderabbitai bot commented Jun 28, 2024

Walkthrough

The updates introduce a comprehensive suite of advanced shaders for rendering eyes, hair, and subsurface scattering effects. These shaders enhance visual realism through customizable properties and intricate functions, utilizing Physically Based Rendering (PBR) techniques. Notable features include detailed calculations for color, anisotropic shading for hair, and iridescence effects, significantly enriching graphical representation in 3D environments.

Changes

File Path Change Summary
.../eye/Eye.gs, .../eye/EyeFunction.glsl Introduces shaders for eye rendering, detailing customizable properties for sclera and iris, along with functions for parallax, color calculation, and normal mapping to enhance material interactions.
.../hair/Hair.gs, .../hair/HairForwardPass.glsl, .../hair/HairLightDirect.glsl Adds shaders for hair rendering, encompassing properties for texture inputs, IOR, and anisotropic effects, along with functions for vertex and fragment processing, direct lighting calculations, and specular contributions based on anisotropic models.
.../sss/SSS.gs, .../sss/SSSFunction.glsl, .../sss/SSSLightDirect.glsl Introduces subsurface scattering shaders, detailing properties for skin scattering effects, curvature textures, and functions for diffuse and specular calculations, enhancing realism in skin rendering.
.../thinfilm/IridescenceFunction.glsl, .../thinfilm/IridescenceIndirectLight.glsl, .../thinfilm/IridescencedirectLight.glsl Adds functions for simulating thin film iridescence, including Fresnel calculations and specular indirect lighting effects, enhancing the rendering of iridescent materials.
.../thinfilm/Iridescene.gs Introduces a shader script defining material properties for iridescence, including parameters for base color and thin film characteristics, structured for forward rendering.

Poem

In realms of shaders, magic spun,
Eyes gleam bright like rising sun.
Hair flows with a lifelike sheen,
Scattering lights in a radiant scene.
Thin films glow with hues so grand,
Rendering wonders, pixel by pixel, hand in hand.
🎨✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@hhhhkrx hhhhkrx changed the title Digtal humen shader Advanced Shader Jul 18, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Outside diff range, codebase verification and nitpick comments (1)
packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl (1)

6-31: Missing definition for FUNCTION_CLEAR_COAT_LOBE

The function or macro FUNCTION_CLEAR_COAT_LOBE is used in the surfaceShading_sss function but its definition could not be found in the repository. Please ensure that FUNCTION_CLEAR_COAT_LOBE is properly defined and imported.

  • File: packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl
  • Line: 19
Analysis chain

Review the surface shading calculation logic.

The function surfaceShading_sss calculates the surface shading using subsurface scattering and mixes the result with diffuse and specular components. The logic appears correct, but ensure SGDiffuseLighting and FUNCTION_CLEAR_COAT_LOBE functions are properly defined and imported.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definitions of `SGDiffuseLighting` and `FUNCTION_CLEAR_COAT_LOBE`.

# Test: Search for the function definitions. Expect: Definitions of the functions.
rg --type glsl -A 5 $'SGDiffuseLighting'
rg --type glsl -A 5 $'FUNCTION_CLEAR_COAT_LOBE'

Length of output: 144


Script:

#!/bin/bash
# Description: Locate GLSL files and search for the definitions of `SGDiffuseLighting` and `FUNCTION_CLEAR_COAT_LOBE`.

# Find all GLSL files in the repository
fd -e glsl > glsl_files.txt

# Search for the function definitions within the located GLSL files
xargs -a glsl_files.txt rg -A 5 'SGDiffuseLighting'
xargs -a glsl_files.txt rg -A 5 'FUNCTION_CLEAR_COAT_LOBE'

Length of output: 2152


Script:

#!/bin/bash
# Description: Search for the definition of `FUNCTION_CLEAR_COAT_LOBE` in the entire repository

# Search for the definition of FUNCTION_CLEAR_COAT_LOBE
rg -A 5 'FUNCTION_CLEAR_COAT_LOBE'

Length of output: 728

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7668eb6 and a689a9a.

Files selected for processing (16)
  • packages/custom-material/src/advanced-shader/eye/Eye.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/eye/EyeForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/eye/EyeFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/Hair.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairLightDirect.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSS.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceIndirectLight.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescencedirectLight.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/Iridescene.gs (1 hunks)
Additional comments not posted (52)
packages/custom-material/src/advanced-shader/thinfilm/IridescencedirectLight.glsl (3)

1-2: Ensure correct inclusion of headers.

The #define FUNCTION_SPECULAR_LOBE specularLobe_iridescence is correctly defined.


3-4: Include necessary headers.

The headers IridescenceFunction.glsl and LightDirectPBR.glsl are correctly included.


6-13: Review the specular lobe calculation logic.

The function specularLobe_iridescence calculates the specular lobe using iridescence and mixes the result with GGX specular BRDF. The logic appears correct, but ensure DirectBDRFIridescence and BRDF_Specular_GGX functions are properly defined and imported.

Verification successful

Verified: The functions DirectBDRFIridescence and BRDF_Specular_GGX are properly defined in the codebase.

  • DirectBDRFIridescence is defined in packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl.
  • BRDF_Specular_GGX is defined in packages/shaderlab/src/shaders/brdf.glsl.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definitions of `DirectBDRFIridescence` and `BRDF_Specular_GGX`.

# Test: Search for the function definitions. Expect: Definitions of the functions.
rg --type glsl -A 5 $'DirectBDRFIridescence'
rg --type glsl -A 5 $'BRDF_Specular_GGX'

Length of output: 141


Script:

#!/bin/bash
# Description: Verify the definitions of `DirectBDRFIridescence` and `BRDF_Specular_GGX`.

# Test: Search for the function definitions. Expect: Definitions of the functions.
rg -A 5 $'DirectBDRFIridescence'
rg -A 5 $'BRDF_Specular_GGX'

Length of output: 4392

packages/custom-material/src/advanced-shader/thinfilm/IridescenceIndirectLight.glsl (3)

1-2: Ensure correct inclusion of headers.

The #define FUNCTION_SPECULAR_IBL evaluateSpecularIBL_iridescence is correctly defined.


3-4: Include necessary headers.

The headers IridescenceFunction.glsl and LightIndirectPBR.glsl are correctly included.


6-17: Review the specular IBL calculation logic.

The function evaluateSpecularIBL_iridescence calculates the specular IBL using iridescence and mixes the result with an environment BRDF approximation. The logic appears correct, but ensure ThinFilmIridescence and envBRDFApprox functions are properly defined and imported.

Verification successful

Verification successful for ThinFilmIridescence and envBRDFApprox functions.

Both functions are properly defined and imported in the codebase:

  • ThinFilmIridescence is defined in packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl.
  • envBRDFApprox is defined in packages/shaderlab/src/shaders/ibl_frag_define.glsl.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definitions of `ThinFilmIridescence` and `envBRDFApprox`.

# Test: Search for the function definitions. Expect: Definitions of the functions.
rg --type glsl -A 5 $'ThinFilmIridescence'
rg --type glsl -A 5 $'envBRDFApprox'

Length of output: 135


Script:

#!/bin/bash
# Description: Verify the definitions of `ThinFilmIridescence` and `envBRDFApprox`.

# Test: Search for the function definitions. Expect: Definitions of the functions.
rg -A 5 'ThinFilmIridescence'
rg -A 5 'envBRDFApprox'

Length of output: 5382

packages/custom-material/src/advanced-shader/hair/HairFunction.glsl (3)

1-8: Ensure correct definition of material properties.

The material properties for hair rendering are correctly defined.


14-17: Review the tangent shifting function.

The function shiftTangent shifts the tangent vector correctly. Ensure that the input vectors are normalized.


19-28: Review the anisotropy specular calculation function.

The function anisotropySpecular calculates the anisotropic specular reflection using the Kajiya-Kay model. The logic appears correct.

packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl (2)

1-2: Ensure correct inclusion of headers.

The #define FUNCTION_SURFACE_SHADING surfaceShading_sss is correctly defined.


3-4: Include necessary headers.

The headers LightDirectPBR.glsl and SSSFunction.glsl are correctly included.

packages/custom-material/src/advanced-shader/hair/HairLightDirect.glsl (3)

6-29: Ensure texture sampling is correct.

The texture sampling logic appears correct, but ensure that material_HairAnisotropyTexture is properly defined and bound in the shader pipeline.


20-24: Ensure material properties are correctly defined.

The material properties material_HairFirstColor, material_HairSecondColor, material_HairFirstWidth, material_HairsFirststrength, material_HairSecondWidth, and material_HairsSecondstrength must be correctly defined and initialized in the shader pipeline.


26-28: Clamp function usage is appropriate.

The usage of clamp to limit the specular color values is appropriate to ensure they stay within the valid range.

packages/custom-material/src/advanced-shader/sss/SSSFunction.glsl (4)

14-42: Ensure correctness of vector operations and exponential calculations.

The vector operations and exponential calculations appear correct, but double-check the mathematical logic to ensure accuracy.


45-52: Ensure correctness of normalization calculations.

The normalization calculations appear correct, but double-check the mathematical logic to ensure accuracy.


54-62: Ensure correctness of vector operations and tone mapping.

The vector operations and tone mapping calculations appear correct, but double-check the mathematical logic to ensure accuracy.


59-61: Tone mapping logic is appropriate.

The tone mapping logic appears appropriate for ensuring the diffuse lobe values stay within the valid range.

packages/custom-material/src/advanced-shader/eye/Eye.gs (3)

2-27: Ensure correctness and completeness of material properties.

The material properties for the sclera and iris appear well-defined and appropriately named. Ensure that all necessary properties are included and correctly initialized.


30-39: Ensure correctness and completeness of conditional macros.

The conditional macros appear well-defined and appropriately named. Ensure that all necessary macros are included and correctly initialized.


43-58: Ensure correctness and completeness of shader passes.

The shader passes for the eye rendering appear well-defined and appropriately named. Ensure that all necessary passes are included and correctly initialized.

packages/custom-material/src/advanced-shader/thinfilm/IridescenceForwardPass.glsl (4)

15-55: Ensure correctness of UV and vertex color calculations.

The UV and vertex color calculations appear correct, but double-check the logic to ensure accuracy.


30-36: Ensure correctness of position calculations.

The position calculations for positionWS and positionVS appear correct, but double-check the logic to ensure accuracy.


57-84: Ensure correctness of surface data initialization and lighting calculations.

The surface data initialization and lighting calculations appear correct, but double-check the logic to ensure accuracy.


75-81: Tone mapping logic is appropriate.

The tone mapping logic appears appropriate for ensuring the color values stay within the valid range.

packages/custom-material/src/advanced-shader/sss/SSSForwardPass.glsl (2)

16-56: LGTM!

The PBRVertex function correctly initializes vertex attributes and varyings. The usage of macros and conditional compilation is appropriate.


58-84: LGTM!

The PBRFragment function correctly initializes surface and BRDF data, evaluates lighting, and sets the fragment color. Proper use of fog and color space conversion is present.

packages/custom-material/src/advanced-shader/thinfilm/Iridescene.gs (3)

3-40: LGTM!

The material properties for base color, thin film, metal roughness, normal, emissive, and occlusion are correctly defined with appropriate ranges and default values.


43-52: LGTM!

The editor macros correctly define conditional macros for various material properties.


55-70: LGTM!

The shader passes correctly define the usage of vertex and fragment shaders and include the iridescence forward pass.

packages/custom-material/src/advanced-shader/sss/SSS.gs (3)

3-40: LGTM!

The material properties for base color, SSS, metal roughness, normal, emissive, and occlusion are correctly defined with appropriate ranges and default values.


44-54: LGTM!

The editor macros correctly define conditional macros for various material properties.


57-71: LGTM!

The shader passes correctly define the usage of vertex and fragment shaders and include the SSS forward pass.

packages/custom-material/src/advanced-shader/hair/HairForwardPass.glsl (2)

16-56: LGTM!

The PBRVertex function correctly initializes vertex attributes and varyings. The usage of macros and conditional compilation is appropriate.


58-106: LGTM!

The PBRFragment function correctly initializes surface and BRDF data, evaluates lighting, and sets the fragment color. Proper use of fog and color space conversion is present.

packages/custom-material/src/advanced-shader/hair/Hair.gs (7)

4-6: Typographical Errors in Property Names

The property names in the "Base" section are consistent and correctly defined.


10-12: Typographical Errors in Property Names

The property names in the "Metal Roughness" section are consistent and correctly defined.


28-29: Typographical Errors in Property Names

The property names in the "Normal" section are consistent and correctly defined.


33-34: Typographical Errors in Property Names

The property names in the "Emissive" section are consistent and correctly defined.


38-45: Typographical Errors in Property Names

The property names in the "Occlusion" and "Common" sections are consistent and correctly defined.


51-58: Typographical Errors in Macro Names

The macro names in the "EditorMacros" section are consistent and correctly defined.


62-77: Typographical Errors in SubShader Definitions

The subshader definitions in the "SubShader" section are consistent and correctly defined.

packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl (6)

1-1: LGTM!

The sqr function is correctly defined.


3-6: LGTM!

The IorToFresnel0 function is correctly defined.


8-12: LGTM!

The Fresnel0ToIor function is correctly defined.


14-29: LGTM!

The EvalSensitivity function is correctly defined.


32-79: LGTM!

The ThinFilmIridescence function is correctly defined.


81-91: LGTM!

The DirectBDRFIridescence function is correctly defined.

packages/custom-material/src/advanced-shader/eye/EyeForwardPass.glsl (2)

16-56: LGTM!

The PBRVertex function is correctly defined.


58-115: LGTM!

The PBRFragment function is correctly defined.

packages/custom-material/src/advanced-shader/eye/EyeFunction.glsl (2)

31-38: LGTM!

The parallaxOffset function is correctly defined.


107-126: Typographical Errors in Property Names

There are several typographical errors in the property names:

  • material_Scleramask should be material_ScleraMask
- sampler2D material_Scleramask;
+ sampler2D material_ScleraMask;

Likely invalid or redundant comment.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a689a9a and 5e56686.

Files selected for processing (16)
  • packages/custom-material/src/advanced-shader/eye/Eye.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/eye/EyeForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/eye/EyeFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/Hair.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairLightDirect.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSS.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceIndirectLight.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescencedirectLight.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/Iridescene.gs (1 hunks)
Files skipped from review due to trivial changes (1)
  • packages/custom-material/src/advanced-shader/eye/EyeForwardPass.glsl
Files skipped from review as they are similar to previous changes (11)
  • packages/custom-material/src/advanced-shader/eye/Eye.gs
  • packages/custom-material/src/advanced-shader/eye/EyeFunction.glsl
  • packages/custom-material/src/advanced-shader/hair/Hair.gs
  • packages/custom-material/src/advanced-shader/hair/HairForwardPass.glsl
  • packages/custom-material/src/advanced-shader/hair/HairLightDirect.glsl
  • packages/custom-material/src/advanced-shader/sss/SSS.gs
  • packages/custom-material/src/advanced-shader/sss/SSSForwardPass.glsl
  • packages/custom-material/src/advanced-shader/sss/SSSFunction.glsl
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceForwardPass.glsl
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceIndirectLight.glsl
  • packages/custom-material/src/advanced-shader/thinfilm/Iridescene.gs
Additional comments not posted (18)
packages/custom-material/src/advanced-shader/thinfilm/IridescencedirectLight.glsl (4)

1-2: Define macro for function specification.

The macro FUNCTION_SPECULAR_LOBE is defined for specularLobe_iridescence.


3-4: Include necessary GLSL files.

The necessary GLSL files BRDF.glsl and IridescenceFunction.glsl are included.


6-13: Review shader function for iridescence.

The function specularLobe_iridescence calculates the specular lobe for iridescence. Ensure that the included functions DirectBDRFIridescence and BRDF_Specular_GGX are correctly implemented and referenced.

  • The function calculates the thin film iridescence using DirectBDRFIridescence.
  • It calculates the BRDF specular using BRDF_Specular_GGX.
  • The two results are mixed based on material_Iridescence.
  • The final specular color is updated with the attenuation irradiance and the mixed factor.

15-15: Include direct PBR lighting.

The file LightDirectPBR.glsl is included for direct PBR lighting calculations.

packages/custom-material/src/advanced-shader/hair/HairFunction.glsl (4)

1-8: Define material properties for hair rendering.

The material properties for hair rendering are defined, including widths, strengths, offsets, and colors for the first and second hair layers.


10-12: Conditional inclusion of hair anisotropy texture.

The hair anisotropy texture is conditionally included based on the MATERIAL_HAS_HAIRANISOTROPY_TEXTURE macro.


14-17: Define function to shift tangent vector.

The function shiftTangent shifts a tangent vector by a given amount along the normal vector. Ensure that the normalization is correctly implemented.


19-28: Define anisotropy specular function using Kajiya-Kay Model.

The function anisotropySpecular calculates the anisotropic specular reflection using the Kajiya-Kay model. Ensure that the calculations for the half-vector, dot product, and smoothstep are correctly implemented.

  • The view direction is retrieved from surfaceData.
  • The half-vector H is calculated.
  • The dot product and sine of the angle between the tangent and half-vector are computed.
  • The directional attenuation is calculated using smoothstep.
  • The final specular reflection is returned.
packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl (4)

1-2: Define macro for function specification.

The macro FUNCTION_SURFACE_SHADING is defined for surfaceShading_sss.


3-5: Include necessary GLSL files.

The necessary GLSL files BRDF.glsl, ReflectionLobe.glsl, and SSSFunction.glsl are included.


7-33: Review shader function for subsurface scattering (SSS).

The function surfaceShading_sss calculates the surface shading for subsurface scattering (SSS). Ensure that the included functions and logic are correctly implemented and referenced.

  • The diffuse and specular colors are initialized.
  • The curvature texture is conditionally sampled based on the MATERIAL_HAS_CURVATEXTURE macro.
  • The skin texture and scatter amount are calculated.
  • The subsurface scattering diffuse lighting is calculated using SGDiffuseLighting.
  • The irradiance is computed.
  • The clear coat lobe is calculated and its attenuation is applied to the irradiance.
  • The diffuse and specular lobes are calculated and added to the final color.

35-35: Include direct PBR lighting.

The file LightDirectPBR.glsl is included for direct PBR lighting calculations.

packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl (6)

8-10: LGTM!

The sqr function is simple and correctly implemented.


12-14: LGTM!

The IorToFresnel0 function uses the correct formula for converting the index of refraction to Fresnel reflectance at normal incidence.


16-19: LGTM!

The Fresnel0ToIor function uses the correct formula for converting Fresnel reflectance at normal incidence to the index of refraction.


21-36: LGTM! But verify the constants.

The EvalSensitivity function appears to be correctly implemented, but the constants used for Gaussian fits should be verified for correctness.


38-86: LGTM! But verify the overall logic.

The ThinFilmIridescence function appears to be correctly implemented, but the overall logic should be verified for correctness.


88-98: LGTM! But verify the calculations.

The DirectBDRFIridescence function appears to be correctly implemented, but the calculations should be verified for correctness.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5e56686 and 6186627.

Files selected for processing (5)
  • packages/custom-material/src/advanced-shader/eye/Eye.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/eye/EyeFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/Hair.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairLightDirect.glsl (1 hunks)
Files skipped from review as they are similar to previous changes (5)
  • packages/custom-material/src/advanced-shader/eye/Eye.gs
  • packages/custom-material/src/advanced-shader/eye/EyeFunction.glsl
  • packages/custom-material/src/advanced-shader/hair/Hair.gs
  • packages/custom-material/src/advanced-shader/hair/HairFunction.glsl
  • packages/custom-material/src/advanced-shader/hair/HairLightDirect.glsl

@GuoLei1990 GuoLei1990 changed the base branch from main to dev/1.3 July 19, 2024 14:39
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6186627 and 11315e3.

Files selected for processing (12)
  • packages/custom-material/src/advanced-shader/eye/Eye.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/eye/EyeFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/Hair.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairForwardPass.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/hair/HairLightDirect.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSS.gs (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceIndirectLight.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescencedirectLight.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/Iridescene.gs (1 hunks)
Files skipped from review as they are similar to previous changes (11)
  • packages/custom-material/src/advanced-shader/eye/Eye.gs
  • packages/custom-material/src/advanced-shader/eye/EyeFunction.glsl
  • packages/custom-material/src/advanced-shader/hair/HairForwardPass.glsl
  • packages/custom-material/src/advanced-shader/hair/HairLightDirect.glsl
  • packages/custom-material/src/advanced-shader/sss/SSS.gs
  • packages/custom-material/src/advanced-shader/sss/SSSFunction.glsl
  • packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceIndirectLight.glsl
  • packages/custom-material/src/advanced-shader/thinfilm/IridescencedirectLight.glsl
  • packages/custom-material/src/advanced-shader/thinfilm/Iridescene.gs
Additional comments not posted (8)
packages/custom-material/src/advanced-shader/hair/Hair.gs (8)

1-7: Base section properties look good.

The properties for IOR, BaseColor, and BaseTexture are correctly defined.


9-13: Metal Roughness section properties look good.

The properties for Metal, Roughness, and RoughnessMetallicTexture are correctly defined.


27-30: Normal section properties look good.

The properties for NormalTexture and NormalIntensity are correctly defined.


32-35: Emissive section properties look good.

The properties for EmissiveColor and EmissiveTexture are correctly defined.


37-41: Occlusion section properties look good.

The properties for OcclusionTexture, OcclusionIntensity, and OcclusionTextureCoord are correctly defined.


43-46: Common section properties look good.

The properties for AlphaCutoff and TilingOffset are correctly defined.


49-59: EditorMacros section looks good.

The conditional macros for various material properties are correctly defined.


62-77: SubShader section looks good.

The subshader with a default pass and a forward pass is correctly defined.

@GuoLei1990 GuoLei1990 added the enhancement New feature or request label Jul 19, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 11315e3 and d4f0b0d.

Files selected for processing (3)
  • packages/custom-material/src/advanced-shader/sss/SSSFunction.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl (1 hunks)
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • packages/custom-material/src/advanced-shader/sss/SSSFunction.glsl
  • packages/custom-material/src/advanced-shader/sss/SSSLightDirect.glsl
  • packages/custom-material/src/advanced-shader/thinfilm/IridescenceFunction.glsl

@GuoLei1990 GuoLei1990 merged commit 31f4bf1 into galacean:dev/1.3 Jul 19, 2024
1 check failed
This was referenced Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request shader
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants