Bugfix Ubuntu ligatures #1597
Merged
Bugfix Ubuntu ligatures #1597
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Finii
force-pushed
the
bugfix/Ubuntu-ligatures
branch
from
f1d1b32
to
1854a98
Compare
[why] The essential glyphs are partially based on which substitution rules the font has. And the config.json configuration is able to remove some substitution rules. So we must first possibly remove rules and then examine them, not the other way around, because now we detect 'essential glyphs' for rules that we later remove. [how] Correct order of operation. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] We have two ranges of glyphs to examine: 1. `0x0000`-`0x017f` is the Latin Extended-A range 2. `0xfb00`-`0xfb06` are 'fi' and other ligatures Both ranges are added to the essential glyphs (with their sub-references, and with other glyphs that are referenced through possub rules). That is correct for range 1, but range 2 - the ligature glyphs - should only become essential if they are indeed used for ligatures by some rule. For example with the UbuntuMono font, that does not have any fi ligature natively, but it DOES have a filled `0xFB01` glyph, we would protect that glyph and its subreferences (i.e. `0xF001`) - needlessly breaking the Font Awesome icons in that font. [how] Add all glyphs of range 1 to the essential glyphs, but add glyphs of range 2 only if they have a possub rule attached. Reported-by: Eli Array Minkoff <eli@planetminkoff.com> Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] The fi and fl ligs are formulated in the font to go through glyphs with codepoint `F001` and `F002`. That is detected and overwriting that glyphs with some (Font Awesome) icons is prevented. The question is: What is more important, to have the fi ligature or to have the fa-music and fa-magnifying_glass icons? [how] We sacrify here Font Nerdism over Icon Nerdism and remove the ligatures before patching. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Finii
force-pushed
the
bugfix/Ubuntu-ligatures
branch
from
1854a98
to
8516c9c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Remove the
fi,fletc ligatures from Ubuntu to free that glyphs for overpatching. At the moment two Font Awesome glyphs (music, magnifying_glass) can not be inserted for that reason.Also fix some code in the patcher that accidentially protected that glyphs albeit no ligatures are active in that font.
Requirements / Checklist
What does this Pull Request (PR) do?
How should this be manually tested?
Any background context you can provide?
Fixes: #1595
Related: #1221
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)