Bug 144472

Summary: [Freetype] Add support for the font-synthesis property
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, mmaxfield
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Martin Robinson 2015-04-30 14:41:59 PDT
r183494 added support for the font-synthesis property for the iOS and Mac ports, but we need this for Freetype ports as well.
Comment 1 Martin Robinson 2015-04-30 14:43:53 PDT
Created attachment 252090 [details]
Patch
Comment 2 Myles C. Maxfield 2015-04-30 18:21:23 PDT
Comment on attachment 252090 [details]
Patch

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

> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:331
> +    if (fontDescription.italic() && fontDescription.fontSynthesis() & FontSynthesisStyle

Why the difference in order between weight and italic?
Comment 3 Martin Robinson 2015-04-30 22:26:12 PDT
Comment on attachment 252090 [details]
Patch

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

>> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:331
>> +    if (fontDescription.italic() && fontDescription.fontSynthesis() & FontSynthesisStyle
> 
> Why the difference in order between weight and italic?

No reason. I think I just did that for some intuitive sense of readability. I'm happy to make it consistent. :)
Comment 4 Martin Robinson 2015-05-01 07:56:05 PDT
Created attachment 252150 [details]
Patch
Comment 5 Sergio Villar Senin 2015-05-01 08:04:03 PDT
Comment on attachment 252150 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [Freetype] Add support for the  font-synthesis property

Nit: extra space here

> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:165
> +    if (descriptionAllowsSyntheticBold && fontDescription.weight() >= FontWeightBold) {

We're doing an extra check for < FontWeightBold here but I guess it's so simple that we can leave with it.
Comment 6 Martin Robinson 2015-05-01 08:09:02 PDT
(In reply to comment #5)
> Comment on attachment 252150 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=252150&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        [Freetype] Add support for the  font-synthesis property
> 
> Nit: extra space here
> 
> > Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:165
> > +    if (descriptionAllowsSyntheticBold && fontDescription.weight() >= FontWeightBold) {
> 
> We're doing an extra check for < FontWeightBold here but I guess it's so
> simple that we can leave with it.

I think in this case we have to check whether the font needs the synthetic bold and also whether the font-synthesis property allows it.
Comment 7 Martin Robinson 2015-05-01 09:01:30 PDT
Committed r183673: <http://trac.webkit.org/changeset/183673>