-
-
Notifications
You must be signed in to change notification settings - Fork 47
Fix outline of gamma, fix pad_glyph(), add more kernings with smarter operation #97
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
base: master
Are you sure you want to change the base?
Changes from all commits
b83e63f
953a344
48dc9dd
45b55b3
d033979
2427542
aa9b690
361ef27
b2d2ae3
d23ddf3
398e894
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,27 +66,25 @@ def _expand_with_variants(font, chars): | |
| def autokern(font): | ||
| all_glyphs = [glyph.glyphname for glyph in font.glyphs() | ||
| if not glyph.glyphname.startswith(' ')] | ||
| ligatures = [name for name in all_glyphs if '_' in name] | ||
| ligatures = [name for name in all_glyphs if name[0] != '_' and '_' in name] | ||
| upper_ligatures = [ligature for ligature in ligatures if ligature.upper() == ligature] | ||
| lower_ligatures = [ligature for ligature in ligatures if ligature.lower() == ligature] | ||
|
|
||
| # Expand the broad letter lists to include accented variants from the outset, | ||
| # so every rule that references `caps`, `lower`, or `all_chars` covers them too. | ||
| caps = _expand_with_variants(font, list('ABCDEFGHIJKLMNOPQRSTUVWXYZ') + upper_ligatures) | ||
| lower = _expand_with_variants(font, list('abcdefghijklmnopqrstuvwxyz') + lower_ligatures) | ||
| all_chars = caps + lower | ||
| caps = list('ABCDEFGHIJKLMNOPQRSTUVWXYZ') | ||
| lower = list('abcdefghijklmnopqrstuvwxyz') | ||
| roman = caps + lower | ||
|
|
||
| font.addLookup('kerning', 'gpos_pair', (), [['kern', [['latn', ['dflt']]]]]) | ||
| font.addLookupSubtable('kerning', 'kern') | ||
|
|
||
| def kern(sep, left, right, **kwargs): | ||
| def kern(sep, left, right, damper=None, **kwargs): | ||
| """Wraps font.autoKern: expands accented variants and leading/trailing ligatures.""" | ||
| def expand(chars, left_side): | ||
| expanded = _expand_with_variants(font, chars) | ||
| seen = set(expanded) | ||
| for glyph in font.glyphs(): | ||
| name = glyph.glyphname | ||
| if '_' not in name: | ||
| if name[0] == '_' or '_' not in name: | ||
| continue | ||
| parts = name.split('_') | ||
| # Left side: ligature's right edge (last component) determines spacing. | ||
|
|
@@ -96,26 +94,59 @@ def expand(chars, left_side): | |
| expanded.append(name) | ||
| seen.add(name) | ||
| return expanded | ||
| font.autoKern('kern', sep, expand(left, left_side=True), expand(right, left_side=False), **kwargs) | ||
| lefts = expand(left, left_side=True) | ||
| rights = expand(right, left_side=False) | ||
| font.autoKern('kern', sep, lefts, rights, **kwargs) | ||
| if damper and damper != 1.0: | ||
| for l in lefts: | ||
| tuples = font[l].getPosSub('kern') | ||
| new_table = [] | ||
| for tup in tuples: | ||
| if tup[1] == 'Pair' and tup[2] in rights: | ||
| font[l].addPosSub('kern', *(tup[2:5] + (int(tup[5] * damper),) + tup[6:])) | ||
|
|
||
| def getkern(left, right): | ||
| c = font[left] | ||
| tuples = c.getPosSub('kern') | ||
| for tup in tuples: | ||
| if tup[1] == 'Pair' and tup[2] == right: | ||
| return tup[5] | ||
| return None | ||
|
|
||
| a = font['_pad_space'].width | ||
| a = max(a - 20, 0) | ||
|
|
||
| # The same combination will be overwritten, so the one written last will take effect. | ||
| # autoKern looks at the outline, so even if you change the padding, it absorbs all of it. | ||
| # Use `+a` when you want to link the spacing after kerning to the padding. | ||
| kern(150, ['/', '\\'], ['/', '\\']) | ||
| kern(60+a, ['s'], set(lower) - {'j', 'f'}, minKern=50) | ||
| # x has diagonal strokes that leave visual space on its left side. | ||
| kern(90+a, set(lower) - {'f'}, ['x'], minKern=40) | ||
| kern(60+a, ['s'], set(lower) - {'i', 'j'}, onlyCloser=True) | ||
| # Overwrite sf and st. (From experience, it is often just right to adopt the larger of the two | ||
| # separation required by the glyphs on the left and right.) | ||
| kern(80+a, set(lower) - {'i', 'j'}, ['f', 't', 'x'], onlyCloser=True) | ||
| # x has diagonal strokes that leave visual space on its right side. (glyph changed in 3dbc5d1) | ||
| kern(80+a, ['x'], set(lower) - {'i', 'j'}, onlyCloser=True) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not keen to add kerning for all letter combinations, and think we should start with changing the bounding box of the character in the first instance. Kerning should then be used when we need specific tweaks between pairs of letters because of the letter forms.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kerning originally refers to the process of tightening the space between characters, and by combining the width adjusted to the bounding box with That said, it caught my attention when it was mentioned, so I checked the kern table, and it turns out that *j is kerned across the board (except for gj, jj, and the like). The LSB could probably be tightened by about 20–30. Formula |
||
| kern(100+a, ['f', 't'], set(lower) - {'i', 'j'}, onlyCloser=True) | ||
| # Set *Y altogether first: CY, OY, etc. will have appropriate values set in the latter part. | ||
| # CY is a notable example where it is better to use the smaller separation value. | ||
| kern(105, roman, ['Y', 'T'], onlyCloser=True, damper=0.75) | ||
| kern(35, caps, ['f'], onlyCloser=True, touch=True, damper=0.9) | ||
| # F/E are separated from T/J so they can use a tighter target gap. | ||
| kern(130, ['F'], set(all_chars) - {'f', 'j'}) | ||
| kern(140, ['E'], ['V', 'W', 'Y']) | ||
| kern(100, ['E'], set(all_chars) - {'f', 'j'}) | ||
| kern(120, ['T', 'J'], ['R']) | ||
| kern(150, ['T', 'J'], set(all_chars) - {'f', 'j'}) | ||
| # C: loosen from the default (was too tight for Ct/Cf/Cj). | ||
| kern(65, ['C'], set(all_chars) - {'f', 'j'}) | ||
| kern(60, ['O'], set(all_chars) - {'f', 'j'}) | ||
| kern(110, ['F'], set(roman) - {'j'}, onlyCloser=True, damper=0.75) | ||
| # Since F and z mesh together and the kerning becomes too large, | ||
| # reuse the kerning value of one of the round letterforms. | ||
| diff_Fo_Fz = getkern('F', 'o') - getkern('F', 'z') | ||
| kern(110 + int(diff_Fo_Fz / 0.75), ['F'], ['z'], onlyCloser=True, damper=0.75) | ||
| kern(90, ['E'], set(roman) - {'j'}, onlyCloser=True, damper=0.75) | ||
| kern(115, ['T', 'J'], set(roman) - {'j'}, onlyCloser=True, damper=0.75) | ||
| kern(105, ['Y'], set(roman) - {'j'}, onlyCloser=True, damper=0.75) | ||
| kern(95, ['V'], set(roman) - {'j'}, onlyCloser=True, damper=0.75) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally I would like to be reviewing the need for new kerning pairs indivitually - for example, adding V against the full set of letters is a change that I would want to consider in isolation ideally. |
||
| kern(65, ['C'], set(roman) - {'j'}, onlyCloser=True, damper=0.75) | ||
| kern(40, ['E', 'C'], ['V'], onlyCloser=True, touch=True) | ||
| kern(60, ['O', 'P'], set(roman) - {'j'}, onlyCloser=True, damper=0.75) | ||
| kern(70, ['D'], set(roman) - {'j'}, onlyCloser=True, damper=0.75) | ||
| kern(150, roman, ['j'], onlyCloser=True, damper=0.75) | ||
| kern(40, ['L'], roman, onlyCloser=True, touch=True, damper=0.9) | ||
|
|
||
|
|
||
| autokern(font) | ||
|
|
@@ -208,7 +239,7 @@ def expand(chars, left_side): | |
| # hinting, which alters the rendered pixel positions of Latin letters. Pin | ||
| # all values here (derived from the Latin+diacritic glyph set) so the | ||
| # hinting is stable regardless of how many non-Latin glyphs are added. | ||
| font.private['BlueValues'] = (-20, 20, 411, 450, 573, 613) | ||
| font.private['BlueValues'] = (-10, 20, 411, 441, 573, 603) | ||
| font.private['OtherBlues'] = (-241, -190) | ||
| font.private['BlueScale'] = 0.0208333 | ||
| font.private['BlueShift'] = 16 | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The gamma change here looks like it is significant, rather than a pixel difference. The location doesn't look correct in the new version?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the charmap before the change, it seems that the position of gamma was shifted as a result of matplotlib's ax.text() trying to fit the outline within the frame. (A similar behavior can also be seen with U+0327. It seems to be an issue at a level different from the font's yMax height.) |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the original handwriting sample, I agree that the spacing is better here. In particular, looking at "unevangelic" the letters are clearly distinct, whereas in the existing sample before this PR, they are on the verge of touching each other. 👍 to the diff. |

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm worried we don't have much visual test coverage of the kerning between accented variants, and therefore want to be sure how how this lands for those characters, even though I can see that the expand call is done within the kern function.
Perhaps we need another sample which covers a bunch of the different languages that we have added support for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, taking that point into consideration, in my previous fork, individual kerning was applied only to basic_latin.