ryanoasis / nerd-fonts

Iconic font aggregator, collection, & patcher. 3,600+ icons, 50+ patched fonts: Hack, Source Code Pro, more. Glyph collections: Font Awesome, Material Design Icons, Octicons, & more
https://NerdFonts.com
Other
54.25k stars 3.64k forks source link

Incorrect display of character 'k' for "FantasqueSansMono" when "ss01" font feature is on #901

Closed Frefreak closed 1 year ago

Frefreak commented 2 years ago

🗹 Requirements

🎯 Subject of the issue

Experienced behavior: If a webpage enables "ss01" font feature, and render font is chosen as "FantasqueSansMono Nerd Font", then what should be displayed as a normal k will show like a "coffee" icon.

Expected behavior:

I see text without emoji

Example symbols:

jjjjkkkkllll

🔧 Your Setup

★ Screenshots (Optional)

I initially noticed it here: I override liberation mono and monospace with FantasqueSansMono Nerd Font using fontconfig's config file, so it's very likely what you see will be fine (because it's not rendered as this font). From the devtools image we can see my browser is rendering using "FantasqueSansMono Nerd Font".

image image

I believe this can be easily reproduced using this html snippet:

<style>
* {
  font-feature-settings: "ss01" on;
  font-family: FantasqueSansMono Nerd Font;
}
</style>
<pre><code>
jjjjkkkkllll
</code></pre>

image

Finii commented 2 years ago

Can not reproduce...

image

Ah, SS01 you say, hold tight ...

image

... examining ...

Finii commented 2 years ago

Source font is ok

image

Finii commented 2 years ago

rule in the original file:

image

Ah, and they put their alternative uniE005 char at

image

Codepoint E005, which is PUA, and we put there the pomicons...

image

They use quite some glyphs there...:

image

*pondering solutions*

Finii commented 2 years ago

While this is easy to fix in fontforge interactively, it seems the necessary actions are not exposed to the Python interface. What I need is a way to access (edit) the current SS01 subtable. I can show it etc, but I can not access (show) the individual rules. I can just add new rules.

Any hint welcome

Other than that, started investigation to use fonttools, but I would rather avoid that. Maybe as postprocess script. Hmm.

Finii commented 2 years ago

Maybe as postprocess script

Ah yes, of course post is too late, it must be pre, but we do not have the 'infrastructure' for pre-stuff yet :roll_eyes:

Finii commented 2 years ago

This is my 'stub' where I try to get access (at least read access) to the sub table:

for filename in sys.argv[1:]:
    fullfile = os.path.basename(filename)
    fname = os.path.splitext(fullfile)[0]

    font = fontforge.open(filename, 1)
    for tab in font.gsub_lookups:
        r = re.match('[\'"][sS][sS]([0-9][0-9])[\'"].*', tab)
        if not r:
            continue
        print('TABS {}'.format(r.groups()[0]))
        print('     : {}'.format(font.getLookupInfo(tab)))
        print('     : {}'.format(font.getLookupInfo(tab)[2]))
        print('     {}'.format(font.lookupSetFeatureList(tab,font.getLookupInfo(tab)[2])))
        for subtable in font.getLookupSubtables(tab):
            print('   - {}'.format(subtable))
            print('     {}'.format(font.getLookupSubtableAnchorClasses(subtable)))
    font.close()
Frefreak commented 2 years ago

Thank you for your work. How font works under the hood is totally new to me so I'm afraid I can't really help anything for now. However I think this might be a good chance to learn those stuffs so I can understand your script and how font patching works. I'm starting with fontforge...

Frefreak commented 2 years ago

I now know some basic usage of fontforge and have briefly read the font-patcher script.

To my understanding the pomicon icons should not be moved otherwise when other projects use one of the pomicons it will display incorrectly. Since it looks like GSUB table specifies substitution by name (e.g. k -> k.noloop when ss01 is on), do we need to programmatically determine there's a k to k.noloop and move k.noloop somewhere else? Is this understanding right?

I tried all the getXXX functions and just like you said none of them seems to do what we want to get info like this: image

Currently the only way I found is through the glyph api like so: image

This would means the script needs to iterate all the glyphs to determine all the mappings (given no prior knowledge), which sounds super inefficient and I don't think should be the way to go. Or can we hard-code the mapping for known fonts, something similar to how src font attrs is doing here?

Finii commented 2 years ago

Thank you for working on it!!

Well, with #914 we already iterate through 'all' glyphs to find refs. Would be easy enough to also look the posSub tables up. And then not patch the ~referenced~ substitution destination glyph.

Iirc I tried to move the ref'ed glyph to some free space and adapt the references, but ... well that seemed a lot of work for very few ref glyphs. So I choose to skip patching that glyph.

Once you start editing the tables it's a can of worms 😒 as they can be extremely complex.

Font based data files are already here, the config. cfg in each individual source font directory. That does not help for self patching though.

And/or --careful could get an optional parameter where to be careful. That can be filled by self patcher persons and config.cfg here in the CI.

But is it really an option to skip some symbols on patching just because they are needed for SS12, which probably noone uses? Maybe SS users are required to burn in the style set with a tool (which you need to do anyhow on all the not-styleset-aware terminals ... and then self patch?

These are tough questions.

Just noticed that the (maybe more important?) PPEM fix #761 does not work with .ttc #783, of course, and that will also consume lots of time. Sigh.

Frefreak commented 2 years ago

Thanks for the explanation.

but ... well that seemed a lot of work for very few ref glyphs.

Indeed, I would prefer skip patching those conflict glyphs too since I don't have a use case for the pomicons. The --careful flag combined with config.cfg seems to be a good way.

But is it really an option to skip some symbols on patching just because they are needed for SS12, which probably noone uses? Maybe SS users are required to burn in the style set with a tool (which you need to do anyhow on all the not-styleset-aware terminals ... and then self patch?

I agree that different users might want different things. For some people those missing symbols "might" be important.

Just noticed that the (maybe more important?) PPEM fix https://github.com/ryanoasis/nerd-fonts/pull/761 does not work with .ttc https://github.com/ryanoasis/nerd-fonts/pull/783, of course, and that will also consume lots of time. Sigh.

That's totally OK. This is just a small issue (almost all the webpages I see don't specify font-features on mono fonts anyways). At worst I can self patch with a --careful flag. Maybe I can even try to help implement the "move to free space" way in the future (needs to take some experiments so no guarantee).

Finii commented 1 year ago

Did not implement the "move to free space" way, but the glyphs will automatically be marked with careful, i.e. not replaced.

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a new issue, complete the issue template with all the details necessary to reproduce, and mention this issue as reference.