Summary: | [Qt] New fast/text/word-space-with-kerning-3.html fails on Qt. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ádám Kallai <kadam> | ||||||||||
Component: | Tools / Tests | Assignee: | Allan Sandfeld Jensen <allan.jensen> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | allan.jensen, dino, enrica, jturcotte, noam, ossy, szledan, webkit.review.bot, zarvai | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 112795, 113179 | ||||||||||||
Bug Blocks: | 87008 | ||||||||||||
Attachments: |
|
Description
Ádám Kallai
2013-03-19 01:14:00 PDT
This seems like an actual bug in the layout system. Is kerning enabled on QT? If so, then there is definitely a bug. The two pieces of content should be identical. If not, then I'm not sure how they could be different :) I think kerning might be always on in the complex font path in Qt. I have a patch to fix that I will see if that fixes it. Created attachment 194018 [details]
Patch
Make complex font path respect WebCore kerning setting
Committed r146331: <http://trac.webkit.org/changeset/146331> Re-opened since this is blocked by bug 112795 The test-case works with Qt 5.1 because both cases goes through the fast font path. There does however seem to be a problem with word-spacing working differently in fast and complex path. Possibly we need to fix the same bug in the Qt complex path that this test was introduced to test in the fast path. Created attachment 194048 [details]
Patch
Second try, this time verified it works when the complex font path is used.
Created attachment 194076 [details]
Patch
Comment on attachment 194076 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194076&action=review > Source/WebCore/platform/graphics/qt/FontQt.cpp:238 > - initFormatForTextLayout(&layout); > + initFormatForTextLayout(&layout, run); > QTextLine line = setupLayout(&layout, run); Maybe you can do this in setupLayout instead? You don't seem to be using any Font member for this logic, and initFormatForTextLayout isn't a fitting name anymore with this. > Source/WebCore/platform/graphics/qt/FontQt.cpp:260 > + if (!range.length) > + return; Maybe this is the reason why, but I'm not quite well following that part. Created attachment 194088 [details]
Patch
Slight clean-up
Comment on attachment 194088 [details]
Patch
Me gusta !
Comment on attachment 194088 [details] Patch Clearing flags on attachment: 194088 Committed r146630: <http://trac.webkit.org/changeset/146630> All reviewed patches have been landed. Closing bug. After the patch 8 test started asserting on Qt debug bots. https://bugs.webkit.org/show_bug.cgi?id=113179 |