-
Notifications
You must be signed in to change notification settings - Fork 24
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
Improve Canvas Text Renderer Performance #319
Conversation
@wouterlucas Could you help me performance test this if you have a chance? |
When the text of a node changes, does that require creating a new texture or do you update the text of the texture? |
|
||
createNode(props: Partial<CoreNodeWritableProps>) { | ||
const resolvedProps = this.resolveNodeDefaults(props); | ||
const node = new CoreNode(this, { |
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.
Seems a waste to clone the resolvedProps
just to add shaderProps: null
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.
Agreed. Will fix.
state.textureNode.texture = texture; | ||
} else { | ||
// Create a new texture node | ||
const textureNode = this.stage.createNode({ |
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.
Why creating a sub node? Is that to decouple the text node size and the texture?
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.
Yeah pretty much. I did do a pass trying to simply update the TextNode's texture
but this was the path of least resistance to be able to do this without changing the size characteristics of canvas TextNodes (i.e. without breaking a ton of tests). Open to suggestions for improvement, however I think this in general will be more performant than what we have right now.
In this case I'm creating a new texture. Generally |
All 3 renderer using Canvas fonts. If you want I can add the same codebase that the PR is from and enable MSDF fonts for comparison. Though it will be fastest. But still a bit away from L2's performance. |
94a56e8
to
63ff8ba
Compare
63ff8ba
to
f443716
Compare
- Canvas Text Textures are now managed properly by the texture manager - Also add willReadFrequently option to text drawing canvas element context
f443716
to
3ac9adf
Compare
Minor changes to text rendering due to eliminating multi-page implementation
TODO
Get multi-page scrolling working again (or this can be deferred)Multi-Page Canvas 2D Text #321