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

font-patcher: Remove CV lookups in Iosevka #750

Closed
wants to merge 1 commit into from

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Jan 3, 2022

[why]
For some reason fontforge is unable to pack all the lookup tables of
Iosevka back into the generated font, although they fit obviously in the
source font.

The lookup tables will become garbled and fontforge says something like:

    Attempt to output 67666 into a 16-bit field.
    It will be truncated and the file may not be useful.

As this also happens if we do nothing, just open and generate the font
without touching, there is not much we can do.

[how]
The only thing we can do is reduce the number of lookup tables that are
in the font file. This is somewhat brute, but at least we end up with a
usable font, albeit with a little bit less features.

Unfortunately the SS (Style Set) tables are not enough, but the CV
(Character Variants) suffice. So all CV lookups are riped out.

Fixes: #694

Requirements / Checklist

What does this Pull Request (PR) do?

Remove the CV lookup tables from all Iosevka variants.
Other fonts in the src/unpatched-fonts/ are not affected/changed.

How should this be manually tested?

Any background context you can provide?

Fontforge bug unexpected behavior. Even in interactive mode we can not open and directly (unmodified) the Iosevka fonts in opentype format. See #694

What are the relevant tickets (if any)?

#694 and possibly #742

Thanks @xsrvmy for the hint in #742 (comment)

Screenshots (if appropriate or helpful)

[why]
For some reason fontforge is unable to pack all the lookup tables of
Iosevka back into the generated font, although they fit obviously in the
source font.

The lookup tables will become garbled and fontforge says something like:

  Attempt to output 67666 into a 16-bit field.
  It will be truncated and the file may not be useful.

As this also happens if we do nothing, just open and generate the font
without touching, there is not much we can do.

[how]
The only thing we can do is reduce the number of lookup tables that are
in the font file. This is somewhat brute, but at least we end up with a
usable font, albeit with a little bit less features.

Unfortunately the SS (Style Set) tables are not enough, but the CV
(Character Variants) suffice. So all CV lookups are riped out.

Fixes: #694

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii
Copy link
Collaborator Author

Finii commented Jan 3, 2022

It can be discussed if that is the right soluion, or if CV is used by anyone (urgently needed).

Maybe we could remove other tables instead. But it's hard to decide on which and make the decision future proof.

@Finii
Copy link
Collaborator Author

Finii commented Jan 3, 2022

Used number of lookup tables to identify Iosevka...

For comparison here the number of lookup of all fonts in src/unpatched-fonts that have more than 41:

 42 src/unpatched-fonts/Noto/Serif/
113 src/unpatched-fonts/Hasklig/
115 src/unpatched-fonts/Monoid/ 
145 src/unpatched-fonts/Lilex/  
170 src/unpatched-fonts/VictorMono/                          
198 src/unpatched-fonts/FantasqueSansMono/                   
221 src/unpatched-fonts/VictorMono/                          
385 src/unpatched-fonts/CascadiaCode/                                   
403 src/unpatched-fonts/FiraCode/                            
403 src/unpatched-fonts/JetBrainsMono/Ligatures/             
429 src/unpatched-fonts/Iosevka/

So that is obviously not a good metrics for total lookup size. But ok-ish for our purpose?

@xsrvmy
Copy link

xsrvmy commented Jan 3, 2022

Iosevka can be manually compiled with cv's selected. So a manual patch is always possible.
On the topic of Iosevka, can you make the script set the WWS family and subfamily? Otherwise the system group the font under Iosekva instead of Iosevka NF

@Finii
Copy link
Collaborator Author

Finii commented Jan 3, 2022

can you make the script set the WWS family and subfamily?

#717 ... it\s ready albeit the not-ready state ;)

No wait, WWS you say? Normally 717 should be enough, no?

@xsrvmy
Copy link

xsrvmy commented Jan 3, 2022

WWS family has to have Nerd Font appended to it, or Iosevka nerd font will be treated as a variant of Iosevka rather than a seperate font.

@Finii
Copy link
Collaborator Author

Finii commented Jan 3, 2022

But you did have a look at 717?

It sets ID 1 & 2 (Family and SubFamily) and 16 & 17 (Typographic Family and Typographic Subfamily/Style).

WWS Family ... normally is not set:

WWS Family Name. Used to provide a WWS-conformant family name in case the entries for IDs 16 and 17 do not conform to the WWS model.

Source

And 717 is specifically designed to make 16/17 (and 1/2) to obey the customary rules)

That is why I ask. Did you try 717 and still need WWS? What constellation?

@xsrvmy
Copy link

xsrvmy commented Jan 4, 2022

On my systems it seems that WWS is overriding typographic family. And let's move this to 717

@xsrvmy
Copy link

xsrvmy commented Jan 4, 2022

Used number of lookup tables to identify Iosevka...

For comparison here the number of lookup of all fonts in src/unpatched-fonts that have more than 41:

 42 src/unpatched-fonts/Noto/Serif/
113 src/unpatched-fonts/Hasklig/
115 src/unpatched-fonts/Monoid/ 
145 src/unpatched-fonts/Lilex/  
170 src/unpatched-fonts/VictorMono/                          
198 src/unpatched-fonts/FantasqueSansMono/                   
221 src/unpatched-fonts/VictorMono/                          
385 src/unpatched-fonts/CascadiaCode/                                   
403 src/unpatched-fonts/FiraCode/                            
403 src/unpatched-fonts/JetBrainsMono/Ligatures/             
429 src/unpatched-fonts/Iosevka/

So that is obviously not a good metrics for total lookup size. But ok-ish for our purpose?

Does this work for the ss0x ttfs?

@Finii
Copy link
Collaborator Author

Finii commented Jan 4, 2022

I do not understand the question?

Stylistic Sets are untouched.

Only Iosevka has that many tables.

Hmm.

??

@xsrvmy
Copy link

xsrvmy commented Jan 4, 2022

I was talking about Iosevka's ttfs that have ss pre-applied. Because their file size is much smaller I wasn't sure if detecting via look up table size would work. I tried this branch on them and it still removes the cv's though, so there is no concern.

@Finii
Copy link
Collaborator Author

Finii commented Jan 4, 2022

Oh. In src/unpatched-fonts/Iosevka/ there are no otf fonts, only ttfs.

It detects the number of lookup tables, not size (which is much more involved to determine).

:-)

@Finii
Copy link
Collaborator Author

Finii commented Jan 5, 2022

It's not only fontforge that has a hard time to handle Iosevka.
Also ttfdump crashes (showttf works).

image

It's of course not Iosevka's fault, but ... it is no easy font either.

@GulajavaMinistudio
Copy link

Screen Shot 2022-01-05 at 23 50 16

Trying the latest commit from repository. It still error to patching latest Iosevka version.

@Finii
Copy link
Collaborator Author

Finii commented Jan 5, 2022

Seems you are on master but the fix has not yet been pulled. You need to switch the branch or cherry-pick the fix-commit?

Or am I wrong?

@GulajavaMinistudio
Copy link

Seems you are on master but the fix has not yet been pulled. You need to switch the branch or cherry-pick the fix-commit?

Or am I wrong?

Oh my bad, i forget that the fix is still in pull request. I will try with switching branch soon.

@GulajavaMinistudio
Copy link

Seems you are on master but the fix has not yet been pulled. You need to switch the branch or cherry-pick the fix-commit?

Or am I wrong?

I'm still use font patcher in master branch, and try to use other Iosevka variant called "Iosevka Fixed" that contain no built in ligature and no built in glyph.
And this run successfully without "truncated error" like previous screenshot with Iosevka and Iosevka Term.

Here is newest screenshot with "Iosevka Fixed" variant
Screen Shot 2022-01-07 at 22 46 43

And here package list link for description about Iosevka Fixed. https://github.com/be5invis/Iosevka/blob/v11.2.4/doc/PACKAGE-LIST.md

I think font patcher will get error if there's some built in ligation and glyph, especially in Iosevka

@xsrvmy
Copy link

xsrvmy commented Jan 7, 2022

No idea if the ligation is the real issue. There are some ligatures that have CVs as well.

@GulajavaMinistudio
Copy link

Trying font patcher from branch "bugfix/Iosevka-lookups" , still not successful to patch Iosevka Font with custom build.
Screen Shot 2022-01-08 at 07 22 41

Still showing

"Done with Patch Sets, generating font...
Internal Error: Attempt to output 65574 into a 16-bit field. It will be truncated and the file may not be useful."

Here i attach Iosevka custom build in this comment, maybe you can try to patch and debugging font patcher with it.
ttf-iosevka-mayukai-custombuild.zip

@Finii
Copy link
Collaborator Author

Finii commented Jan 8, 2022

I think font patcher will get error if there's some built in ligation and glyph, especially in Iosevka

Ligatures are no problem, and I persoanlly like the 'programming' ligatures - and use Nerd Fonts with ligatures ever since.

The problem is that Iosevka has a lot lookup tables, and when we (i.e. fontforge) re-packs them runs out of space.
In my opinion ligatures are supported by almost all fontends people can use, and should not be removed without need. On the other hand most programs struggle to support advanced OpenType features like Stylistic Sets (SS) and Alternative Glyps - thats why I thought IF we remove something, let it be those.

@Finii
Copy link
Collaborator Author

Finii commented Jan 8, 2022

Here i attach Iosevka custom build in this comment, maybe you can try to patch and debugging font patcher with it.

Hmm, patching is no problem with Regular. And that files has only 157 tables, so it would not need the bugfix/Iosevka-lookups anyhow (i.e. the branch does not handle that font any different then before).

Dropping of tables would be shown where the red arrow is:

image

and comes out without error:

image

I tried with fontforge:

 Version: 20201107
 Based on sources from 2021-01-15 15:55 UTC-ML-D-GDK3.

and HEAD (self built).


Ah, you have Italic in the name, so you probably did this?

image

which also has no issues here

image

@Finii
Copy link
Collaborator Author

Finii commented Jan 8, 2022

This is how it looks if tables are removed:

image

Maybe you can include the top in your screenshots, i.e. the invocation (command line) and the first few lines until patching starts.

@GulajavaMinistudio
Copy link

Maybe you can include the top in your screenshots, i.e. the invocation (command line) and the first few lines until patching starts.

Here's my additional screenshot and command that i use.
fontforge -script ./font-patcher /Users/gulajavministudio/Documents/WebstormProject/Iosevka-11.2.4/dist/iosevka-mayukai-sonata-terminal-ss03-v574/ttf/iosevka-mayukai-sonata-italic.ttf --careful --complete
Screen Shot 2022-01-09 at 01 46 44

If i use --careful tag, the result is failed with internal error "truncated" like before.
Screen Shot 2022-01-09 at 01 48 16


But if i use without --careful tag, the result is successfully patched.
Screen Shot 2022-01-09 at 01 52 09
The result is success
Screen Shot 2022-01-09 at 01 52 19

@Finii
Copy link
Collaborator Author

Finii commented Jan 8, 2022

Can reproduce.

@Finii
Copy link
Collaborator Author

Finii commented Jan 11, 2022

At least the old and your new issue appear at the same spot when writing the subtables...:

image

@Finii
Copy link
Collaborator Author

Finii commented Jan 11, 2022

I more and more think this is a fontforge bug.

The GSUB table is big, one can not use short to represent indices within them...:

True Type Font File Dumper: v 0.5.5
Copyright 1996-1998 ollie@ms1.hinet.net
Dumping File:iosevka-regular.ttf


Offset Table
------------
         sfnt version:           1.0
         number of tables: 18
   0. 'DSIG' - checksum = 0x00000001, offset = 0x004f3018, len =         8
   1. 'GDEF' - checksum = 0x13de42bd, offset = 0x0000012c, len =       718
   2. 'GPOS' - checksum = 0x0050d227, offset = 0x000003fc, len =    455900
   3. 'GSUB' - checksum = 0xa79d9c51, offset = 0x0006f8d8, len =    179962
   4. 'OS/2' - checksum = 0x71108694, offset = 0x0009b7d4, len =        96
   5. 'cmap' - checksum = 0x01d678f7, offset = 0x0009b834, len =      5772
   6. 'cvt ' - checksum = 0x337614d2, offset = 0x004f206c, len =       200
   7. 'fpgm' - checksum = 0x622f037f, offset = 0x004f2134, len =      3596
   8. 'gasp' - checksum = 0x00000010, offset = 0x004f2064, len =         8
   9. 'glyf' - checksum = 0xbd7c4a19, offset = 0x0009cec0, len =   3977248
  10. 'head' - checksum = 0x1ae21d3f, offset = 0x00467ee0, len =        54
  11. 'hhea' - checksum = 0x049b52f3, offset = 0x00467f18, len =        36
  12. 'hmtx' - checksum = 0x82097cfe, offset = 0x00467f3c, len =     91238
  13. 'loca' - checksum = 0x87347f18, offset = 0x0047e3a4, len =     96724

GPOS is even bigger, is processed before GSUB, and has no issue although it is generated by the same function!

Continuing to examine whilst doing Zoom 🥱

@Finii
Copy link
Collaborator Author

Finii commented Jan 11, 2022

Found. Creating patch over there.

@Finii
Copy link
Collaborator Author

Finii commented Jan 11, 2022

fontforge would be fixed with fontforge/fontforge#4883, but then the question is how long will it take to be pulled, released, distributed, ...

I'm not sure what we should do with this issue. This PR does not work reliably, maybe I can come up with an intermediate solution.

@Finii
Copy link
Collaborator Author

Finii commented Jan 24, 2022

Fontforge merged the bugfix into MASTER, so in principle we could generate the patched Iosevka with a self-build fontforge.

But somehow we need a solution for self patcher that use an older fontforge, at least a warning or something...

@Finii Finii mentioned this pull request Aug 19, 2022
2 tasks
@Finii
Copy link
Collaborator Author

Finii commented Aug 19, 2022

Fixed via #884

@Finii Finii closed this Aug 19, 2022
@Finii Finii deleted the bugfix/Iosevka-lookups branch March 16, 2024 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some Iosevka variants broken
3 participants