-
Notifications
You must be signed in to change notification settings - Fork 3.8k
font-patcher: Remove obsolete metadata on glyph exchange #711
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
Conversation
[why] When we overwrite a glyph that originally had some special handling, be it a substitution or position table entry (GPOS/GSUB), that special handling is usually not appropriate anymore and has to be removed. If we need special lookup table entries for the new glyph we would have to add them later anyhow, because we can not rely on their existance. In Issue #509 it was a ligature entry, that replaced 'f' followed by 'i' with the 'fi' ligature. The ligature glyph is overwritten by us with a telephone symbol and the substitution table entry makes no sense anymore. [how] If we overwrite a preexisting codepoint we remove it from all lookup tables. Thanks to all other reporters with details. Fixes: #509 #254 Reported-by: mangelozzi <mangelozzi@gmail.com> Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
|
This looks similar to what I came up with but just cleaner 😊 . I have one question though.. I had this extra check but I think it may be redundant: isSymbolALigature = fontforge.IsLigature(int(copiedToSlot, 16))
if isSymbolALigature:
self.sourceFont[currentSourceFontGlyph].removePosSub("*")thoughts on |
|
I guess we want to remove any special handling on that glyph, not just ligature related stuff. For example if a position variation had been imposed on the original glyph, it will be most likely be inappropriate for the new glyph. At least that was my reasoning. Any special instructions regarding that glyph must be obsolete if there is something else. Even if we could reuse that table entries, we would need to provide them along with the glyph a-new, because we can not rely on all fonts havoing them. |
|
Oh and I checked via |
|
@gardotd426 Ah that's because the fonts need to be rebuilt. Only a limited subset of fonts are being rebuilt with changes to the patcher currently and Inconsolata isn't one of them. See the assets in https://github.com/ryanoasis/nerd-fonts/releases/tag/2.2.0-RC for the current list |
|
Ah I was gonna say, some of the fonts that I knew used to show the issue
weren't showing it anymore, so I was confused. Alright well that explains a
lot, thanks.
…On Thu, Dec 16, 2021 at 2:03 AM Ryan L McIntyre ***@***.***> wrote:
@gardotd426 <https://github.com/gardotd426> Ah that's because the fonts
need to be rebuilt.
Only a limited subset of fonts are being rebuilt with changes to the
patcher currently and Inconsolata isn't one of them.
See the assets in
https://github.com/ryanoasis/nerd-fonts/releases/tag/2.2.0-RC for the
current list
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#711 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AM5Y332CQ2ZG4RFNOKXWPELURGFNVANCNFSM5J6R7IAA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
Yeah sorry if it's confusing. You'd have to patch it yourself at this moment to get the fix. |
|
This repo has it fixed: https://github.com/ToxicFrog/Ligaturizer/releases |
[why] When a certain 'higher codepoint' glyph is needed for a substitution or ligature rule of a basic glyph and we replace the 'higher codepoint' glyph with a symbol that stylistic set or ligature will be broken. [how] We can not determine if a certain glyph is the _target_ of a pos-sub rule (at least I could not find a way). What we do is remove all pos-sub entries that _start_ at a symbol-patched glyph [1], but that is not the same. Instead of walking through all substitution tables we just examine the 'basic glyphs' and also protect all glyphs that they reference through most of the possub tables. In fact I encountered only "Substitution" entries and never "Ligature" entries, but we handle both alike. "Pair", "AltSub", and "MultSub" are not handled, but could be added if need be. [1] #711 Fixes: #901 Reported-by: Xiangyu Zhu <frefreak.zxy@gmail.com> Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
font-patcher: Remove obsolete metadata on glyph exchange
[why] When a certain 'higher codepoint' glyph is needed for a substitution or ligature rule of a basic glyph and we replace the 'higher codepoint' glyph with a symbol that stylistic set or ligature will be broken. [how] We can not determine if a certain glyph is the _target_ of a pos-sub rule (at least I could not find a way). What we do is remove all pos-sub entries that _start_ at a symbol-patched glyph [1], but that is not the same. Instead of walking through all substitution tables we just examine the 'basic glyphs' and also protect all glyphs that they reference through most of the possub tables. In fact I encountered only "Substitution" entries and never "Ligature" entries, but we handle both alike. "Pair", "AltSub", and "MultSub" are not handled, but could be added if need be. [1] ryanoasis#711 Fixes: ryanoasis#901 Reported-by: Xiangyu Zhu <frefreak.zxy@gmail.com> Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
When we overwrite a glyph that originally had some special handling, be
it a substitution or position table entry (GPOS/GSUB), that special
handling is usually not appropriate anymore and has to be removed.
If we need special lookup table entries for the new glyph we would have
to add them later anyhow, because we can not rely on their existance.
In Issue #509 it was a ligature entry, that replaced 'f' followed by 'i'
with the 'fi' ligature. The ligature glyph is overwritten by us with a
telephone symbol and the substitution table entry makes no sense
anymore.
[how]
If we overwrite a preexisting codepoint we remove it from all lookup
tables.
Thanks to all other reporters with details.
Fixes: #509 #254
Reported-by: mangelozzi mangelozzi@gmail.com
Signed-off-by: Fini Jastrow ulf.fini.jastrow@desy.de
Requirements / Checklist
./font-patcher Inconsolata.otf --fontawesome --octicons --pomicons./gotta-patch-em-all-font-patcher\!.sh HermitWhat does this Pull Request (PR) do?
Remove obsolete ligature (and other) information on any glyph we replace.
How should this be manually tested?
Patch for example
ffi)and look if the
filigatures were removed: Either infontforgeor by installing the font and typing text in a ligature aware environment (i.e.writer) and check that there is no ligature forfianymore.Any background context you can provide?
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)