Bug 96490 - [Qt4] QOpenGLContext used in GraphicsContext3DQt.cpp (regression r126635)
Summary: [Qt4] QOpenGLContext used in GraphicsContext3DQt.cpp (regression r126635)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 420+
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: QtWebkit23
  Show dependency treegraph
 
Reported: 2012-09-12 03:28 PDT by Allan Sandfeld Jensen
Modified: 2012-09-14 06:03 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.00 KB, patch)
2012-09-14 05:46 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2012-09-12 03:28:21 PDT
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.
Comment 1 Allan Sandfeld Jensen 2012-09-12 03:50:21 PDT
The commit has now been merged into the qtwebkit-2.3-staging branch. I have disabled USE(3D_GRAPHICS) until this bug is closed.
Comment 2 Allan Sandfeld Jensen 2012-09-12 07:19:06 PDT
(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.
Comment 3 Simon Hausmann 2012-09-12 07:44:03 PDT
(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().
Comment 4 Allan Sandfeld Jensen 2012-09-14 05:17:28 PDT
(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.
Comment 5 Allan Sandfeld Jensen 2012-09-14 05:46:06 PDT
Created attachment 164121 [details]
Patch

Patch proposed for landing.
Comment 6 Allan Sandfeld Jensen 2012-09-14 05:47:15 PDT
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.
Comment 7 Allan Sandfeld Jensen 2012-09-14 06:03:20 PDT
Landed in qtwebkit-2.3-staging.