Bug 102603 – Enable proper kerning and ligatures support for HarfBuzz-ng complex text path

• Status: ASSIGNED
• Component: Text

[Eli Fidler]

Currently, the HarfBuzz-ng complex text path has a lot of bugs and missing functionality in kerning and ligature support. Here are some examples:

The HarfBuzz-ng complex text path automatically enables the OpenType GPOS "kern" feature. This means that the width of a run depends on whether we got sent to the complex path (which might have been because of a combining character). See Dan Bernstein's comment at https://bugs.webkit.org/show_bug.cgi?id=101009#c31 . I believe this is the issue blocking 100794. I think this is also causing bad results for 

TrueType "kern" table kerning is not supported at all.

-webkit-font-kerning does basically nothing.

-webkit-font-variant-ligatures does basically nothing.

[Eli Fidler]

That should have been "...bad results for lots of layout tests".

[Eli Fidler]

I have a patch to fix this on the BlackBerry port. We use HarfBuzz-ng, but with a backend that isn't upstreamed yet (HarfBuzzNGFaceIType.cpp). 

I'll need someone to help implement TrueType kerning support for the other HarfBuzzNGFace* implementations (Cairo, CoreText, Skia).

[Eli Fidler]

Created attachment 174837 [details]
Patch

[Eli Fidler]

for Chromium folk, https://code.google.com/p/chromium/issues/detail?id=41990 also looks related

[Dominik Röttsches (drott)]

Comment on attachment 174837 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174837&action=review

Looks interesting, thanks for the patch.

> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzNGFaceCairo.cpp:124
> +    // FIXME: get the TrueType "kern" table kerning adjustment

Could you show the Blackberry implementation of this function that you have, just to have some kind of template of what is needed here? 
Looking at harfbuzzGetGlyphExtents for example, you can see how we get from hbFont to cairo_scaled_font_t, and if needed, we can get to the underlying freetype font from there to get this kerning information.

[Eli Fidler]

static hb_position_t harfbuzzGetGlyphHorizontalKerning(hb_font_t *hbFont, void *fontData, hb_codepoint_t leftGlyph, hb_codepoint_t rightGlyph, void *userData)
{
    FS_STATE* font = reinterpret_cast<FontPlatformData*>(fontData)->font();

    FS_FIXED dx = 0, dy;
    if (FS_get_kerning(font, leftGlyph, rightGlyph, &dx, &dy) != SUCCESS)
        return dx;

    return dx;
}

static hb_position_t harfbuzzGetGlyphVerticalKerning(hb_font_t *hbFont, void *fontData, hb_codepoint_t topGlyph, hb_codepoint_t bottomGlyph, void *userData)
{
    FS_STATE* font = reinterpret_cast<FontPlatformData*>(fontData)->font();

    FS_FIXED dx, dy = 0;
    if (FS_get_kerning(font, topGlyph, bottomGlyph, &dx, &dy) != SUCCESS)
        return dy;

    return dy;
}

[Dominik Röttsches (drott)]

