Bug 33299 - [Gtk] FreeType backend does not respect FC_MATRIX property
Summary: [Gtk] FreeType backend does not respect FC_MATRIX property
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Major
Assignee: Martin Robinson
URL:
Keywords: PlatformOnly
Depends on: 36548
Blocks: 46216
  Show dependency treegraph
 
Reported: 2010-01-06 21:05 PST by Dino Morelli
Modified: 2010-10-07 10:47 PDT (History)
5 users (show)

See Also:


Attachments
sample HTML document illustrating CSS italic issue (835 bytes, text/html)
2010-01-06 21:05 PST, Dino Morelli
no flags Details
Patch for this issue (24.79 KB, patch)
2010-09-21 12:11 PDT, Martin Robinson
cfleizach: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dino Morelli 2010-01-06 21:05:33 PST
Created attachment 46018 [details]
sample HTML document illustrating CSS italic issue

Text that should display as italic is not rendering italic. Example HTML document attached.

This is happening in both the Midori and uzbl browsers on Arch Linux 32-bit with libwebkit 1.1.15.4
Comment 1 Dino Morelli 2010-01-06 21:17:03 PST
Versions of webkit browsers tested on the Arch Linux system:

midori 0.2.2-1
uzbl-git 20091205-1
Comment 2 Martin Robinson 2010-09-17 15:35:19 PDT
The issue here seems to be that we are totally ignoring the FC_MATRIX property of the FcPattern we use to create the font.
Comment 3 Martin Robinson 2010-09-17 15:36:41 PDT
The FC_MATRIX property is *very* lightly documented, but we can observe it in Pango.
Comment 4 Martin Robinson 2010-09-21 12:11:29 PDT
Created attachment 68273 [details]
Patch for this issue
Comment 5 Alexander Butenko 2010-09-28 10:08:58 PDT
ping?
Comment 6 Martin Robinson 2010-09-28 10:39:57 PDT
I'll see if I can find someone to review this today.
Comment 7 chris fleizach 2010-10-05 10:04:38 PDT
Comment on attachment 68273 [details]
Patch for this issue

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

r=me, but you should address the "new SimpleFontData" thing, even though its existing code

> WebCore/platform/graphics/cairo/SimpleFontDataCairo.cpp:89
>  

shouldn't this be adoptRef or something. Naked news are no longer in favor i believe.
Comment 8 Martin Robinson 2010-10-05 10:33:28 PDT
(In reply to comment #7)

> shouldn't this be adoptRef or something. Naked news are no longer in favor i believe.

Thanks for the review! I don't know if adoptRef is appropriate here because SimpleFontData isn't a descendant of RefCounted. In particular, it seems like 
m_smallCapsFontData is owned only by SimpleFontData.
Comment 9 Martin Robinson 2010-10-05 11:51:36 PDT
(In reply to comment #8)

> Thanks for the review! I don't know if adoptRef is appropriate here because SimpleFontData isn't a descendant of RefCounted. In particular, it seems like  m_smallCapsFontData is owned only by SimpleFontData.

Chris, given this, do you still mind if I land the patch as-is?
Comment 10 Martin Robinson 2010-10-07 10:19:21 PDT
I'm almost 100% certain about the SimpleFontData issue, so I'll land as-is. If this is wrong though, we can roll it out ASAP.
Comment 11 Martin Robinson 2010-10-07 10:47:46 PDT
Committed r69320: <http://trac.webkit.org/changeset/69320>