-
-
Notifications
You must be signed in to change notification settings - Fork 310
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 sprite mask system #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
50a7e7e
324c9e5
27e6bf7
1b337e7
a44e230
98efec7
c8be7f1
1f190f0
a5c0445
bc21358
e2efa9d
c320b31
501eb00
a010ade
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 | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,178 @@ | ||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||
* @title SpriteMaskCustomStencil | ||||||||||||||||||||||||||||||||||||||||||
* @category SpriteMask | ||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||||||||
AssetType, | ||||||||||||||||||||||||||||||||||||||||||
Camera, | ||||||||||||||||||||||||||||||||||||||||||
CompareFunction, | ||||||||||||||||||||||||||||||||||||||||||
Layer, | ||||||||||||||||||||||||||||||||||||||||||
Script, | ||||||||||||||||||||||||||||||||||||||||||
Sprite, | ||||||||||||||||||||||||||||||||||||||||||
SpriteMask, | ||||||||||||||||||||||||||||||||||||||||||
SpriteMaskInteraction, | ||||||||||||||||||||||||||||||||||||||||||
SpriteRenderer, | ||||||||||||||||||||||||||||||||||||||||||
StencilOperation, | ||||||||||||||||||||||||||||||||||||||||||
Texture2D, | ||||||||||||||||||||||||||||||||||||||||||
Vector3, | ||||||||||||||||||||||||||||||||||||||||||
WebGLEngine | ||||||||||||||||||||||||||||||||||||||||||
} from "@galacean/engine"; | ||||||||||||||||||||||||||||||||||||||||||
import { initScreenshot, updateForE2E } from "./.mockForE2E"; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// Create engine | ||||||||||||||||||||||||||||||||||||||||||
WebGLEngine.create({ canvas: "canvas" }).then((engine) => { | ||||||||||||||||||||||||||||||||||||||||||
engine.canvas.resizeByClientSize(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// Create root entity | ||||||||||||||||||||||||||||||||||||||||||
const rootEntity = engine.sceneManager.activeScene.createRootEntity(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// Create camera | ||||||||||||||||||||||||||||||||||||||||||
const cameraEntity = rootEntity.createChild("Camera"); | ||||||||||||||||||||||||||||||||||||||||||
cameraEntity.transform.setPosition(0, 0, 50); | ||||||||||||||||||||||||||||||||||||||||||
const camera = cameraEntity.addComponent(Camera); | ||||||||||||||||||||||||||||||||||||||||||
camera.cullingMask = Layer.Layer0; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// Create sprite and mask | ||||||||||||||||||||||||||||||||||||||||||
engine.resourceManager | ||||||||||||||||||||||||||||||||||||||||||
.load([ | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
// Sprite texture | ||||||||||||||||||||||||||||||||||||||||||
url: "https://gw.alipayobjects.com/mdn/rms_7c464e/afts/img/A*rgNGR4Vb7lQAAAAAAAAAAAAAARQnAQ", | ||||||||||||||||||||||||||||||||||||||||||
type: AssetType.Texture2D | ||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
// Mask texture | ||||||||||||||||||||||||||||||||||||||||||
url: "https://gw.alipayobjects.com/mdn/rms_7c464e/afts/img/A*qyhFT5Un5AgAAAAAAAAAAAAAARQnAQ", | ||||||||||||||||||||||||||||||||||||||||||
type: AssetType.Texture2D | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
]) | ||||||||||||||||||||||||||||||||||||||||||
.then((textures: Texture2D[]) => { | ||||||||||||||||||||||||||||||||||||||||||
const pos = new Vector3(); | ||||||||||||||||||||||||||||||||||||||||||
const scale = new Vector3(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// Create sprites. | ||||||||||||||||||||||||||||||||||||||||||
const sprite = new Sprite(engine, textures[0]); | ||||||||||||||||||||||||||||||||||||||||||
const maskSprite = new Sprite(engine, textures[1]); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// create a sprite renderer, and write stencil | ||||||||||||||||||||||||||||||||||||||||||
pos.set(0, 0, 0); | ||||||||||||||||||||||||||||||||||||||||||
scale.set(5, 5, 5); | ||||||||||||||||||||||||||||||||||||||||||
const writeStencilSR = addSpriteRenderer( | ||||||||||||||||||||||||||||||||||||||||||
pos, | ||||||||||||||||||||||||||||||||||||||||||
scale, | ||||||||||||||||||||||||||||||||||||||||||
sprite, | ||||||||||||||||||||||||||||||||||||||||||
SpriteMaskInteraction.None, | ||||||||||||||||||||||||||||||||||||||||||
Layer.Layer0, | ||||||||||||||||||||||||||||||||||||||||||
Layer.Layer0, | ||||||||||||||||||||||||||||||||||||||||||
0 | ||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||
const writeStencilMaterial = writeStencilSR.getInstanceMaterial(); | ||||||||||||||||||||||||||||||||||||||||||
const writeStencilState = writeStencilMaterial.renderState.stencilState; | ||||||||||||||||||||||||||||||||||||||||||
writeStencilState.enabled = true; | ||||||||||||||||||||||||||||||||||||||||||
writeStencilState.writeMask = 0xff; | ||||||||||||||||||||||||||||||||||||||||||
writeStencilState.passOperationFront = StencilOperation.IncrementSaturate; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// create a sprite renderer, mask interaction is none, and read stencil | ||||||||||||||||||||||||||||||||||||||||||
pos.set(3, 3, 0); | ||||||||||||||||||||||||||||||||||||||||||
const readStencilSR = addSpriteRenderer( | ||||||||||||||||||||||||||||||||||||||||||
pos, | ||||||||||||||||||||||||||||||||||||||||||
scale, | ||||||||||||||||||||||||||||||||||||||||||
sprite, | ||||||||||||||||||||||||||||||||||||||||||
SpriteMaskInteraction.None, | ||||||||||||||||||||||||||||||||||||||||||
Layer.Layer0, | ||||||||||||||||||||||||||||||||||||||||||
Layer.Layer0, | ||||||||||||||||||||||||||||||||||||||||||
1 | ||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||
readStencilSR.color.set(1, 0, 0, 1); | ||||||||||||||||||||||||||||||||||||||||||
const readStencilMaterial = readStencilSR.getInstanceMaterial(); | ||||||||||||||||||||||||||||||||||||||||||
const readStencilState = readStencilMaterial.renderState.stencilState; | ||||||||||||||||||||||||||||||||||||||||||
readStencilState.enabled = true; | ||||||||||||||||||||||||||||||||||||||||||
readStencilState.referenceValue = 1; | ||||||||||||||||||||||||||||||||||||||||||
readStencilState.compareFunctionFront = CompareFunction.LessEqual; | ||||||||||||||||||||||||||||||||||||||||||
readStencilState.compareFunctionBack = CompareFunction.LessEqual; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// create a sprite renderer, mask interaction is not none | ||||||||||||||||||||||||||||||||||||||||||
pos.set(5, -3, 0); | ||||||||||||||||||||||||||||||||||||||||||
const maskSR = addSpriteRenderer( | ||||||||||||||||||||||||||||||||||||||||||
pos, | ||||||||||||||||||||||||||||||||||||||||||
scale, | ||||||||||||||||||||||||||||||||||||||||||
sprite, | ||||||||||||||||||||||||||||||||||||||||||
SpriteMaskInteraction.VisibleOutsideMask, | ||||||||||||||||||||||||||||||||||||||||||
Layer.Layer0, | ||||||||||||||||||||||||||||||||||||||||||
Layer.Layer0, | ||||||||||||||||||||||||||||||||||||||||||
2 | ||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||
maskSR.color.set(0, 1, 0, 1); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// create a sprite mask | ||||||||||||||||||||||||||||||||||||||||||
pos.set(20, 0, 0); | ||||||||||||||||||||||||||||||||||||||||||
addMask(pos, maskSprite, Layer.Layer0, Layer.Layer0); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// create a sprite renderer, and read stencil | ||||||||||||||||||||||||||||||||||||||||||
pos.set(20, 10, 0); | ||||||||||||||||||||||||||||||||||||||||||
scale.set(3, 3, 3); | ||||||||||||||||||||||||||||||||||||||||||
const readStencilSR2 = addSpriteRenderer( | ||||||||||||||||||||||||||||||||||||||||||
pos, | ||||||||||||||||||||||||||||||||||||||||||
scale, | ||||||||||||||||||||||||||||||||||||||||||
sprite, | ||||||||||||||||||||||||||||||||||||||||||
SpriteMaskInteraction.None, | ||||||||||||||||||||||||||||||||||||||||||
Layer.Layer0, | ||||||||||||||||||||||||||||||||||||||||||
Layer.Layer0, | ||||||||||||||||||||||||||||||||||||||||||
4 | ||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||
readStencilSR2.color.set(1, 0.5, 0.8, 1); | ||||||||||||||||||||||||||||||||||||||||||
const readStencilMaterial2 = readStencilSR2.getInstanceMaterial(); | ||||||||||||||||||||||||||||||||||||||||||
const readStencilState2 = readStencilMaterial2.renderState.stencilState; | ||||||||||||||||||||||||||||||||||||||||||
readStencilState2.enabled = true; | ||||||||||||||||||||||||||||||||||||||||||
readStencilState2.referenceValue = 1; | ||||||||||||||||||||||||||||||||||||||||||
readStencilState2.compareFunctionFront = CompareFunction.Greater; | ||||||||||||||||||||||||||||||||||||||||||
readStencilState2.compareFunctionBack = CompareFunction.Greater; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
updateForE2E(engine, 100, 100); | ||||||||||||||||||||||||||||||||||||||||||
initScreenshot(engine, camera); | ||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
engine.run(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||
* Add sprite renderer and set mask interaction and layer. | ||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||
function addSpriteRenderer( | ||||||||||||||||||||||||||||||||||||||||||
pos: Vector3, | ||||||||||||||||||||||||||||||||||||||||||
scale: Vector3, | ||||||||||||||||||||||||||||||||||||||||||
sprite: Sprite, | ||||||||||||||||||||||||||||||||||||||||||
maskInteraction: SpriteMaskInteraction, | ||||||||||||||||||||||||||||||||||||||||||
maskLayer: number, | ||||||||||||||||||||||||||||||||||||||||||
layer: number, | ||||||||||||||||||||||||||||||||||||||||||
priority: number | ||||||||||||||||||||||||||||||||||||||||||
): SpriteRenderer { | ||||||||||||||||||||||||||||||||||||||||||
const entity = rootEntity.createChild("Sprite"); | ||||||||||||||||||||||||||||||||||||||||||
entity.layer = layer; | ||||||||||||||||||||||||||||||||||||||||||
const renderer = entity.addComponent(SpriteRenderer); | ||||||||||||||||||||||||||||||||||||||||||
const { transform } = entity; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
transform.position = pos; | ||||||||||||||||||||||||||||||||||||||||||
transform.scale = scale; | ||||||||||||||||||||||||||||||||||||||||||
renderer.sprite = sprite; | ||||||||||||||||||||||||||||||||||||||||||
renderer.maskInteraction = maskInteraction; | ||||||||||||||||||||||||||||||||||||||||||
renderer.maskLayer = maskLayer; | ||||||||||||||||||||||||||||||||||||||||||
renderer.priority = priority; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
return renderer; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||
* Add sprite mask and set influence layers, include mask animation script. | ||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||
function addMask<T extends Script>(pos: Vector3, sprite: Sprite, layer: number, influenceLayers: number): void { | ||||||||||||||||||||||||||||||||||||||||||
const entity = rootEntity.createChild(`Mask`); | ||||||||||||||||||||||||||||||||||||||||||
entity.layer = layer; | ||||||||||||||||||||||||||||||||||||||||||
const mask = entity.addComponent(SpriteMask); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
// entity.addComponent(scriptType); | ||||||||||||||||||||||||||||||||||||||||||
entity.transform.position = pos; | ||||||||||||||||||||||||||||||||||||||||||
mask.sprite = sprite; | ||||||||||||||||||||||||||||||||||||||||||
mask.influenceLayers = influenceLayers; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+168
to
+177
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. Remove unused generic type parameter 'T extends Script' from 'addMask' function The generic type parameter Apply this diff to remove the unused generic type parameter: - function addMask<T extends Script>(pos: Vector3, sprite: Sprite, layer: number, influenceLayers: number): void {
+ function addMask(pos: Vector3, sprite: Sprite, layer: number, influenceLayers: number): void { 📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: codecov/patch
|
||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -213,6 +213,13 @@ | |
threshold: 0.2 | ||
} | ||
}, | ||
SpriteMask: { | ||
CustomStencil: { | ||
category: "SpriteMask", | ||
caseFileName: "spriteMask-customStencil", | ||
threshold: 0.3 | ||
} | ||
}, | ||
Comment on lines
+216
to
+222
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 Add test coverage for the new There are no existing tests covering the newly added 🔗 Analysis chainConsider adding test coverage for the new configuration. The static analysis tool indicates that the newly added lines are not covered by tests. To ensure the reliability and correctness of the configuration, it's recommended to add appropriate test coverage for these new lines. To verify the current test coverage and identify areas that need additional tests, you can run the following command: Would you like assistance in generating test cases for the new 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check test coverage for the E2E_CONFIG object
# Test: Search for tests related to E2E_CONFIG
rg -i "E2E_CONFIG" --type ts --glob "**/*test*.ts"
# Test: Check if there are any existing tests for SpriteMask category
rg -i "SpriteMask" --type ts --glob "**/*test*.ts"
Length of output: 10608 🧰 Tools🪛 GitHub Check: codecov/patch
|
||
Text: { | ||
TypedText: { | ||
category: "Text", | ||
|
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) { | ||
|
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.
Avoid reusing 'pos' and 'scale' variables to prevent unintended side effects
The variables
pos
andscale
are reused when setting the position and scale of multiple entities. Sincetransform.position
andtransform.scale
assign the object references, subsequent modifications topos
andscale
will unintentionally affect the previously assigned entities. This can lead to unexpected behavior in the scene.Consider creating new
Vector3
instances for each entity to avoid this issue. Apply the following changes:In the main block after line 52:
Repeat for each position and scale:
In the
addSpriteRenderer
function, modify lines 155-156:In the
addMask
function, modify line 174:Also applies to: 155-156, 174-174