WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
145492
[WinCairo] ClearType should be enabled on Cairo port
https://bugs.webkit.org/show_bug.cgi?id=145492
Summary
[WinCairo] ClearType should be enabled on Cairo port
Karlen Simonyan
Reported
2015-05-30 02:06:06 PDT
Created
attachment 253958
[details]
Cairo port text rendering behaviour for custom fonts with and without ClearType Overview: by default Cairo port uses CAIRO_ANTIALIAS_GRAY for text antialiasing, which results to using ANTIALIASED_QUALITY for Unicscribe text rendering on Windows platform instead of CLEARTYPE_QUALITY (corresponding Cairo antialiasing mode is CAIRO_ANTIALIAS_BEST). Steps to reproduce: Open any web page with embedded fonts (
http://www.apple.com/mac/
, for example). Actual result: some amount of text is rendered thin and semi-transparent (as it should NOT be). Expected result: smooth text as shown on the right side of the attachment (wincairo_text_rendering_diff.png).
Attachments
Cairo port text rendering behaviour for custom fonts with and without ClearType
(52.27 KB, image/png)
2015-05-30 02:06 PDT
,
Karlen Simonyan
no flags
Details
ClearType fix for custom fonts
(681 bytes, patch)
2015-05-30 02:22 PDT
,
Karlen Simonyan
no flags
Details
Formatted Diff
Diff
ClearType fix for custom fonts (v2)
(676 bytes, patch)
2015-07-06 14:15 PDT
,
Karlen Simonyan
no flags
Details
Formatted Diff
Diff
ClearType fix for custom fonts (v3)
(676 bytes, patch)
2015-07-06 14:20 PDT
,
Karlen Simonyan
no flags
Details
Formatted Diff
Diff
ClearType fix for custom fonts (v4)
(787 bytes, patch)
2015-07-07 13:11 PDT
,
Karlen Simonyan
achristensen
: review-
Details
Formatted Diff
Diff
ClearType fix for custom fonts (v5)
(1.45 KB, patch)
2015-07-08 12:51 PDT
,
Karlen Simonyan
bfulgham
: review+
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Karlen Simonyan
Comment 1
2015-05-30 02:22:57 PDT
Created
attachment 253959
[details]
ClearType fix for custom fonts
Jinyoung Hur
Comment 2
2015-07-03 09:58:21 PDT
It seems that this patch might help improving the quality of custom font rendering on Windows. Is there anybody to review this?
Jinyoung Hur
Comment 3
2015-07-05 17:48:41 PDT
Comment on
attachment 253959
[details]
ClearType fix for custom fonts Please fix the Bot Status, "patch does not apply...". It seems that your patch has a CR(carriage return) character, which is a violation of the WebKit coding convention. After correction, review flags should be set as "?" in order to be reviewed by any reviewers.
Karlen Simonyan
Comment 4
2015-07-06 14:15:44 PDT
Created
attachment 256237
[details]
ClearType fix for custom fonts (v2)
Karlen Simonyan
Comment 5
2015-07-06 14:20:00 PDT
Created
attachment 256238
[details]
ClearType fix for custom fonts (v3)
Karlen Simonyan
Comment 6
2015-07-06 14:26:02 PDT
Comment on
attachment 256238
[details]
ClearType fix for custom fonts (v3)
>Index: FontPlatformDataCairoWin.cpp >=================================================================== >--- Source/WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp (revision 186376) >+++ Source/WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp (working copy) >@@ -77,7 +77,7 @@ > // We force antialiasing and disable hinting to provide consistent > // typographic qualities for custom fonts on all platforms. > cairo_font_options_set_hint_style(options, CAIRO_HINT_STYLE_NONE); >- cairo_font_options_set_antialias(options, CAIRO_ANTIALIAS_GRAY); >+ cairo_font_options_set_antialias(options, CAIRO_ANTIALIAS_BEST); > > if (syntheticOblique()) { > static const float syntheticObliqueSkew = -tanf(14 * acosf(0) / 90);
Karlen Simonyan
Comment 7
2015-07-06 14:27:05 PDT
Comment on
attachment 256238
[details]
ClearType fix for custom fonts (v3)
>Index: FontPlatformDataCairoWin.cpp >=================================================================== >--- Source/WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp (revision 186376) >+++ Source/WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp (working copy) >@@ -77,7 +77,7 @@ > // We force antialiasing and disable hinting to provide consistent > // typographic qualities for custom fonts on all platforms. > cairo_font_options_set_hint_style(options, CAIRO_HINT_STYLE_NONE); >- cairo_font_options_set_antialias(options, CAIRO_ANTIALIAS_GRAY); >+ cairo_font_options_set_antialias(options, CAIRO_ANTIALIAS_BEST); > > if (syntheticOblique()) { > static const float syntheticObliqueSkew = -tanf(14 * acosf(0) / 90);
Karlen Simonyan
Comment 8
2015-07-06 14:28:54 PDT
(In reply to
comment #3
)
> Comment on
attachment 253959
[details]
> ClearType fix for custom fonts > > Please fix the Bot Status, "patch does not apply...". > It seems that your patch has a CR(carriage return) character, which is a > violation of the WebKit coding convention. > After correction, review flags should be set as "?" in order to be reviewed > by any reviewers.
Thanks Jinyoung for good point!
Jinyoung Hur
Comment 9
2015-07-06 19:27:30 PDT
You're welcome! However, your patch still looks not ready for a review. It seems that you have uploaded your patch manually rather than use the patch-uploading script, "webkit-patch". I believe the sign, "patch does not apply...", would disappear if you revise your patch using the script. There is an explanation how to use it in this page. (
https://www.webkit.org/coding/contributing.html
)
Karlen Simonyan
Comment 10
2015-07-07 13:11:33 PDT
Created
attachment 256315
[details]
ClearType fix for custom fonts (v4)
WebKit Commit Bot
Comment 11
2015-07-07 13:14:08 PDT
Attachment 256315
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp:80: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 1 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alex Christensen
Comment 12
2015-07-07 17:06:51 PDT
The patch needs a ChangeLog entry. Use perl Tools/Scripts/prepare-ChangeLog to prepare an example
Alex Christensen
Comment 13
2015-07-08 10:17:41 PDT
Comment on
attachment 256315
[details]
ClearType fix for custom fonts (v4) This is likely an improvement that should go into WebKit, but please prepare a ChangeLog entry like the changes on trac.webkit.org
Karlen Simonyan
Comment 14
2015-07-08 12:51:46 PDT
Created
attachment 256400
[details]
ClearType fix for custom fonts (v5)
WebKit Commit Bot
Comment 15
2015-07-08 12:54:47 PDT
Attachment 256400
[details]
did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/graphics/win/FontPlatformDataCairoWin.cpp:80: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Karlen Simonyan
Comment 16
2015-07-08 13:13:59 PDT
(In reply to
comment #13
)
> Comment on
attachment 256315
[details]
> ClearType fix for custom fonts (v4) > > This is likely an improvement that should go into WebKit, but please prepare > a ChangeLog entry like the changes on trac.webkit.org
Hi Alex! I've prepared new patch (
https://bugs.webkit.org/attachment.cgi?id=256400
) with ChangeLog entry. Hope everything is correct for now. Thank you.
Brent Fulgham
Comment 17
2015-07-08 14:41:59 PDT
Comment on
attachment 256400
[details]
ClearType fix for custom fonts (v5) View in context:
https://bugs.webkit.org/attachment.cgi?id=256400&action=review
This seems reasonable. r=me.
> Source/WebCore/ChangeLog:8 > + No new tests (OOPS!).
Do we not have any tests that exercise this code? At the very least, I would expect we would need to rebaseline test due to the improved quality of the font rendering.
Alex Christensen
Comment 18
2015-07-09 11:42:51 PDT
http://trac.webkit.org/changeset/186599
> Do we not have any tests that exercise this code? At the very least, I would expect we would need to rebaseline test due to the improved quality of the font rendering.
WinCairo does not have layout test expectations right now. If it did, we would need to rebaseline some tests.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug