|Summary:||[Qt] Support -webkit-font-smoothing|
|Product:||WebKit||Reporter:||Allan Sandfeld Jensen <allan.jensen>|
|Component:||Layout and Rendering||Assignee:||Allan Sandfeld Jensen <allan.jensen>|
|Severity:||Normal||CC:||noam, ossy, pierre.rossi, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Allan Sandfeld Jensen 2013-01-03 03:24:36 PST
Qt currently does not respond to the -webkit-font-smoothing CSS property. It should be relatively simple to connect it to corresponding QFont properties though.
Comment 2 Pierre Rossi 2013-01-03 05:09:20 PST
Comment on attachment 181163 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181163&action=review LGTM but I'm just curious about the tests. Post-landing gardening expected ? > Source/WebCore/ChangeLog:10 > + Covered by existing tests. Is it really ? No test expectations to update ?
Comment 3 Allan Sandfeld Jensen 2013-01-03 05:13:26 PST
(In reply to comment #2) > (From update of attachment 181163 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=181163&action=review > > LGTM but I'm just curious about the tests. Post-landing gardening expected ? > > > Source/WebCore/ChangeLog:10 > > + Covered by existing tests. > > Is it really ? No test expectations to update ? Actually this is somewhat complicated. First of all it only touches the image results, and they are not currently clean. Secondly it differs between WK2 and WK1. It turns out WebKitTestRunner always sets Font::ShouldUseSmoothing to false. I am considering making it ignore Font::ShouldUseSmoothing() to begin with. That should make it possible to only update the tests that explicitly disable font smoothing.
Comment 4 Allan Sandfeld Jensen 2013-01-03 05:15:27 PST
Oh yeah, and finally the image differences are often too small for Qt ImageDiff to pick them up. See bug #94782
Comment 5 Allan Sandfeld Jensen 2013-01-03 05:26:54 PST
Created attachment 181165 [details] Patch Ignore Font::shouldUseSmoothing for now.
Comment 7 Jocelyn Turcotte 2013-03-19 10:30:07 PDT
Comment on attachment 193839 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193839&action=review > Source/WebCore/platform/graphics/qt/FontPlatformDataQt.cpp:79 > +#else Those two logics are independent and one of them should be removed soon. I'm of the opinion that it would make it cleaner to have #if < 5.1 featureA #endif #if >= 5.1 featureB #endif No strong feeling though.
Comment 9 Allan Sandfeld Jensen 2013-03-20 03:52:22 PDT
Comment on attachment 193839 [details] Patch The two ifdefs are both to minimize baselines and each branch sets StyleStrategy so they are related.
Comment 10 Jocelyn Turcotte 2013-03-20 04:09:43 PDT
Comment on attachment 193839 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193839&action=review > Source/WebCore/platform/graphics/qt/FontPlatformDataQt.cpp:77 > // Kept enabled for Qt < 5.1 to maintain stable baselines for 5.0. If you feel like it, you could reword this comment to cover the added block, or add a new one. r=me as discussed.
Comment 11 Allan Sandfeld Jensen 2013-03-20 04:24:38 PDT
Committed r146324: <http://trac.webkit.org/changeset/146324>