Fea fix mark mkmk

[cjdunn]

I removed the empty mark and mkmk feature code so fontmake would build these features automatically. So far it looks like it's working. If you need these to be built differently please let us know.

[davelab6]

@cjdunn please resolve conflicts :)

[m4rc1e]

@cjdunn Its much better. The mark positioning issues on the 'i' variants has been solved.

This issue still persists though.

before

after

---

In order to help you folks. I've added a new branch called mf-regression. This contains a dir called regression-tests. You'll find a script to generate the diff images and a set of diffs for the Thin, Reg and Black weights for a range of platforms/browsers. I will generate diff images for all the styles tomorrow.

This branch shouldn't be merged into the master. It is solely there for testing purposes. Perhaps we can start adding unit/functional tests and merge it into the master once its better.

cc @davelab6

[m4rc1e]

Inspect supplied diff images and solve the last remaining gpos issues.

[cjdunn]

@davelab6 I saw those merge conflicts, but they appear to all be binary files so I was confused. Thought you guys might know why the conflict was happening. I'll fix now.

@m4rc1e Glad to hear the issue with the i accents are resolved. Can you provide a live html proof? It's hard to tell which glyphs you are referencing in those blurry images. Thanks.

[dberlow]

Looks like licorice fondue to me too.

…

On Tue, Apr 3, 2018 at 1:39 PM, CJ Dunn ***@***.***> wrote: @davelab6 <https://github.com/davelab6> I saw those merge conflicts, but they appear to all be binary files so I was confused. Thought you guys might know why the conflict was happening. I'll fix now. @m4rc1e <https://github.com/m4rc1e> Glad to hear the issue with the i accents are resolved. Can you provide a live html proof? It's hard to tell which glyphs you are referencing in those blurry images. Thanks. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#5 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB3ajq6AjRclHTlFqOcD94J8ozgTritbks5tk7POgaJpZM4TCHsl> .

[cjdunn]

@davelab6 I overwrote the .ttfs in the master_ttf_interpolatable folder with the old ones from the Master branch so it's not showing a conflict anymore. I still don't know why one set of binaries will "merge" and another set wont, but anyway GitHub says it's OK now.

[m4rc1e]

@cjdunn Check these combos:
ǥ᪻
ŋ᪼
đ᪼
ǥ᪼

Before:
http://gf-regression.com/screenshot/222e38dc-9cf7-4f22-b8df-6cd54663cb26/glyphs-all/before/80

After:
http://gf-regression.com/screenshot/222e38dc-9cf7-4f22-b8df-6cd54663cb26/glyphs-all/after/80

[cjdunn]

Thanks @m4rc1e this is helpful! Will take a look

[davelab6]

(I guess the merge conflict was caused by Sam moving things around; I guess that a rebase would do the trick to bring your branch back into merge compatibility :)

[sberlow]

Geez, cleaning things up never pays. ;) -- Sam Berlow General Manager Font Bureau 151 Beach Road Tisbury, Ma. 02568 508-693-2425 www.fontbureau.com

[cjdunn]

@m4rc1e
I tracked down the issue: The anchors in /eng /dcroat and /gbar didn't match those in the related combining marks. They were named 'parenthesses.w1' instead of 'markparenthesses.w1' etc. I duplicated the anchors with the correct names. I didn't delete the old ones in case they were used by something else (these kind of duplicates seem to happen elsewhere too so I followed that example).

The /uni1ABC and related glyphs (uni1ABC.w1 etc) seem to be attaching now:

Will push my changes now

[cjdunn]

I removed those TTFs so it wouldn't complain about a merge conflict. But every time I build, those TTFs will get written again. Is anyone opposed to me adding a .gitignore? Or, I'm open to other solutions

[davelab6]

If those TTFs are just build artifacts, it seems reasonable to add them to gitignore

[cjdunn]

@davelab6 OK, I added a .gitignore for all files in that folder, so hopefully that will avoid future conflicts

[m4rc1e]

@cjdunn cheers its solved.

I'm still reviewing and I'll be adding peices of #4 as they come up.

[cjdunn]

@m4rc1e glad to hear the previous issue is solved.

I got a notification about an issue in Roboto-Thin with what looks like uni1AB5 "Combining X-X below" and I can see that it doesn't match the previous Roboto. I can make it match the old one (see version 1 screenshot) but it appears to be mechanically stretched, and the way I had it (see version 2 screenshot) appears to match the Roman. Please confirm how you would like to proceed.

Version 1 (Roboto-Thin looks mechanically stretched)

Version 2 (Roboto-Thin looks closer to Roboto-Regular)

Please confirm which version you prefer, thanks.

Will look forward to hearing what else you find after you finish your review, but just wanted to let you know what I found so far.

[davelab6]

Version 2 please

[davelab6]

Should this PR be closed?

[cjdunn]

@davelab6 I have fixed the original issue related to mark and mkmk features. Please merge and close if you are happy with my changes.

[davelab6]

I delegate the checking to @m4rc1e :) But I believe this is good to merge, so I am doing so

[davelab6]

Oh hmm, 'rebase and merge' has conflicts, so I am holding off for now.

@m4rc1e what do you reckon? :)

[anthrotype]

It will always have merge conflicts because binary ttf files are not meant to be checked in a git source code repository. They are the output of the build, not the source files.

[m4rc1e]

@davelab6 I'm pretty certain this can be closed. #8 has included these changes. @cjdunn is my assumption correct?

[cjdunn]

@m4rc1e Yes, #8 has included these changes. Please feel free to close this if you are happy with #8

[m4rc1e]

Thanks closing