Window.createImageBitmap(HTMLCanvasElement) should copy the front buffer of WebGL, not back buffer

Window.createImageBitmap(HTMLCanvasElement) should copy the front buffer of WebGL, not back buffer

BUG=438986

[dshwang, 2014-12-04 16:49:41]

dongseong.hwang@intel.com changed reviewers:
+ junov@chromium.org, kbr@chromium.org

[dshwang, 2014-12-04 16:49:41]

@kbr, could you review?

Although the w3c bug [1] is not resolved yet, I'm willing to change
implementation
[1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=27513

Rationals
1. The spec doesn't specify, so we can choose our preference.
2. All render objects requests front buffer except for ImageBitmap
 e.g. HTMLCanvasElement::paint(), CSSCanvasValue::image(), Editor::copyImage(),
Editor::copy(), HitTestResult::absoluteImageURL(), WebViewImpl::saveImageAt(),
etc..
3. If ImageBitmap copy the front buffer, HTMLCanvasElement::m_copiedImage always
store the front buffer.
4. HTMLCanvasElement::m_copiedImage already has a bug not to copy real back
buffer in some case. HTMLCanvasElement::m_copiedImage should be cleared after
DrawingBuffer::prepareMailbox(), because the back buffer is invalidated. To fix
it, we need to add dirty callback from DrawingBuffer to WebGLRenderingContext
like https://codereview.chromium.org/766333003
5. From implementation perspective, it makes our world more clean.

[Ken Russell (switch to Gerrit), 2014-12-05 02:56:06]

Thanks for pointing this out and working on it.

A change definitely needs to be made here, to the spec as well, but I think the
fix needs to be more subtle. If a user does the following all in a single frame:

 1. Draws some WebGL to a canvas
 2. Turns that canvas into an ImageBitmap
 3. When the ImageBitmap is resolved, draws it to a different canvas's 2D
context

the ImageBitmap should conceptually contain the rendering results from (1),
wouldn't you say?

I think we need to specify this as something like: if the WebGL context's
results have been modified since the last time its results were presented to the
web page compositor, then the most recent results are used (i.e., the back
buffer); otherwise, the front buffer is used.

Is this feasible to implement? I think there are enough state bits on the
WebGLRenderingContextBase to track this.

[dshwang, 2014-12-05 10:10:44]

On 2014/12/05 02:56:06, Ken Russell wrote:
> Thanks for pointing this out and working on it.
> 
> A change definitely needs to be made here, to the spec as well, but I think
the
> fix needs to be more subtle. If a user does the following all in a single
frame:
> 
>  1. Draws some WebGL to a canvas
>  2. Turns that canvas into an ImageBitmap
>  3. When the ImageBitmap is resolved, draws it to a different canvas's 2D
> context
> 
> the ImageBitmap should conceptually contain the rendering results from (1),
> wouldn't you say?

I agree. (1) should belong to the results.

> I think we need to specify this as something like: if the WebGL context's
> results have been modified since the last time its results were presented to
the
> web page compositor, then the most recent results are used (i.e., the back
> buffer); otherwise, the front buffer is used.

After reading your explanation, I think copying back buffer is better than
copying front buffer.
And this subtle way might be better than copying back buffer.

> Is this feasible to implement? I think there are enough state bits on the
> WebGLRenderingContextBase to track this.

It's feasible while it makes code more complicated mainly because we cache the
rendering results until next frame.

By the way, canvas.drawImage(WebGLCanvas) and webgl.texImage2D(WebGLCanvas) copy
transparent buffer since the last time its results were presented to the web
page compositor.
Do we need to give this special care to only ImageBuffer? I think ImageBuffer
should behave similar to canvas.drawImage(WebGLCanvas) and
webgl.texImage2D(WebGLCanvas).
I mean copying back buffer for ImageBuffer is not bad, instead of subtle care.

[Ken Russell (switch to Gerrit), 2014-12-05 20:22:32]

On 2014/12/05 10:10:44, dshwang wrote:
> On 2014/12/05 02:56:06, Ken Russell wrote:
> > Thanks for pointing this out and working on it.
> > 
> > A change definitely needs to be made here, to the spec as well, but I think
> the
> > fix needs to be more subtle. If a user does the following all in a single
> frame:
> > 
> >  1. Draws some WebGL to a canvas
> >  2. Turns that canvas into an ImageBitmap
> >  3. When the ImageBitmap is resolved, draws it to a different canvas's 2D
> > context
> > 
> > the ImageBitmap should conceptually contain the rendering results from (1),
> > wouldn't you say?
> 
> I agree. (1) should belong to the results.
> 
> > I think we need to specify this as something like: if the WebGL context's
> > results have been modified since the last time its results were presented to
> the
> > web page compositor, then the most recent results are used (i.e., the back
> > buffer); otherwise, the front buffer is used.
> 
> After reading your explanation, I think copying back buffer is better than
> copying front buffer.
> And this subtle way might be better than copying back buffer.
> 
> > Is this feasible to implement? I think there are enough state bits on the
> > WebGLRenderingContextBase to track this.
> 
> It's feasible while it makes code more complicated mainly because we cache the
> rendering results until next frame.
> 
> By the way, canvas.drawImage(WebGLCanvas) and webgl.texImage2D(WebGLCanvas)
copy
> transparent buffer since the last time its results were presented to the web
> page compositor.
> Do we need to give this special care to only ImageBuffer? I think ImageBuffer
> should behave similar to canvas.drawImage(WebGLCanvas) and
> webgl.texImage2D(WebGLCanvas).
> I mean copying back buffer for ImageBuffer is not bad, instead of subtle care.

Right, I don't know what I was thinking. All of these APIs should behave
identically, so I don't think any changes should be made in this area.

[dshwang, 2014-12-05 20:33:23]

On 2014/12/05 20:22:32, Ken Russell wrote:
> On 2014/12/05 10:10:44, dshwang wrote:
> > On 2014/12/05 02:56:06, Ken Russell wrote:
> > > Thanks for pointing this out and working on it.
> > > 
> > > A change definitely needs to be made here, to the spec as well, but I
think
> > the
> > > fix needs to be more subtle. If a user does the following all in a single
> > frame:
> > > 
> > >  1. Draws some WebGL to a canvas
> > >  2. Turns that canvas into an ImageBitmap
> > >  3. When the ImageBitmap is resolved, draws it to a different canvas's 2D
> > > context
> > > 
> > > the ImageBitmap should conceptually contain the rendering results from
(1),
> > > wouldn't you say?
> > 
> > I agree. (1) should belong to the results.
> > 
> > > I think we need to specify this as something like: if the WebGL context's
> > > results have been modified since the last time its results were presented
to
> > the
> > > web page compositor, then the most recent results are used (i.e., the back
> > > buffer); otherwise, the front buffer is used.
> > 
> > After reading your explanation, I think copying back buffer is better than
> > copying front buffer.
> > And this subtle way might be better than copying back buffer.
> > 
> > > Is this feasible to implement? I think there are enough state bits on the
> > > WebGLRenderingContextBase to track this.
> > 
> > It's feasible while it makes code more complicated mainly because we cache
the
> > rendering results until next frame.
> > 
> > By the way, canvas.drawImage(WebGLCanvas) and webgl.texImage2D(WebGLCanvas)
> copy
> > transparent buffer since the last time its results were presented to the web
> > page compositor.
> > Do we need to give this special care to only ImageBuffer? I think
ImageBuffer
> > should behave similar to canvas.drawImage(WebGLCanvas) and
> > webgl.texImage2D(WebGLCanvas).
> > I mean copying back buffer for ImageBuffer is not bad, instead of subtle
care.
> 
> Right, I don't know what I was thinking. All of these APIs should behave
> identically, so I don't think any changes should be made in this area.

I agree. I close it as WontFix