Bug 112704 - [Qt] Enable kerning by default
Summary: [Qt] Enable kerning by default
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on: 106013
Blocks:
  Show dependency tree / graph
 
Reported: 2013-03-19 09:09 PDT by Allan Sandfeld Jensen
Modified: 2013-03-20 07:21 PDT (History)
2 users (show)

See Also:

Attachments
Patch (5.47 KB, patch)
2013-03-19 10:46 PDT, Allan Sandfeld Jensen 		no flags Details | Formatted Diff | Diff
Patch (5.57 KB, patch)
2013-03-20 03:56 PDT, Allan Sandfeld Jensen 		no flags Details | Formatted Diff | Diff
Patch (5.56 KB, patch)
2013-03-20 04:15 PDT, Allan Sandfeld Jensen 		jturcotte: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2013-03-19 09:09:53 PDT 
Since kerning is now possible in the fast font-path, we should enable kerning by default as we do in the rest of Qt.
Comment 1 Allan Sandfeld Jensen 2013-03-19 10:46:13 PDT 
Created attachment 193858 [details]
Patch
Comment 2 Jocelyn Turcotte 2013-03-20 03:47:54 PDT 
Comment on attachment 193858 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193858&action=review

> Source/WebKit/qt/WebCoreSupport/InitWebCoreQt.cpp:77
> +        WebCore::Font::setDefaultTypesettingFeatures(WebCore::Kerning);

Comment please :)
Comment 3 Allan Sandfeld Jensen 2013-03-20 03:56:36 PDT 
Created attachment 194014 [details]
Patch
Comment 4 Jocelyn Turcotte 2013-03-20 04:00:09 PDT 
Comment on attachment 194014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194014&action=review

> Tools/DumpRenderTree/qt/TestRunnerQt.cpp:91
> +    DumpRenderTreeSupportQt::setDefaultTypesettingFeatures(0);

This is a bit stretched, passing an int knowing that the caller will call it with 0 and won't have to include WebCore headers.
DumpRenderTreeSupportQt doesn't seem to expose any other WebCore type that way, so maybe this should be exposed as a "disableAll" method or as individual bool settings for kerning and ligatures independently?
Comment 5 Jocelyn Turcotte 2013-03-20 04:05:40 PDT 
Pierre, I'd also like to know if this looks fine to you, could you have a look?
Comment 6 Allan Sandfeld Jensen 2013-03-20 04:08:56 PDT 
(In reply to comment #4)
> (From update of attachment 194014 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194014&action=review
> 
> > Tools/DumpRenderTree/qt/TestRunnerQt.cpp:91
> > +    DumpRenderTreeSupportQt::setDefaultTypesettingFeatures(0);
> 
> This is a bit stretched, passing an int knowing that the caller will call it with 0 and won't have to include WebCore headers.
> DumpRenderTreeSupportQt doesn't seem to expose any other WebCore type that way, so maybe this should be exposed as a "disableAll" method or as individual bool settings for kerning and ligatures independently?

I will make it disable all. This is all we really need since the layout expectations simply expects how it used to be, with all off by default.
Comment 7 Allan Sandfeld Jensen 2013-03-20 04:15:42 PDT 
Created attachment 194015 [details]
Patch
Comment 8 Jocelyn Turcotte 2013-03-20 06:44:49 PDT 
Comment on attachment 194015 [details]
Patch

14:32 < elproxy> LGTM.
r=me
Comment 9 Allan Sandfeld Jensen 2013-03-20 07:21:18 PDT 
Committed r146340: <http://trac.webkit.org/changeset/146340>