Bug 50147

Summary: [Qt] 4 fast/text tests fail with Qt >= 4.7.1
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: allan.jensen, andersca, cshu, joseph.wild, jturcotte, laszlo.gombos, ossy, vestbo
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 46248, 79666    
Attachments:
Description Flags
Contains text dump of QtTestBrowser using webkit test fonts (-use-test-fonts) and without none

Description Csaba Osztrogonác 2010-11-29 04:16:03 PST
fast/text/format-control.html
fast/text/justification-padding-mid-word.html
fast/text/zero-width-characters.html
fast/text/international/khmer-selection.html
Comment 1 Joe Wild 2011-03-29 06:21:45 PDT
I'm wondering if these 2 errors are related and are caused by fonts.
I'll take a deeper look.

https://bugs.webkit.org/show_bug.cgi?id=50146
Bug 50146 - [Qt] 4 css2.1/t1202-counter tests fail with Qt 4.7.1 

And in this one the display of t1202-counter-09-b.html in the
QtTestBrowser looks correct.  But when I add -use-test-fonts the
display is empty.
  $ $B/QtTestBrowser LayoutTests/css2.1/t1202-counter-09-b.html 
  $ $B/QtTestBrowser -use-test-fonts LayoutTests/css2.1/t1202-counter-09-b.html 

https://bugs.webkit.org/show_bug.cgi?id=50147
Bug 50147 - [Qt] 4 fast/text tests fail with Qt 4.7.1 

When I display the format-control.html in the QtTestBrowser it looks correct,
But when I use the WebKit test fonts with the -use-test-fonts the zero width
chars are displayed as spaces.

  $ $B/QtTestBrowser LayoutTests/fast/text/format-control.html
  $ $B/QtTestBrowser -use-test-fonts LayoutTests/fast/text/format-control.html
Comment 2 Joe Wild 2011-03-29 16:52:19 PDT
X is char 8204 (0x200C), zero width whitespace non-joining

without -use-test-fonts
JPW: Font::floatWidthForSimpleText  string =  "fXi" 
Index =  0 ch =  'f' Width =  13 
Index =  1 ch =  'X' Width =  0 
Index =  2 ch =  'i' Width =  11 

with -use-test-fonts
JPW: Font::floatWidthForSimpleText  string =  "fXi" 
Index =  0 ch =  'f' Width =  13 
Index =  1 ch =  'X' Width =  10 
Index =  2 ch =  'i' Width =  11 

So shows QFontMetrics wrong/different for WebKit Test Fonts.

I can see about 50 fonts normally and only 5 fonts
with -use-test-fonts.  Not sure why this is.  I would
expect the webkit test fonts to be added in addition to the
other fonts.
Comment 3 Joe Wild 2011-03-30 14:25:30 PDT
Created attachment 87613 [details]
Contains text dump of QtTestBrowser using webkit test fonts (-use-test-fonts) and without

So it looks like the webkit test fonts do not have fonts for chars
0x800C and 0x800D, and that is why these test are failing.

After I load the WebKit test fonts, I only see these 5 fonts with no
characters in the higher ranges (if I am reading things correctly).

Can anyone verify this or show me how I can?  And if true, I guess
this is an error the Qt WEBKIT_TESTFONTS located here:

   clone from git://gitorious.org/qtwebkit/testfonts.git

Attached is the dumps of the fonts when Webkit Test fonts are loaded
and when they are not.
Comment 4 Joe Wild 2011-03-31 11:15:41 PDT
I no longer think this is a font issue.  I think it was introduced
in the middle of 2010 with Qt 4.7.  floatWidthForSimpleText was
introduced in r61002.

If I change the call to width() in floatWidthForSimpleText to be
similar to the complex path (floatWidthForComplexText), then this
seems to work.  I would also assume taking the complex path would work
since the 2 routines are almost identical other than the call to
width().

#if 0
    int w = QFontMetrics(font()).width(string, -1, Qt::TextBypassShaping);
#else
    int w  = QFontMetrics(font()).width(string);
#endif

Do you have any advice on how to best correct?  Should we be taking the
complex path for zero-width chars?  Or is there something deeper wrong?
Comment 5 Jocelyn Turcotte 2011-04-01 04:31:00 PDT
(In reply to comment #4)
> Do you have any advice on how to best correct?  Should we be taking the
> complex path for zero-width chars?  Or is there something deeper wrong?

As far as I know the main difference between the fast and the complex path is that the complex path will use all the OpenType information to position the glyphs, while the simple path only use horizontal "advance widths".

What seems to happen in your case is that Qt do not get the right advance width value when loading the test font, which is calculated correctly somehow when using harfbuzz in the complex path. At this level this code in Qt is quite different among platforms, so you could also try to see if you correctly get the zero-width on other platforms for these fonts.

A workaround would be to modify Font::codePath to force the complex path for these characters, but that would be a rather ugly idea.
Comment 6 Joe Wild 2011-04-01 16:25:05 PDT
https://bugs.webkit.org/show_bug.cgi?id=50147
Bug 50147 - [Qt] 4 fast/text tests fail with Qt 4.7.1 

https://bugs.webkit.org/show_bug.cgi?id=50146
Bug 50146 - [Qt] 4 css2.1/t1202-counter tests fail with Qt 4.7.1 

I am not sure where to go next with these errors.  I think they are both
caused by the WebKit Test Fonts from 

   clone from git://gitorious.org/qtwebkit/testfonts.gi

Does anyone know who I should report this against or how I can help fix?

I think these 2 errors are related to text rendering/font issues that
have shown up in the changes for Qt 4.7.  I think the real problem is
that the webkit fonts do not have all the info needed for the new Qt
4.7 code to work correctly.  I was able to solve the Georgian fonts
not rendering by moving this font to the webkit test font directory.

  $ pwd
  /home/jwild/tools/qtwebkit-testfonts
  $ cp /usr/share/fonts/truetype/ttf-dejavu/DejaVuSerif.ttf .

When I examine the fonts available without the webkit
fonts:

   * there are 50 fonts that match the pattern.
   * many of the fonts have the Georgian language code (ka) set.
     one of the failing tests is not diplaying Georgian text.
   * There seems to be a larger range of characters defined
     for most fonts.

It looks like this code started breaking the end of 2010.
It seems to be a combination of things.  Around June of 2010
the text rendering was redone to also provide simple text rendering
(floatWidthFromSimpleText).  Then near the end of 2010, this code
was enabled with the switch to Qt 4.7.

If this is wrong, please bounce it back to me.  The whole font thing
is a bit of black magic to me.
Comment 7 Csaba Osztrogonác 2011-04-01 16:29:29 PDT
I cc-ed our (test)font expert Tor Arne. Have you got any idea?
Comment 8 Csaba Osztrogonác 2012-05-18 06:05:38 PDT
The bug is still valid, but 2 of them pass with Qt 5

fails with Qt 4.8:
Regressions: Unexpected text diff mismatch : (4)
  fast/text/format-control.html = TEXT
  fast/text/international/khmer-selection.html = TEXT
  fast/text/justification-padding-mid-word.html = TEXT
  fast/text/zero-width-characters.html = TEXT

fails with Qt 5:
Regressions: Unexpected text diff mismatch : (2)
  fast/text/format-control.html = TEXT
  fast/text/international/khmer-selection.html = TEXT
Comment 9 Allan Sandfeld Jensen 2012-10-22 02:05:06 PDT
The two remaining tests now pass in QtWebKit2.3
Comment 10 Csaba Osztrogonác 2012-10-23 00:35:06 PDT
Reopen, because the tests are still in qt/TestExpectations.
If they are passing, please unskip them too.