fix: Text updateTransform cannot be triggered before Render #6751
Conversation
|
Could you check out the failing test? |
packages/text/src/Text.ts
Outdated
| @@ -165,6 +165,11 @@ export class Text extends Sprite | |||
| { | |||
| const style = this._style; | |||
|
|
|||
| if (!style) | |||
There was a problem hiding this comment.
I get the updateText in the set text method, but why the check for no style here?
There was a problem hiding this comment.
this.text triggers the set before this.style
I changed the content of the |
|
Would it be possible to add a new unit test for this case? If we do find a better solution, would be good to prevent a regression. |
hmm, there's no need to add new unit tests. I'm just correcting that value. // use: https://pixijs.download/dev/pixi-legacy.js
const label = new PIXI.Text("\n");
label.updateText() // Without this line，output: 3
console.log(label.canvas.width) // output: 1 |
|
Oh I get it! Awesome. |
packages/text/src/Text.ts
Outdated
| @@ -713,6 +713,7 @@ export class Text extends Sprite | |||
| } | |||
| this._text = text; | |||
| this.dirty = true; | |||
| this.updateText(true); | |||
There was a problem hiding this comment.
I am a bit wary of this change, as I think it'll bring performance issues for fast changing text.
Text in PixiJS has always been 'lazy'. So if you updated text a million times within 1 frame..... it doesn't matter, the texture for text only got updated when it comes through to render.
Now we're saying, every time you change the text, it's going to update that texture immediately.
That's a different performance profile which may negatively effect content out there - I'm thinking of high scores or numbers counting up super fast.
There was a problem hiding this comment.
It is worth considering. Another option is to call updateTransform manually in updateTexture.
But that's just repeating the calculation. So it's a temporary solution.
I'll try to optimize it later.
|
need _recursivePostUpdateTransform or something like that |
3Q, updated |
packages/text/src/Text.ts
Outdated
| @@ -394,6 +394,9 @@ export class Text extends Sprite | |||
|
|
|||
| baseTexture.setRealSize(canvas.width, canvas.height, this._resolution); | |||
|
|
|||
| // Recursively updates transform of all objects from the root to this one | |||
| (this as any)._recursivePostUpdateTransform(); | |||
There was a problem hiding this comment.
Much better fix, however, can you please remove as any. Keeping it like this masks these API when we have to refactor things later.
The approach to deal with private APIs that are used internally, is to remove the access modifier (in this case private) from the original method. We can use documentation flagging to make sure it's not "public" for users.
There was a problem hiding this comment.
yes,. this really should not be private
fix: #6744
fixed: https://jsfiddle.net/JetLu/w501jpxk/
Only in step
2willText.updateTextbe triggered, but it's too late.Text.updateText->Text.updateTexture->Sprite._onTextureUpdate->Transform.onChangeTextupdatetransformproperty lags behindrender.This PR may be only a temporary solution.