(In reply to comment #6)
> static hb_position_t harfbuzzGetGlyphHorizontalKerning(hb_font_t *hbFont, void *fontData, hb_codepoint_t leftGlyph, hb_codepoint_t rightGlyph, void *userData)
> [...]

Alright, I think these values can be retrieved quite easily using http://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html#FT_Get_Kerning - I can look into it, but can't say when I get to it, yet.

[Dominik Röttsches (drott)]

Created attachment 175630 [details]
Backend Implementation for Cairo/Freetype

Eli, you may use this one on top of your patch. Attribution in the ChangeLog would be appreciated.
I quickly tried with font-kerning.html and some other tests in fast/text and it does something that looks expected. 
How can we verify the results more precisely?

[Eli Fidler]

Sorry Dominik, I haven't had a chance to try out your patch yet. I'd expect that with out patches combined, you'd see basic kerning working (like fast/text/font-kerning.html), GPOS kerning not enabled automatically (i.e. only when text-rendering:optimizelegibility or some explicit feature enable is specified),
and ligatures working. 

We don't really have a featureful font accessible to all DRT ports, so it's hard to layout test this.

[Dominik Röttsches (drott)]

(In reply to comment #9)

> We don't really have a featureful font accessible to all DRT ports, so it's hard to layout test this.

Is there an open source one with a permissive open source license? For EFL & GTK, a font pack is downloaded. We can add any fonts with a compatible license to that font pack and test with it.

[Tony Chang]

(In reply to comment #10)
> (In reply to comment #9)
> 
> > We don't really have a featureful font accessible to all DRT ports, so it's hard to layout test this.
> 
> Is there an open source one with a permissive open source license? For EFL & GTK, a font pack is downloaded. We can add any fonts with a compatible license to that font pack and test with it.

In the past, bashi has created custom fonts to test features using shareware or open source font creation tools.  I'm not sure how hard this is to do.

[Kenichi Ishibashi]

Hi Eli, Dominik,

Thank you for the patch. Skia doesn't provide an API to retrieve kerning information so harfbuzzNGFaceSkia can't support this at this time (Maybe Skia doesn't want to add such API). Please just leave TODOs for harfbuzzNGFaceSkia.
 
(In reply to comment #11)
> In the past, bashi has created custom fonts to test features using shareware or open source font creation tools.  I'm not sure how hard this is to do.

I used fontforge, but creating a font isn't a trivial task.

I think we can use arial font. I confirmed that Mac's arial and Ubuntu's arial have kern table.

[Dominik Röttsches (drott)]

(In reply to comment #12)

> Thank you for the patch. Skia doesn't provide an API to retrieve kerning information so harfbuzzNGFaceSkia can't support this at this time (Maybe Skia doesn't want to add such API). Please just leave TODOs for harfbuzzNGFaceSkia.

I don't know about this area, just out of curiosity, is there a way to access the underlying "raw" Freetype handle through Skia? In that case, we could retrieve this info..?

[Kenichi Ishibashi]

(In reply to comment #13)
> I don't know about this area, just out of curiosity, is there a way to access the underlying "raw" Freetype handle through Skia? In that case, we could retrieve this info..?

No. Skia doesn't expose FT_Face. I asked Skia guys to expose it, but they didn't want to do.

[Adam Barth]

Comment on attachment 174837 [details]
Patch

Marking r- to kick out of the EWS queue.

[Dominik Röttsches (drott)]

Eli, are you still working on this? If it's blocked for Skia right now - how about landing parts of this patch and enabling it on EFL & GTK?

[Eli Fidler]

Sorry, I haven't had a chance to get back to this upstream. Hopefully I can split it out soon.

[Dominik Röttsches (drott)]

Let me know if you could use any help with that.

[Eli Fidler]

Created attachment 188214 [details]
Patch

[Eli Fidler]

ok, this patch is cleaned up and merges the Cairo backend. It's not ready for review yet, since fast/text/ipa-tone-letters.html is failing and I can't figure out why. I'm worried that width calculation of complex text runs might be broken for all HarfBuzzNG ports (i.e. not calculated in the same way as simple text, or other complex text ports), but I haven't managed to track it down yet.

My EFL build seems flaky, so maybe I'm doing something wrong.

[Dominik Röttsches (drott)]

(In reply to comment #20)
> ok, this patch is cleaned up and merges the Cairo backend. It's not ready for review yet, since fast/text/ipa-tone-letters.html is failing and I can't figure out why. I'm worried that width calculation of complex text runs might be broken for all HarfBuzzNG ports (i.e. not calculated in the same way as simple text, or other complex text ports), but I haven't managed to track it down yet.

Do you have any other symptoms except this one failing? Any other pointers what to look for? I can add it to my list and try to take a look as well.

> My EFL build seems flaky, so maybe I'm doing something wrong.

Send me an email if you like - I can help getting it built. We use Ubuntu 12.04, and a command line like:
./Tools/Scripts/build-webkit --efl --cmakeargs="-DSHARED_CORE=ON" --debug
should work fine on such a system.

[WebKit Review Bot]

Comment on attachment 188214 [details]
Patch

Attachment 188214 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16775835

New failing tests:
svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorStart.svg
html5lib/generated/run-tests16-data.html
svg/W3C-I18N/text-anchor-dirRTL-anchorMiddle.svg
svg/W3C-I18N/text-anchor-dirLTR-anchorEnd.svg
http/tests/cache/subresource-failover-to-network.html
svg/W3C-I18N/text-anchor-dirLTR-anchorMiddle.svg
svg/W3C-I18N/text-anchor-dirNone-anchorEnd.svg
svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorEnd.svg
svg/W3C-I18N/text-anchor-dirRTL-anchorStart.svg
inspector/audits/audits-panel-functional.html
fast/loader/text-document-wrapping.html
svg/W3C-I18N/text-anchor-no-markup.svg
svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorMiddle.svg
fast/layers/no-clipping-overflow-hidden-added-after-transition.html
svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorStart.svg
fast/forms/datalist/update-range-with-datalist.html
svg/W3C-I18N/text-anchor-dirNone-anchorMiddle.svg
svg/W3C-I18N/text-dirLTR-ubOverride.svg
svg/W3C-I18N/text-anchor-inherited-dirRTL-anchorEnd.svg
svg/W3C-I18N/text-anchor-inherited-dirLTR-anchorMiddle.svg
svg/W3C-I18N/tspan-dirLTR-ubOverride-in-default-context.svg
fast/layers/no-clipping-overflow-hidden-added-after-transform.html
fast/forms/month/month-appearance-l10n.html
svg/W3C-I18N/tspan-dirLTR-ubOverride-in-ltr-context.svg
svg/W3C-I18N/text-anchor-dirLTR-anchorStart.svg
fast/loader/javascript-url-in-object.html
svg/W3C-I18N/text-anchor-dirNone-anchorStart.svg
svg/W3C-I18N/text-anchor-dirRTL-anchorEnd.svg
fast/layers/no-clipping-overflow-hidden-hardware-acceleration.html
fast/text/ipa-tone-letters.html