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

[Document] post-processing related #2511

Merged
merged 4 commits into from
Jan 24, 2025

Conversation

zhuxudong
Copy link
Member

@zhuxudong zhuxudong commented Jan 15, 2025

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:

Summary by CodeRabbit

  • Documentation

    • Added postProcessMask property to camera component documentation in English and Chinese
    • Introduced new documentation for custom post-processing effects in English and Chinese
    • Updated shader API documentation with package reference changes and new BTDF section
    • Restructured post-processing configuration documentation with clearer modes and explanations
  • Refactor

    • Updated image rendering with custom <Image> component
    • Improved formatting of documentation tables and code snippets

@zhuxudong zhuxudong self-assigned this Jan 15, 2025
Copy link

coderabbitai bot commented Jan 15, 2025

Walkthrough

This pull request introduces documentation updates for camera and post-processing features across English and Chinese documentation. The changes include adding a new postProcessMask property to camera component documentation, creating new documentation for custom post-processing effects, updating post-processing configuration sections, and refactoring shader API documentation with package reference updates.

Changes

File Path Change Summary
docs/en/graphics/camera/component.md Added postProcessMask property to camera component documentation
docs/en/graphics/postProcess/customPostProcess.mdx New documentation for custom post-processing effects, introducing CustomPass and CustomEffect classes
docs/en/graphics/postProcess/postProcess.mdx Restructured post-processing configuration sections, renamed headings, added mode descriptions
docs/en/graphics/shader/shaderAPI.mdx Updated package references from @galacean/engine-toolkit to @galacean/engine-shader-shaderlab
docs/zh/* (corresponding Chinese files) Mirrored changes from English documentation

Sequence Diagram

sequenceDiagram
    participant Camera
    participant PostProcessMask
    participant PostProcessEffect
    participant CustomPass
    
    Camera->>PostProcessMask: Configure post-processing mask
    PostProcessMask-->>PostProcessEffect: Determine active effects
    PostProcessEffect->>CustomPass: Apply custom rendering
    CustomPass->>Camera: Render modified scene
Loading

Possibly related PRs

Suggested Labels

post processing

Suggested Reviewers

  • GuoLei1990
  • cptbtptpbcptdtptp

Poem

🐰 Hop, hop, through shader's light,
Post-processing now shines so bright!
Masks and passes, effects galore,
Camera's magic we now explore!
Rendering dreams with rabbit's might! 🎨


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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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 generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

@zhuxudong zhuxudong added the documentation Improvements or additions to documentation label Jan 15, 2025
@zhuxudong zhuxudong added this to the 1.4 milestone Jan 15, 2025
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.58%. Comparing base (9ae8641) to head (606778b).
Report is 35 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2511      +/-   ##
==========================================
+ Coverage   68.52%   68.58%   +0.06%     
==========================================
  Files         957      957              
  Lines      100041   100125      +84     
  Branches     8544     8572      +28     
==========================================
+ Hits        68555    68675     +120     
+ Misses      31230    31194      -36     
  Partials      256      256              
Flag Coverage Δ
unittests 68.58% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

🧹 Nitpick comments (9)
docs/zh/graphics/postProcess/postProcess.mdx (1)

44-44: Consider revising the Chinese grammar.

The phrase "具体的后处理" should be "具体地后处理" as it modifies a verb.

-具体的后处理效果配置,请参考 [后处理效果列表]
+具体地后处理效果配置,请参考 [后处理效果列表]
🧰 Tools
🪛 LanguageTool

[uncategorized] ~44-~44: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:具体"地"后处理
Context: ...4.51.12.gif" /> 具体的后处理效果配置,请参考 [后处理效果列表](/docs/graphics/pos...

(wb4)

docs/en/graphics/postProcess/postProcess.mdx (1)

57-57: Standardize the spelling of "anti-aliasing".

The term "anti-aliasing" should be spelled without a hyphen as "antialiasing" for consistency with technical documentation standards.

-multi-sampling anti-aliasing settings
+multi-sampling antialiasing settings

Also applies to: 77-77

🧰 Tools
🪛 LanguageTool

[misspelling] ~57-~57: This word is normally spelled as one.
Context: ...aaSamples) | Adjusts the multi-sampling anti-aliasing settings to improve image quality, such...

(EN_COMPOUNDS_ANTI_ALIASING)

docs/en/graphics/postProcess/customPostProcess.mdx (5)

6-10: Consider enhancing the introduction with use cases.

While the technical explanation is clear, it would be helpful to add examples of when custom post-processing might be needed (e.g., night vision effects, color grading, etc.) to help readers understand the practical applications.


45-45: Add comments explaining the grayscale coefficients.

The line uses standard luminance coefficients (0.299, 0.587, 0.114) for RGB to grayscale conversion. Consider adding a comment explaining these coefficients and why they're used instead of a simple average.


65-67: Consider adding error handling for texture operations.

The onRender method should include error handling for cases where srcTexture might be null or invalid, as this could happen in edge cases during scene transitions or resource loading.


102-103: Consider using a lower default intensity value.

The default intensity of 0.8 might be too strong for most use cases. Consider using a more moderate default value (e.g., 0.5) to provide a better out-of-the-box experience.


141-141: Consider adding a performance optimization note.

The mix operation is performed on every pixel. For large viewports, you might want to add a note about the potential performance impact and suggest optimizations (e.g., using a lower resolution render target for the effect).

docs/en/graphics/camera/component.md (1)

84-84: Enhance postProcessMask property documentation.

Consider adding:

  1. Examples of common mask values
  2. How to combine multiple masks
  3. Default value and behavior
docs/en/core/scene.md (1)

157-159: Remove empty code block.

The empty code block at the end of the file serves no purpose and should be removed.

🧰 Tools
🪛 Markdownlint (0.37.0)

158-158: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ae8641 and f001ed6.

📒 Files selected for processing (8)
  • docs/en/core/scene.md (5 hunks)
  • docs/en/graphics/camera/component.md (1 hunks)
  • docs/en/graphics/postProcess/customPostProcess.mdx (1 hunks)
  • docs/en/graphics/postProcess/postProcess.mdx (1 hunks)
  • docs/zh/core/scene.md (4 hunks)
  • docs/zh/graphics/camera/component.md (1 hunks)
  • docs/zh/graphics/postProcess/customPostProcess.mdx (1 hunks)
  • docs/zh/graphics/postProcess/postProcess.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/graphics/postProcess/customPostProcess.mdx

[style] ~25-~25: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... The algorithm is not special, but you need to pay attention to renderer_BlitTexture...

(REP_NEED_TO_VB)


[style] ~80-~80: This phrasing could be wordy, so try replacing it with something more concise.
Context: ...ss logic, such as whether the intensity is greater than 0: ```ts showLineNumbers {2-9} class C...

(MORE_THAN_EXCEEDS)


[style] ~99-~99: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...intensity. The data to be fused is also very simple. The engine has encapsulated a series o...

(EN_WEAK_ADJECTIVE)


[uncategorized] ~99-~99: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...of post-processing parameters, such as [floating point type parameters](/apis/core/#PostProces...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

docs/zh/graphics/postProcess/postProcess.mdx

[uncategorized] ~44-~44: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:具体"地"后处理
Context: ...4.51.12.gif" /> 具体的后处理效果配置,请参考 [后处理效果列表](/docs/graphics/pos...

(wb4)

docs/en/graphics/postProcess/postProcess.mdx

[misspelling] ~57-~57: This word is normally spelled as one.
Context: ...aaSamples) | Adjusts the multi-sampling anti-aliasing settings to improve image quality, such...

(EN_COMPOUNDS_ANTI_ALIASING)


[misspelling] ~77-~77: This word is normally spelled as one.
Context: ...ost-processing is enabled and strict on anti-aliasing performance. The higher the value, the ...

(EN_COMPOUNDS_ANTI_ALIASING)

🪛 Markdownlint (0.37.0)
docs/en/core/scene.md

158-158: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: build (22.x, windows-latest)
  • GitHub Check: codecov
  • GitHub Check: e2e (22.x)
🔇 Additional comments (9)
docs/zh/graphics/postProcess/postProcess.mdx (3)

6-6: Improved clarity by focusing on camera rendering.

The description now correctly emphasizes that post-processing operates on camera-rendered results rather than scene-rendered results, which is more accurate and aligns better with the actual implementation.


8-12: Enhanced visual comparison with before/after examples.

The use of the <Comparison> component effectively demonstrates the impact of post-processing by showing side-by-side comparisons.


19-20: Well-structured documentation of post-processing modes and properties.

The documentation clearly explains the two modes (global and local) and provides a comprehensive table of properties with detailed descriptions and API links.

Also applies to: 26-30

docs/zh/core/scene.md (2)

22-22: Improved image reference with proper styling.

The image reference includes proper styling attributes and alt text, enhancing accessibility and presentation.


44-45: Enhanced table formatting for better readability.

The table structure has been improved with consistent formatting and clear column headers.

Also applies to: 48-49

docs/en/graphics/postProcess/postProcess.mdx (1)

76-79: Well-structured best practices section.

The best practices section provides clear, actionable recommendations with performance implications and alternatives.

🧰 Tools
🪛 LanguageTool

[misspelling] ~77-~77: This word is normally spelled as one.
Context: ...ost-processing is enabled and strict on anti-aliasing performance. The higher the value, the ...

(EN_COMPOUNDS_ANTI_ALIASING)

docs/zh/graphics/postProcess/customPostProcess.mdx (2)

27-49: Well-structured shader implementation example.

The shader code example is clear and includes helpful comments about the built-in renderer_BlitTexture variable.


82-92: Good example of custom validation logic.

The code demonstrates how to implement custom validation logic for post-processing effects, including checking both the active state and effect intensity.

docs/zh/graphics/camera/component.md (1)

84-84: Well-documented new postProcessMask property.

The new property is properly documented with a clear description and links to relevant documentation.

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

🧹 Nitpick comments (8)
docs/en/graphics/postProcess/postProcess.mdx (5)

15-30: Well-structured configuration section with comprehensive property documentation!

The clear distinction between modes and the property table greatly improves readability. Consider adding example values or default values to the property table for even better clarity.


32-47: Consider enhancing the mixing rules example with visual aids.

While the explanation is thorough, consider:

  1. Adding a diagram showing the blending transition in local mode
  2. Formatting the mixing formula using math notation (e.g., using LaTeX or similar)

66-79: Excellent camera configuration documentation with a minor formatting suggestion.

The property table provides clear descriptions with proper API links. Consider standardizing the term "anti-aliasing" to "antialiasing" throughout the documentation for consistency.

🧰 Tools
🪛 LanguageTool

[misspelling] ~74-~74: This word is normally spelled as one.
Context: ...aaSamples) | Adjusts the multi-sampling anti-aliasing settings to improve image quality, such...

(EN_COMPOUNDS_ANTI_ALIASING)


81-89: Clear viewport documentation with an important warning.

Consider adding a comma after "independent" in the sentence: "The camera in the viewport is independent, but has post-processing configurations..."

🧰 Tools
🪛 LanguageTool

[uncategorized] ~83-~83: Possible missing comma found.
Context: ...he camera in the viewport is independent but has post-processing configurations simi...

(AI_HYDRA_LEO_MISSING_COMMA)


91-96: Excellent performance optimization guidelines!

Consider enhancing this section by:

  1. Adding approximate performance impact numbers where possible
  2. Including device-specific recommendations (e.g., which settings work best for different mobile tiers)
🧰 Tools
🪛 LanguageTool

[misspelling] ~94-~94: This word is normally spelled as one.
Context: ...ost-processing is enabled and strict on anti-aliasing performance. The higher the value, the ...

(EN_COMPOUNDS_ANTI_ALIASING)

docs/zh/graphics/postProcess/postProcess.mdx (3)

17-31: Comprehensive configuration documentation with clear API references!

The two-mode explanation and property table provide excellent technical detail. Consider adding a code example demonstrating the configuration of these properties in practice.


34-44: Clear explanation of blending rules with practical examples!

The blending behavior is well-explained with a practical example. Consider adding a visual diagram to illustrate the blending distance concept for better understanding.


59-59: Fix grammatical structure in Chinese text.

Change "具体的后处理" to "具体地后处理" to follow the correct Chinese grammar pattern for adverbial modification.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~59-~59: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:具体"地"后处理
Context: ...4.51.12.gif" /> 具体的后处理效果配置,请参考 [后处理效果列表](/docs/graphics/pos...

(wb4)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f001ed6 and 429b1da.

📒 Files selected for processing (2)
  • docs/en/graphics/postProcess/postProcess.mdx (1 hunks)
  • docs/zh/graphics/postProcess/postProcess.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/graphics/postProcess/postProcess.mdx

[misspelling] ~74-~74: This word is normally spelled as one.
Context: ...aaSamples) | Adjusts the multi-sampling anti-aliasing settings to improve image quality, such...

(EN_COMPOUNDS_ANTI_ALIASING)


[uncategorized] ~83-~83: Possible missing comma found.
Context: ...he camera in the viewport is independent but has post-processing configurations simi...

(AI_HYDRA_LEO_MISSING_COMMA)


[misspelling] ~94-~94: This word is normally spelled as one.
Context: ...ost-processing is enabled and strict on anti-aliasing performance. The higher the value, the ...

(EN_COMPOUNDS_ANTI_ALIASING)

docs/zh/graphics/postProcess/postProcess.mdx

[uncategorized] ~59-~59: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:具体"地"后处理
Context: ...4.51.12.gif" /> 具体的后处理效果配置,请参考 [后处理效果列表](/docs/graphics/pos...

(wb4)

⏰ 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/postProcess/postProcess.mdx (2)

6-12: Great improvements to the introduction and visual comparison!

The terminology change is more precise, and the new Comparison component provides a clearer before/after visualization of post-processing effects.


49-63: Excellent component addition guide with visual demonstrations!

The combination of text instructions and GIF demonstrations provides clear guidance for both preset and manual component addition.

docs/zh/graphics/postProcess/postProcess.mdx (3)

6-14: Well-structured introduction with effective visual comparison!

The terminology change from "场景渲染" to "相机渲染" provides better technical accuracy, and the before/after comparison effectively demonstrates the visual impact of post-processing.


49-72: Excellent usage guide with clear visual demonstrations!

The step-by-step instructions and property explanations are comprehensive and well-documented with supporting visuals.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~59-~59: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:具体"地"后处理
Context: ...4.51.12.gif" /> 具体的后处理效果配置,请参考 [后处理效果列表](/docs/graphics/pos...

(wb4)


74-80: Clear viewport documentation with important export behavior warning!

The section effectively explains viewport-specific settings and properly warns users about the distinction between viewport display and exported results.

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

🧹 Nitpick comments (4)
docs/zh/graphics/shader/shaderAPI.mdx (1)

339-340: Documentation improvement suggestion.

Consider adding more details about the BRDF structures (SurfaceData and BRDFData) to help developers understand their purpose and usage better.

docs/en/graphics/shader/shaderAPI.mdx (3)

281-285: LGTM! Consider enhancing function documentation.

The updated function signatures improve the lighting model's flexibility by separating diffuse and specular components. This is a good improvement.

Consider adding brief descriptions for the new output parameters:

  • totalDiffuseColor: Purpose and expected range
  • totalSpecularColor: Purpose and expected range

344-346: Enhance BTDF section documentation.

While the section provides basic information, consider enhancing it with:

  • Brief explanation of BTDF (Bidirectional Transmittance Distribution Function)
  • Common use cases
  • Example usage code

350-350: Improve writing clarity.

Replace "a large number of" with "numerous" or specify the exact number of variables for better precision.

🧰 Tools
🪛 LanguageTool

[style] ~350-~350: Specify a number, remove phrase, or simply use “many” or “numerous”
Context: .../BTDF.glsl)。 ### FragmentPBR Contains a large number of variables passed from the CPU, such as ...

(LARGE_NUMBER_OF)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 429b1da and e188ede.

📒 Files selected for processing (2)
  • docs/en/graphics/shader/shaderAPI.mdx (8 hunks)
  • docs/zh/graphics/shader/shaderAPI.mdx (8 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/graphics/shader/shaderAPI.mdx

[style] ~350-~350: Specify a number, remove phrase, or simply use “many” or “numerous”
Context: .../BTDF.glsl)。 ### FragmentPBR Contains a large number of variables passed from the CPU, such as ...

(LARGE_NUMBER_OF)

⏰ 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 (11)
docs/zh/graphics/shader/shaderAPI.mdx (9)

6-7: Package name update looks good!

The package name has been correctly updated from @galacean/engine-toolkit to @galacean/engine-shader-shaderlab in the warning callout.


151-151: Source code link update is accurate.

The Common.glsl source code link has been properly updated to point to the new repository path.


215-215: Source code link update is accurate.

The Shadow.glsl source code link has been properly updated to point to the new repository path.


252-252: Source code link update is accurate.

The AttributesPBR.glsl source code link has been properly updated to point to the new repository path.


256-256: Source code link update is accurate.

The VaryingsPBR.glsl source code link has been properly updated to point to the new repository path.


276-282: Function signature updates and additions look good!

The following changes have been properly implemented:

  1. Added sheen lobe function definition
  2. Updated surfaceShading function signature with separate diffuse and specular color outputs
  3. Added sheenLobe function signature with proper parameters

304-309: Function signature updates and additions look good!

The following changes have been properly implemented:

  1. Added sheen IBL function definition
  2. Added evaluateSheenIBL function signature with proper parameters

341-343: New BTDF section is well-structured.

The new section about transmission and refraction functions has been properly added with appropriate source code reference.


361-361: Documentation reference is helpful.

Good addition of the reference to ForwardPassPBR for understanding file organization.

docs/en/graphics/shader/shaderAPI.mdx (2)

152-152: Verify the validity of updated source code URLs.

All shader source file URLs have been updated to point to the new package structure. Let's verify these URLs are accessible.

Also applies to: 216-216, 253-253, 257-257, 261-261, 292-292, 317-317, 342-342, 350-350

✅ Verification successful

URLs are valid and point to correct file locations

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the shader files exist in the new locations
fd -e glsl -p -I "Common.glsl|Shadow.glsl|AttributesPBR.glsl|VaryingsPBR.glsl|LightDirectPBR.glsl|LightIndirectPBR.glsl|VertexPBR.glsl|BRDF.glsl|FragmentPBR.glsl" packages/shader-shaderlab/src/shaders/

Length of output: 766


6-8: Verify package references across the codebase.

The package name has been updated from @galacean/engine-toolkit to @galacean/engine-shader-shaderlab. Let's verify this change is consistent across the codebase.

✅ Verification successful

Package reference update is correct

The change from @galacean/engine-toolkit to @galacean/engine-shader-shaderlab in the shader documentation is correct. The new package appears to be a specialized package for shader functionality, while the old package references in other files are for different toolkit features.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining references to the old package name
rg -l "@galacean/engine-toolkit"

# Search for the new package name to ensure consistent usage
rg -l "@galacean/engine-shader-shaderlab"

Length of output: 4102

docs/zh/core/scene.md Outdated Show resolved Hide resolved
docs/zh/core/scene.md Show resolved Hide resolved
docs/zh/graphics/postProcess/customPostProcess.mdx Outdated Show resolved Hide resolved
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

🔭 Outside diff range comments (1)
docs/zh/core/scene.mdx (1)

Line range hint 8-10: Fix syntax error in code example.

The code example has a syntax error in the variable declaration and uses incorrect operation.

-const scene1, scene2;
+const scene1 = /* scene instance */;
+const scene2 = /* scene instance */;

