-
-
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
Add GUI Infrastructure #2375
base: dev/1.4
Are you sure you want to change the base?
Add GUI Infrastructure #2375
Changes from 213 commits
9c1b561
a44de9e
d504369
06c8e53
60b240a
123dde4
563bfbe
a2a236b
bfd7435
51de522
88bb053
554b80a
339f7eb
c648e0c
26308cf
28d9579
139f247
a0ed23b
b550930
2d86283
8e028fb
40b1e88
eee57cf
4be33d5
0d3f67b
61bfb35
9dd5bf1
371550b
ed074ff
08214c2
094b5e6
04eb47c
650b135
3f0cfa0
7e33812
c265204
d442be2
7a09c01
901ba13
b256155
10bb95b
b62df3a
0dbad8d
9e50d30
ebd27df
c98748f
7649de4
68657e8
c95c0c2
8f36498
10fc066
c9ac2a5
5dc393d
d667d99
caa0a38
8f77a72
f08d477
7f4a460
cfe01a5
83cbf4a
c353ee3
fbf6ed1
997f29f
68f8993
96e9621
f9e67ec
48bcccf
c65299a
0acf868
5871137
999e348
fa5a17f
8fae435
332ff64
3710d41
1714428
8bcde48
a0e2dc5
f232172
09e975c
bfb78bc
a35fc8f
169f3eb
436d4b1
ae911ae
2621b7e
0b2b12d
239feed
8388166
520da7b
54ff8f1
d8393e8
836e752
169aa09
c09d5e7
df3b5f8
3ada1f6
6edbf7b
fbaa8aa
26b3b94
bae7a9a
7f613d9
271a77c
9575b36
054ded6
cef8757
1772f93
a1969c3
78059b3
7d57894
59d5c86
531e2b3
67eea0f
41401e4
0e48dbb
4cb99c8
e2a74ca
ef592f6
80e88c8
edc5953
6a0962e
b4748af
3d8cbc2
c880f70
0455188
44447f3
98e7887
4b5dc36
7f6798d
7347e62
59ce725
c502f6d
e0e85c6
70cc80d
8ca4efb
ba35d56
10d4bf5
c100342
e51478c
bbbc3ff
4c9794b
30348f1
8583bf9
43687db
754a7f4
8332c31
6b07a12
1bf8334
3f46739
dc293bd
9860e71
2ab14d6
beb3593
aa0d174
af0f2d7
0b45a7f
7cb710c
e4f0d93
854410d
5cb8b48
ce13c8e
e9f2bee
38edcbf
9d407d2
043d9fa
0aa5514
6475f15
6a7bf93
0b99f79
75bc0c1
beb506b
fdca466
e3f0aae
37359f0
31529a9
1203df7
42dcd13
5fef2a1
78add14
fd2dc9d
c9177bf
e71a6da
1457cdd
c9b3157
477af1f
7ccffd3
11691f6
4934498
f0ebdd0
fa07ecd
7ae6558
52c0d28
5931066
a197c1e
189a1e7
aa6aff8
077e163
40d926f
334bd4a
8d0ee7a
fc9302b
2dce250
4e83314
012b61e
f7dacdc
41075e6
2f4ce0f
04bb5b3
f237f6a
20a1d30
5279a74
6a87332
a5e0642
d88532f
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 |
---|---|---|
@@ -1,11 +1,20 @@ | ||
import { Vector2 } from "@galacean/engine-math"; | ||
import { Renderer } from "../../Renderer"; | ||
|
||
/** | ||
* @internal | ||
*/ | ||
export interface ISpriteAssembler { | ||
resetData(renderer: Renderer, vertexCount?: number): void; | ||
updatePositions?(renderer: Renderer): void; | ||
updatePositions?( | ||
renderer: Renderer, | ||
width: number, | ||
height: number, | ||
pivot: Vector2, | ||
flipX?: boolean, | ||
flipY?: boolean | ||
): void; | ||
updateUVs?(renderer: Renderer): void; | ||
updateColor?(renderer: Renderer): void; | ||
updateColor?(renderer: Renderer, alpha?: number): void; | ||
updateAlpha?(renderer: Renderer, alpha: number): void; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import { BoundingBox, Matrix } from "@galacean/engine-math"; | ||
import { BoundingBox, Matrix, Vector2 } from "@galacean/engine-math"; | ||
import { StaticInterfaceImplement } from "../../base/StaticInterfaceImplement"; | ||
import { UIImage } from "../../ui"; | ||
import { SpriteMask } from "../sprite"; | ||
import { SpriteRenderer } from "../sprite/SpriteRenderer"; | ||
import { ISpriteAssembler } from "./ISpriteAssembler"; | ||
|
@@ -12,7 +13,7 @@ export class SimpleSpriteAssembler { | |
static _rectangleTriangles = [0, 1, 2, 2, 1, 3]; | ||
static _worldMatrix = new Matrix(); | ||
|
||
static resetData(renderer: SpriteRenderer | SpriteMask): void { | ||
static resetData(renderer: SpriteRenderer | SpriteMask | UIImage): void { | ||
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. I noticed that we now have three related types (SpriteRenderer, SpriteMask, and UIImage). To improve readability and maintainability, would it be better to define a single type for these renderable components, such as Renderable? This way, we can avoid redundancy and make future updates easier. 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. Good idea, you should try to pr |
||
const manager = renderer._getChunkManager(); | ||
const lastSubChunk = renderer._subChunk; | ||
lastSubChunk && manager.freeSubChunk(lastSubChunk); | ||
|
@@ -21,16 +22,23 @@ export class SimpleSpriteAssembler { | |
renderer._subChunk = subChunk; | ||
} | ||
|
||
static updatePositions(renderer: SpriteRenderer | SpriteMask): void { | ||
const { width, height, sprite } = renderer; | ||
const { x: pivotX, y: pivotY } = sprite.pivot; | ||
static updatePositions( | ||
renderer: SpriteRenderer | SpriteMask | UIImage, | ||
width: number, | ||
height: number, | ||
pivot: Vector2, | ||
flipX: boolean = false, | ||
flipY: boolean = false | ||
): void { | ||
const { sprite } = renderer; | ||
const { x: pivotX, y: pivotY } = pivot; | ||
// Renderer's worldMatrix | ||
const worldMatrix = SimpleSpriteAssembler._worldMatrix; | ||
const { elements: wE } = worldMatrix; | ||
// Parent's worldMatrix | ||
const { elements: pWE } = renderer.entity.transform.worldMatrix; | ||
const sx = renderer.flipX ? -width : width; | ||
const sy = renderer.flipY ? -height : height; | ||
const sx = flipX ? -width : width; | ||
const sy = flipY ? -height : height; | ||
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. I ran a performance test comparing the ternary operator version and a simplified multiplication version for sx and sy. The results showed that the multiplication approach is consistently faster. Since both versions produce the same output, I recommend switching to the multiplication method to improve performance, especially in cases where this function is called frequently. Here’s the updated version: const sx = width * (flipX ? -1 : 1); 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. |
||
(wE[0] = pWE[0] * sx), (wE[1] = pWE[1] * sx), (wE[2] = pWE[2] * sx); | ||
(wE[4] = pWE[4] * sy), (wE[5] = pWE[5] * sy), (wE[6] = pWE[6] * sy); | ||
(wE[8] = pWE[8]), (wE[9] = pWE[9]), (wE[10] = pWE[10]); | ||
|
@@ -74,15 +82,25 @@ export class SimpleSpriteAssembler { | |
vertices[offset + 28] = top; | ||
} | ||
|
||
static updateColor(renderer: SpriteRenderer): void { | ||
static updateColor(renderer: SpriteRenderer, alpha: number = 1): void { | ||
const subChunk = renderer._subChunk; | ||
const { r, g, b, a } = renderer.color; | ||
const finalAlpha = a * alpha; | ||
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. Above three lines and following line can be optimized using destructuring to reduce redundancy and improve readability. Here’s an optimized version: const { _subChunk: { chunk: { vertices } }, color: { r, g, b, a } } = renderer; |
||
const vertices = subChunk.chunk.vertices; | ||
for (let i = 0, o = subChunk.vertexArea.start + 5; i < 4; ++i, o += 9) { | ||
vertices[o] = r; | ||
vertices[o + 1] = g; | ||
vertices[o + 2] = b; | ||
vertices[o + 3] = a; | ||
vertices[o + 3] = finalAlpha; | ||
} | ||
} | ||
|
||
static updateAlpha(renderer: SpriteRenderer, alpha: number = 1): void { | ||
const subChunk = renderer._subChunk; | ||
const finalAlpha = renderer.color.a * alpha; | ||
const vertices = subChunk.chunk.vertices; | ||
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. Above three lines can be optimized using destructuring to make it more concise and avoid repeated references to renderer and subChunk. Here's a suggested improvement: 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. I personally think this is not good for readability. |
||
for (let i = 0, o = subChunk.vertexArea.start + 5; i < 4; ++i, o += 9) { | ||
vertices[o + 3] = finalAlpha; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
import { Matrix } from "@galacean/engine-math"; | ||
import { Matrix, Vector2 } from "@galacean/engine-math"; | ||
import { StaticInterfaceImplement } from "../../base/StaticInterfaceImplement"; | ||
import { UIImage } from "../../ui"; | ||
import { SpriteMask } from "../sprite"; | ||
import { SpriteRenderer } from "../sprite/SpriteRenderer"; | ||
import { ISpriteAssembler } from "./ISpriteAssembler"; | ||
|
||
|
@@ -14,7 +16,7 @@ export class SlicedSpriteAssembler { | |
]; | ||
static _worldMatrix = new Matrix(); | ||
|
||
static resetData(renderer: SpriteRenderer): void { | ||
static resetData(renderer: SpriteRenderer | UIImage): void { | ||
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. Same as the previous comment regarding SpriteRenderer | SpriteMask | UIImage. To avoid redundancy, consider using the same Renderable type here as well. See the comment in packages/core/src/2d/assembler/SimpleSpriteAssembler.ts |
||
const manager = renderer._getChunkManager(); | ||
const lastSubChunk = renderer._subChunk; | ||
lastSubChunk && manager.freeSubChunk(lastSubChunk); | ||
|
@@ -23,8 +25,15 @@ export class SlicedSpriteAssembler { | |
renderer._subChunk = subChunk; | ||
} | ||
|
||
static updatePositions(renderer: SpriteRenderer): void { | ||
const { width, height, sprite } = renderer; | ||
static updatePositions( | ||
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. Same observation as mentioned earlier regarding the updatePositions function. Since the added parameters (width, height, pivot, flipX, and flipY) extend its functionality beyond just position updates, I would recommend renaming the function here as well to something like updateTransform or updateDimensionsAndPosition. This will help clarify its current role. |
||
renderer: SpriteRenderer | SpriteMask | UIImage, | ||
width: number, | ||
height: number, | ||
pivot: Vector2, | ||
flipX: boolean = false, | ||
flipY: boolean = false | ||
): void { | ||
const { sprite } = renderer; | ||
const { border } = sprite; | ||
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. This two lines can be optimized using destructuring to make it more concise and avoid repeated: const { sprite: { border } } = renderer; 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. Is this a subjective preference? 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. Regarding the destructuring style, it’s indeed a subjective choice that depends on individual coding style and readability. If the original style aligns with project guidelines or personal preference, then let’s keep it as it is. |
||
// Update local positions. | ||
const spritePositions = sprite._getPositions(); | ||
|
@@ -73,16 +82,16 @@ export class SlicedSpriteAssembler { | |
} | ||
|
||
// Update renderer's worldMatrix. | ||
const { x: pivotX, y: pivotY } = renderer.sprite.pivot; | ||
const localTransX = renderer.width * pivotX; | ||
const localTransY = renderer.height * pivotY; | ||
const { x: pivotX, y: pivotY } = pivot; | ||
const localTransX = width * pivotX; | ||
const localTransY = height * pivotY; | ||
// Renderer's worldMatrix. | ||
const worldMatrix = SlicedSpriteAssembler._worldMatrix; | ||
const { elements: wE } = worldMatrix; | ||
// Parent's worldMatrix. | ||
const { elements: pWE } = renderer.entity.transform.worldMatrix; | ||
const sx = renderer.flipX ? -1 : 1; | ||
const sy = renderer.flipY ? -1 : 1; | ||
const sx = flipX ? -1 : 1; | ||
const sy = flipY ? -1 : 1; | ||
(wE[0] = pWE[0] * sx), (wE[1] = pWE[1] * sx), (wE[2] = pWE[2] * sx); | ||
(wE[4] = pWE[4] * sy), (wE[5] = pWE[5] * sy), (wE[6] = pWE[6] * sy); | ||
(wE[8] = pWE[8]), (wE[9] = pWE[9]), (wE[10] = pWE[10]); | ||
|
@@ -118,7 +127,7 @@ export class SlicedSpriteAssembler { | |
renderer._bounds.transform(worldMatrix); | ||
} | ||
|
||
static updateUVs(renderer: SpriteRenderer): void { | ||
static updateUVs(renderer: SpriteRenderer | UIImage): void { | ||
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. Same as commented above. |
||
const subChunk = renderer._subChunk; | ||
const vertices = subChunk.chunk.vertices; | ||
const spriteUVs = renderer.sprite._getUVs(); | ||
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. Above three lines can be optimized using destructuring to make it more concise and avoid repeated references to renderer and subChunk. Here's a suggested improvement: const { _subChunk: { chunk: { vertices } }, sprite } = renderer; |
||
|
@@ -131,15 +140,25 @@ export class SlicedSpriteAssembler { | |
} | ||
} | ||
|
||
static updateColor(renderer: SpriteRenderer): void { | ||
static updateColor(renderer: SpriteRenderer | UIImage, alpha: number = 1): void { | ||
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. Same as commented above. |
||
const subChunk = renderer._subChunk; | ||
const { r, g, b, a } = renderer.color; | ||
const finalAlpha = a * alpha; | ||
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. Above three lines and following line can be optimized using destructuring to reduce redundancy and improve readability. Here’s an optimized version: const { _subChunk: { chunk: { vertices } }, color: { r, g, b, a } } = renderer; |
||
const vertices = subChunk.chunk.vertices; | ||
for (let i = 0, o = subChunk.vertexArea.start + 5; i < 16; ++i, o += 9) { | ||
vertices[o] = r; | ||
vertices[o + 1] = g; | ||
vertices[o + 2] = b; | ||
vertices[o + 3] = a; | ||
vertices[o + 3] = finalAlpha; | ||
} | ||
} | ||
|
||
static updateAlpha(renderer: SpriteRenderer, alpha: number = 1): void { | ||
const subChunk = renderer._subChunk; | ||
const finalAlpha = renderer.color.a * alpha; | ||
const vertices = subChunk.chunk.vertices; | ||
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. Above three lines and following line can be optimized using destructuring to reduce redundancy and improve readability. Here’s an optimized version: const { _subChunk: { chunk: { vertices } }, color: { a } } = renderer; |
||
for (let i = 0, o = subChunk.vertexArea.start + 5; i < 16; ++i, o += 9) { | ||
vertices[o + 3] = finalAlpha; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
import { MathUtil, Matrix } from "@galacean/engine-math"; | ||
import { MathUtil, Matrix, Vector2 } from "@galacean/engine-math"; | ||
import { DisorderedArray } from "../../DisorderedArray"; | ||
import { Logger } from "../../base"; | ||
import { StaticInterfaceImplement } from "../../base/StaticInterfaceImplement"; | ||
import { UIImage } from "../../ui"; | ||
import { SpriteTileMode } from "../enums/SpriteTileMode"; | ||
import { Sprite } from "../sprite"; | ||
import { SpriteRenderer } from "../sprite/SpriteRenderer"; | ||
|
@@ -18,7 +19,7 @@ export class TiledSpriteAssembler { | |
static _uvRow = new DisorderedArray<number>(); | ||
static _uvColumn = new DisorderedArray<number>(); | ||
|
||
static resetData(renderer: SpriteRenderer, vertexCount: number): void { | ||
static resetData(renderer: SpriteRenderer | UIImage, vertexCount: number): void { | ||
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. Same as commented above. |
||
if (vertexCount) { | ||
const manager = renderer._getChunkManager(); | ||
const lastSubChunk = renderer._subChunk; | ||
|
@@ -33,8 +34,15 @@ export class TiledSpriteAssembler { | |
} | ||
} | ||
|
||
static updatePositions(renderer: SpriteRenderer): void { | ||
const { width, height, sprite, tileMode, tiledAdaptiveThreshold: threshold } = renderer; | ||
static updatePositions( | ||
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. Same observation as mentioned earlier regarding the updatePositions function. Since the added parameters (width, height, pivot, flipX, and flipY) extend its functionality beyond just position updates, I would recommend renaming the function here as well to something like updateTransform or updateDimensionsAndPosition. This will help clarify its current role. |
||
renderer: SpriteRenderer | UIImage, | ||
width: number, | ||
height: number, | ||
pivot: Vector2, | ||
flipX: boolean = false, | ||
flipY: boolean = false | ||
): void { | ||
const { sprite, tileMode, tiledAdaptiveThreshold: threshold } = renderer; | ||
// Calculate row and column | ||
const { _posRow: posRow, _posColumn: posColumn, _uvRow: uvRow, _uvColumn: uvColumn } = TiledSpriteAssembler; | ||
const maxVertexCount = renderer._getChunkManager().maxVertexCount; | ||
|
@@ -64,16 +72,16 @@ export class TiledSpriteAssembler { | |
); | ||
TiledSpriteAssembler.resetData(renderer, vertexCount); | ||
// Update renderer's worldMatrix | ||
const { x: pivotX, y: pivotY } = renderer.sprite.pivot; | ||
const localTransX = renderer.width * pivotX; | ||
const localTransY = renderer.height * pivotY; | ||
const { x: pivotX, y: pivotY } = pivot; | ||
const localTransX = width * pivotX; | ||
const localTransY = height * pivotY; | ||
// Renderer's worldMatrix | ||
const { _worldMatrix: worldMatrix } = TiledSpriteAssembler; | ||
const { elements: wE } = worldMatrix; | ||
// Parent's worldMatrix | ||
const { elements: pWE } = renderer.entity.transform.worldMatrix; | ||
const sx = renderer.flipX ? -1 : 1; | ||
const sy = renderer.flipY ? -1 : 1; | ||
const sx = flipX ? -1 : 1; | ||
const sy = flipY ? -1 : 1; | ||
let wE0: number, wE1: number, wE2: number; | ||
let wE4: number, wE5: number, wE6: number; | ||
(wE0 = wE[0] = pWE[0] * sx), (wE1 = wE[1] = pWE[1] * sx), (wE2 = wE[2] = pWE[2] * sx); | ||
|
@@ -139,7 +147,7 @@ export class TiledSpriteAssembler { | |
renderer._bounds.transform(worldMatrix); | ||
} | ||
|
||
static updateUVs(renderer: SpriteRenderer): void { | ||
static updateUVs(renderer: SpriteRenderer | UIImage): void { | ||
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. Same as commented above. |
||
const { _posRow: posRow, _posColumn: posColumn, _uvRow: uvRow, _uvColumn: uvColumn } = TiledSpriteAssembler; | ||
const rowLength = posRow.length - 1; | ||
const columnLength = posColumn.length - 1; | ||
|
@@ -173,16 +181,27 @@ export class TiledSpriteAssembler { | |
} | ||
} | ||
|
||
static updateColor(renderer: SpriteRenderer): void { | ||
static updateColor(renderer: SpriteRenderer | UIImage, alpha: number = 1): void { | ||
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. Same as commented above. |
||
const subChunk = renderer._subChunk; | ||
const { r, g, b, a } = renderer.color; | ||
const finalAlpha = a * alpha; | ||
const vertices = subChunk.chunk.vertices; | ||
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. Above four lines can be optimized using destructuring to reduce redundancy and improve readability. Here’s an optimized version: const { _subChunk: { chunk: { vertices } }, color: { r, g, b, a } } = renderer; 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. Destructuring assignment is a concise syntax that is suitable for shallow objects and simple property extraction. It is the recommended approach, especially when dealing with properties that are certain to exist. 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. 👌 Thanks for the insight! Since destructuring is a subjective choice, let’s keep the original approach as it aligns with your style. |
||
const vertexArea = subChunk.vertexArea; | ||
for (let i = 0, o = vertexArea.start + 5, n = vertexArea.size / 9; i < n; ++i, o += 9) { | ||
vertices[o] = r; | ||
vertices[o + 1] = g; | ||
vertices[o + 2] = b; | ||
vertices[o + 3] = a; | ||
vertices[o + 3] = finalAlpha; | ||
} | ||
} | ||
|
||
static updateAlpha(renderer: SpriteRenderer, alpha: number = 1): void { | ||
const subChunk = renderer._subChunk; | ||
const finalAlpha = renderer.color.a * alpha; | ||
const vertices = subChunk.chunk.vertices; | ||
const vertexArea = subChunk.vertexArea; | ||
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. It can be optimized using destructuring to reduce redundancy and improve readability. Here’s an optimized version: const { _subChunk: { chunk: { vertices }, vertexArea }, color: { a } } = renderer; |
||
for (let i = 0, o = vertexArea.start + 5, n = vertexArea.size / 9; i < n; ++i, o += 9) { | ||
vertices[o + 3] = finalAlpha; | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -238,8 +238,9 @@ export class SpriteMask extends Renderer { | |
} | ||
|
||
protected override _updateBounds(worldBounds: BoundingBox): void { | ||
if (this.sprite) { | ||
SimpleSpriteAssembler.updatePositions(this); | ||
const { sprite } = this; | ||
if (sprite) { | ||
SimpleSpriteAssembler.updatePositions(this, this.width, this.height, sprite.pivot, this._flipX, this._flipY); | ||
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. To improve readability and reduce repeated references to this, I suggest destructuring the properties before calling the method. Here’s an example of how you could simplify it: const { width, height, _flipX, _flipY } = this; This reduces redundancy and makes the code cleaner. 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. Destructuring assignment usually takes out the attributes to be reused to avoid repeated reading, which does not exist here. The code itself is not very complex and can be written in one line, so I don't think it is necessary 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. 👌 Agreed. We can use this.xx here directly~ |
||
} else { | ||
worldBounds.min.set(0, 0, 0); | ||
worldBounds.max.set(0, 0, 0); | ||
|
@@ -250,7 +251,8 @@ export class SpriteMask extends Renderer { | |
* @inheritdoc | ||
*/ | ||
protected override _render(context: RenderContext): void { | ||
if (!this.sprite?.texture || !this.width || !this.height) { | ||
const { _sprite: sprite } = this; | ||
if (!sprite?.texture || !this.width || !this.height) { | ||
singlecoder marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return; | ||
} | ||
|
||
|
@@ -266,7 +268,7 @@ export class SpriteMask extends Renderer { | |
|
||
// Update position | ||
if (this._dirtyUpdateFlag & RendererUpdateFlags.WorldVolume) { | ||
SimpleSpriteAssembler.updatePositions(this); | ||
SimpleSpriteAssembler.updatePositions(this, this.width, this.height, sprite.pivot, this._flipX, this._flipY); | ||
singlecoder marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this._dirtyUpdateFlag &= ~RendererUpdateFlags.WorldVolume; | ||
} | ||
|
||
|
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.
After adding parameters like width, height, pivot, and flip, it seems that the function is no longer just updating the position. Should we consider renaming the function, or adding a new method for these updates?
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.
This method just for update vertex position, can you give a other case?
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.
This is my problem. I saw that the input parameters changed, and I thought the function accusation also changed.
Please revolve this comment.