Bug 93252 – Make GraphicsSurface double buffered by default.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Zeno Albisser]

The current GraphicsSurface implementation is mainly used for WebGL.
To achieve a reasonable performance, WebGL requires at least two buffers. Therefore we currently create two GraphicsSurfaces on mac so we can switch between them.
On Linux nevertheless we only create a single GraphicsSurface, because the GraphicsSurfaceGLX is backed by an XWindow which already has a front and a back buffer.

We should unify this behaviour and only create a single GraphicsSurface which always provides two buffers on the writing side.
For mac this would mean that a GraphicsSurface will be backed by two IOSurfaces that will be used alternated.

[Zeno Albisser]

Created attachment 156673 [details]
patch for feedback.

[Noam Rosenthal]

Comment on attachment 156673 [details]
patch for feedback.

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

Make sure this doesn't break WebGL on WebKit1.

> Source/WebCore/ChangeLog:9
> +        two provide a front and a back buffer.

two -> to

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:267
> -    const QOpenGLContext* currentContext = QOpenGLContext::currentContext();
> -    QSurface* currentSurface = 0;
> -    if (currentContext != m_platformContext) {
> -        currentSurface = currentContext->surface();
> +    if (QOpenGLContext::currentContext() != m_platformContext)
>          m_platformContext->makeCurrent(m_surface);
> -    }
> +
>      blitMultisampleFramebuffer();
> -    if (currentSurface)
> -        const_cast<QOpenGLContext*>(currentContext)->makeCurrent(currentSurface);
> +    m_platformContext->makeCurrent(m_surface);

This will break WebGL on WebKit1.

[Zeno Albisser]

Comment on attachment 156673 [details]
patch for feedback.

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

>> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:267
>> +    m_platformContext->makeCurrent(m_surface);
> 
> This will break WebGL on WebKit1.

Good point! I'll look at this again.

Do you agree with the general direction of the patch?

[Noam Rosenthal]

(In reply to comment #3)
> (From update of attachment 156673 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156673&action=review
> 
> >> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:267
> >> +    m_platformContext->makeCurrent(m_surface);
> > 
> > This will break WebGL on WebKit1.
> 
> Good point! I'll look at this again.
> 
> Do you agree with the general direction of the patch?

Definitely! Good work.

[Zeno Albisser]

Comment on attachment 156673 [details]
patch for feedback.

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

>>>> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:267
>>>> +    m_platformContext->makeCurrent(m_surface);
>>> 
>>> This will break WebGL on WebKit1.
>> 
>> Good point! I'll look at this again.
>> 
>> Do you agree with the general direction of the patch?
> 
> Definitely! Good work.

I was wrong about this hunk. It is actually wrong and also unnecessary. - I will remove it.

[Zeno Albisser]

Created attachment 157177 [details]
patch for review.

[Zeno Albisser]

Comment on attachment 157177 [details]
patch for review.

Patch needs to be rebased first.

[Zeno Albisser]

Created attachment 157189 [details]
patch for review.

[Early Warning System Bot]

Comment on attachment 157189 [details]
patch for review.

Attachment 157189 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13454490

[Early Warning System Bot]

Comment on attachment 157189 [details]
patch for review.

Attachment 157189 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13456479

[Zeno Albisser]

Created attachment 157213 [details]
patch for review.

[Kenneth Russell]

Comment on attachment 157213 [details]
patch for review.

Interesting. This looks good to me for what it's worth, but Noam should review it.

[Zeno Albisser]

Comment on attachment 157213 [details]
patch for review.

Clearing flags on attachment: 157213

Committed r125773: <http://trac.webkit.org/changeset/125773>

[Zeno Albisser]

All reviewed patches have been landed.  Closing bug.