Bug 84531 – [Qt][WK2] Snapshot API

• Component: Platform
• Status: RESOLVED INVALID

[Noam Rosenthal]

Support creating a snapshot out of the current webview, to be put into a QImage or saved into a PNG.

[Helder Correia]

Created attachment 147020 [details]
Patch

[Helder Correia]

Comments about the API proposal and approaches taken are appreciated. Also, I wrote tst_renderToDataURL.qml and modified tst_qquickwebview.cpp (not in the patch), but unfortunately I couldn't see any pixels in both cases - like I do in MiniBrowser and qmlscene. There's something that I'm not aware of in the tests, so I'd appreciate any hints or even complements to the patch. Thanks in advance.

[Kenneth Rohde Christiansen]

Comment on attachment 147020 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147020&action=review

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1401
> +    return QString::fromLatin1("data:image/") + QString::fromLatin1(format)
> +           + QString::fromLatin1(";base64,") + QString::fromLatin1(formattedImage.toBase64());

Maybe use the StringBuilder and then make it a QUrl ?

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:320
> +    Q_INVOKABLE QString renderToDataURL(const char* format = "png", int quality = -1) const;

URL is not Qt style, would be Url. Shouldn't it return an QUrl ?

> Tools/MiniBrowser/qt/qml/BrowserWindow.qml:260
> +                        webView.experimental.renderToFile(path)

Would it be interesting to decide what scale to render the page at? or what areas of the page to render?

[Allan Sandfeld Jensen]

Comment on attachment 147020 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147020&action=review

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:319
> +    Q_INVOKABLE bool renderToFile(const QString& path, const char* format = "png", int quality = -1) const;

I would prefer if it rendered to a QFile instead of a path. That would be a more flexible interface, and would avoid having us create new files inside the API.

[Alexis Menard (darktears)]

Comment on attachment 147020 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147020&action=review

I think the example of the BrowserWindow should me more straightforward. Why passing through a file here? Wouldn't it be possible to set the source of the image directly from the onClicked signal?  So it could look a bit more transparent.

>> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:319
>> +    Q_INVOKABLE bool renderToFile(const QString& path, const char* format = "png", int quality = -1) const;
> 
> I would prefer if it rendered to a QFile instead of a path. That would be a more flexible interface, and would avoid having us create new files inside the API.

Well QFile doesn't map in QML...

>> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:320
>> +    Q_INVOKABLE QString renderToDataURL(const char* format = "png", int quality = -1) const;
> 
> URL is not Qt style, would be Url. Shouldn't it return an QUrl ?

Yes so that you can put it into a qml image element.

>> Tools/MiniBrowser/qt/qml/BrowserWindow.qml:260
>> +                        webView.experimental.renderToFile(path)
> 
> Would it be interesting to decide what scale to render the page at? or what areas of the page to render?

That could be done using some kinda JQuery syntax, or selectors.

[Simon Hausmann]

Comment on attachment 147020 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147020&action=review

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:160
> +    context.setShareContext(QOpenGLContext::currentContext());

Does this even work with the threaded scenegraph renderer? The context of that one lives in the render thread (so QOpenGLContext::currentContext() will return 0 in the GUI thread). It's the resources of the scene graph opengl context that you'd probably want to access though in order to get access to its textures (avoiding re-upload?), but you cannot access it from the gui thread.

>>> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:320
>>> +    Q_INVOKABLE QString renderToDataURL(const char* format = "png", int quality = -1) const;
>> 
>> URL is not Qt style, would be Url. Shouldn't it return an QUrl ?
> 
> Yes so that you can put it into a qml image element.

What is the use-case for this API, i.e. who needs it?

[Kenneth Rohde Christiansen]

> >> URL is not Qt style, would be Url. Shouldn't it return an QUrl ?
> > 
> > Yes so that you can put it into a qml image element.
> 
> What is the use-case for this API, i.e. who needs it?

We could let the user select a part of an image and at the same time show how the launcher icon will look, directly from qml. That is one use case

[Alexis Menard (darktears)]

