Bug 96490 – [Qt4] QOpenGLContext used in GraphicsContext3DQt.cpp (regression r126635)

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Allan Sandfeld Jensen]

The patch for bug #78672 introduced the GraphicsContext3D::RenderToCurrentGLContext render-style, but added Qt5 specific code to handle it. It should be too difficult to express that in Qt4.8 terms, but I will leave it to someone who knows what they are doing.

[Allan Sandfeld Jensen]

The commit has now been merged into the qtwebkit-2.3-staging branch. I have disabled USE(3D_GRAPHICS) until this bug is closed.

[Allan Sandfeld Jensen]

(In reply to comment #0)
> The patch for bug #78672 introduced the GraphicsContext3D::RenderToCurrentGLContext render-style, but added Qt5 specific code to handle it. It should be too difficult to express that in Qt4.8 terms, but I will leave it to someone who knows what they are doing.

Make that should not be too difficult. The code in question is this:
    if (renderStyle == GraphicsContext3D::RenderToCurrentGLContext) {
        m_platformContext = QOpenGLContext::currentContext();
        m_surface = m_platformContext->surface();
        return;
    }

I was thinking it could be solved for Qt4 as
        m_platformContext = const_cast<QGLContext*>(QGLContext::currentContext());
        m_surface = dynamic_cast<QGLWidget*>(m_platformContext->device());

But without knowing the details, I would rather not do const casts and rely on assumptions of what type m_platformContext->device() is.

[Simon Hausmann]

(In reply to comment #2)
> (In reply to comment #0)
> > The patch for bug #78672 introduced the GraphicsContext3D::RenderToCurrentGLContext render-style, but added Qt5 specific code to handle it. It should be too difficult to express that in Qt4.8 terms, but I will leave it to someone who knows what they are doing.
> 
> Make that should not be too difficult. The code in question is this:
>     if (renderStyle == GraphicsContext3D::RenderToCurrentGLContext) {
>         m_platformContext = QOpenGLContext::currentContext();
>         m_surface = m_platformContext->surface();
>         return;
>     }
> 
> I was thinking it could be solved for Qt4 as
>         m_platformContext = const_cast<QGLContext*>(QGLContext::currentContext());
>         m_surface = dynamic_cast<QGLWidget*>(m_platformContext->device());
> 
> But without knowing the details, I would rather not do const casts and rely on assumptions of what type m_platformContext->device() is.

I'd go for that dynamic_cast. You can ASSERT() if you want to be on the safer side, but I think chances are extremely small.

Another option would be to leave it as a QPaintDevice* altogether. For deletion you're going to get a QObject* m_surfaceOwner from trunk soon (but even then QPaintDevice's virtual destructor should do the trick). And otherwise I think m_surface is only used for the *IfNeeded() part of makeCurrentIfNeeded().

[Allan Sandfeld Jensen]

(In reply to comment #3)
> Another option would be to leave it as a QPaintDevice* altogether. For deletion you're going to get a QObject* m_surfaceOwner from trunk soon (but even then QPaintDevice's virtual destructor should do the trick). And otherwise I think m_surface is only used for the *IfNeeded() part of makeCurrentIfNeeded().

I did try to redefine the surface to QPaintDevice, but there was a handfull of places where QGLWidget specific functions were used.

[Allan Sandfeld Jensen]

Created attachment 164121 [details]
Patch

Patch proposed for landing.

[Allan Sandfeld Jensen]

Should this be applied to Trunk also? We still have a bunch of HAVE(QT5) hooks, and as long as it is small stuff we could try to keep the diff between the branches small.

[Allan Sandfeld Jensen]

Landed in qtwebkit-2.3-staging.