Implement transferToImageBitmap() in WebGLRenderingContext

third_party/WebKit/Source/modules/webgl/WebGLRenderingContext.cpp

Index: third_party/WebKit/Source/modules/webgl/WebGLRenderingContext.cpp
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContext.cpp b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContext.cpp
index cc46e806bbfa070435ca4cde303a74fd5a772a27..8d079724ee0ac5f9944b921479e903fc89799608 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContext.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContext.cpp
@@ -85,6 +85,9 @@ static bool shouldCreateContext(WebGraphicsContext3DProvider* contextProvider)
 CanvasRenderingContext* WebGLRenderingContext::Factory::create(ScriptState* scriptState, OffscreenCanvas* offscreenCanvas, const CanvasContextCreationAttributes& attrs)
 {
     WebGLContextAttributes attributes = toWebGLContextAttributes(attrs);
+    // transferToImageBitmap calls paintRenderingResultsToImageData, which
+    // requires this bit to be false;
+    attributes.setPremultipliedAlpha(false);

[Ken Russell (switch to Gerrit), 2016/05/10 23:11:42]

I don't think it's legal to just set this all the time for WebGL contexts. It
will break rendering of certain applications which assume the compositor and the
WebGL app behave in a certain way. Unfortunately, I don't think this is or can
be covered by a WebGL conformance test, but we should add a reference test that
verifies the behavior of premultipliedAlpha:true and false for WebGL-rendered
canvases. Can you help with that or should I write them?

[xidachen, 2016/05/11 00:17:52]

I did hesitate when I added this statement. The only reason I put it here is
that there is a "ASSERT(!m_premultipliedAlpha);" in the method
DrawingBuffer::paintRenderingResultsToImageData. So without this line, I am
always hitting that ASSERT. Is there a way to avoid hitting that ASSERT without
setting false here? I'd like to remove this like we can avoid that ASSERT.

[Ken Russell (switch to Gerrit), 2016/05/11 01:00:41]

It's likely that that code path has never been tested, and that the assertion
was merely put in place to force re-evaluation of the code if anyone hit it.

This code path would probably take effect if someone called toDataURL() on a
WebGL-rendered canvas where the context was created with premultiplyAlpha:true.
Also, clearly, your new code is hitting it.

In order to generalize it, a lossy "unmultiplication" of the alpha channel would
have to be done during the readback. Thinking about it, something like this must
be being done for 2D canvas when fetching the ImageData for the backing store.
Can you talk with junov@ and confirm?

In some sense, there's no problem with adding support for AlphaDoUnmultiply in
DrawingBuffer::readBackFramebuffer -- but if we do this, we need to add tests.

Alternatively, we could work on implementing this the right way, and avoid the
readbacks entirely when creating the ImageBitmap.

[xidachen, 2016/05/11 01:07:08]

Yeah, it makes more sense to implement this using another approach. I just took
another look at:
WebGLRenderingContextBase::paintRenderingResultsToImageData
Apparent it returns nullptr when the premultipliedAlpha=true.

     OwnPtr<WebGraphicsContext3DProvider> contextProvider(createWebGraphicsContext3DProvider(scriptState, attributes, 1));
     if (!shouldCreateContext(contextProvider.get()))
         return nullptr;
@@ -147,8 +150,7 @@ void WebGLRenderingContext::setOffscreenCanvasGetContextResult(OffscreenRenderin
 
 ImageBitmap* WebGLRenderingContext::transferToImageBitmap(ExceptionState& exceptionState)
 {
-    NOTIMPLEMENTED();
-    return nullptr;
+    return transferToImageBitmapBase();
 }
 
 void WebGLRenderingContext::registerContextExtensions()