[webkit-reviews] review granted: [Bug 43070] [chromium] Let PlatformContextSkia draw to a GLES2Canvas in addition to an SkCanvas : [Attachment 62739] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted James Robinson
<jamesr at chromium.org>'s request for review:
Bug 43070: [chromium] Let PlatformContextSkia draw to a GLES2Canvas in addition
to an SkCanvas
https://bugs.webkit.org/show_bug.cgi?id=43070

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebCore/platform/graphics/skia/PlatformContextSkia.cpp:679
 +  void PlatformContextSkia::setGLES2Context(WebCore::GLES2Context* context,
const WebCore::IntSize& size)
This code should really be within the WebCore namespace :)

WebCore/platform/graphics/skia/PlatformContextSkia.cpp:212
 +	, m_GPUCanvas(0)
style-nit: m_gpuCanvas <- acronym at the start of a term should be lowercase.

WebCore/platform/graphics/skia/PlatformContextSkia.cpp:687
 +	if (m_useGPU) {
nit: consider returning early if !m_useGPU to reduce indentation of the
interesting
part of this function.	of course, if you don't like this style for the shorter

methods and prefer consistency, then ignore this nit.

WebCore/platform/graphics/skia/PlatformContextSkia.cpp:689
 +		// Depending on the blend mode we need to one of a few things:
nit: "need to _do_ one..."

WebCore/platform/graphics/skia/PlatformContextSkia.cpp:744
 +	    if (m_backingStoreState == Hardware) {
nit: no brackets around single line statements

WebCore/platform/graphics/skia/PlatformContextSkia.cpp:762
 +	// FIXME: Keep a texture around for this rather than constantly
creating/destroying  one.
nit: "destroying  one"	<- one space instead of two

WebCore/platform/graphics/skia/PlatformContextSkia.cpp:775
 +	WTF::OwnArrayPtr<uint32_t> buf(new uint32_t[width]);
ordinarily people drop the WTF:: prefix for OwnArrayPtr.  the header
file has a using WTF::OwnArrayPtr.

R=me