Bug 60079

Summary: REGRESSION(r85013): platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html is failing
Product: WebKit Reporter: Tony Chang <tony>
Component: Tools / TestsAssignee: Kenichi Ishibashi <bashi>
Status: RESOLVED FIXED    
Severity: Normal CC: agl, evan, simonjam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Screenshot of firefox
none
Patch V0 tony: review+

Description Tony Chang 2011-05-03 16:17:47 PDT
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?
Comment 1 Tony Chang 2011-05-03 16:18:13 PDT
I'm going to mark the test as failing for now.
Comment 2 Kenichi Ishibashi 2011-05-05 19:30:33 PDT
Hi Tony,

(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.
Comment 3 Kenichi Ishibashi 2011-05-10 03:12:44 PDT
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.
Comment 4 Kenichi Ishibashi 2011-05-10 03:13:32 PDT
Created attachment 92934 [details]
Screenshot of firefox
Comment 5 Kenichi Ishibashi 2011-05-10 03:25:54 PDT
Created attachment 92937 [details]
Patch V0
Comment 6 Tony Chang 2011-05-10 09:51:20 PDT
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?
Comment 7 Evan Martin 2011-05-10 10:14:38 PDT
+agl FYI, we're deleting some of your code
Comment 8 Evan Martin 2011-05-10 10:18:36 PDT
Comment on attachment 92937 [details]
Patch V0

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

Looks good to me too.

> LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html:10
> +<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.
Comment 9 Adam Langley 2011-05-10 10:19:04 PDT
(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 10 Tony Chang 2011-05-10 10:38:14 PDT
Comment on attachment 92937 [details]
Patch V0

I will land this for you.  I want to make sure we get both the 32bit (hardy) and 64bit (lucid) versions right.
Comment 11 Tony Chang 2011-05-10 11:12:44 PDT
Committed r86171: <http://trac.webkit.org/changeset/86171>
Comment 12 Tony Chang 2011-05-10 11:15:04 PDT
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).
Comment 13 Kenichi Ishibashi 2011-05-10 17:50:17 PDT
Hi guys,

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)