Add pbr document #2523
Add pbr document #2523
Conversation
WalkthroughThis pull request involves significant documentation changes regarding Physically Based Rendering (PBR) in the graphics shader section. The existing 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 (
|
github-actions
bot
added
the
documentation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2523 +/- ##
==========================================
- Coverage 68.60% 68.58% -0.02%
==========================================
Files 957 957
Lines 100125 100125
Branches 8573 8568 -5
==========================================
- Hits 68687 68667 -20
- Misses 31182 31202 +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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
docs/en/graphics/shader/builtins/pbr.mdx (2)
1-4: Consider providing full English translationThe document currently mixes Chinese and English content. Consider providing a complete English version or maintaining separate language files for better accessibility to international users.
8-24: Enhance parameter documentation with code examplesWhile the parameter documentation is comprehensive, adding code snippets demonstrating the usage of each parameter would make it more practical for developers. Consider adding examples like:
const material = new PBRMaterial(); material.metallic = 0.8; material.roughness = 0.2;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/en/graphics/shader/builtins/pbr.md(0 hunks)docs/en/graphics/shader/builtins/pbr.mdx(1 hunks)docs/en/graphics/shader/builtins/thin.mdx(0 hunks)
💤 Files with no reviewable changes (2)
- docs/en/graphics/shader/builtins/thin.mdx
- docs/en/graphics/shader/builtins/pbr.md
|
|
||
| 我们调节材质的金属度，可以发现，金属度越大，周围的环境越清晰，并且开始从白色纯色变成彩色。这是因为电介质（即金属度为 1 时）材质会将光线 100% 全部反射出物体表面，即反射出彩色的周边环境： | ||
|
|
||
| <img src="https://gw.alipayobjects.com/zos/OasisHub/711f8b97-247c-465e-8cf2-4896b0c78534/metal.gif" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance image accessibility
While the documentation includes helpful visual examples, consider:
- Adding
alttext to all images for screen readers - Ensuring images have appropriate contrast ratios
- Providing text descriptions of the visual changes shown in animations
Example improvement:
<Image
+ alt="Demonstration of metallic parameter effect showing increasing reflection of environment"
src="https://gw.alipayobjects.com/zos/OasisHub/711f8b97-247c-465e-8cf2-4896b0c78534/metal.gif"
/>Also applies to: 49-51, 67-69, 89-91, 106-109, 120-122, 126-128, 142-145, 151-153
| | [glossiness](/apis/core/#PBRMaterial-glossiness) | 光泽度。模拟光滑程度，与粗糙度相反。(注，只有关闭金属粗糙工作流才生效)。 | | ||
| | [specularGlossinessTexture](/apis/core/#PBRMaterial-specularGlossinessTexture) | 高光光泽度纹理。搭配高光光泽度使用，是相乘的关系。 | | ||
|
|
||
| > **注**：PBR 必须开启[环境光](/docs/graphics-light-ambient) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move critical requirements to the beginning
The note about PBR requiring ambient light is crucial for functionality but is currently placed at the end of the document. Consider moving this requirement to the beginning of the document and expanding it to explain why ambient light is necessary for PBR.
| - 首先需要分别打开 Camera 下和场景中的 Opaque Texture： | ||
|
|
||
| <Image | ||
| src="https://mdn.alipayobjects.com/huamei_9ahbho/afts/img/A*_tBbRYNaBPMAAAAAAAAAAAAADgDwAQ/original" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make prerequisites more prominent
The requirement to enable Opaque Texture in both Camera and scene settings is crucial for transmission to work. Consider moving these prerequisites to the beginning of the Transmission section and highlighting them in a warning callout, similar to how you've handled other important notes.
+<Callout type="warning">
+Prerequisites for Transmission:
+- Enable Opaque Texture in Camera settings
+- Enable Opaque Texture in scene settings
+</Callout>Committable suggestion skipped: line range outside the PR's diff.
| @@ -0,0 +1,175 @@ | |||
| --- | |||
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
docs/en/graphics/shader/builtins/pbr.mdx (2)
4-4: Improve language clarity and consistency.Remove the hyphen in "physically-based" as it's redundant when using an adverb ending in "-ly".
-PBR incorporates energy conservation, is physically-based, and introduces IBL +PBR incorporates energy conservation, is physically based, and introduces IBL🧰 Tools
🪛 LanguageTool
[uncategorized] ~4-~4: The hyphen in physically-based is redundant.
Context: ...BR incorporates energy conservation, is physically-based, and introduces IBL (Image-Based Lighti...(ADVERB_LY_HYPHEN_FIX)
8-25: Enhance parameter table readability.The parameter table is well-structured but could benefit from consistent formatting:
- Some descriptions end with periods while others don't
- Some links use parentheses while others don't
Also, consider using more concise phrasing:
-Works in conjunction with metallic and roughness values +Works with metallic and roughness values -Used in conjunction with emissive color +Used with emissive color🧰 Tools
🪛 LanguageTool
[style] ~12-~12: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...re) | Metallic-Roughness Texture: Works in conjunction with metallic and roughness values, using a ...(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~13-~13: A determiner appears to be missing. Consider inserting it.
Context: ... Base Color Texture = Final Base Color. Base color represents the material's reflect...(AI_EN_LECTOR_MISSING_DETERMINER)
[style] ~17-~17: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...issiveTexture) | Emissive Texture: Used in conjunction with emissive color（[emissiveFactor](/apis/c...(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
docs/zh/graphics/shader/builtins/pbr.mdx (2)
4-4: Consider adding a table of contents.The document is quite extensive. Adding a table of contents would help users navigate through different sections more easily.
PBR 全称是 **Physically Based Rendering**，中文意思是**基于物理的渲染**，最早由迪士尼在 2012 年提出，后来被游戏界广泛使用。跟传统的 **Blinn-Phong** 等渲染方法相比，PBR 遵循能量守恒，符合物理规则，美术们只需要调整几个简单的参数，即使在复杂的场景中也能保证正确的渲染效果。PBR 遵循能量守恒，是基于物理的渲染，并且引入了 [IBL](/docs/graphics-light-ambient) 模拟全局光照，通过金属度、粗糙度等参数，更加方便地调节渲染效果。 +## 目录 + +- [PBR 基础参数介绍](#pbr-基础参数介绍) +- [PBRMaterial](#pbrmaterial) + - [Anisotropy](#anisotropy) + - [Sheen](#sheen) + - [Thin Film Iridescence](#thin-film-iridescence) + - [Transmission](#transmission) + - [Refraction](#refraction) +- [PBRSpecularMaterial](#pbrspecularmaterial)
8-19: Consider adding default values for parameters.The basic parameters table is well-structured, but it would be helpful to include default values for each parameter.
| 参数 | 应用 | | :-- | :-- | -| [metallic](/apis/core/#PBRMaterial-metallic) | 金属度。模拟材质的金属程度，金属值越大，镜面反射越强，即能反射更多周边环境。 | +| [metallic](/apis/core/#PBRMaterial-metallic) | 金属度。模拟材质的金属程度，金属值越大，镜面反射越强，即能反射更多周边环境。默认值：0。 | -| [roughness](/apis/core/#PBRMaterial-roughness) | 粗糙度。模拟材质的粗糙程度，粗糙度越大，微表面越不平坦，镜面反射越模糊。 | +| [roughness](/apis/core/#PBRMaterial-roughness) | 粗糙度。模拟材质的粗糙程度，粗糙度越大，微表面越不平坦，镜面反射越模糊。默认值：1。 |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/en/graphics/shader/builtins/pbr.mdx(1 hunks)docs/zh/graphics/shader/builtins/pbr.md(0 hunks)docs/zh/graphics/shader/builtins/pbr.mdx(1 hunks)docs/zh/graphics/shader/builtins/thin.mdx(0 hunks)
💤 Files with no reviewable changes (2)
- docs/zh/graphics/shader/builtins/thin.mdx
- docs/zh/graphics/shader/builtins/pbr.md
🧰 Additional context used
🪛 LanguageTool
docs/en/graphics/shader/builtins/pbr.mdx
[uncategorized] ~4-~4: The hyphen in physically-based is redundant.
Context: ...BR incorporates energy conservation, is physically-based, and introduces IBL (Image-Based Lighti...
(ADVERB_LY_HYPHEN_FIX)
[style] ~12-~12: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...re) | Metallic-Roughness Texture: Works in conjunction with metallic and roughness values, using a ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~13-~13: A determiner appears to be missing. Consider inserting it.
Context: ... Base Color Texture = Final Base Color. Base color represents the material's reflect...
(AI_EN_LECTOR_MISSING_DETERMINER)
[style] ~17-~17: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...issiveTexture) | Emissive Texture: Used in conjunction with emissive color（[emissiveFactor](/apis/c...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[duplication] ~37-~37: Possible typo: you repeated a word.
Context: ...rial)。 ## PBRMaterial #### Anisotropy Anisotropy refers to the material's property of re...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~95-~95: This phrasing could be wordy, so try replacing it with something more concise.
Context: ...e](/apis/core/#PBRMaterial-iridescence) is greater than 0, and enable the thin-film effect - Ad...
(MORE_THAN_EXCEEDS)
[uncategorized] ~174-~174: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... multiplied together. | > Note：PBR require [ambient lighting](/docs/graphics-light...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
⏰ 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/en/graphics/shader/builtins/pbr.mdx (3)
28-28: Add alt text for accessibility.The image demonstrating metallic property effects needs alt text for screen readers.
174-174: 🛠️ Refactor suggestionFix grammar and move critical requirement.
- Fix the verb agreement in the note about ambient lighting.
- Consider moving this critical requirement to a more prominent location, such as the beginning of the document.
-> **Note**：PBR require [ambient lighting](/docs/graphics-light-ambient) to be enabled. +> **Note**：PBR requires [ambient lighting](/docs/graphics-light-ambient) to be enabled.Likely invalid or redundant comment.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~174-~174: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... multiplied together. | > Note：PBR require [ambient lighting](/docs/graphics-light...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
104-109: 🛠️ Refactor suggestionMove transmission prerequisites to the beginning of the section.
The requirements for enabling Opaque Texture in both Camera and Scene settings are crucial for transmission to work. Consider moving these prerequisites to the beginning of the Transmission section.
#### Transmission <Callout type="info">Describes the portion of light passing through an object rather than being reflected or absorbed. This phenomenon occurs in transparent or translucent materials when part of the light passes through the surface.</Callout> +<Callout type="warning"> +Prerequisites for transmission: +- Enable Opaque Texture in Camera settings +- Enable Opaque Texture in Scene settings +</Callout> <ImageLikely invalid or redundant comment.
docs/zh/graphics/shader/builtins/pbr.mdx (2)
1-3: LGTM! Well-structured frontmatter.The MDX frontmatter is correctly formatted with the title "PBR".
174-175: LGTM! Good reference to additional resources.The link to the material usage tutorial provides a good next step for users.
| | [glossiness](/apis/core/#PBRMaterial-glossiness) | 光泽度。模拟光滑程度，与粗糙度相反。(注，只有关闭金属粗糙工作流才生效)。 | | ||
| | [specularGlossinessTexture](/apis/core/#PBRMaterial-specularGlossinessTexture) | 高光光泽度纹理。搭配高光光泽度使用，是相乘的关系。 | | ||
|
|
||
| > **注**：PBR 必须开启[环境光](/docs/graphics-light-ambient) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding more details about ambient light requirement.
The note about ambient light requirement is crucial but could be expanded to explain why it's necessary and how to properly set it up.
-> **注**：PBR 必须开启[环境光](/docs/graphics-light-ambient)
+> **注意**：PBR 必须开启[环境光](/docs/graphics-light-ambient)，因为：
+> - PBR 需要环境光来模拟全局光照
+> - 环境光提供了间接光照，对于金属和光滑表面的渲染效果尤为重要
+> - 没有环境光的场景中，PBR 材质可能显示异常或黑暗
+>
+> 请确保在使用 PBR 材质时：
+> 1. 在场景中添加环境光组件
+> 2. 设置适当的环境光强度
+> 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.
| > **注**：PBR 必须开启[环境光](/docs/graphics-light-ambient) | |
| > **注意**：PBR 必须开启[环境光](/docs/graphics-light-ambient)，因为： | |
| > - PBR 需要环境光来模拟全局光照 | |
| > - 环境光提供了间接光照，对于金属和光滑表面的渲染效果尤为重要 | |
| > - 没有环境光的场景中，PBR 材质可能显示异常或黑暗 | |
| > | |
| > 请确保在使用 PBR 材质时： | |
| > 1. 在场景中添加环境光组件 | |
| > 2. 设置适当的环境光强度 | |
| > 3. 提供环境贴图（推荐）以获得更真实的效果 |
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