Skip to content

Fix outline of gamma, fix pad_glyph(), add more kernings with smarter operation#97

Open
dummy-index wants to merge 11 commits into
ipython:masterfrom
dummy-index:fix/kerning_etc
Open

Fix outline of gamma, fix pad_glyph(), add more kernings with smarter operation#97
dummy-index wants to merge 11 commits into
ipython:masterfrom
dummy-index:fix/kerning_etc

Conversation

@dummy-index
Copy link
Copy Markdown
Contributor

  • The gamma outline was broken, caused by combination of the wavy outline and changeWeight(). (a part of Minor kerning improvements #70)
  • If you align positions using the values from boundingBox() without having called addExtrema() and then round them, the extrema end up in positions that are difficult to round, making it hard to converge. It's better to call addExtrema() first. (However, it seems that for a few characters, the positions of the extrema are still changing)
  • Due to the above changes, the effect of automatic hinting changed, so I adjusted the BlueValues and the y-direction positions of A and N. Also, while I was at it, I corrected the lifting of j that had occurred in 3dbc5d1.
  • Regarding pad_glyph(), I modified the algorithm and added comments. Due to this bug, the letter-spacing of some characters had become excessively tight.
  • Added numerous kernings. Improved processing.

Copy link
Copy Markdown
Member

@pelson pelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it is possible to separate the kerning changes proposed from the non-kerning changes that are here? The latter are ready to merge 👍, but I think the kerning changes need us to be on the same page, and for us to individually asses the merit of each full-set kerning change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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?


# 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)
Copy link
Copy Markdown
Member

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?

# 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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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.

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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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.

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 this pull request may close these issues.

2 participants