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

accented glyphs are not composites #169

Closed
laerm0 opened this issue Jul 4, 2019 · 8 comments
Closed

accented glyphs are not composites #169

laerm0 opened this issue Jul 4, 2019 · 8 comments

Comments

@laerm0
Copy link

laerm0 commented Jul 4, 2019

Hi –

I was taking a look at @thundernixon's fork to figure out build issues I am having on a different VF and I noticed that the accented glyphs are not composites. This may be intended; if so, can you tell me why? I'm trying to debug poor accent alignment of instances of a VF so maybe you've discovered some info I could use. If not intended, well, now you know. :)

@thundernixon
Copy link
Contributor

thundernixon commented Jul 12, 2019

I dug around a bit in the code to seek out an answer, and found this comment in fontbuild:

# We decompose any glyph with two or more components to make sure
# that fontTools varLib is able to produce properly-slanting interpolation.

When I commented out line 194 to avoid decomposing glyphs, I found out what that means. In a wakamai fondue test:

changing the slant axis in a test version of Inter

As far as I can tell, components that are made with reflected transformations suffer from slanting in the incorrection direction.

So, for now, it clearly makes sense to continue to decompose glyphs for the full variable font. However, if this could be solved (whether in VarLib or in Inter), this would probably be a really big win for future versions of Inter. In my export tests, the decomposed version is 1.3 MB, compared to just 720 KB for the version that keeps components (a 43% filesize reduction). Even when both are compressed to woff2, keeping components takes the file from 392 KB to 290 KB (26% reduction).

As a big plus, I think this would probably also save significant filesize in all versions of Inter (roman-only VF as well as static fonts).

@thundernixon
Copy link
Contributor

Hoping for an extremely easy fix, I corrected the Italic angle to -10 (as per the OT spec), then rebuilt. No such luck. Slanting still happens in the wrong direction. I can't find an open issue for varLib, so I'm writing one.

@thundernixon
Copy link
Contributor

thundernixon commented Jul 12, 2019

Oh. This may be completely obvious to others, but it just clicked for me what is probably happening here.

At all locations across the slant axis, the q is a direct, mirrored component of p. The q is simply a mirror of the slanted p, so it leans the opposite way. This is in contrast to what one would intuitively expect: an interpolation between the upright and slanted q (from a mirrored, upright p to the mirrored and forward-slanted p).

@thundernixon
Copy link
Contributor

thundernixon commented Jul 12, 2019

Upon further testing, I think I may be relatively close to a solution, though I probably need help from @rsms to know whether it's breaking something that I'm not catching.

In fontbuild, the composedGlyphIsNonTrivial function checks whether a component has transformations that may not work well for varLib. However, I think that it casts far too wide a net, and results in nearly every glyph with components being decomposed. So, I changed a few things:

  • Instead of decomposing a component if any scaling is happening, I only decompose a component if xScale < 0 or yScale < 0 (that is, if it is mirrored on the x or y axis).
  • I think that yAxisIsNonTrivial was getting triggered on any glyph with an italic angle, due to lines 182-187 in fontbuild. In turn, this added almost all component glyphs to a list for decomposition. I commented this out on lines 79–80.

Here's a permalink to my modifed composedGlyphIsNonTrivial check.

After building a font with these modifications, I have an output VF that is 734 KB. Clearly, a few glyphs are still decomposing (such as the q), but most are preserved.

I made a filter in GlyphsApp to see all glyphs with components, then copied this into a quick test of the new font in Wakamai Fondue. I don't see anything that seems to be slanting in the wrong direction. However, I would want @rsms to help look for problems, as he obviously knows this design best.

image

One thing that suggests that this change has no regression is a comparison of before/after font files, for all glyphs that have components in the source. No glyphs shift between these two tests (the GIF below has two screenshots, showing before and after font files):

inter-components

Here's a permalink to the built VF with most components left intact. Rasmus, when you get a chance, could you please take a look and let me know if you see anything that still seems to be broken along the slant axis?

@rsms
Copy link
Owner

rsms commented Jul 18, 2019

Excellent work @thundernixon! I just built #171 locally and have tested a bunch of composed glyphs — it all seems to work fine!

@rsms
Copy link
Owner

rsms commented Jul 18, 2019

BTW, I'm not seeing a dramatic size decrease. When doing a clean build rm -rf build && make var on your branch #171 and then looking at build/fonts/var/Inter.var.ttf, it's 1.2MB and the woff2 is 384kB. Comparing this to the same files when building without the changes to misc/fontbuild the ttf is 1.3MB and the woff2 393kB. Maybe I'm missing something..?

I downloaded your build product Inter-with_components.var.ttf and it is indeed much smaller than what I'm seeing. Were there perhaps changes you forgot to include in #171 @thundernixon ?

@rsms
Copy link
Owner

rsms commented Jul 18, 2019

I think I found the discrepancy — in your repo the test for components is commented out, but not in the PR branch.

https://github.com/thundernixon/inter/blob/d02b6e4b38da62860183593100ee354785f5b653/misc/fontbuild#L67-L68

[edit] Building with those lines disabled indeed yields the expected results. Doing some testing on glyphs now and will amend the patch with the two lines removed if all checks out.

@thundernixon
Copy link
Contributor

Oh, wow, good catch! Yes, I was digging around and confused at the discrepancy. I believe that is the difference. I've updated the PR. If you try to build with that, are things looking more accurate to my file size results?

@rsms rsms closed this as completed in 3de54b5 Jul 18, 2019
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 a pull request may close this issue.

3 participants