(In reply to comment #6)
> (From update of attachment 147020 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=147020&action=review
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:160
> > +    context.setShareContext(QOpenGLContext::currentContext());
> 
> Does this even work with the threaded scenegraph renderer? The context of that one lives in the render thread (so QOpenGLContext::currentContext() will return 0 in the GUI thread). It's the resources of the scene graph opengl context that you'd probably want to access though in order to get access to its textures (avoiding re-upload?), but you cannot access it from the gui thread.
> 
> >>> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:320
> >>> +    Q_INVOKABLE QString renderToDataURL(const char* format = "png", int quality = -1) const;
> >> 
> >> URL is not Qt style, would be Url. Shouldn't it return an QUrl ?
> > 
> > Yes so that you can put it into a qml image element.
> 
> What is the use-case for this API, i.e. who needs it?

A thumbnail of the webpage you want on the moment.

[Simon Hausmann]

I wonder about the use-case for this API, because I think the use-case might affect the implementation quite a bit. Various questions come to my mind due to the lack of a clearer description of the use-case:

    1) Why choose the WebView's viewport?
    2) How extensible is this API for the use-case of intending to take a snapshot of the entire web page?
    3) What is the expectation on the caller side in terms of performance? Is this expected to be a fast function or rather slow? If the latter, should it be async?


Similarly I can see different approaches of grabbing the content. It is tempting to re-use the textures the texturemapper has, but I think the only reliable way of accessing the textures is through the render thread, i.e. from within. 

You could theoretically create a sharing gl context that shares resources with QQuickCanvas::openglContext(), but since you're using the entire texture mapper for rendering, wouldn't it be likely that you end up in a situation you are calling the texture mapper at a point in time when the render thread is _also_ calling the same instance? Is the texture mapper thread-safe on that level?

A completely different approach would be the one we took on the N9, which is sending a message to the web process for taking a snapshot with a content rect as parameter. The web process then renders the content in software into shared memory (the usual WK2 mechanism) and asynchronously returns a message to the uiprocess, indicating that it is done.

[Simon Hausmann]

(In reply to comment #8)
> (In reply to comment #6)
> > (From update of attachment 147020 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=147020&action=review
> > 
> > > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:160
> > > +    context.setShareContext(QOpenGLContext::currentContext());
> > 
> > Does this even work with the threaded scenegraph renderer? The context of that one lives in the render thread (so QOpenGLContext::currentContext() will return 0 in the GUI thread). It's the resources of the scene graph opengl context that you'd probably want to access though in order to get access to its textures (avoiding re-upload?), but you cannot access it from the gui thread.
> > 
> > >>> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:320
> > >>> +    Q_INVOKABLE QString renderToDataURL(const char* format = "png", int quality = -1) const;
> > >> 
> > >> URL is not Qt style, would be Url. Shouldn't it return an QUrl ?
> > > 
> > > Yes so that you can put it into a qml image element.
> > 
> > What is the use-case for this API, i.e. who needs it?
> 
> A thumbnail of the webpage you want on the moment.

Sorry, my comment was specific to the data URL API. What is the use-case of encoding a thumbnail as a data url?

[Noam Rosenthal]

(In reply to comment #9)
> I wonder about the use-case for this API, because I think the use-case might affect the implementation quite a bit. Various questions come to my mind due to the lack of a clearer description of the use-case:
> 
>     1) Why choose the WebView's viewport?
>     2) How extensible is this API for the use-case of intending to take a snapshot of the entire web page?
>     3) What is the expectation on the caller side in terms of performance? Is this expected to be a fast function or rather slow? If the latter, should it be async?

The first use-case I care about is support for pixel-tests. In that case, the Webview viewport is exactly what you need. We want to test exactly what the user would see, not tell the web-page to render a "special edition" :)

> 
> 
> Similarly I can see different approaches of grabbing the content. It is tempting to re-use the textures the texturemapper has, but I think the only reliable way of accessing the textures is through the render thread, i.e. from within. 
> 
> You could theoretically create a sharing gl context that shares resources with QQuickCanvas::openglContext(), but since you're using the entire texture mapper for rendering, wouldn't it be likely that you end up in a situation you are calling the texture mapper at a point in time when the render thread is _also_ calling the same instance? Is the texture mapper thread-safe on that level?
> 
We can (and should) find a way to block the render thread from painting while we're doing this.

