976173 - preserveDrawingBuffer: false breaks printing of Canvas / WebGL

Status: NEW

(Core :: Graphics: CanvasWebGL, enhancement, P5)

Description

[Michael Davidson]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.107 Safari/537.36

Steps to reproduce:

Go to www.google.com/maps (opt in to the new Google Maps)
Ensure you're not in lite mode
Try to print


Actual results:

The canvas isn't printed


Expected results:

The canvas should be printed (and is in Chrome).

I've verified this is due to preserveDrawingBuffer:false set when we get the context.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Can we not draw such webgl canvases to a 2d canvas?

http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLCanvasElement.cpp?rev=fed727c12315&mark=274-274,286-287#270

(I know very little about webgl)

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

I understand this reproduces at least on Windows and OS X

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that shows that we can only paint a gl canvas to a 2d one until composite has happened if preserveDrawingBuffer:false

But note that Chrome has the same behavior as we do for this testcase...

Comment 4

[Boris Zbarsky [:bzbarsky]]

It's possible that printing will need a different way to capture the state of gl canvases.  :(

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

No, this bug almost certainly falls into the general bucket of "we implement preserveDrawingBuffer:false in the wrong way by not waiting for the next frame to actually start rendering, like we should, before clearing the framebuffer". There is no need to add any specific code paths for printing. Jeff Gilbert or Dan Glastonbury would know more.

Comment 6

[Kelsey Gilbert [:jgilbert]]

(In reply to Benoit Jacob [:bjacob] from comment #5)
> No, this bug almost certainly falls into the general bucket of "we implement
> preserveDrawingBuffer:false in the wrong way by not waiting for the next
> frame to actually start rendering, like we should, before clearing the
> framebuffer". There is no need to add any specific code paths for printing.
> Jeff Gilbert or Dan Glastonbury would know more.

There's nothing really 'wrong' about what we do WRT pDB:false. Generally, we haven't don't clear a pDB:false frame until about 2-3 frames after it was generated, when we reuse that surface.

The mechanics of pDB:false don't lend themselves to printing, or toDataURL, for that matter. Don't use the pDB:false optimization if you need to do these things. While printing is not mentioned in the spec, our code effectively handles toDataURL and printing similarly, I believe.

"Testcase that shows that we can only paint a gl canvas to a 2d one until composite has happened if preserveDrawingBuffer:false" is almost exactly what the spec says should happen.

Comment 7

[Kelsey Gilbert [:jgilbert]]

I'll note that while we're spec-compliant, this behavior is not spec-mandated, so we could fix it, if possible.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

I didn't realize that our behavior here was still spec-compliant; I thought that the spec now mandated preserving the drawing buffer until the next frame starts rendering (i.e. another draw or clear operation is made). Sorry if that was wrong. Can you just explain, since the present testcase doesn't do any draw or clear after the initial frame, why we have to clear the drawing buffer in this case?

Comment 9

[Kelsey Gilbert [:jgilbert]]

(In reply to Benoit Jacob [:bjacob] from comment #8)
> I didn't realize that our behavior here was still spec-compliant; I thought
> that the spec now mandated preserving the drawing buffer until the next
> frame starts rendering (i.e. another draw or clear operation is made). Sorry
> if that was wrong. Can you just explain, since the present testcase doesn't
> do any draw or clear after the initial frame, why we have to clear the
> drawing buffer in this case?

We don't have the same frame simultaneously in both content and the compositor.
It doesn't matter that the user hasn't drawn, but rather that they drew on the previous frame, and we need to composite that frame. We should be able to add logic to 'hold open a read lock' for the previous frame, but this does add complexity. 

Spec on pDB:False:
 If this flag is false, attempting to perform operations using this context as a source image after the rendering function has returned can lead to undefined behavior. This includes readPixels or toDataURL calls, or using this context as the source image of another context's texImage2D or drawImage call. 

Basically, if when you return from a rendering function, it's like calling SwapBuffers. (unless we haven't Presented to the compositor yet) With pDB:false, your data isn't still there, because you have a new surface to draw into from calling SwapBuffers.

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

Got it, thanks. The present bug is RESOLVED INVALID, then. Could we flag the drawingbuffer state as undefined and generate a helpful warning when the result of an operation is consequently undefined (like, here, getting at the drawing buffer's pixels) ?

Comment 11

[Kelsey Gilbert [:jgilbert]]

(In reply to Benoit Jacob [:bjacob] from comment #10)
> Got it, thanks. The present bug is RESOLVED INVALID, then. Could we flag the
> drawingbuffer state as undefined and generate a helpful warning when the
> result of an operation is consequently undefined (like, here, getting at the
> drawing buffer's pixels) ?

That's a decent idea then. I'm leaving this as 'NEW', since we *could* fix it, even if we don't have to. I have lowered the importance to 'enhancement', which makes it clear from bugzilla search results that this is not a priority, even if it's still open.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Jeff, we now can do async operations from printing more easily, do you know what's a good time to read back the 2D / WebGL buffer if any? This may be easy-ish to fix.

Comment 15

[Kelsey Gilbert [:jgilbert]]

Printing needs to ask the compositor for readback, not webgl. Webgl doesn't per se have access to the data after Presentation with PDB:false.

Comment 17

[Thomas Wisniewski [:twisniewski]]

This is making printouts of Mapbox maps blank, as discovered at https://github.com/webcompat/web-bugs/issues/111242

Given that, perhaps we should we bump the priority of this bug?