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

[bugzilla-daemon at webkit.org]

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
https://bugs.webkit.org/show_bug.cgi?id=69530

Attachment 110192: Patch
https://bugs.webkit.org/attachment.cgi?id=110192&action=review

------- 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