The attached patch updates the WebKit sources to support proper rendering of text using the Cairo back-end.
Created attachment 19102 [details] Patch to add font support to Cairo/Win backend.
(In reply to comment #1) > Created an attachment (id=19102) [edit] > Patch to add font support to Cairo/Win backend. > I didn't have the chance to look over this fully yet. Some things I spotted: + static cairo_font_options_t* fontOptions; + if (!fontOptions) + fontOptions = cairo_font_options_create(); ^ Uninitialised variable, this will break in release builds. You should be fine with just: static cairo_font_options_t* fontOptions = cairo_font_options_create(); or you could keep the logic (it'll become useful later when Win implements central font options) and assign 0. +#if PLATFORM(CG) CGContextSaveGState(pctx); IntRect printRect = printerRect(printDC); @@ -2740,6 +2741,9 @@ HRESULT STDMETHODCALLTYPE WebFrame::spoo CGContextEndPage(pctx); CGContextRestoreGState(pctx); +#else + notImplemented(); +#endif ^ I'm already working on printing abstraction, so it's good that you just stubbed it out and didn't try to implement it. The rename of cgFont() to platformFont() is a clever shortcut, I like that :-) +#if PLATFORM(CG) typedef struct CGColor* CGColorRef; -typedef struct CGContext* CGContextRef; +typedef struct CGContext* ContextRef; +typedef const CGColorRef ColorRef; +#elif PLATFORM(CAIRO) +#include "GraphicsContext.h" +typedef WebCore::GraphicsContext* ContextRef; +typedef const WebCore::Color* ColorRef; +#endif ^ I'd just do #else here, though it doesn't matter much. I think there are a few more worthwhile code sharing opportunities here before, but didn't study the split closely enough to be sure yet.
"Uninitialised variable, this will break in release builds." Incorrect. Statics inside functions in C++ are initialized to 0.
(In reply to comment #3) > "Uninitialised variable, this will break in release builds." > > Incorrect. Statics inside functions in C++ are initialized to 0. > Technically correct but code like this gets moved around and might well end up in a pure C file some day. I'm pretty sure gcc will not zero stack-allocated pointers on some platforms unless you explicitly enable C99. I wouldn't write new code that relies on this feature.
(In reply to comment #2) > Some things I spotted: > > + static cairo_font_options_t* fontOptions; > + if (!fontOptions) > + fontOptions = cairo_font_options_create(); > > ^ Uninitialised variable, this will break in release builds. > You should be fine with just: > > static cairo_font_options_t* fontOptions = cairo_font_options_create(); Oh -- I didn't even notice I had done this. I'll fix this immediately.
Created attachment 19118 [details] Revised patch based on alp's comments Update against today's SVN and incorporating Alp's suggestions.
Created attachment 19199 [details] Second revision based on Alp and others comments. Revised to increase code sharing between CG/Cairo.
Created attachment 19231 [details] Third revision based on dhyatt and mitzpettel's comments * Switched from "platformFont" to explicit "cgFont" and "fontFace" members * Removed use of "platformPrivateInit/Destroy/etc." and switched to more descriptive names (initGDIFont/platformCommonDestroy/etc."
Comment on attachment 19231 [details] Third revision based on dhyatt and mitzpettel's comments + bool fontCreationFailed = (result->cgFont() == 0); This should just say + bool fontCreationFailed = !result->cgFont(); (similarly for the Cairo version). +#include <wtf/RetainPtr.h> should come last in FontCustomPlatformDataCairo.cpp. + FontCustomPlatformDataCairo(cairo_font_face_t* fontFace) + : m_fontFace(fontFace) + {} The initialization should be indented and the braces go on separate lines. +FontCustomPlatformDataCairo* createFontCustomPlatformData(SharedBuffer* buffer); No need to name the argument. + static cairo_font_options_t* fontOptions = 0; + if (!fontOptions) + fontOptions = cairo_font_options_create(); + cairo_font_options_set_antialias(fontOptions, CAIRO_ANTIALIAS_SUBPIXEL); No need to initialize the static variable to 0. But more importantly, Darin pointed out that you probably meant to include cairo_font_options_set_antialias() in the if block so that you only do it once. SimpleFontData::platformDestroy() in SimpleFontDataCairoWin.cpp should call platformCommonDestroy().
Created attachment 19234 [details] Fourth revision based on mitzpettel's comments * Corrects formatting * SimpleFontData::platformDestroy() in SimpleFontDataCairoWin.cpp now calls platformCommonDestroy(). * cairo_font_options_set_antialias() now only called once during the initialization.
Comment on attachment 19234 [details] Fourth revision based on mitzpettel's comments r=me Please remove this line when landing: + WARNING: NO TEST CASES ADDED OR CHANGED
Landed in <http://trac.webkit.org/projects/webkit/changeset/30441>.