Bug 35569

Summary: [Qt] Avoid calling QFont::detach too often from FontQt
Product: WebKit Reporter: Holger Freyther <zecke>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
First version. hausmann: review+

Description Holger Freyther 2010-03-02 02:32:28 PST
Currently the QFont Font::font() method tries to apply the wordspacing and letterspacing set on the font. This value is not stored in the FontPlatformData and must be set on the QFont each time, also we try to have the actual font data not in WebCore::Font.

This creates at least one issue: We have to apply the word and letter spacing each time which will always certainly end up in a call to QFont::detach. One solution would be to put a QFont into the WebCore::Font but this is a against the separation we try to achieve with the FontPlatformData. Another one is to not call these functions if we have the default letter and wordspacing.

This is giving us a small improvement on the text classes.
Comment 1 Holger Freyther 2010-03-02 02:37:32 PST
Created attachment 49791 [details]
First version.

Add two if's. I'm sleepy right now so be extra careful when reviewing.
Comment 2 WebKit Review Bot 2010-03-02 02:41:59 PST
Attachment 49791 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/qt/FontQt.cpp:222:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/qt/FontQt.cpp:224:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Simon Hausmann 2010-03-03 12:45:45 PST
Comment on attachment 49791 [details]
First version.

Nice catch :)
Comment 4 Holger Freyther 2010-03-04 04:30:10 PST
Landed in r55518.