Delete msaa in bloom and RenderBufferStoreAction #2170
Conversation
WalkthroughThe recent update focuses on simplifying the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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
Outside diff range, codebase verification and nitpick comments (1)
packages/core/src/RenderPipeline/PipelineUtils.ts (1)
161-162: EnsureblitTexturecalls reflect the new signature:The following calls to
blitTextureneed to be updated to match the new signature:
- packages/core/src/postProcess/PostProcessManager.ts
PipelineUtils.blitTexture(engine, srcTexture, destTarget, 0, camera.viewport, this._uberMaterial);- packages/core/src/postProcess/effects/BloomEffect.ts
PipelineUtils.blitTexture(engine, srcTexture, this._mipDownRT[0], undefined, undefined, this._bloomMaterial, 0);- Multiple other instances in
BloomEffect.ts- packages/core/src/RenderPipeline/BasicRenderPipeline.ts
PipelineUtils.blitTexture(engine, <Texture2D>internalColorTarget.getColorTexture(0), cameraRenderTarget, 0, camera.viewport);- packages/core/src/RenderPipeline/OpaqueTexturePass.ts
PipelineUtils.blitTexture(this.engine, <Texture2D>this._cameraColorTexture, this._renderTarget);Please update these calls to ensure they match the new method signature without the
renderBufferStoreActionparameter.Analysis chain
Change Approved: Simplified Method Signature
The
renderBufferStoreActionparameter has been removed from theblitTexturemethod, simplifying its signature and internal logic.Ensure that all calls to
blitTextureare updated to reflect this change.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `blitTexture` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type js -A 5 $'PipelineUtils.blitTexture'Length of output: 45
Script:
#!/bin/bash # Description: Verify all function calls to `blitTexture` match the new signature. # Search for calls to the `blitTexture` method across the codebase. rg --type ts 'blitTexture\(' -A 5Length of output: 3835
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/core/src/RenderPipeline/PipelineUtils.ts (4 hunks)
- packages/core/src/RenderPipeline/index.ts (1 hunks)
- packages/core/src/postProcess/effects/BloomEffect.ts (8 hunks)
Additional comments not posted (7)
packages/core/src/RenderPipeline/index.ts (1)
3-3: Change Approved: Removed ExportThe export of
RenderBufferStoreActionhas been removed, which aligns with the PR's objective to delete its usage.packages/core/src/postProcess/effects/BloomEffect.ts (6)
265-265: Change Approved: Simplified Texture CreationThe
msaaSamplesparameter has been set to 1, simplifying the texture creation logic.
278-278: Change Approved: Simplified Texture CreationThe
msaaSamplesparameter has been set to 1, simplifying the texture creation logic.
287-287: Change Approved: RemovedRenderBufferStoreActionThe
RenderBufferStoreActionparameter has been removed from theblitTexturemethod call, aligning with the PR's objective.
307-307: Change Approved: RemovedRenderBufferStoreActionThe
RenderBufferStoreActionparameter has been removed from theblitTexturemethod call, aligning with the PR's objective.
316-316: Change Approved: RemovedRenderBufferStoreActionThe
RenderBufferStoreActionparameter has been removed from theblitTexturemethod call, aligning with the PR's objective.
340-340: Change Approved: RemovedRenderBufferStoreActionThe
RenderBufferStoreActionparameter has been removed from theblitTexturemethod call, aligning with the PR's objective.
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
New Features
Refactor