[webkit-reviews] review denied: [Bug 43608] [chromium] Implement GLES2Canvas/Texture in terms of GraphicsContext3D instead of direct OpenGL calls : [Attachment 63691] Patch

[bugzilla-daemon at webkit.org]

Stephen White <senorblanco at chromium.org> has denied James Robinson
<jamesr at chromium.org>'s request for review:
Bug 43608: [chromium] Implement GLES2Canvas/Texture in terms of
GraphicsContext3D instead of direct OpenGL calls
https://bugs.webkit.org/show_bug.cgi?id=43608

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

------- Additional Comments from Stephen White <senorblanco at chromium.org>
WebCore/platform/graphics/chromium/GLES2Canvas.cpp:117
 +	    m_context->deleteBuffer(m_quadVertices);
While you're there, you should probably change this to m_quadIndices (bug).

WebCore/platform/graphics/chromium/GLES2Canvas.cpp:138
 +	m_context->prepareTexture();
I think this prepareTexture() call is wrong.

WebCore/platform/graphics/chromium/GLES2Canvas.cpp:167
 +	m_context->prepareTexture();
Same as above, prepareTexture() can go away (I think).

WebCore/platform/graphics/chromium/GLES2Texture.cpp:70
 +	    return 0;
I think this will leak the texture.  If you're going to early-return here, you
should deleteTexture().  It might be better to just move this check to the
start of the function.

WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:801
 +	    platformContext()->restore();
Rather than adding a platformContext()->restore here, I'd prefer that this
block be moved above platformContext()->save().

Looks good other than that.