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

Add config option to configure font fallback #1557

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

Yaraslaut
Copy link
Member

@Yaraslaut Yaraslaut commented Jul 2, 2024

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 list
none will make fallback list empty
list will put in fallback list fonts in the same order as they appear in configuration given that they are found

@github-actions github-actions bot added frontend Contour Terminal Emulator (GUI frontend) VT: rasterizer Rendering of the terminal into a pixmap using `terminal_renderer` library fonts font rasterization and text shaping API and platform implementations labels Jul 2, 2024
@Yaraslaut Yaraslaut force-pushed the improvement/configurable_font_fallback branch from 485ae45 to 72b9bfa Compare July 2, 2024 15:25
@Yaraslaut Yaraslaut marked this pull request as draft July 2, 2024 16:20
if (_d->tryShapeWithFallback(
font, fontInfo, hbBuf, hbFont, script, presentation, codepoints, clusters, result))
auto shapingFunction = [&](auto&&... args) {
if (useFontFallback)
Copy link
Member

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)

Copy link
Member Author

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

@Yaraslaut Yaraslaut force-pushed the improvement/configurable_font_fallback branch 2 times, most recently from 8e6e46b to 192251c Compare July 5, 2024 14:08
@github-actions github-actions bot removed the VT: rasterizer Rendering of the terminal into a pixmap using `terminal_renderer` library label Jul 5, 2024
@Yaraslaut Yaraslaut force-pushed the improvement/configurable_font_fallback branch from 192251c to 0b17aac Compare July 5, 2024 14:09
@Yaraslaut Yaraslaut marked this pull request as ready for review July 5, 2024 14:12
@Yaraslaut Yaraslaut force-pushed the improvement/configurable_font_fallback branch from 0b17aac to 37c033a Compare July 5, 2024 14:16
@Yaraslaut Yaraslaut changed the title Add config option to disable font fallback Add config option to configure font fallback Jul 8, 2024
@Yaraslaut Yaraslaut force-pushed the improvement/configurable_font_fallback branch 2 times, most recently from 7f01423 to 2b6c86c Compare July 25, 2024 08:41
src/text_shaper/coretext_locator.mm Outdated Show resolved Hide resolved
src/text_shaper/coretext_locator.mm Outdated Show resolved Hide resolved
src/text_shaper/fontconfig_locator.cpp Outdated Show resolved Hide resolved
src/text_shaper/fontconfig_locator.cpp Outdated Show resolved Hide resolved
@Yaraslaut Yaraslaut force-pushed the improvement/configurable_font_fallback branch 5 times, most recently from ad29132 to ea3ce1a Compare July 28, 2024 19:37
@Yaraslaut Yaraslaut force-pushed the improvement/configurable_font_fallback branch 2 times, most recently from 9a8b35c to 4a0d49f Compare August 5, 2024 16:00
@Yaraslaut Yaraslaut force-pushed the improvement/configurable_font_fallback branch from 4a0d49f to a06b9c4 Compare August 5, 2024 16:03
Copy link
Member

@christianparpart christianparpart left a 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 :)

@christianparpart christianparpart merged commit b86961d into master Aug 7, 2024
39 checks passed
@christianparpart christianparpart deleted the improvement/configurable_font_fallback branch August 7, 2024 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fonts font rasterization and text shaping API and platform implementations frontend Contour Terminal Emulator (GUI frontend)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants