Fix ColorWriteMask property type of BlendState in ShaderLab
#2651
Conversation
WalkthroughThe changes introduce the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Case
participant Parser as ShaderContentParser
participant Engine as Engine Types
Test->>Parser: Provide shader content with ColorWriteMask
Parser->>Engine: Resolve ColorWriteMask from _engineType
Engine-->>Parser: Return ColorWriteMask enum
Parser-->>Test: Use ColorWriteMask enum in parsed data
Suggested labels
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/zh/graphics/material/shaderLab/global.mdx (1)
35-35: UpdateBlendStatedocumentation to useColorWriteMaskenum
Changing the type annotation fromfloattoColorWriteMask.XXXaligns the docs with the runtime API.Consider adding an API link (e.g.,
[ColorWriteMask](/apis/galacean/#ColorWriteMask)) for consistency with other enum references.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/src/shader-lab/shaders/demo.shaderis excluded by!**/*.shader
📒 Files selected for processing (5)
docs/zh/graphics/material/shaderLab/global.mdx(1 hunks)packages/shader-lab/src/common/Keywords.ts(1 hunks)packages/shader-lab/src/contentParser/KeywordMap.ts(1 hunks)packages/shader-lab/src/contentParser/ShaderContentParser.ts(2 hunks)tests/src/shader-lab/ShaderLab.test.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: e2e (22.x)
- GitHub Check: codecov
- GitHub Check: build (22.x, windows-latest)
🔇 Additional comments (6)
packages/shader-lab/src/contentParser/KeywordMap.ts (1)
27-27: Add mapping for ColorWriteMask keyword
Mapping"ColorWriteMask"toEKeyword.GS_ColorWriteMaskaccurately introduces the new enum to the parser's keyword map.packages/shader-lab/src/common/Keywords.ts (1)
100-100: IntroduceGS_ColorWriteMaskenum member
AddingGS_ColorWriteMaskin theEKeywordenum enables recognition of theColorWriteMaskkeyword in parser and keyword mapping. Ensure its position aligns with the existing ordering of galacean-internal keywords.packages/shader-lab/src/contentParser/ShaderContentParser.ts (2)
17-17: ImportColorWriteMaskfrom engine package
IncludingColorWriteMaskin the import is necessary for the parser to reference the enum values at runtime.
63-63: IncludeColorWriteMaskin_engineTypemapping
AddingColorWriteMaskto the static_engineTypeensuresShaderContentParsercan resolveColorWriteMask.XXXreferences correctly.tests/src/shader-lab/ShaderLab.test.ts (2)
3-3: ImportColorWriteMaskinto test suite
Bringing inColorWriteMaskfrom@galacean/engine-coreis required to reference the enum in assertions.
191-191: AssertColorWriteMask.Noneinstead of numeric literal
Using the enum value improves readability and ensures test resilience against changes in underlying numeric values.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2651 +/- ##
==========================================
- Coverage 68.90% 68.90% -0.01%
==========================================
Files 961 961
Lines 100604 100615 +11
Branches 8715 8714 -1
==========================================
+ Hits 69322 69328 +6
- Misses 31022 31027 +5
Partials 260 260
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Fix the doc error and runtime implementation of
ColorWriteMask.BlendState state { Enabled: bool; ColorBlendOperation: BlendOperation.XXX; AlphaBlendOperation: BlendOperation.XXX; SourceColorBlendFactor: BlendFactor.XXX; SourceAlphaBlendFactor: BlendFactor.XXX; DestinationColorBlendFactor: BlendFactor.XXX; DestinationAlphaBlendFactor: BlendFactor.XXX; // should be ColorWriteMask.XXX; ColorWriteMask: float; BlendColor: vec4; AlphaToCoverage: bool; }TODO