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

Mechanism to define priority fallback fonts #1777

Closed
wants to merge 11 commits into from

Conversation

saurtron
Copy link
Collaborator

@saurtron saurtron commented Nov 18, 2024

This PR is for reference only, I think it's better to merge #1790 directly (this PR also contained there).

Work done

  • Introduce lua AddFallbackFont(vfspath) and ClearFallbackFonts().
  • Backend at CFontTexture::AddFallbackFont and CFontTexture::ClearFallbackFonts.

Related issues

While this PR won't directly fix all of the above, the proposed mechanism will allow fixing all those easily by having games providing priority fallback fonts for languages and emojis, and using the proposed api.

Testing

  • Get this bar branch test-fallback-fonts into your BAR.sdd
    • This branch has functionality for enabling/disabling one fallback through mouse left click, and also runs an automatic procedure to write into chat and enable/disable fallbacks.
  • Open skirmish
  • Once it starts, the test widget will automatically run the following procedure:
    • Write something into chat, like: "blah🔥" (at frame 3)
      • it will use your os fallback
    • Enable fallbacks and write into chat (at frame 150)
      • should say "Fallback fonts, true"
      • The fire glyph should change to the provided font
    • Disable fallbacks and write into chat (at frame 450)
      • should say "Fallback fonts, false"
      • The fire glyph should change to the os font again

You will notice disabling doesn't really work well, for the branch where fallbacks are enabled and disabled, use this branch instead: #1790

You can further play with it by using left mouse button to enable/disable fallback, and writing your own characters into chat.

Discussion

  • I added the lua methods inside Lua/LuaFonts.cpp as that seems to be the best place, only drawback is it results in methods being under gl. namespace which is a bit awkward maybe.
  • While the mechanism is simple I think this will suffice for application needs.
    • Applications can use the api like gl.AddFallbackFont("font/NotoEmoji-VariableFont_wght.ttf") to add fallbacks, priority will be the same as order of fallback addition.
    • Allow to ClearFallbackFonts just in case.
    • Note gl.LoadFont doesn't currently add fonts to gameFonts, this is on purpose as right now we don't need those included in fontconfig listings, at least not for this PR. The idea is the game will be using a specific font for render in different places, and having all the fallbacks "for free" with every font.
  • Can be made more sophisticated on top of this implementation:
    • For example could allow declaring priority fallback fonts just for certain families or languages, but I believe this isn't really needed atm.
    • Can be expanded later by including more parameters to AddFallbackFont.
  • I strive to use safe fontconfig mechanisms that work well with the library:
    • While it's possible to add font definition patterns to the fontconfig application list through FcConfigGetFonts, this is not recommended: "This font set is owned by the library and must not be modified or freed."
      • We need this since the allowed methods FcConfigAppFontAddDir and FcConfigAppFontAddFile work with actual fs paths, but we need to load from VFS.
      • I'm using our own set gameFontSet, first searching on it with FcFontSetSort, and then falling back to FcFontSort to search system fonts, this allows having our own set in addition to the fontconfig ones with minimal changes.
      • The documentation is not very clear here, and "must not be modified" may not mean we can't add patterns to the set, but it might so safest to have our own separate set imo.
    • We could use xml priority matches as described here, but that doesn't allow us to clear the prioritization, and also according to documentation for FcConfigBuildFonts: "Note that any changes to the configuration after this call (through FcConfigParseAndLoad or FcConfigParseAndLoadFromMemory) have indeterminate effects"
      • I'm using a default pattern with priority font families prepended instead fallbackPattern, this seems to work fine.
    • There's one thing I'm not sure about, you may have noticed the commented method: FcConfigSubstitute(FtLibraryHandler::GetFCConfig(), pattern, FcMatchScan) after adding the font to our set. I see the library doing this when it adds fonts through FcConfigAppFontAddFile, but tbh have no idea if it's actually needed in this case, on my tests it doesn't seem to help or hurt.
  • I have respected USE_FONTCONFIG #ifdefs and added my own to maintain compatibility, but I think that's broken atm since FcConfig *config pointer isn't protected by an #ifdef. I believe we might actually want to remove the USE_FONTCONFIG ifdefs.
  • Not sure where the documentation for the lua methods should go, I don't see it for LoadFonts or DeleteFont, I'll take a look. Added ldoc, will keep an eye for Convert LDoc to lua-language-server #1775
  • This should avoid needing Luaui reload when changing fallbacks, but ClearFallbackFonts might want to do some atlas cleaning after being called.
  • We might also want to add system priority fonts by family name for fonts everyone should have in order to avoid distribution by games. This is possible but not sure really needed atm since it's always going to be more fragile due to users not actually having the fonts. If wanted a new method AddSystemFallbackFont(familyname) could be added.

More notes on testing

(for a direct procedure see "Testing" above)

The general procedure is finding a few characters you know render different on your os and some other font. Try without the fallback, then try with fallbacks.

  • Make sure you have this branch.
  • Add the font to BAR.sdd/fonts/.
  • Call gl.AddFallbackFont("fonts/XXXXX") in some widget you know will be loaded, like advplayerslist.
  • I'm using font/NotoEmoji-VariableFont_wght.ttf and FreeSerif.otf with the "🔥" character.
  • Also DejaVuSerif.ttf and NotoSansMono-Bold.ttf with "⟵".

fc-list or some similar program can be used to list fonts your os has with some character, like: fc-list :charset=27f5, fc-list :charset=1f81a or fc-list :charset=1F525.

@sprunk
Copy link
Collaborator

sprunk commented Nov 18, 2024

Not sure where the documentation for the lua methods should go, I don't see it for LoadFonts or DeleteFont, I'll take a look.

It goes above the function but there's an ongoing effort to change its format, see #1775.

@saurtron
Copy link
Collaborator Author

saurtron commented Nov 18, 2024

ongoing effort to change its format

Ok for now I added ldoc format since it's what's used currently, I'll keep an eye on the conversion effort in case I need to change this, or to let them know to convert this too.

Actually the font module is not included into the ldoc generation, but I think adding it should go into a different PR.

can still sometimes have it's own priorities.
@saurtron
Copy link
Collaborator Author

saurtron commented Nov 19, 2024

Changed the search mechanism a bit to first search our fallback fonts and then the system ones, since after some testing I found out fontconfig otherwise still sometimes prioritizes unexpectedly (for example it kept choosing note emoji colour instead of my selected note emoji regular, even when the search pattern looked like it should prioritize the regular one).

I'll keep testing patterns to try and revert to the old search mechanism but not sure it will be feasible due to how fontconfig works, also current way is a bit more efficient when priority fallbacks are defined since it will at first be searching only on the game font list, and only resort to search on the system fonts when needing it.

@saurtron saurtron added the fonts label Dec 2, 2024
@saurtron saurtron marked this pull request as draft December 9, 2024 15:56
@lhog
Copy link
Collaborator

lhog commented Dec 16, 2024

Does #1790 supersede this one?

@saurtron
Copy link
Collaborator Author

Does #1790 supersede this one?

Yes, although I left this one so both can be compared so its easier to see the two implementation steps, also in case atlas refresh has some problem could always fall back to this one.

@saurtron saurtron closed this Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants