-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Update shaderlab doc #2505
base: dev/1.4
Are you sure you want to change the base?
Update shaderlab doc #2505
Conversation
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
Sequence DiagramsequenceDiagram
participant Developer
participant ShaderEditor
participant Material
participant UIScript
Developer->>ShaderEditor: Define Editor Properties
ShaderEditor->>Material: Configure Material Inspector
Developer->>UIScript: Implement ShaderUIScript
UIScript->>Material: Register Property/Macro Callbacks
Material-->>UIScript: Trigger Callbacks on 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 (
|
There was a problem hiding this 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
🧹 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 @@
## dev/1.4 #2505 +/- ##
===========================================
- Coverage 68.53% 68.51% -0.02%
===========================================
Files 957 957
Lines 100057 100057
Branches 8552 8547 -5
===========================================
- Hits 68575 68555 -20
- Misses 31226 31246 +20
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. |
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
Editor
directiveNew Features
ShaderUIScript
class for material property and macro change callbacks