This patch special-cases drawing of text-shadows on the chromium port to let GDI draw the text and text shadow, by passing Skia. This only occurs when both the text fill color and shadow color are opaque and the text shadow is 0 blur.
Created attachment 29088 [details] Let GDI draw text-shadow on opaque colors and 0 blur
Comment on attachment 29088 [details] Let GDI draw text-shadow on opaque colors and 0 blur Clearing review flag as it was set to + by a non-reviewer.
Comment on attachment 29088 [details] Let GDI draw text-shadow on opaque colors and 0 blur >Index: ChangeLog ... >+ Reviewed by NOBODY (OOPS!). >+ >+ Special-case drawing text-shadow on win32, to let GDI draw in cases when both fill & >+ shadow color are opaque and the shadow has no blur. replace tabs with spaces. >Index: platform/graphics/chromium/FontChromiumWin.cpp >+ IntSize shadow_size; >+ int shadow_blur; >+ Color shadow_color; please use webkit style for naming variables: http://webkit.org/coding/coding-style.html >Index: platform/graphics/skia/SkiaFontWin.cpp >+bool windowsCanHandleDrawTextShadow(WebCore::GraphicsContext *context) { >+ IntSize shadow_size; >+ int shadow_blur; >+ Color shadow_color; ditto
See: http://webkit.org/coding/coding-style.html for information on WebKit coding style.
Created attachment 29092 [details] Spaces removed, variables now reflect webkit style guide
Comment on attachment 29092 [details] Spaces removed, variables now reflect webkit style guide The bug url should be in the ChangeLog. Much stronger would be to ASSERT these conditions instead of just makign a comment: + // Note: If the fillColor or shadowColor are ever non-opaque, we should + // never reach this code. SkiaFontWin::windowsCanHandleTextDrawing() will + // have set m_useGDI == false and the above check will have passed and + // the this function returned. tempColor isn't a very descriptiveName. textColor would probably be better. savedColor or savedTextColor would be better than "saveColor". +bool windowsCanHandleDrawTextShadow(WebCore::GraphicsContext *context) { { should be on its own line. "context" is not needed here (nor on the line after) as it provides no additional description to purpose of the argument (See style guide). +bool windowsCanHandleDrawTextShadow(GraphicsContext *context);
Comment on attachment 29092 [details] Spaces removed, variables now reflect webkit style guide I can't really comment on if this is the right approach or not. Maybe fishd or brettw can.
Created attachment 29101 [details] Changes from Comment #6 (Eric Seidel) -ASSERT()s used rather than second check of windowCanHandleDrawTextShadow() -tempColor -> textColor -saveColor -> savedTextColor -formatting -"context" removed from function declaration of both *canHandle*Text functions
I pre-reviewed this approach with brettw and made his corrections before submitting here. (In reply to comment #7) > (From update of attachment 29092 [details] [review]) > I can't really comment on if this is the right approach or not. Maybe fishd or > brettw can. >
Comment on attachment 29101 [details] Changes from Comment #6 (Eric Seidel) Looks fine. Thanks!
Landed as: http://trac.webkit.org/changeset/42131