[webkit-reviews] review granted: [Bug 69530] re-add support for GDI text behind a compile flag : [Attachment 110192] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 10 12:57:31 PDT 2011

James Robinson <jamesr at chromium.org> has granted Mike Reed <reed at google.com>'s
request for review:
Bug 69530: re-add support for GDI text behind a compile flag

Attachment 110192: Patch

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110192&action=review

Awesome! Thanks so much.

r=me with a few nits to address before landing

> Source/WebCore/ChangeLog:11
> +	   Reverts back to using GDI for text (when possible)
> +	   but keeps skia-text version behind a compile-flag. If/when we can
> +	   resolve the outstanding soft-clip and intl-performance bugs with the

> +	   skia version, we may change the compile-flag to reenable skia.

Please add a link to the commit where this code was deleted so people can look
at the original history if they need to (you can link to revisions in WebKit
with URLs of the form http://trac.webkit.org/changeset/12345).

> Source/WebCore/platform/graphics/chromium/FontChromiumWin.cpp:449
> +//	 graphicsContext->platformContext()->prepareForSoftwareDraw();
> +

don't check this in commented out - I don't think you need it, so it can just
be deleted.

> Source/WebCore/platform/graphics/chromium/FontChromiumWin.cpp:469
> +	   int curLen = std::min(kMaxBufferLength, numGlyphs - glyphIndex);

nit: webkit style is to put a 'using namespace std' declaration at the top of
the file and have this call be just min()

> Source/WebCore/platform/graphics/skia/SkiaFontWin.cpp:47
> +bool windowsCanHandleDrawTextShadow(GraphicsContext *context)

nit: WebKit style is the * goes with the type name, so GraphicsContext* context

More information about the webkit-reviews mailing list