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

"Overeager" glyph search for certain characters breaks fallback in Blink #152

Closed
drott opened this issue Oct 21, 2015 · 7 comments
Closed

Comments

@drott
Copy link
Collaborator

drott commented Oct 21, 2015

When working on shaper driven run segmentation in Blink I am observing a few situations in which the shaper seems to be a bit overeager in finding a renderable character.

  1. The first situation is using the ASCII parentheses instead of a .notdef for the fullwidth parentheses: ./hb-shape --shapers=ot /Library/Fonts/Times\ New\ Roman.ttf``../test/shaping/hb-unicode-encode U+FF08 results in [parenleft=0+682]. In CJK texts that leads to slightly broken spacing, since for example the fallback chain has Times first, then a CJK font. HarfBuzz will render the parentheses with Times, then the CJK text with the fallback font, leading to uneven spacing and a suboptimal mix of fonts. If HarfBuzz would just give up here and return .notdef, the fonts would not be mixed, leading to a more pleasant appearance.
  2. The second example that showed in Blink tests: ./hb-shape --shapers=ot /Library/Fonts/Arial.ttf 𝒞 results in [C=0+1479] - so the mathematical script C is converted to a Latin capital C. In Blink's svg/text/non-bmp-positioning-lists.svg layout test however, the mathematical script is expected and was previously shown due to functioning system fallback for such symbols.
@jfkthame
Copy link
Collaborator

Sounds like you're allowing harfbuzz to use compatibility decompositions. IMO, that's generally not desirable for text shaping; it will lead to a lot of undesirable substitutions.

I'd recommend setting up unicode callbacks that provide ONLY canonical (de)compositions.

@khaledhosny
Copy link
Collaborator

You need to disable the compatibility decomposition Unicode function.

@khaledhosny
Copy link
Collaborator

I had argued that HarfBuzz shouldn’t even try to do compatibility decomposition in the first place, it almost always results in the wrong glyph, but it is hidden in most big clients as they do fallback before shaping.

@drott
Copy link
Collaborator Author

drott commented Oct 21, 2015

Thanks @khaledhosny for taking a look so quickly. Would you perhaps have some more instructions on how I would do that?

Blink's hb_font_t and hb_face_t logic is in https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp&sq=package:chromium&l=281&rcl=1445426673 - for glyph retrieval we defer to harfBuzzGetGlyph which passes it to Skia, unless a variation selector is encountered. Is that the function you refer to?

Or do I need to change compile time options to prevent HarfBuzz from doing that?

@khaledhosny
Copy link
Collaborator

Blink does not seem to call hb_buffer_set_unicode_funcs anywhere, so it presumably uses one of HarfBuzz’s Unicode functions providers. This is what I did in LibreOffice as we were using the ICU functions already (by the means of just including hb-icu.h).

@drott
Copy link
Collaborator Author

drott commented Oct 21, 2015

Great, yes - I just started browsing HarfBuzz' code and was getting somewhere near. Thanks for the pointers, I'll try that in Blink. Closing the issue for now. Thanks for your help.

@drott drott closed this as completed Oct 21, 2015
@behdad
Copy link
Member

behdad commented Oct 21, 2015

Sorry about this. Yes, I need to disable the compat stuff... Will do this time.

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

4 participants