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).
Created attachment 253959 [details] ClearType fix for custom fonts
It seems that this patch might help improving the quality of custom font rendering on Windows. Is there anybody to review this?
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.
Created attachment 256237 [details] ClearType fix for custom fonts (v2)
Created attachment 256238 [details] ClearType fix for custom fonts (v3)
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);
(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!
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)
Created attachment 256315 [details] ClearType fix for custom fonts (v4)
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.
The patch needs a ChangeLog entry. Use perl Tools/Scripts/prepare-ChangeLog to prepare an example
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
Created attachment 256400 [details] ClearType fix for custom fonts (v5)
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.
(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.
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.
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.