这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@Finii
Copy link
Collaborator

@Finii Finii commented Dec 13, 2021

[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

  • Read the Contributing Guidelines
  • Read or at least glanced at the FAQ
  • Read or at least glanced at the Wiki
  • Scripts execute without error (if necessary):
    • If any of the scripts were modified they have been tested and execute without error, e.g.:
      • ./font-patcher Inconsolata.otf --fontawesome --octicons --pomicons
      • ./gotta-patch-em-all-font-patcher\!.sh Hermit
  • Extended the README and documentation if necessary, e.g. You added a new font please update the table

What 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

  • Overpass
  • M+ (also ffi)
  • Inconsolata
  • Ubuntu
  • Monaco
  • Share Tech Mono (?)

and look if the fi ligatures were removed: Either in fontforge or by installing the font and typing text in a ligature aware environment (i.e. writer) and check that there is no ligature for fi anymore.

Any background context you can provide?

What are the relevant tickets (if any)?

Screenshots (if appropriate or helpful)

[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>
@ryanoasis
Copy link
Owner

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 fontforge.IsLigature any familiarity?

@Finii
Copy link
Collaborator Author

Finii commented Dec 15, 2021

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.
So if the new glyph needs some table entries we need to provide them and can not safely reuse old entries. Old entries are just ... old.

@ryanoasis ryanoasis merged commit a457461 into master Dec 16, 2021
@ryanoasis ryanoasis deleted the bufix/ligature-leftover branch December 16, 2021 04:46
@gardotd426
Copy link

Unfortunately the issue still persists on Inconsolata Nerd Font.

I literally erased my entire ~/.local/share/fonts/NerdFonts directory, did a git pull in the repo, then ran ./install.sh and reinstalled everything. Inconsolata still shows:

Screenshot_20211216_003438

@gardotd426
Copy link

Oh and I checked via fc-cache, I have no other Inconsolata Nerd Fonts located in any other locations. The only ones are the ones from the new ./install.sh.

@ryanoasis
Copy link
Owner

@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

@gardotd426
Copy link

gardotd426 commented Dec 16, 2021 via email

@ryanoasis
Copy link
Owner

Yeah sorry if it's confusing. You'd have to patch it yourself at this moment to get the fix.
Maybe I'll add Inconsolata to the RC so we can prove it out 😄

@marcelluseasley
Copy link

This repo has it fixed: https://github.com/ToxicFrog/Ligaturizer/releases

Finii added a commit that referenced this pull request Jan 28, 2023
[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>
LNKLEO pushed a commit to LNKLEO/Nerd that referenced this pull request Nov 24, 2023
font-patcher: Remove obsolete metadata on glyph exchange
LNKLEO pushed a commit to LNKLEO/Nerd that referenced this pull request Nov 24, 2023
[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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fi shows as a telephone with Inconsolata NF

5 participants