Bug 91386 – Layout Test fast/text/descent-clip-in-scaled-page.html is failing on linux since it was added

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Vsevolod Vlasov]

The following layout test is failing on linux

fast/text/descent-clip-in-scaled-page.html

Probable cause:

http://trac.webkit.org/changeset/122651/

[Vsevolod Vlasov]

Flakiness dashboard: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Ftext%2Fdescent-clip-in-scaled-page.html

[Vsevolod Vlasov]

Skipped: http://trac.webkit.org/changeset/122721

[Tony Chang]

wangxianzhu, can you investigate?  I wonder how it passed the EWS bots . . .

[Xianzhu Wang]

(In reply to comment #3)
> wangxianzhu, can you investigate?  I wonder how it passed the EWS bots . . .

OK. I might be flaky.

[Tony Chang]

Why would it be flaky?

It may be possible that we're not resetting fontconfig properly between tests. You may be able to repro by running other tests before your test.

[Tony Chang]

Xianzhu, is it possible that the expected result is wrong?  Doesn't your patch move the baseline up a pixel?

This test is consistently failing.  If we can't figure out why the test is failing, I think we should rollout the whole patch since it's not doing what we're expecting.

[Tony Chang]

(In reply to comment #6)
> Xianzhu, is it possible that the expected result is wrong?  Doesn't your patch move the baseline up a pixel?

Actually, that's not what's happening since only the bottom line is different.

If I understand your patch correctly, it only moves the baseline for glyphs that overflow. Does that result in words with a different baseline for a few letters?

Also, it looks like the test is failing on Mac because it also does subpixel text rendering and the bottom of the text (a line of grey pixels) is clipped. I'm starting to think this workaround for only Chromium Linux & Android is incorrect.

[Tony Chang]

For reference, the original bug is https://bugs.webkit.org/show_bug.cgi?id=88684 .

[Xianzhu Wang]

(In reply to comment #7)
> (In reply to comment #6)
> > Xianzhu, is it possible that the expected result is wrong?  Doesn't your patch move the baseline up a pixel?
> 
> Actually, that's not what's happening since only the bottom line is different.
> 
> If I understand your patch correctly, it only moves the baseline for glyphs that overflow. Does that result in words with a different baseline for a few letters?
>

The baseline movement applies to all glyphs of a particular font displayed in the same size. The similar patch has been in chromium-android downstream for nearly one year (without the layout test) and it does resolve the issue of font bottom truncation.

I guess the failure on chromium-linux is caused by layout before layoutTestController.setSubpixelTextPositioning(). I'll verify that and if it is, fix the test.

> Also, it looks like the test is failing on Mac because it also does subpixel text rendering and the bottom of the text (a line of grey pixels) is clipped. I'm starting to think this workaround for only Chromium Linux & Android is incorrect.

The method of the change might also applies to Mac, but the change itself applies only to chromium-linux and chromium-android because it is Skia and Harfbuzz specific. For Mac it needs another fix in its graphics code. However I think the final fix should be WebCore's support of sub-pixel ascent/descent.

[Tony Chang]

(In reply to comment #9)
> I guess the failure on chromium-linux is caused by layout before layoutTestController.setSubpixelTextPositioning(). I'll verify that and if it is, fix the test.

Xianzhu, have you had a chance to investigate this?

[Xianzhu Wang]

(In reply to comment #10)
> (In reply to comment #9)
> > I guess the failure on chromium-linux is caused by layout before layoutTestController.setSubpixelTextPositioning(). I'll verify that and if it is, fix the test.
> 
> Xianzhu, have you had a chance to investigate this?

Sorry for no progress by now. Hopefully I will get a chance in this or next week.

[Tony Chang]

Out of curiosity, how does iOS handle this?

[Xianzhu Wang]

Created attachment 155628 [details]
Patch

[Xianzhu Wang]

Comment on attachment 155628 [details]
Patch

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

> LayoutTests/fast/text/descent-clip-in-scaled-page-expected.html:10
> +       enabled. */

Note: these comments were copied from platform/chromium-linux/fast/text/chromium-linux-text-subpixel-positioning.html.

[Tony Chang]

Comment on attachment 155628 [details]
Patch

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

>> LayoutTests/fast/text/descent-clip-in-scaled-page-expected.html:10
>> +       enabled. */
> 
> Note: these comments were copied from platform/chromium-linux/fast/text/chromium-linux-text-subpixel-positioning.html.

Nit: It doesn't really seem necessary to copy this comment into the -expected.html file.

[Tony Chang]

Did you find anything out about iOS?

[Xianzhu Wang]

(In reply to comment #12)
> Out of curiosity, how does iOS handle this?

Though the bug is visible on chromium-linux using the Ahem font, it is invisible when using other normal fonts. It seems that the other fonts has some spacing below the descent part of the glyphs within the line box. The descent in the font metrics is larger than the solid descent shown in the glyph images.

The fonts on Android, just like the Ahem font, seem to have no or small vertical spacing below the descent, so it's visible when the descent is clipped.

Wondering if iOS has its own solution to the issue, or just because its fonts are not like the Android fonts.

[Xianzhu Wang]

Created attachment 155633 [details]
for landing

[Xianzhu Wang]

Comment on attachment 155628 [details]
Patch

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

>>> LayoutTests/fast/text/descent-clip-in-scaled-page-expected.html:10
>>> +       enabled. */
>> 
>> Note: these comments were copied from platform/chromium-linux/fast/text/chromium-linux-text-subpixel-positioning.html.
> 
> Nit: It doesn't really seem necessary to copy this comment into the -expected.html file.

Done.

[Xianzhu Wang]

(In reply to comment #17)
> (In reply to comment #12)
> > Out of curiosity, how does iOS handle this?
> 
> Though the bug is visible on chromium-linux using the Ahem font, it is invisible when using other normal fonts. It seems that the other fonts has some spacing below the descent part of the glyphs within the line box. The descent in the font metrics is larger than the solid descent shown in the glyph images.
> 
> The fonts on Android, just like the Ahem font, seem to have no or small vertical spacing below the descent, so it's visible when the descent is clipped.
> 
> Wondering if iOS has its own solution to the issue, or just because its fonts are not like the Android fonts.

Another factor is the VDMX table in the font. The problematic rounding of ascent/descent only occurs when the font doesn't contain the table (which is the case for some Android fonts and the Ahem font).

[Tony Chang]

(In reply to comment #20)
> (In reply to comment #17)
> > The fonts on Android, just like the Ahem font, seem to have no or small vertical spacing below the descent, so it's visible when the descent is clipped.
> > 
> > Wondering if iOS has its own solution to the issue, or just because its fonts are not like the Android fonts.
> 
> Another factor is the VDMX table in the font. The problematic rounding of ascent/descent only occurs when the font doesn't contain the table (which is the case for some Android fonts and the Ahem font).

Maybe this is a bug in the Android fonts?  How does layout using the native Android SDK (i.e., the Java API) handle this?

[Xianzhu Wang]

(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #17)
> > > The fonts on Android, just like the Ahem font, seem to have no or small vertical spacing below the descent, so it's visible when the descent is clipped.
> > > 
> > > Wondering if iOS has its own solution to the issue, or just because its fonts are not like the Android fonts.
> > 
> > Another factor is the VDMX table in the font. The problematic rounding of ascent/descent only occurs when the font doesn't contain the table (which is the case for some Android fonts and the Ahem font).
> 
> Maybe this is a bug in the Android fonts?  How does layout using the native Android SDK (i.e., the Java API) handle this?

I don't think it's a bug of Android fonts. There are just differences. The problem is that the descent was rounded and caused problems when displaying Android fonts. Android just doesn't round the descent so there is no problem.

[WebKit Review Bot]

Comment on attachment 155633 [details]
for landing

Rejecting attachment 155633 [details] from commit-queue.

New failing tests:
platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html
Full output: http://queues.webkit.org/results/13387958

[WebKit Review Bot]

Created attachment 155675 [details]
Archive of layout-test-results from gce-cq-02

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: gce-cq-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Xianzhu Wang]

Comment on attachment 155633 [details]
for landing

Try CQ again. I think the failure was just flakyness unrelated to this change.

[WebKit Review Bot]

Comment on attachment 155633 [details]
for landing

Clearing flags on attachment: 155633

Committed r124340: <http://trac.webkit.org/changeset/124340>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.