After http://trac.webkit.org/changeset/85013, I landed some new baselines in http://trac.webkit.org/changeset/85674 . The tests I rebaselined seem fine. However, platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html seems incorrect now. The backslash-looking mark is no longer connected to the glyph.
Bashi, can you take a look at this?
I'm going to mark the test as failing for now.
(In reply to comment #0)
> Bashi, can you take a look at this?
Sorry for late response. I'm on vacation until Mar 8 so I'll take a look after the vacation.
The function getOutlinePoint() in platform/graphics/chromium/HarfbuzzSkia.cpp misuses SkPaint::getTextPath(). Harfbuzz calls this function to get the position of an anchor point(the point to connect two or more glyphs). The anchor point is specified by the outline point index. The function uses SkPaint::getTextPath() to get outline points, but the number of outline points of the result is different from those of original outline information of the glyph because Skia reorganizes them from original information and sometimes inserts additional points. This means we cannot use SkPaint::getTextPath() to get the right position of anchor points.
As far as I checked, the function is only called to get the position of anchor points. So we can take a workaround that just returning HB_Err_Ok with resultingNumPoints = 0 so that harfbuzz uses the design coordinate value pair (See Get_Anchor() in harfbuzz-gops.c of the harfbuzz library). I'll post a patch soon.
Note that harfbuzz doesn't call this function for Hindi fonts which are used in layout tests as of r85879. Harfbuzz calls the function only the font has OpenType AnchorTable format 2 (Lucida fonts contain it, but Lohit fonts don't).
BTW, The backslash-looking mark (U+0947, DEVANAGARI VOWEL SIGN E) should be located above the vertical line of the base glyph (U+0915, DEVANAGARI LETTER KA), not the right side of the character. I'll attach a screenshot of the rendering result of Firefox.
Created attachment 92934 [details]
Screenshot of firefox
Created attachment 92937 [details]
This looks fine to me, but I'll let Evan also take a look.
(In reply to comment #3)
> Note that harfbuzz doesn't call this function for Hindi fonts which are used in layout tests as of r85879. Harfbuzz calls the function only the font has OpenType AnchorTable format 2 (Lucida fonts contain it, but Lohit fonts don't).
I see. Is there a different free font we can use that has an OpenType AnchorTable format 2?
+agl FYI, we're deleting some of your code
Comment on attachment 92937 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=92937&action=review
Looks good to me too.
> +<p>The backslash-looking mark should connect into the the character on the center, not be positioned off to the side.</p>
I feel like I've seen (in some other font, perhaps?) this character connect slightly to the right of the center, but still touching the glyph.
But I think your new text is fine; if we end up using that other font we can just change this text again. It's more likely my memory is of a bug.
(In reply to comment #7)
> +agl FYI, we're deleting some of your code
Sounds good. I generally recommend deleting all of my code in this area.
Comment on attachment 92937 [details]
I will land this for you. I want to make sure we get both the 32bit (hardy) and 64bit (lucid) versions right.
Committed r86171: <http://trac.webkit.org/changeset/86171>
BTW, on Hardy 32, the DEVANAGARI VOWEL SIGN E does not touch but I checked in the result anyway because (1) we're dropping hardy support in the next couple weeks and (2) it's not clear to me if this is a bug in the code or just a bug in the font (the version in Lucid is much newer).
Thank you for review and comments.
(In reply to comment #6)
> I see. Is there a different free font we can use that has an OpenType AnchorTable format 2?
Thank you for landing the patch. I checked fonts that I have and found some fonts (e.g. DejaVuSans fonts) contain AnchorTable format 2. I'm not sure how they are used in such fonts, but the point index in AnchorTable format 2 is just for fine-tuning the location of the anchor point so I think harfbuzz would be able to locate glyphs appropriately even if getOutlinePoint() doesn't provide the position. (For more details, please refer the section of Anchor Table on http://www.microsoft.com/typography/otspec/gpos.htm)