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

Change to mark-zeroing in HB 1.2.0 has regressed some fonts due to bad GDEF tables #264

Closed
jfkthame opened this issue Jun 14, 2016 · 18 comments

Comments

@jfkthame
Copy link
Collaborator

See mozilla bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=1279925
https://bugzilla.mozilla.org/show_bug.cgi?id=1279693

I confirmed that reverting b3582a8 (and the followup eaadcbb) fixes the problem with IPA characters in Tahoma, using the font shipped with Windows 8.1. (This seems to be a long-standing font bug, given that we also have an old report at https://bugzilla.mozilla.org/show_bug.cgi?id=653993.)

@behdad
Copy link
Member

behdad commented Jun 14, 2016

Ack. Sorry for the late response; been up in the air. Shall we blacklist some more fonts?

@jfkthame
Copy link
Collaborator Author

n/p, I'm also away from my usual desk this week.

I'm not sure if the individual table-based blacklist is the best approach here, given that this seems to be a longstanding bug that has existed in numerous releases of Tahoma/TahomaBold; it'll get pretty ugly to enumerate them one by one. I have at least 8 problematic fonts on hand already, without going in search of versions from Windows 7 or from older/newer MacOSX releases than what I'm currently running.

How much would we lose if we reverted to zeroing by Unicode for Latin? (And for Tibetan, per bug 1279693.)

@behdad
Copy link
Member

behdad commented Jul 21, 2016

I merged this, but keeping issue open to discuss further changes.

@behdad behdad reopened this Jul 21, 2016
@dequis
Copy link

dequis commented Aug 19, 2016

Oh hi there, spent a couple of hours doing regression tests between firefox 46 and 47 and then between harfbuzz 1.1.3 and 1.2.0* and I just noticed this is my issue!

Segoe UI and Noto Sans seem to turn the backtick character into a combining character after b3582a8. The blacklisted stuff in master doesn't help.

This happens mainly in github, most other websites don't use those fonts.

*For some reason doing make clean && make && sudo make install while bisecting led me to wrong results, as if the binary didn't change. Took me way too long to realize ./configure was needed too. Not the smoothest bisection.

@behdad
Copy link
Member

behdad commented Sep 2, 2016

@jfkthame given more reports that came in, what do you think we should do here?

@jfkthame
Copy link
Collaborator Author

jfkthame commented Sep 2, 2016

Remind me, if we switch the generic shaper back to zeroing by Unicode instead of GDEF, what will that break? Anything?

@behdad
Copy link
Member

behdad commented Sep 2, 2016

Remind me, if we switch the generic shaper back to zeroing by Unicode instead of GDEF, what will that break? Anything?

Last time it was breaking minor scripts. So, if we change, we should change just Latin or LGC. But I prefer to either 1. respect what font designer put into font, or 2. figure out what heuristics DirectWrite is using and possibly emulate that.

I hear DirectWrite might be skipping shaping if there's no default-on feature in the font (globally) other than liga and kern. I'll ask for clarification. Note that I'm NOT suggesting we skip shaping or necessarily use this heuristic to decide whether to ignore GDEF. I just want to better understand why DirectWrite does not have the same bug.

@jfkthame
Copy link
Collaborator Author

jfkthame commented Sep 5, 2016

@dequis What version of Segoe UI are you using where you see this problem?

@dequis
Copy link

dequis commented Sep 5, 2016

fc-query shows fontversion: 350618(i)(s) (full output)

It was taken from windows 8.0 or 8.1, don't remember which one i had at the time.

$ md5sum segoeui.ttf
b9e9836ef247552bfd77832dfac2d8d8  segoeui.ttf

@jfkthame
Copy link
Collaborator Author

jfkthame commented Sep 6, 2016

That's Segoe UI from Win8.1, I believe. But I'm not seeing this issue with that font. Are you sure you're entering the (spacing) ASCII grave character, U+0060? Might you have a keyboard layout that's actually generating a combining grave accent, U+0300?

@dequis
Copy link

dequis commented Sep 6, 2016

Okay, my bad, infinality's variant of fontconfig must have been replacing Segoe UI (which is what github specifies in its font-family) with Noto Sans. After replacing with stock fontconfig/freetype2/cairo/etc github looks noticeably different and the issue is not present when that font is selected, it's just Noto Sans.

The character is definitely fine as it's what I use for markdown code blocks. Oh man it feels so good to be able to write those normally again.

:D:D:D

@behdad
Copy link
Member

behdad commented Sep 7, 2016

@dequis what version of Noto Sans is that? Where did you get it from?

@dequis
Copy link

dequis commented Sep 7, 2016

Uh, yeah, about that... This is embarrassing. I had a really old package, ttf-google-fonts-git-20130806. I noticed this after posting my last comment and tried to rebuild a new one yesterday but got a kernel panic probably because of overheating when compressing the package. So I'll report back if it turns out that the new one fixes it and that I'm an idiot. I apologize in advance.

Current version is fontversion: 68157(i)(s) (fc-query output and md5sum)

@dequis
Copy link

dequis commented Sep 7, 2016

fontversion: 69468(i)(s) isn't affected.

However, building a new ttf-google-fonts-git package didn't help, because it grabs from the google/fonts git repo which still has the same NotoSans-Regular.ttf version from three years ago, 68157, which has this issue. So, not 100% my fault! Yay!

If it matters, the exact file I had (uploaded here) was subtly different from that one, even though both have the same version and the same issue. That git repo didn't even exist back in 2013 apparently (initial commit says 2015). I don't know.

fc-query output and md5sum of the other 68157 from the git repo, for diffing

@behdad
Copy link
Member

behdad commented Sep 7, 2016

cc @davelab6

@KrasnayaPloshchad
Copy link

KrasnayaPloshchad commented Sep 27, 2016

I just want to better understand why DirectWrite does not have the same bug.

I think making more improvements on hb-directwrite would get help.

@ebraminio
Copy link
Collaborator

I think making more improvements on hb-directwrite would get help.

What improvements it needs? Perhaps I can help.

@behdad
Copy link
Member

behdad commented Mar 2, 2017

Closing until another push comes in.

@behdad behdad closed this as completed Mar 2, 2017
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