Bug 47033 - [GTK] REGRESSION: FreeType backend does not respect XSettings font settings after r68558
Summary: [GTK] REGRESSION: FreeType backend does not respect XSettings font settings a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 47134
  Show dependency tree / graph
 
Reported: 2010-10-01 17:39 PDT by Martin Robinson
Modified: 2010-10-14 11:53 PDT (History)
0 users

See Also:

Attachments
Patch for this issue (20.05 KB, patch)
2010-10-01 17:55 PDT, Martin Robinson 		no flags Details | Formatted Diff | Diff
Patch updated to ToT (18.83 KB, patch)
2010-10-07 12:31 PDT, Martin Robinson 		no flags Details | Formatted Diff | Diff
Patch with updated comments (19.05 KB, patch)
2010-10-14 11:12 PDT, Martin Robinson 		zecke: 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 Martin Robinson 2010-10-01 17:39:58 PDT 
It appears that when subpixel smoothing is turned on in XSettings configuration and FontConfig has no preference, grayscale anti-aliasing is still used. Regression tests did not catch this bug, because previously all pixel tests used grayscale XSettings.
Comment 1 Martin Robinson 2010-10-01 17:55:16 PDT 
Created attachment 69550 [details]
Patch for this issue
Comment 2 Martin Robinson 2010-10-07 12:31:20 PDT 
Created attachment 70129 [details]
Patch updated to ToT
Comment 3 Holger Freyther 2010-10-14 10:46:40 PDT 
Comment on attachment 70129 [details]
Patch updated to ToT

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

The patch looks fine, I have just one minor comment.

> WebCore/ChangeLog:11
> +        override the setting if it was previously off.

Why is that the case? What would happen if we turn it on again?

> WebCore/platform/graphics/cairo/FontPlatformDataFreeType.cpp:93
> +            cairo_font_options_set_antialias(options, CAIRO_ANTIALIAS_GRAY);

The else if comes a bit unexpected. Could you move the comment to the top?
Comment 4 Martin Robinson 2010-10-14 11:12:58 PDT 
Created attachment 70753 [details]
Patch with updated comments
Comment 5 Holger Freyther 2010-10-14 11:28:48 PDT 
Comment on attachment 70753 [details]
Patch with updated comments

Looks good.
Comment 6 Martin Robinson 2010-10-14 11:53:13 PDT 
Committed r69786: <http://trac.webkit.org/changeset/69786>