-
-
Notifications
You must be signed in to change notification settings - Fork 300
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
Refactor mask #2369
base: dev/1.4
Are you sure you want to change the base?
Refactor mask #2369
Changes from all commits
3ff8a7b
4afe44b
603ba2a
0ae6484
1a89b5d
a5f1ab9
7ad02b4
e97c4aa
2f2615b
47c9c7e
e52b60a
8a48797
a311f2d
9a06859
f6f373b
f6b35a9
47d670a
8c0fc37
e71b5d6
5df85bc
0a09fee
82f7f82
12d40f3
028e4b0
2b92fe1
3d10522
5f50e58
f96f891
ea9e05c
6e6eab5
ce3efed
bd38b58
18ea656
894dff9
f0c6d4b
4f4218d
397ac0e
d29b823
8a34428
61558b1
3fd4e4f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,6 @@ | |
import { Renderer, RendererUpdateFlags } from "../../Renderer"; | ||
import { assignmentClone, deepClone, ignoreClone } from "../../clone/CloneManager"; | ||
import { ShaderProperty } from "../../shader/ShaderProperty"; | ||
import { CompareFunction } from "../../shader/enums/CompareFunction"; | ||
import { ISpriteAssembler } from "../assembler/ISpriteAssembler"; | ||
import { SimpleSpriteAssembler } from "../assembler/SimpleSpriteAssembler"; | ||
import { SlicedSpriteAssembler } from "../assembler/SlicedSpriteAssembler"; | ||
|
@@ -257,7 +256,6 @@ | |
|
||
set maskInteraction(value: SpriteMaskInteraction) { | ||
if (this._maskInteraction !== value) { | ||
this._updateStencilState(this._maskInteraction, value); | ||
this._maskInteraction = value; | ||
} | ||
} | ||
|
@@ -269,7 +267,7 @@ | |
super(entity); | ||
this.drawMode = SpriteDrawMode.Simple; | ||
this._dirtyUpdateFlag |= SpriteRendererUpdateFlags.Color; | ||
this.setMaterial(this._engine._spriteDefaultMaterial); | ||
this.setMaterial(this._engine._basicResources.spriteDefaultMaterial); | ||
this._onSpriteChange = this._onSpriteChange.bind(this); | ||
//@ts-ignore | ||
this._color._onValueChanged = this._onColorChanged.bind(this); | ||
|
@@ -334,7 +332,7 @@ | |
} | ||
// @todo: This question needs to be raised rather than hidden. | ||
if (material.destroyed) { | ||
material = this._engine._spriteDefaultMaterials[this._maskInteraction]; | ||
material = this._engine._basicResources.spriteDefaultMaterial; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add test coverage for material fallback logic The line Consider adding a unit test to cover this scenario to ensure that the fallback to the default material behaves as expected when the original material is destroyed. Would you like assistance in creating a test case for this? 🧰 Tools🪛 GitHub Check: codecov/patch
|
||
} | ||
|
||
// Update position | ||
|
@@ -395,28 +393,6 @@ | |
this._dirtyUpdateFlag &= ~SpriteRendererUpdateFlags.AutomaticSize; | ||
} | ||
|
||
private _updateStencilState(from: SpriteMaskInteraction, to: SpriteMaskInteraction): void { | ||
const material = this.getMaterial(); | ||
const { _spriteDefaultMaterials: spriteDefaultMaterials } = this._engine; | ||
if (material === spriteDefaultMaterials[from]) { | ||
this.setMaterial(spriteDefaultMaterials[to]); | ||
} else { | ||
const { stencilState } = material.renderState; | ||
if (to === SpriteMaskInteraction.None) { | ||
stencilState.enabled = false; | ||
stencilState.writeMask = 0xff; | ||
stencilState.referenceValue = 0; | ||
stencilState.compareFunctionFront = stencilState.compareFunctionBack = CompareFunction.Always; | ||
} else { | ||
stencilState.enabled = true; | ||
stencilState.writeMask = 0x00; | ||
stencilState.referenceValue = 1; | ||
stencilState.compareFunctionFront = stencilState.compareFunctionBack = | ||
to === SpriteMaskInteraction.VisibleInsideMask ? CompareFunction.LessEqual : CompareFunction.Greater; | ||
} | ||
} | ||
} | ||
|
||
@ignoreClone | ||
private _onSpriteChange(type: SpriteModifyFlags): void { | ||
switch (type) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,6 @@ import { Renderer } from "../../Renderer"; | |
import { TransformModifyFlags } from "../../Transform"; | ||
import { assignmentClone, deepClone, ignoreClone } from "../../clone/CloneManager"; | ||
import { ShaderData, ShaderProperty } from "../../shader"; | ||
import { CompareFunction } from "../../shader/enums/CompareFunction"; | ||
import { ShaderDataGroup } from "../../shader/enums/ShaderDataGroup"; | ||
import { Texture2D } from "../../texture"; | ||
import { FontStyle } from "../enums/FontStyle"; | ||
|
@@ -250,7 +249,6 @@ export class TextRenderer extends Renderer { | |
set maskInteraction(value: SpriteMaskInteraction) { | ||
if (this._maskInteraction !== value) { | ||
this._maskInteraction = value; | ||
this._setDirtyFlagTrue(DirtyFlag.MaskInteraction); | ||
} | ||
} | ||
|
||
|
@@ -294,7 +292,7 @@ export class TextRenderer extends Renderer { | |
const { engine } = this; | ||
this._font = engine._textDefaultFont; | ||
this._addResourceReferCount(this._font, 1); | ||
this.setMaterial(engine._textDefaultMaterial); | ||
this.setMaterial(engine._basicResources.textDefaultMaterial); | ||
//@ts-ignore | ||
this._color._onValueChanged = this._onColorChanged.bind(this); | ||
} | ||
|
@@ -394,11 +392,6 @@ export class TextRenderer extends Renderer { | |
return; | ||
} | ||
|
||
if (this._isContainDirtyFlag(DirtyFlag.MaskInteraction)) { | ||
this._updateStencilState(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before implement is different! |
||
this._setDirtyFlagFalse(DirtyFlag.MaskInteraction); | ||
} | ||
|
||
if (this._isContainDirtyFlag(DirtyFlag.SubFont)) { | ||
this._resetSubFont(); | ||
this._setDirtyFlagFalse(DirtyFlag.SubFont); | ||
|
@@ -437,29 +430,6 @@ export class TextRenderer extends Renderer { | |
camera._renderPipeline.pushRenderElement(context, renderElement); | ||
} | ||
|
||
private _updateStencilState(): void { | ||
const material = this.getInstanceMaterial(); | ||
const stencilState = material.renderState.stencilState; | ||
const maskInteraction = this._maskInteraction; | ||
|
||
if (maskInteraction === SpriteMaskInteraction.None) { | ||
stencilState.enabled = false; | ||
stencilState.writeMask = 0xff; | ||
stencilState.referenceValue = 0; | ||
stencilState.compareFunctionFront = stencilState.compareFunctionBack = CompareFunction.Always; | ||
} else { | ||
stencilState.enabled = true; | ||
stencilState.writeMask = 0x00; | ||
stencilState.referenceValue = 1; | ||
const compare = | ||
maskInteraction === SpriteMaskInteraction.VisibleInsideMask | ||
? CompareFunction.LessEqual | ||
: CompareFunction.Greater; | ||
stencilState.compareFunctionFront = compare; | ||
stencilState.compareFunctionBack = compare; | ||
} | ||
} | ||
|
||
private _resetSubFont(): void { | ||
const font = this._font; | ||
this._subFont = font._getSubFont(this.fontSize, this.fontStyle); | ||
|
@@ -762,8 +732,7 @@ enum DirtyFlag { | |
LocalPositionBounds = 0x2, | ||
WorldPosition = 0x4, | ||
WorldBounds = 0x8, | ||
MaskInteraction = 0x10, | ||
Color = 0x20, | ||
Color = 0x10, | ||
|
||
Position = LocalPositionBounds | WorldPosition | WorldBounds, | ||
Font = SubFont | Position | ||
Comment on lines
+735
to
738
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Incomplete Removal of The search revealed multiple remaining references to
Please address these remaining references to ensure the 🔗 Analysis chainVerify the impact of DirtyFlag enum changes. The Please ensure that:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for any remaining references to DirtyFlag.MaskInteraction
echo "Searching for DirtyFlag.MaskInteraction references:"
rg "DirtyFlag\.MaskInteraction" --type ts
# Search for uses of DirtyFlag.Color to ensure it's being used correctly
echo "Searching for DirtyFlag.Color usage:"
rg "DirtyFlag\.Color" --type ts
Length of output: 41441 |
||
|
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.
💡 Codebase verification
Residual old import paths detected
The script found an occurrence of the old import path in
packages/core/src/RenderPipeline/MaskManager.ts
. Please update this reference to ensure consistency across the codebase.🔗 Analysis chain
Verify import path consistency across the codebase
The import path for
SpriteMaskLayer
has been updated. Ensure that this change is consistent across the entire codebase to prevent any import-related issues.Run the following script to check for any remaining occurrences of the old import path:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 178
Script:
Length of output: 156