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

Fix Font (texture) handling #882

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix Font (texture) handling #882

wants to merge 1 commit into from

Conversation

o-p-a
Copy link

@o-p-a o-p-a commented Jul 26, 2024

When ES renders characters on the screen, it first uploads the glyphs to a texture and caches them.
The size of each texture is 2048x512, and when one is used up, allocate another texture and used.
If you are an alphabetic region, you probably won't need multiple textures.
For example users East Asian countries who use Chinese characters, one texture is often not enough.
(This is even more noticeable on high-resolution displays, where one character has more pixels.)
When this happens, ES will not work properly. This PR will fix this.


In TextComponent, the text to be drawn is stored in the TextCache.
If the glyphs used in the text span multiple textures for the reasons mentioned above, there will be more than one VertexList holding the vertices and textureId.
This is constructed by Font::buildTextCache(), and the result of the calculation is set to cache->vertexLists at the end of the function, but the increment of i is forgotten and it is only stored in the first element.
As a result, ES will abort. I will correct this by incrementing i appropriately.

However, this alone will not result in correct drawing.

Because, when (reasons mentioned above) a new texture is pushed back into mTextures, reallocation occurs when the std::vector is resized elements, and the texture is destroyed even if it is in use.
FontTexture has a default copy ctor and dtor, so they are called when reallocate.
The ctor simply copies the members, and the dtor destroys the associated texture.
...Oh, I just wanted to reallocate it, but it destroyed the texture!

I considered defining an appropriate copy constructor to fix this, but unfortunately mTextures[].textureId is referenced by pointer from the TextCache's vertexLists, so even if I did that, it would be necessary to update the TextCache's pointer every time mTextures is reallocated.

It's a bit of a hasty job, but I reserved 10 elements when generating the Font to prevent rearrangement. I think 10 is a sufficient number even in kanji-using countries.
I also set up a guard to prevent any attempt to exceed 10.

@pjft
Copy link
Collaborator

pjft commented Jul 26, 2024

I like this intent, thank you! Can we get this to be tested out by folks in the forums, hopefully even folks who'd use East Asian characters? I'm away for a few weeks, so I really won't be able to test this (well, I also wouldn't be using the character set for it), but I appreciate putting this together as it is indeed an issue for such character sets.

Thanks.

@o-p-a
Copy link
Author

o-p-a commented Jul 29, 2024

Thank you for reply!
I have created this thread on the forum.
I hope it will be merged once it is confirmed as OK.

@pjft
Copy link
Collaborator

pjft commented Jul 29, 2024

Thanks. Let me know how it goes, otherwise I'll ping this in September and we'll take it from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants