Keep track of missing glyphs

[ecraven]

I've patched imgui locally to keep track of which glyphs are missing in a font, and provide that info to the library user, so that the font can be re-created with an appropriate glyph_range.
I'd love for this to get into mainline imgui. Should I hide things behind an #ifdef, so people can disable it at compile-time, or is a runtime check good enough, for providing an actual pull request for you all to look at?

[ocornut]

Runtime check is probably only good enough if done very efficiently in a way that doesn't harm the performance of the CalcTextSize function. However that feature would pretty much be overlapping with the more desirable atlas update requirement.

If you let a frame elapse before rendering then those glyphs will be missing for one frame, so in theory the data needs to be computed immediately.

[ecraven]

I'm currently using this for a font merged from multiple TTFs, so I don't think imgui could update things correctly internally :-/ you are correct that glyphs are missing for a frame, but that's better than having to load all of CJK in multiple fonts and sizes, which I'd have to do right now :-/
Implementation-wise, I've just added code after the if in ImFont::FindGlyph (but before the return FallbackGlyph;). Not sure whether that would impede performance in some way :-/

[ocornut]

FindGlyph() is probably the best place to put it in term of performances, unfortunately CalcTextSize() doesn't uses it (it rather uses the densely packaged IndexXAdvance[] array), so that would also create an issue for one frame where both the Calc and Draw functions would use the fallback width.

It's actually a fairly bearable side-effect, and it avoid a lots of the trouble of getting it right, so it might be a great fit in term of the project philosophy. Maybe it is the best solution and those who want to avoid artefact that request glyphs to be loaded ahead.

I'll have to think about it. For now but it should be a reasonably easy patch to maintain locally if you are only adding to FindGlyph() - function is trivial and not likely to change.

Can you post of the gist of your changes here? (not as a formal PR, just a loose patch/dump of code) for reference.

[ecraven]

https://github.com/ecraven/imgui/tree/missing-glyphs
The vector could be a boolean set, which would add about 8k per font, but might be faster. I'm not hung up on the actual code, just the general idea, I'd love to have a way to find out which glyphs are currently being rendered, but not in a font.
What I have also done locally (not shown in the commit, to make it easier to see what is going on) is add a second optional parameter to FindGlyph, so that it only registers missing glyphs in ImFont::RenderChar and RenderText.

[chetan-prime]

Hi @ecraven . I've integrated your patch into my local copy of ImGui but am having issues. It keeps putting the glyphs back into the MissingGlyphs vector even after they are merged into the existing font.
This a gist for the code I'm using in the master while loop - https://gist.github.com/chetan-prime/b8ab0382c64a2bf0500a3ea561106cdb

Expected Result:
Missing Glyphs added to ImFont

Actual Output:
The Glyphs don't get added or FindGlyph() keeps adding them back to the MissingGlyphs vector . I can't see the missing glyphs on screen also.

The below keeps looping endlessly -

ImFont::FindGlyph(20116)/adding 20116 to MissingGlyphsVector
ImFont::FindGlyph(26376)/adding 26376 to MissingGlyphsVector
ImFont::FindGlyph(36130)/adding 36130 to MissingGlyphsVector
ImFont::FindGlyph(23500)/adding 23500 to MissingGlyphsVector
ImFont::FindGlyph(21916)/adding 21916 to MissingGlyphsVector
ImFont::FindGlyph(27426)/adding 27426 to MissingGlyphsVector
ImFont::FindGlyph(20320)/adding 20320 to MissingGlyphsVector
ImFont::FindGlyph(20170)/adding 20170 to MissingGlyphsVector
ImFont::FindGlyph(22825)/adding 22825 to MissingGlyphsVector
ImFont::FindGlyph(26159)/adding 26159 to MissingGlyphsVector
ImFont::FindGlyph(20010)/adding 20010 to MissingGlyphsVector
ImFont::FindGlyph(22909)/adding 22909 to MissingGlyphsVector
ImFont::FindGlyph(26085)/adding 26085 to MissingGlyphsVector
ImFont::FindGlyph(23376)/adding 23376 to MissingGlyphsVector
glyph_ranges[0/1]=20116
glyph_ranges[2/3]=26376
glyph_ranges[4/5]=36130
glyph_ranges[6/7]=23500
glyph_ranges[8/9]=21916
glyph_ranges[10/11]=27426
glyph_ranges[12/13]=20320
glyph_ranges[14/15]=20170
glyph_ranges[16/17]=22825
glyph_ranges[18/19]=26159
glyph_ranges[20/21]=20010
glyph_ranges[22/23]=22909
glyph_ranges[24/25]=26085
glyph_ranges[26/27]=23376
missing_new.size()=0/AreGlyphsMissing=0/font=1b70180/ifont=1b70180

[chetan-prime]

I have it solved, @ecraven
For anyone else that has the same issue using this patch, I'd like to confirm that the issue was solved by calling ResetMissingGlyphs() AFTER calling GetTexDataAsRGBA32(..).
Calling GetTexDataAsRGBA32 to recreate the font texture seems to add the missing glyphs back into the MissingGlyphs vector (I think this isn't easy to avoid as the process of regenerating the textures must make a lot of calls to FindGlyph). Anyway the solution was dead simple. Works great now

[ocornut]

It's a really good news.! I may look into making an official version of something like that before implementing the full thing.

[ecraven]

I have it solved, @ecraven For anyone else that has the same issue using this patch, I'd like to confirm that the issue was solved by calling ResetMissingGlyphs() AFTER calling GetTexDataAsRGBA32(..). Calling GetTexDataAsRGBA32 to recreate the font texture seems to add the missing glyphs back into the MissingGlyphs vector (I think this isn't easy to avoid as the process of regenerating the textures must make a lot of calls to FindGlyph). Anyway the solution was dead simple. Works great now

Sorry, this is a detail I neglected to mention, I actually changed FindGlyph locally to take a second parameter (bool registerMissing = false), to make it only register missing glyphs on "external" calls to FindGlyph, not internal imgui bookkeeping ones. I'll try to start an actual PR over the next week.

[m9710797]

Hi all,

I was about to make a similar feature request, but then found this existing issue.
So +1 from my side for adding something like this in the official version :-)

The idea/motivation for my request was very similar. But I had a different implementation in mind:

• Instead of adding an extra bool and ImVector inside ImFont (and corresponding getter/setters), I'd add a customizable callback function. Maybe with this prototype and default implementation:

const ImFont::Glyph* defaultMissingGlyphCallback(const ImFont* font, ImWchar missing) {
    return font->FallbackGlyph;
}

• Users can then override this callback to build similar functionality as in ecraven's patch. Or still other functionality.

This callback approach might be less intrusive (smaller diff compared to current code). And less overhead for the majority of the users who don't need this.

Thanks for considering this.

[ocornut]

Closing this as the work on dynamic_fonts (#8465) is solving this and making the question obsolete.