> A completely different approach would be the one we took on the N9, which is sending a message to the web process for taking a snapshot with a content rect as parameter. The web process then renders the content in software into shared memory (the usual WK2 mechanism) and asynchronously returns a message to the uiprocess, indicating that it is done.

I thought about this too. That API exists, problem we had on the N9 was that that codepath flattens all the layers, which results in a different result than what you see in the viewport. 
It's possible to fix it by enabling a software path for 3D transforms in WebCore, and would also be the right way to take "full page" snapshots.

I think that maybe instead of providing a generic "snapshot API" we should create a C++-only viewport snapshot API specifically for pixel-tests, and then use the existing WebProcess-side API for other types of snapshots.

[Simon Hausmann]

(In reply to comment #11)
> (In reply to comment #9)
> > I wonder about the use-case for this API, because I think the use-case might affect the implementation quite a bit. Various questions come to my mind due to the lack of a clearer description of the use-case:
> > 
> >     1) Why choose the WebView's viewport?
> >     2) How extensible is this API for the use-case of intending to take a snapshot of the entire web page?
> >     3) What is the expectation on the caller side in terms of performance? Is this expected to be a fast function or rather slow? If the latter, should it be async?
> 
> The first use-case I care about is support for pixel-tests. In that case, the Webview viewport is exactly what you need. We want to test exactly what the user would see, not tell the web-page to render a "special edition" :)

Ahh, I see. Thanks! That makes more sense now.
 
> > 
> > 
> > Similarly I can see different approaches of grabbing the content. It is tempting to re-use the textures the texturemapper has, but I think the only reliable way of accessing the textures is through the render thread, i.e. from within. 
> > 
> > You could theoretically create a sharing gl context that shares resources with QQuickCanvas::openglContext(), but since you're using the entire texture mapper for rendering, wouldn't it be likely that you end up in a situation you are calling the texture mapper at a point in time when the render thread is _also_ calling the same instance? Is the texture mapper thread-safe on that level?
> > 
> We can (and should) find a way to block the render thread from painting while we're doing this.

I don't think that's possible right now. The functionality exists "within", meaning QQuickCanvas::grabFrameBuffer() takes care of telling the render thread to render the next frame into a QImage, then blocks the gui thread until that happens and then returns. Perhaps that's all we need for the pixel tests though, if we make sure that in WTR the WebView is the only element, there are no margins, etc. - I think that's actually the case. If that's doable then we don't need to change anything in Source/WebKit and only change WTR.
 
> > A completely different approach would be the one we took on the N9, which is sending a message to the web process for taking a snapshot with a content rect as parameter. The web process then renders the content in software into shared memory (the usual WK2 mechanism) and asynchronously returns a message to the uiprocess, indicating that it is done.
> 
> I thought about this too. That API exists, problem we had on the N9 was that that codepath flattens all the layers, which results in a different result than what you see in the viewport. 
> It's possible to fix it by enabling a software path for 3D transforms in WebCore, and would also be the right way to take "full page" snapshots.

Right, sounds like we want that, although not for the pixel tests.
 
> I think that maybe instead of providing a generic "snapshot API" we should create a C++-only viewport snapshot API specifically for pixel-tests, and then use the existing WebProcess-side API for other types of snapshots.

Indeed, those are two different use-cases and we don't need a public API for the pixel test get-me-that-viewport functionality.

[Viatcheslav Ostapenko]

Comment on attachment 147020 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147020&action=review

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:137
> +QImage QQuickWebPage::toImage() const

There is a problem with this API.
When page is not visible and layer tree is not up to date (or completely missing), we still want to be able to show snapshot of the page.

Also, rendering of objects created on another thread requires synchronization.

[Misha]

Comment on attachment 147020 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147020&action=review

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:168
> +    QImage image(rect.width(), rect.height(), QImage::Format_ARGB32_Premultiplied);

I think API should also provide the ability to set image format. 32 bits for thumbnail is not always necessary and 16 bit will work as well.

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1396
> +    bool ok = image.save(&buffer, format, quality);

This IO will block UI. Why do we do thumbnail on the UIProcess side and not on the WebProcess?

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.