Bug 35388 – [Qt] GraphicsLayer: support webGL

• Component: WebGL
• Status: RESOLVED FIXED

[Noam Rosenthal]

as QtWebkit now supports WebGL, AC (GraphicsLayerQt) needs to support it as well

[Noam Rosenthal]

Apparently this depends on using a shared-context inside Qt's WebGL; When that patch comes through this one would be easy.

[Tor Arne Vestbø]

Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should be used for bugs/features in the public QtWebKit API layer, not to signify that the bug is specific to the Qt port of WebKit

    http://trac.webkit.org/wiki/QtWebKitBugs#Component

[Noam Rosenthal]

This would work the best if GraphicsContext3DQt gave me an API along the lines of
GraphicsContext3DQt::paint(QPainter*, const QRectF &), and could figure out on its own whether to draw a GL texture or to flatten the FB to an image.

[Jarkko Sakkinen]

I run into same problem as Noamr: no a way to access internal GL context of Qt. However, I did already initial tryout patch on how get WebGL context atleast visible when AC is enabled (see http://bugreports.qt.nokia.com/browse/QTWEBKIT-91).

[Jarkko Sakkinen]

Completed simple patch for this. Submitting probably today after I have done style checking etc.

[Jarkko Sakkinen]

Created attachment 51742 [details]
Initial AC support for WebGL. 

Works with rasterizer graphics system. In order to get it work with opengl graphics system there should be a way to acquire the GL context used by Qt so that shared GL context could be created. However, this will not affect to the integration with AC (shared context is created inside GraphicsContext3D and paint method will have two branches depending on paint engine).

[Csaba Osztrogonác]

Attachment 51742 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/1388014

[Jarkko Sakkinen]

Created attachment 51744 [details]
Fixed compilation error.

[Csaba Osztrogonác]

Attachment 51744 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/1419001

[Jarkko Sakkinen]

Created attachment 51751 [details]
Compilation fix.

[Csaba Osztrogonác]

Attachment 51751 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/1429002

[Jarkko Sakkinen]

Created attachment 51765 [details]
Fixed caging of 3D_CANVAS code in GraphicsLayerQt.cpp

[Early Warning System Bot]

Attachment 51765 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/1626018

[Jarkko Sakkinen]

Created attachment 51826 [details]
Compilation fix

[Simon Hausmann]

Ouch, the entire FBO contents copied with glReadPixels into a QImage and then put back onto the screen without content caching with drawImage and setCacheMode(NoCache) ?

That sounds like it's going to be really slow, quite the opposite of what AC is trying to achieve.

One way would be to introduce a code path to blit the FBO directly if we're painting with OpenGL anyway, and only otherwise fall back.

[Jarkko Sakkinen]

Simon: see comments #1 and #6 and I couldn't agress less :) Here's the deal. How can I get pointer to OpenGL context of Qt paint engine in the *initialization* of GraphicsContext3D? This is needed in order to create a shared OpenGL context. Otherwise it is impossible to do this without readpixels. 

I submitted this patch at this point merely to pinpoint the issue here than suggest it to be integrated as such :)

[Jarkko Sakkinen]

FBO blit directly can only be done if we have shared contexes (then the texture is known to other context too), otherwise it is simply nondoable.

[Jarkko Sakkinen]

One way to make a workaround with this that I've been thinking: If we can assume that GraphicsContext3D gets GraphicsContext3D::paint call before any other OpenGL calls then I can postpone the context creation into first paint call (then I can trust that QGLContext::current() gives the context of the paint engine).

[Jarkko Sakkinen]

Another idea: create a dummy QPixmap and painter to that in constructor and call some paint call to that. Would this internally do the necessary context switch? Then I could just grab the context using QGLContext::currentContext(). I think I'll test these two approaches as soon as I have some extra time.

[Jarkko Sakkinen]

Did a test application:

#include <QtGui>
#include <QtOpenGL/QtOpenGL>

int main(int argc, char** argv)
{
    QApplication app(argc, argv);

    qDebug() << QGLContext::currentContext();
    QPixmap temp(10, 10);
    QPainter p(&temp);
    qDebug() << QGLContext::currentContext();

    return 0;
}

Output for rasterizer gs:

0x0 
0x0 

Output for opengl gs:

0x0 
0x88442f0 

So my second approach seems to work. I'm going to try that next in GraphicsContext3D.

[Simon Hausmann]

Hmm, it sounds like you need another parameter that's passed through during GraphicsContext3D creation then, telling you if the QWebPage's view is a GL widget or not.

If it is a GL widget, then you could create a QGLContext that is shared with the GL widget's one. Otherwise you simply create your own QGLContext. I don't see why it should be necessary to create a QGLWidget.

Then in beginPaint() you can either paint quickly thanks to context sharing, or we need to convert the texture into a pixmap and draw that.

I must be missing something :)

[Eric Seidel (no email)]

Comment on attachment 51826 [details]
Compilation fix

we don't name arguments that don't add clarity:
 656         void paint(QPainter* painter) const;

Why the double-spacing?
496 {
 497     m_internal->m_glWidget->makeCurrent();
 498 
 499     m_internal->bindFramebuffer(GraphicsContext3D::FRAMEBUFFER, m_internal->m_mainFbo);
 500 
 501     glReadPixels(/* x */ 0, /* y */ 0, m_currentWidth, m_currentHeight, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, m_internal->m_pixels.bits());
 502     painter->drawImage(/* x */ 0, /* y */ 0, m_internal->m_pixels.transformed(QMatrix().rotate(180)));
 503 
 504     m_internal->bindFramebuffer(GraphicsContext3D::FRAMEBUFFER, m_internal->m_currentFbo);
 505 }

This should need { } to have local variables, no?
   case Canvas3DContentType:
 417         m_gc3D->paint(painter);
 418         GraphicsContext gc(painter);
 419         m_layer->paintGraphicsLayerContents(gc, option->exposedRect.toAlignedRect());
 420         break;
 421 #endif

Why the extra new line?
             break;
 565 
 566 #endif


Double spacing again?

 void GraphicsLayerQt::setContentsToGraphicsContext3D(const GraphicsContext3D* ctx)
 922 {
 923     if (ctx == m_impl->m_gc3D)
 924         return;
 925 
 926     m_impl->m_pendingContent.contentType = GraphicsLayerQtImpl::Canvas3DContentType;
 927 
 928     m_impl->m_gc3D = ctx;
 929 
 930     m_impl->notifyChange(GraphicsLayerQtImpl::ContentChange);
 931 }
 932 

Besides the nits this change looks good.  r- for the nits.

[Jarkko Sakkinen]

Simon Hausmann: no you're not and QGLWidget is easy and that could be easily done in initialization. But I'm trying to do a solution that works both with QGLWidget and the case where the grahicssystem is opengl (graphicssystem is opengl and webview is a qwidget).

I'm going to finish this patch (I have some ideas how to solve that last pending issue) after this is accepted: https://bugs.webkit.org/show_bug.cgi?id=37026

[Jarkko Sakkinen]

Sorry for the typo. I ment "no you're not missing anything" :)

[Jarkko Sakkinen]

Created separate bug for shared context: https://bugs.webkit.org/show_bug.cgi?id=37070.

[Jarkko Sakkinen]

Got my build fix for Windows into upstream. I've been travelling alot during last three weeks and have only my Windows laptop with me. That's why this hasn't progressed. Now I'm able to continue with this work.

[Jarkko Sakkinen]

Now support for shared context with QGLWidget viewport is in upstream so it is possible to continue with this patch.

[Jarkko Sakkinen]

Created attachment 54874 [details]
drawTexture()

[Jarkko Sakkinen]

Created attachment 54875 [details]
changelog fix

[Jarkko Sakkinen]

Created attachment 54876 [details]
3D CSS transformation and WebGL canvas.

[Kenneth Rohde Christiansen]

So this doesn't support the case where the graphicssystem is opengl, but only when the graphics view is basked by the QGLWidget?

Do we need any new patches in QtLauncher to make it possible to actually test this?

[Jarkko Sakkinen]

Yes, only QGLWidget viewport is supported because there is no standard way to create shared context with graphicssystem OpenGL context. No changes needed for QtLauncher for now.

[Jarkko Sakkinen]

NOTE: from AC point of view things won't change if there comes a solution to do shared context with graphicssystem. It will just call paint() as it does now.

[Jarkko Sakkinen]

NOTE2: it requires AC to be enabled in order to use drawTexture(), otherwise it fallbacks to readPixels. Also, Apple implementation supports complete GPU acceleration only if AC is enabled.

[Noam Rosenthal]

Comment on attachment 54875 [details]
changelog fix

> +    QWebPageClient* webPageClient = m_internal->m_hostWindow->platformPageClient();
> +    QGLWidget* ownerGLWidget  = m_internal->getOwnerGLWidget(webPageClient);

Shouldn't we just call QGLWidget* ownerGLWidget = qobject_cast<QGLWidget*>(painter->paintDevice()); ?

[Noam Rosenthal]

I meant painter->device(), of course.

[Noam Rosenthal]

Disregard my last two comments - it's not possible :)

[WebKit Commit Bot]

Comment on attachment 54875 [details]
changelog fix

Clearing flags on attachment: 54875

Committed r58678: <http://trac.webkit.org/changeset/58678>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.