-  z = x - y
+  return x + y
🧹 Nitpick comments (4)
docs/zh/core/scene.mdx (1)

62-64: Fix inconsistent variable usage in code example.

The code example uses a string literal "..." instead of the declared sceneUrl variable.

-engine.resourceManager.load({ type: AssetType.Scene, url: "..." }).then((scene) => {
+engine.resourceManager.load({ type: AssetType.Scene, url: sceneUrl }).then((scene) => {
docs/en/core/scene.mdx (1)

157-159: Remove empty code blocks.

These empty code blocks serve no purpose and should be removed.

docs/zh/graphics/postProcess/customPostProcess.mdx (1)

71-75: Optimize grayscale calculation in shader.

The grayscale calculation could be optimized using built-in GLSL functions.

-    vec4 color = texture2D(renderer_BlitTexture, v_uv);
-    float grayScale = 0.299 * color.r + 0.587 * color.g + 0.114 * color.b;
-    gl_FragColor = vec4(vec3(grayScale), 1.0);
+    vec4 color = texture2D(renderer_BlitTexture, v_uv);
+    float grayScale = dot(color.rgb, vec3(0.299, 0.587, 0.114));
+    gl_FragColor = vec4(vec3(grayScale), 1.0);
docs/en/graphics/postProcess/customPostProcess.mdx (1)

53-53: Improve documentation clarity.

Consider rephrasing to avoid repetition and improve clarity:
"The implementation is straightforward, with renderer_BlitTexture serving as the built-in variable that contains the post-processing result from the previous Pass (Bloom and Tonemapping in this case)."

🧰 Tools
🪛 LanguageTool

[style] ~53-~53: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... The algorithm is not special, but you need to pay attention to renderer_BlitTexture...

(REP_NEED_TO_VB)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e188ede and 606778b.

📒 Files selected for processing (4)
  • docs/en/core/scene.mdx (5 hunks)
  • docs/en/graphics/postProcess/customPostProcess.mdx (1 hunks)
  • docs/zh/core/scene.mdx (4 hunks)
  • docs/zh/graphics/postProcess/customPostProcess.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/graphics/postProcess/customPostProcess.mdx

[style] ~53-~53: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... The algorithm is not special, but you need to pay attention to renderer_BlitTexture...

(REP_NEED_TO_VB)


[style] ~108-~108: This phrasing could be wordy, so try replacing it with something more concise.
Context: ...ss logic, such as whether the intensity is greater than 0: ```ts showLineNumbers {2-9} filenam...

(MORE_THAN_EXCEEDS)


[style] ~127-~127: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...intensity. The data to be fused is also very simple. The engine has encapsulated a series o...

(EN_WEAK_ADJECTIVE)


[uncategorized] ~127-~127: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...of post-processing parameters, such as [floating point type parameters](/apis/core/#PostProces...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: codecov
  • GitHub Check: e2e (22.x)
🔇 Additional comments (2)
docs/en/core/scene.mdx (1)

62-64: Fix inconsistent variable usage in code example.

The code example uses a string literal "..." instead of the declared sceneUrl variable.

docs/en/graphics/postProcess/customPostProcess.mdx (1)

71-75: Optimize grayscale calculation in shader.

The grayscale calculation could be optimized using built-in GLSL functions.

Comment on lines +93 to +95
onRender(_, srcTexture: Texture2D, dst: RenderTarget): void {
Blitter.blitTexture(this.engine, srcTexture, dst, undefined, undefined, this._blitMaterial, 0);
}
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 error handling for texture operations.

The onRender method should validate the input texture before processing.

   onRender(_, srcTexture: Texture2D, dst: RenderTarget): void {
+    if (!srcTexture || !dst) {
+      console.warn("Invalid texture or render target in CustomPass.onRender");
+      return;
+    }
     Blitter.blitTexture(this.engine, srcTexture, dst, undefined, undefined, this._blitMaterial, 0);
   }
📝 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
onRender(_, srcTexture: Texture2D, dst: RenderTarget): void {
Blitter.blitTexture(this.engine, srcTexture, dst, undefined, undefined, this._blitMaterial, 0);
}
onRender(_, srcTexture: Texture2D, dst: RenderTarget): void {
if (!srcTexture || !dst) {
console.warn("Invalid texture or render target in CustomPass.onRender");
return;
}
Blitter.blitTexture(this.engine, srcTexture, dst, undefined, undefined, this._blitMaterial, 0);
}

@eyworldwide eyworldwide merged commit 89f3ffd into galacean:main Jan 24, 2025
9 checks passed
This was referenced Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants