Summary: | Disable hinting when webkit-font-smoothing:antialiased is used on Mac. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | bungeman | ||||||||
Component: | Platform | Assignee: | bungeman | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | mitz, reed, senorblanco, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
bungeman
2012-10-01 12:53:54 PDT
Created attachment 166529 [details]
Patch
are we putting this on hold, pending the search for other mechanisms that are more explicit? Created attachment 167371 [details]
Patch
USE(SKIA_ON_MAC_CHROMIUM) is currently always true when FontSkia.cpp is included. However, this documents for the future merging of the various 'Font' implementations that this is Mac specific. Requesting re-evaluation now that the Skia changes have settled down. works for me Comment on attachment 167371 [details]
Patch
I think we prefer #if PLATFORM(CHROMIUM) && OS(DARWIN) in platform/graphics/skia. SKIA_ON_MAC_CHROMIUM only seems to be used in cross-platform code, and should be replaced with the above now anyway.
Comment on attachment 167371 [details] Patch Attachment 167371 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14183532 New failing tests: http/tests/workers/terminate-during-sync-operation.html Created attachment 167457 [details]
Patch
Changed test to '#if OS(DARWIN)' since this is the desired behavior, Chromium or not (though Chromium is the only one currently using this file). It would be nice if the test could be more like '#if SKIA_FONT_HOST(CORE_GRAPHICS)' since that's what we're really getting at here. The webkit-font-smoothing css property is just exposing a CoreGraphics API. It just happens to be that currently 'OS(DARWIN)' will have the same value as the ephemeral 'SKIA_FONT_HOST(CORE_GRAPHICS)'. That's why I proposed using SKIA_ON_MAC_CHROMIUM, as it seemed closer to 'SKIA_FONT_HOST(CORE_GRAPHICS)'. The test '#if OS(DARWIN)' is fine for now, but I have modified the comment to clarify the situation. Comment on attachment 167457 [details]
Patch
OK. r=me
Comment on attachment 167457 [details] Patch Clearing flags on attachment: 167457 Committed r130800: <http://trac.webkit.org/changeset/130800> All reviewed patches have been landed. Closing bug. |