Bug 102603

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

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

Created attachment 174837 [details] Patch

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

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.

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; }

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

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?

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.

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

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.

(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..?

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

Comment on attachment 174837 [details] Patch Marking r- to kick out of the EWS queue.

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?

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

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

Created attachment 188214 [details] Patch

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.

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

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