Bug 79402 - [Qt] Font related problem with newer Qt5
Summary: [Qt] Font related problem with newer Qt5
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Blocker
Assignee: Simon Hausmann
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 79328
  Show dependency treegraph
 
Reported: 2012-02-23 14:40 PST by Csaba Osztrogonác
Modified: 2012-02-24 06:49 PST (History)
6 users (show)

See Also:


Attachments
png with the lateset pinned Qt5 hash (11.83 KB, image/png)
2012-02-23 14:44 PST, Csaba Osztrogonác
no flags Details
png with the lateset Qt5 hash (12.18 KB, image/png)
2012-02-23 14:44 PST, Csaba Osztrogonác
no flags Details
Patch (1.66 KB, patch)
2012-02-24 03:32 PST, Simon Hausmann
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2012-02-23 14:40:07 PST
The latest pinned Qt5 hash works fine - 1baa51c3d06dfaf08afe16e3e7dd77a63052a419 
I tried to do update it to 59eebee2600505b2d207d651fb5aa0da979f0f6a, but I got
a strange font related problem. Almost all tests ~5500 fail, because the
default font is changed for some reason. :-/ We should find the reason
and fix it before updating. I don't thinks if skipping ~5500 tests is
a good idea and we can't check ~5500 manually and then maintain ~5500 
expected files for Qt 4.8 and Qt 5.0 parallel. :(

Simon, Balázs, you are font experts. Could you check what happened?
Comment 1 Csaba Osztrogonác 2012-02-23 14:41:09 PST
I'll attach png results soon.
Comment 2 Csaba Osztrogonác 2012-02-23 14:44:09 PST
Created attachment 128559 [details]
png with the lateset pinned Qt5 hash
Comment 3 Csaba Osztrogonác 2012-02-23 14:44:39 PST
Created attachment 128561 [details]
png with the lateset Qt5 hash
Comment 4 Csaba Osztrogonác 2012-02-23 14:47:31 PST
I forgot to mention, if you would like to debug, you have to apply the buildfix from bug79328 for the newer Qt5.
Comment 5 Simon Hausmann 2012-02-24 03:22:04 PST
I spent a bit of time looking into this and this is what I think happened:

There are two differences that we can observe, a difference in _metrics_ (*-expected.txt) as well as in the _visual rendering_ (attached png files).

I do believe the difference in the png files is _not_ caused by any _recent_ changes (i.e. from last week's qt5 pinned hash to current one), but by some more general changes in Qt 5. I don't think we should do anything about them until we drop support for Qt 4.

The difference in _metrics_ I believe is caused by an intentional behavioural change in Qt 5 http://codereview.qt-project.org/#change,15169 (or by one of its follow-up patches).

I'm looking into compensation for this in WebKit to give us Qt 4 like results. In the future perhaps we can make the Qt 4 rendering behave like Qt 5 (in terms of metrics) and rebase all our results (or drop qt4 and rebase anyway).
Comment 6 Simon Hausmann 2012-02-24 03:32:19 PST
Created attachment 128701 [details]
Patch
Comment 7 Csaba Osztrogonác 2012-02-24 06:39:14 PST
Comment on attachment 128701 [details]
Patch

Clearing flags on attachment: 128701

Committed r108788: <http://trac.webkit.org/changeset/108788>
Comment 8 Csaba Osztrogonác 2012-02-24 06:39:22 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Balazs Kelemen 2012-02-24 06:49:14 PST
> I do believe the difference in the png files is _not_ caused by any _recent_ changes (i.e. from last week's qt5 pinned hash to current one), but by some more general changes in Qt 5. I don't think we should do anything about them until we drop support for Qt 4.
> 

I believe the png files are just very out-of-date, AFAIK pixel tests do not pass even with Qt4.