Skip to content
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 emoji SVG parsing by caching #100300

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

edwin0cheng
Copy link
Contributor

@edwin0cheng edwin0cheng commented Dec 12, 2024

This PR adds a cache for SVG parsing in the Text Server.

Basically, when we first encounter the document, we parse it as before, but we also note the offsets of other glyphs and store the remaining XML. The next time we see another glyph, we can simply parse that glyph node and insert it back into the stored XML.

Although it is still very slow in the case mentioned in #100278 (which is related to font rendering), the time difference is still significant (on my machine):

Before : 4 mins +
After: 35 secs

Fixes #100278

@edwin0cheng
Copy link
Contributor Author

edwin0cheng commented Dec 12, 2024

Seem like Windows build fail due to using designated initializers 😞
Fixed now.

@edwin0cheng edwin0cheng force-pushed the improve_emoji branch 2 times, most recently from 9fcebc1 to 05bddda Compare December 13, 2024 05:55
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works as expected.

The same change should be also applied to modules/text_server_fb/thorvg_svg_in_ot.* (exactly same code).

@edwin0cheng
Copy link
Contributor Author

And I add fb textserver part too.

@edwin0cheng
Copy link
Contributor Author

@bruvzg , do you think these actually fixes #88809 and #100278 ? It still be very slow in #100278 case.

@bruvzg
Copy link
Member

bruvzg commented Dec 16, 2024

do you think these actually fixes #88809 and #100278 ? It still be very slow in #100278 case.

Not sure if a lot can be done for pre-rendering, pre-rendering option only exist because it is slow and primarily for MSDF, it should not be used for emojis in a first place.

#88809 actually is not fully related, it's likely caused by fallback lookup for the most part.

@edwin0cheng edwin0cheng requested a review from bruvzg December 16, 2024 15:56
@akien-mga
Copy link
Member

Looks good! Could you squash the commits? See PR workflow for instructions.

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Dec 18, 2024
Basically, when we first encounter the document, we parse it as before, but we also note the offsets of other glyphs and store the remaining XML. The next time we see another glyph, we can simply parse that glyph node and insert it back into the stored XML.
@edwin0cheng
Copy link
Contributor Author

Looks good! Could you squash the commits? See PR workflow for instructions.

@akien-mga Sure, Done.

@akien-mga akien-mga merged commit d458253 into godotengine:master Dec 18, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@edwin0cheng edwin0cheng deleted the improve_emoji branch December 19, 2024 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Displaying Emoji in custom font is very slow
5 participants