Bug 27663

Summary: FontPlatformData for Qt requires mapping from FontWeight to QFont::Weight
Product: WebKit Reporter: Mike Fenton <mifenton>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: manyoso
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Path to add toQFontWeight functionality.
manyoso: review-
Revised Patch manyoso: review+

Description Mike Fenton 2009-07-24 12:25:57 PDT
Based on FIXME's found in graphics/qt/FontPlatformDataQt.cpp and graphics/qt/FontCacheQt.cpp an update is required to map FontWeight to QFont::Weight values.
Comment 1 Mike Fenton 2009-07-24 12:32:44 PDT
Created attachment 33461 [details]
Path to add toQFontWeight functionality.
Comment 2 Adam Treat 2009-07-24 12:59:49 PDT
Comment on attachment 33461 [details]
Path to add toQFontWeight functionality.

> +    static inline QFont::Weight toQFontWeight(FontWeight fontWeight)
> +    {
> +        QFont::Weight weight;

Unnecessary.

> +        switch (fontWeight) {
> +        case FontWeight100:
> +        case FontWeight200:
> +            weight = QFont::Light;
> +            break;

return QFont::Light;

> +        case FontWeight600:
> +            weight = QFont::DemiBold;

return QFont::DemiBold;

And so on...

> -    if (description.weight() >= FontWeight600) {
> -        m_font.setWeight(QFont::Bold);
> -        m_bold = true;
> -    } else
> -        m_font.setWeight(QFont::Normal);
> +
> +    m_font.setWeight(toQFontWeight(description.weight()));
> +    m_bold = m_font.bold();

Seems unnecessary to keep 'm_bold' with what you are doing.  m_font.bold() stores the state.

However, this appears to be a behavior change.  Are you sure it is correct?

How did you go about determining the proper mapping?

Did you run the layout tests before and after to see if anything changed?

Cheers,
Adam
Comment 3 Mike Fenton 2009-07-27 11:07:04 PDT
Created attachment 33556 [details]
Revised Patch

Patch has been updated.

Based on feedback, toQFontWeight now returns values directly within intermediary variable.

Switch bold calculation to >Normal from >=Bold to ensure logic matches calculation provided by QFont.bold().

This does not break any tests.

m_bold variable is required by FontPlatformData and can not be removed without affecting other platforms.
Comment 4 Adam Treat 2009-07-27 11:29:46 PDT
Comment on attachment 33556 [details]
Revised Patch

Looks good to me.
Comment 5 Adam Treat 2009-07-27 14:32:12 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/platform/graphics/qt/FontCacheQt.cpp
	M	WebCore/platform/graphics/qt/FontPlatformData.h
	M	WebCore/platform/graphics/qt/FontPlatformDataQt.cpp
Committed r46428
	M	WebCore/ChangeLog
	M	WebCore/platform/graphics/qt/FontCacheQt.cpp
	M	WebCore/platform/graphics/qt/FontPlatformData.h
	M	WebCore/platform/graphics/qt/FontPlatformDataQt.cpp
r46428 = 4e8d024866454361b6a4ba28eff659a91cc1517e (git-svn)
No changes between current HEAD and refs/remotes/git-svn
Resetting to the latest refs/remotes/git-svn
http://trac.webkit.org/changeset/46428