[webkit-reviews] review granted: [Bug 34719] WebGL rendering results must be made available to Canvas.toDataURL and 2D drawImage : [Attachment 60482] revised patch: responding to Simon Fraser's review

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Zhenyao Mo
<zmo at google.com>'s request for review:
Bug 34719: WebGL rendering results must be made available to Canvas.toDataURL
and 2D drawImage
https://bugs.webkit.org/show_bug.cgi?id=34719

Attachment 60482: revised patch: responding to Simon Fraser's review
https://bugs.webkit.org/attachment.cgi?id=60482&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>

WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:256
 +	// BEGIN: the same code as in
WebGraphicsContext3DDefaultImpl.cpp::readBackFramebuffer
Either share the code, or remove the comment. As-is, the comment is not useful.


Can you share code with GraphicsContext3D::readPixels()?

WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:290
 +	CGImageRef cgImage = CGImageCreate(m_currentWidth,
You should use RetainPtr for the dataProvider, colorSpace and cgImage.

Do you need to save/restore the state in the imageBuffer->context()?

WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:307
 +	// We want to completely overwrite the previous frame's rendering
results.

Is that described in a spec anywhere?

r=me