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

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

Closed
3 tasks done
Frefreak opened this issue Aug 30, 2022 · 12 comments · Fixed by #1089
Closed
3 tasks done

Comments

@Frefreak
Copy link

🗹 Requirements

  • I have searched the issues for my issue and found nothing related and/or helpful
  • I have searched the FAQ for help
  • I have searched the Wiki for help

🎯 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

  • Which font are you using (e.g. Anonymice Powerline Nerd Font Complete.ttf)?
    • Please give the full filename Fantasque Sans Mono Regular Nerd Font Complete.ttf
    • Where did you get the file from (download link, self patched, source downloaded from link...) archlinux's AUR package
  • Which terminal emulator are you using (e.g. iterm2, urxvt, gnome, konsole)? alacritty (but this issue happens on webpage)
  • Are you using OS X, Linux or Windows? And which specific version or distribution? archlinux

★ 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
Copy link
Collaborator

Finii commented Aug 30, 2022

Can not reproduce...

image

Ah, SS01 you say, hold tight ...

image

... examining ...

@Finii
Copy link
Collaborator

Finii commented Aug 30, 2022

Source font is ok

image

@Finii
Copy link
Collaborator

Finii commented Aug 30, 2022

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
Copy link
Collaborator

Finii commented Sep 1, 2022

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
Copy link
Collaborator

Finii commented Sep 1, 2022

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 🙄

@Finii
Copy link
Collaborator

Finii commented Sep 1, 2022

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
Copy link
Author

Frefreak commented Sep 1, 2022

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
Copy link
Author

Frefreak commented Sep 14, 2022

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
Copy link
Collaborator

Finii commented Sep 14, 2022

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
Copy link
Author

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 #761 does not work with .ttc #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 added a commit that referenced this issue 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.

Fixes: #901

Reported-by: Xiangyu Zhu <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
Finii added a commit that referenced this issue 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 <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii mentioned this issue Jan 28, 2023
2 tasks
@Finii
Copy link
Collaborator

Finii commented Jan 28, 2023

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

@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2023
LNKLEO pushed a commit to LNKLEO/Nerd that referenced this issue 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 <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants