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

[fix-nonhinting] Changing maxp maxSizeOfInstructions when adding prep table #912

Open
kalapi opened this issue Apr 5, 2024 · 10 comments
Open

Comments

@kalapi
Copy link
Contributor

kalapi commented Apr 5, 2024

Hello!

I'd like to preface this by saying that i'm unsure whether or not this needs to be implemented/fixed, but since the maxp table handles memory requirements I was thinking that it might be important to get right.

Issue description

When I run gftools fix-unhinted a prep table is added to the font but the maxSizeOfInstructions remains unchanged. The prep table generated for me has 7 lines(bytes) of instructions and thus this should reflect in the maxp table

Interested to know your thoughts and thanks in advance for your help!

@simoncozens
Copy link
Contributor

the maxp table handles memory requirements

Does it, though? :-) I mean, historically, perhaps. But these days I don't know if any TrueType implementations are going to barf because the maxSizeOfInstructions is wrong. Worth noting that font["maxp"].recalc() in fontTools computes everything apart from the maxSizeOfInstructions.

@vitorsr
Copy link

vitorsr commented Sep 28, 2024

@simoncozens, wouldn't you rather kindly consider deprecating this fix-nonhinting "hack"?

Today it has virtually no impact on almost all renderers but can cause confusion somewhere in the pipeline on the hinting state of an unhinted TrueType font.

As an example fontconfig checks for the existence of a prep table to generate the fonthashint pattern element (added here [1]) which may influence the not autohinting of unhinted fonts either directly or indirectly.

(I have opened an issue on fontconfig about this [2].)

gftools is not the only project to apply this patch to unhinted fonts but it is one of the most impactful in connection to web fonts.

[1] https://gitlab.freedesktop.org/fontconfig/fontconfig/-/commit/80047ed8e8b63153ad2014f731453eb47c79c296

[2] https://gitlab.freedesktop.org/fontconfig/fontconfig/-/issues/426

@simoncozens
Copy link
Contributor

I'm going to defer to @m4rc1e here since he actually understands hinting somewhat. (I don't.)

@m4rc1e
Copy link
Collaborator

m4rc1e commented Sep 30, 2024

tbh, I have no clue how to calculate maxSizeOfInstructions and it seems everyone else is also pretty perplexed by it, fonttools/fonttools#902.

@m4rc1e
Copy link
Collaborator

m4rc1e commented Sep 30, 2024

Regarding @vitorsr's question about deprecating gftools fix-non-hinting. This does sound possible in 2024 since Win XP, 8 etc are all EOL so we shouldn't be worrying about them. I'll make some tests.

@vitorsr
Copy link

vitorsr commented Sep 30, 2024

Thanks, @simoncozens.

@m4rc1e - Akira Tagoh has submitted a patch following my issue report and it should be able to resolve the fonthashint pattern element false positive for unhinted fonts "fixed" with fix-nonhinting:

https://gitlab.freedesktop.org/fontconfig/fontconfig/-/commit/5e058033fe6b4caddd4eddd3a9e0289ef69c5748

This resolves the issue on FreeType-fontconfig platforms.

It still would be a good idea to eventually deprecate it. This only matters for deliberate unhinted monochrome rendering at very small sizes or very light weights, but has the downside of generating false positives for presence of hinting in unhinted fonts.

@vitorsr
Copy link

vitorsr commented Sep 30, 2024

@m4rc1e, still on the topic of hinting - Google Fonts is shipping unhinted static instances for hinted variable sources I think because of a fonttools call in gftools that defaults to perform overlap removal which strips hints:

https://github.com/googlefonts/gftools/blob/v0.9.70/Lib/gftools/instancer.py#L28-L40

The generated static instance will preserve all hinting tables such as fpgm, cvt, prep and others applicable if the variable source has them but glyphs will be stripped of instructions following overlap removal. The result is that it effectively generates unhinted statics.

Note that the overlap argument (OverlapMode.REMOVE) deviates from the fonttools default (OverlapMode.KEEP_AND_SET_FLAGS):

https://github.com/fonttools/fonttools/blob/4.54.1/Lib/fontTools/varLib/instancer/__init__.py#L1576


While I am unsure the gftools instancer is called on the builder, the call to fontmake defers to its default:

https://github.com/googlefonts/gftools/blob/v0.9.70/Lib/gftools/builder/recipeproviders/googlefonts.py#L194-L195

... which is also to remove overlaps:

https://github.com/googlefonts/fontmake/blob/v3.9.0/Lib/fontmake/font_project.py#L535

And the remove_overlaps (True) argument differs from the ufo2ft default for removeOverlaps (False):

https://github.com/googlefonts/ufo2ft/blob/v3.3.0/Lib/ufo2ft/_compilers/baseCompiler.py#L40

The UFO-built interpolated instance with removed overlaps will have invalid hinting upon attempted instruction transfer or compilation.


I previously made a rant about it in google/fonts#7007 (comment) - apologies for the fuss.

I do recall keeping overlaps has cross-platform issues that need to be solved by setting correct flags on glyf elements.

However, it would be interesting to investigate how to reliably apply hinting especially for expensive hand-tuned VTT instructions.

Because as is I am seeing unhinted Google Fonts web fonts everywhere all the time.

@m4rc1e
Copy link
Collaborator

m4rc1e commented Sep 30, 2024

@vitorsr welcome to the rabbit hole ;-)

If we decide to keep overlaps, we are able to keep the hints. However, some printers dislike overlaps and simply setting glyf table bits is also causing some unintended consequences for some of our platforms.

@vitorsr
Copy link

vitorsr commented Sep 30, 2024

welcome to the rabbit hole ;-)

😭

Maybe consider then adding a step in the builder to run ttfautohint on detected breakage (say, nonempty cvt, fpgm, gasp, prep but empty instructions on basic Latin glyf elements)?

In any case, thanks for the attention. Really appreciate your, Simon's and the GF team's work.

@davelab6
Copy link
Member

consider deprecating this fix-nonhinting "hack"?

I am open to this

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

No branches or pull requests

5 participants