-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Add config option to configure font fallback #1557
Add config option to configure font fallback #1557
Conversation
485ae45
to
72b9bfa
Compare
src/text_shaper/open_shaper.cpp
Outdated
if (_d->tryShapeWithFallback( | ||
font, fontInfo, hbBuf, hbFont, script, presentation, codepoints, clusters, result)) | ||
auto shapingFunction = [&](auto&&... args) { | ||
if (useFontFallback) |
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.
Hmmm 🤔
Is this really useful, to be able to completely turn off font fallback?
Even if, we should not do something like if (useFontFallback) ... else ...
in the rendering hot path, but rely on function pointers / virtual dispatch instead. I know this sounds like a micro optimization, but would be cleaner IMHO on top of it, too.
Ignoring that, I think we could instead promote the mock
font locator instead, because there you can explicitly declare how many fonts (font files) to pick in order for fallback. If one would not specify any more than a single font, that would be the same as having no font fallback at all.
But none of it is actually addressing the point in #225.
We could instead work on embedding fonts (Nerd Fonts and standard emoji fonts), which would help people like @eproxus to not require font patching (nerd font), or always come with the same experience when enabling something like use_builtin_emoji: true
(or emoji: builtin
or not specifying emoji:
key at all, indicating that the builtin should be used)
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.
I updated PR, now we configure fallback on the stage of font_locator and also can specify list of fonts that use for fallback
8e6e46b
to
192251c
Compare
192251c
to
0b17aac
Compare
0b17aac
to
37c033a
Compare
7f01423
to
2b6c86c
Compare
ad29132
to
ea3ce1a
Compare
9a8b35c
to
4a0d49f
Compare
4a0d49f
to
a06b9c4
Compare
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.
Thank you, @Yaraslaut.
In the long run, we should try to also extend the documentation as we go with the code changes. This is a user-facing configuration change, which would fit nicely into our website's documentation as well :)
Refs #225
Refs #1374
PR adds configuration option to disable font fallback
Implemented following logic: fallback entry in the config has several options:
auto
none
or a list of fonts[ "A" , "B" ]
auto
will insert all fonts in fallback listnone
will make fallback list emptylist
will put in fallback list fonts in the same order as they appear in configuration given that they are found