748135 - Add takeScreenshot() to nsIDOMWindowUtils

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, enhancement)

Description

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

This way we don't need a canvas to capture screenshots.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

We can probably remove canvas.mozFetchAsStream(), I don't think anything other than the thumbnail service uses it. I'd file a follow-up for it.

Comment 2

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

The method needs a security check.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

(In reply to Olli Pettay [:smaug] from comment #2)
> The method needs a security check.

Added.

Another thing I noticed is that the screenshots captured aren't scaled down as they were before. I wasn't able to find the code responsible for this in .drawWindow()...

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v3

Added a gfxContext->Scale() call to let the thumbnail be scaled down accordingly.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v4

Simplified the patch again because imgIEncoder already implements nsIAsyncInputStream.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

I did some measurements and the results are a bit surprising:

The old approach that creates a canvas, calls drawWindow() and mozFetchAsStream() - that still extracts and encodes image data sync - takes on average 50ms.

The new approach that calls takeScreenshot() and does the same sync image data encoding takes on average 150ms.

Is this maybe because the Azure/Skia backend accelerates the resize/scale operation for canvases? BTW, my machine doesn't support HW accel. It would be interesting to know if mobile actually took a performance hit by implementing bug 724210. They don't seem to scale down and moved the PNG encoding to a different thread (which we could also do?) so that still might be a win...

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 617840 [details] [diff] [review]
patch v4

Looks good to me. You should get review from a DOM peer (Olli?) and get feedback from some gfx person about your perf findings, I guess!

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Adding khuey and hope he has some idea about the perf issues I'm seeing.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'm not the right person to ask about gfx perf.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Hmm, that's what I thought. Adding roc to hopefully lighten us up regarding comment #6.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

I spoke to Jeff&Joe about this - they weren't sure about why there was a perf difference, but suggested that using RenderDocument still wasn't optimal. They suggested instead adding an API to directly retrieve the drawn data (asynchronously), and then using that instead (which should work fine as long as we only want to capture the selected tab). Would need more feedback from them about how exactly to do that (Jeff suggested discussing it with Bas and roc), but it might be best to WONTFIX this and just focus on that instead.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

Marking as WONTFIX based on comment #11.

Comment 13

[Timothy Nikkel (:tnikkel)]

If you pass the widget_layers flag and the data you are asking for is actually retained we shouldn't need to do any painting, just composite the layer tree to the output context. However there are some things that might prevent that from happening. For the content to be retained it has to actually be visible on screen when you make the call, and you can't pass any flags to drawWindow or RenderDocument that ask for something different then what is drawn on screen.

Comment 14

[Timothy Nikkel (:tnikkel)]

For example, background tabs don't have any retained content because they are not visible.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

The "but it might actually be slower or lower quality than normal" comment in the IDL is slightly worrying, but it sounds like we should get a bug on file to consider using DRAWWINDOW_USE_WIDGET_LAYERS? We currently only care about capturing thumbnails of displayed pages.

Comment 16

[Timothy Nikkel (:tnikkel)]

I guess it might be slower if its in GPU memory and we have to do readback? I'm not sure. I'm also not sure about the quality part.

Comment 17

[Timothy Nikkel (:tnikkel)]

The two flags in this patch (RENDER_IGNORE_VIEWPORT_SCROLLING, RENDER_DOCUMENT_RELATIVE) are the bad kind that make the widget layers flag not work so well.