Summary: | Layout Test fast/text/descent-clip-in-scaled-page.html is failing on linux since it was added | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Vsevolod Vlasov <vsevik> | ||||||||
Component: | Tools / Tests | Assignee: | Xianzhu Wang <wangxianzhu> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | derat, tony, wangxianzhu, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Vsevolod Vlasov
2012-07-16 07:05:24 PDT
Flakiness dashboard: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Ftext%2Fdescent-clip-in-scaled-page.html wangxianzhu, can you investigate? I wonder how it passed the EWS bots . . . (In reply to comment #3) > wangxianzhu, can you investigate? I wonder how it passed the EWS bots . . . OK. I might be flaky. 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. 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. (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. For reference, the original bug is https://bugs.webkit.org/show_bug.cgi?id=88684 . (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. (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? (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. Out of curiosity, how does iOS handle this? Created attachment 155628 [details]
Patch
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. 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. Did you find anything out about iOS? (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. Created attachment 155633 [details]
for landing
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. (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). (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? (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. 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 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
Comment on attachment 155633 [details]
for landing
Try CQ again. I think the failure was just flakyness unrelated to this change.
Comment on attachment 155633 [details] for landing Clearing flags on attachment: 155633 Committed r124340: <http://trac.webkit.org/changeset/124340> All reviewed patches have been landed. Closing bug. |