Update shaderlab doc #2505
Update shaderlab doc #2505
Conversation
Sway007
added
documentation
WalkthroughThis pull request introduces comprehensive documentation updates for ShaderLab, focusing on shader syntax, editor properties, and Multi-Render Target (MRT) support. The changes include adding an Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/zh/graphics/shader/shaderLab/syntax/intro.mdx (1)
22-25: Consider adding a brief comment about the Editor block's purpose.While the placement is correct, adding a brief inline comment about the Editor block's purpose would help readers understand its role in the shader structure.
Editor { + // Customize material properties and Inspector UI ... }docs/zh/graphics/shader/shaderLab/syntax/pass.mdx (2)
106-112: Consider adding a note about gl_FragData limitations.While the example is correct, it would be helpful to mention that gl_FragData has a maximum array size limitation that varies by hardware.
116-127: Add performance considerations for MRT usage.The GLSL 300 example with structure is well-documented, but consider adding a note about performance implications of using multiple render targets.
docs/zh/graphics/shader/shaderLab/syntax/editor.mdx (2)
211-253: Add error handling in the UIScript example.The example would benefit from proper error handling for potential null values and type checking.
this.onPropertyChanged("material_NormalTexture", (material: Material, value) => { + if (!material?.shaderData) { + console.warn("Invalid material or shader data"); + return; + } const shaderData = material.shaderData; if (value) { shaderData.enableMacro("MATERIAL_HAS_NORMALTEXTURE"); } else { shaderData.disableMacro("MATERIAL_HAS_NORMALTEXTURE"); } })
257-260: Consider adding a code example for global variable declaration.The warning about global variables is important, but it would be more helpful with a concrete example showing how to properly declare the variables.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/zh/graphics/shader/shaderLab/syntax/_meta.json(1 hunks)docs/zh/graphics/shader/shaderLab/syntax/editor.mdx(1 hunks)docs/zh/graphics/shader/shaderLab/syntax/intro.mdx(1 hunks)docs/zh/graphics/shader/shaderLab/syntax/pass.mdx(1 hunks)docs/zh/graphics/shader/shaderLab/syntax/shader.mdx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (22.x, windows-latest)
- GitHub Check: e2e (22.x)
- GitHub Check: codecov
🔇 Additional comments (5)
docs/zh/graphics/shader/shaderLab/syntax/_meta.json (1)
4-4: LGTM!The addition of the "editor" entry properly maps to the new Editor documentation section.
docs/zh/graphics/shader/shaderLab/syntax/intro.mdx (1)
11-14: LGTM!The image source has been updated to use the WebP format, which provides better compression and faster loading times.
docs/zh/graphics/shader/shaderLab/syntax/pass.mdx (1)
101-104: LGTM! Clear introduction to MRT support.The introduction clearly states compatibility with both GLSL 100 and GLSL 300 syntax.
docs/zh/graphics/shader/shaderLab/syntax/shader.mdx (1)
21-21: LGTM! Clear reference to Editor documentation.The reference to the Editor directive is clear and properly links to the detailed documentation.
docs/zh/graphics/shader/shaderLab/syntax/editor.mdx (1)
7-37: LGTM! Well-structured Editor block example.The example clearly demonstrates the structure of Properties, Macros, and UIScript sections.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2505 +/- ##
==========================================
+ Coverage 69.01% 69.04% +0.02%
==========================================
Files 957 957
Lines 100078 100078
Branches 8560 8564 +4
==========================================
+ Hits 69070 69100 +30
+ Misses 30752 30722 -30
Partials 256 256
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
* fix: characterController create error
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
docs/zh/graphics/shader/shaderLab/syntax/pass.mdx (1)
102-127: LGTM! Comprehensive MRT documentationThe MRT documentation clearly explains both GLSL 100 and 300 syntax methods with proper examples.
Consider adding version-specific notes
It would be helpful to explicitly mention which GLSL version is recommended for different scenarios.
docs/en/graphics/shader/shaderLab/syntax/shader.mdx (2)
21-21: Add a direct link to Editor documentationConsider adding a direct link to the Editor documentation section for better navigation.
128-130: Add precision qualifier examplesConsider adding examples demonstrating when to use different precision qualifiers (lowp/mediump/highp).
docs/zh/graphics/shader/shaderLab/syntax/editor.mdx (2)
242-250: Improve readability and maintainability of angle calculations.The angle calculations use magic numbers and could be simplified. Consider extracting constants and using more descriptive variable names.
- const PI2 = Math.PI * 2; - const rotationRad = (Math.atan2(anisotropyInfo.y, anisotropyInfo.x) + PI2 ) % PI2; - shaderData.setFloat("anisotropyRotation", rotationRad * (180 / Math.PI)) + const TWO_PI = Math.PI * 2; + const RADIANS_TO_DEGREES = 180 / Math.PI; + const rotationRad = (Math.atan2(anisotropyInfo.y, anisotropyInfo.x) + TWO_PI) % TWO_PI; + const rotationDeg = rotationRad * RADIANS_TO_DEGREES; + shaderData.setFloat("anisotropyRotation", rotationDeg);
257-260: Consider adding a code example for global variable declaration.The warning about manual global variable declaration is important but could be more helpful with a concrete example.
Consider adding a code example showing how to properly declare global variables in the ShaderPass, like:
// In ShaderPass uniform vec4 material_BaseColor; uniform vec4 material_EmissiveColor;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/en/graphics/shader/shaderLab/syntax/_meta.json(1 hunks)docs/en/graphics/shader/shaderLab/syntax/editor.mdx(1 hunks)docs/en/graphics/shader/shaderLab/syntax/intro.mdx(2 hunks)docs/en/graphics/shader/shaderLab/syntax/pass.mdx(1 hunks)docs/en/graphics/shader/shaderLab/syntax/shader.mdx(1 hunks)docs/zh/graphics/shader/shaderLab/syntax/editor.mdx(1 hunks)docs/zh/graphics/shader/shaderLab/syntax/pass.mdx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e (22.x)
🔇 Additional comments (7)
docs/en/graphics/shader/shaderLab/syntax/_meta.json (1)
4-4: LGTM! Addition of Editor entryThe new entry follows the existing naming pattern and properly integrates with the documentation structure.
docs/en/graphics/shader/shaderLab/syntax/intro.mdx (1)
11-14: LGTM! Image format optimizationThe switch to WEBP format is a good optimization for web delivery.
docs/en/graphics/shader/shaderLab/syntax/pass.mdx (1)
102-129: LGTM! Consistent MRT documentationThe English documentation accurately mirrors the Chinese version with proper technical terminology.
docs/en/graphics/shader/shaderLab/syntax/shader.mdx (4)
33-46: LGTM! Comprehensive BlendState documentationThe BlendState properties are well-documented with clear parameter descriptions.
52-58: LGTM! Clear DepthState documentationThe DepthState properties are accurately documented with proper types.
64-79: LGTM! Detailed StencilState documentationThe StencilState documentation comprehensively covers all properties with proper types.
97-115: LGTM! Clear example with both assignment typesThe example effectively demonstrates both constant and variable assignments for BlendState.
| Editor { | ||
| ... | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Add link to Editor block documentation
The Editor block documentation exists at editor.mdx in the same directory, but there's no link to it from the introduction. Add a reference to help users find the detailed documentation.
🔗 Analysis chain
Verify Editor block documentation
The Editor block is introduced in the syntax framework, but ensure that there's a corresponding link to its detailed documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Editor documentation exists
fd -t f "editor.mdx" docs/en/graphics/shader/shaderLab/syntax/
Length of output: 114
Script:
#!/bin/bash
# Check intro.mdx for links to editor documentation
cat docs/en/graphics/shader/shaderLab/syntax/intro.mdx | grep -i "editor" -A 2 -B 2
Length of output: 272
| * @param material - The material which the shader is bound to | ||
| */ | ||
| onMaterialShaderSwitched(material: Material): void {} |
There was a problem hiding this comment.
Fix JSDoc parameter documentation.
The JSDoc comments use @parma instead of the correct @param tag. This should be fixed for better documentation tooling support.
/**
* Register property change callback.
- * @parma propertyName - Property name
- * @parma onChanged - Fired on property changed
+ * @param propertyName - Property name
+ * @param onChanged - Fired on property changed
*/
/**
* Register macro change callback.
- * @parma macroName - Macro name
- * @parma onChanged - Fired on macro changed
+ * @param macroName - Macro name
+ * @param onChanged - Fired on macro changed
*/Also applies to: 175-177, 181-183, 187-189
| ```glsl | ||
| Editor { | ||
| Properties { | ||
| material_BaseColor("Offset unit scale", Color) = (1,1,1,1); |
There was a problem hiding this comment.
Fix inconsistent property description.
The property name material_BaseColor suggests a base color property, but the description "Offset unit scale" seems incorrect and misleading. Consider updating the description to match the property's purpose, e.g., "Base Color".
- material_BaseColor("Offset unit scale", Color) = (1,1,1,1);
+ material_BaseColor("Base Color", Color) = (1,1,1,1);📝 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.
| material_BaseColor("Offset unit scale", Color) = (1,1,1,1); | |
| material_BaseColor("Base Color", Color) = (1,1,1,1); |
| /** | ||
| * @language zh | ||
| * Comments description | ||
| */ | ||
|
|
||
| /** | ||
| * @language en | ||
| * 注释描述 | ||
| */ |
There was a problem hiding this comment.
Fix swapped language descriptions in comments.
The multilingual comments have their descriptions in the wrong language sections. The Chinese description appears under the English tag and vice versa.
/**
* @language zh
- * Comments description
+ * 注释描述
*/
/**
* @language en
- * 注释描述
+ * Comments description
*/📝 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.
| /** | |
| * @language zh | |
| * Comments description | |
| */ | |
| /** | |
| * @language en | |
| * 注释描述 | |
| */ | |
| /** | |
| * @language zh | |
| * 注释描述 | |
| */ | |
| /** | |
| * @language en | |
| * Comments description | |
| */ |
| <Callout typ="warning"> | ||
| Currently, only Int and Float types support enumeration, and type mixing is not supported. For example, the following | ||
| enumeration mixes Float and Int, and will not be correctly parsed. ```glsl propertyName("Property Description", | ||
| Enum("Item1":1, "Item2":2.0, "Item3": 3)) = 2.0; ``` | ||
| </Callout> |
There was a problem hiding this comment.
Fix typo in Callout component attribute.
The Callout component has a typo in its type attribute (typ instead of type).
-<Callout typ="warning">
+<Callout type="warning">📝 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.
| <Callout typ="warning"> | |
| Currently, only Int and Float types support enumeration, and type mixing is not supported. For example, the following | |
| enumeration mixes Float and Int, and will not be correctly parsed. ```glsl propertyName("Property Description", | |
| Enum("Item1":1, "Item2":2.0, "Item3": 3)) = 2.0; ``` | |
| </Callout> | |
| <Callout type="warning"> | |
| Currently, only Int and Float types support enumeration, and type mixing is not supported. For example, the following |
Please check if the PR fulfills these requirements
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:
Summary by CodeRabbit
Documentation
EditordirectiveNew Features
ShaderUIScriptclass for material property and macro change callbacks