Bug 106319 – [EFL] [WebGL] Remove any GLX dependencies from X11WindowResources.

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Kalyan]

After https://bugs.webkit.org/show_bug.cgi?id=105136, we have GLX dependencies in X11WindowResources class. This is wrong and we should try to avoid any GL calls here( We could be using either GLX or EGL). 

we differ the setgeometry calls till we actually blit the texture contents to FBO in our platformLayer(GraphicsContext3DPrivate). We eventually call swapbuffers after the blit operation anyway. So, is the issue here that the buffers are not resized properly before the blit operation happens ?? (i.e calling glviewport with right attributes in GLPlatformSurface::setGeometry).

[Viatcheslav Ostapenko]

(In reply to comment #0)
> After https://bugs.webkit.org/show_bug.cgi?id=105136, we have GLX dependencies in X11WindowResources class. This is wrong and we should try to avoid any GL calls here( We could be using either GLX or EGL). 
> 
> we differ the setgeometry calls till we actually blit the texture contents to FBO in our platformLayer(GraphicsContext3DPrivate). We eventually call swapbuffers after the blit operation anyway. So, is the issue here that the buffers are not resized properly before the blit operation happens ?? (i.e calling glviewport with right attributes in GLPlatformSurface::setGeometry).

As I wrote in 105136, glViewport doesn't help.

[Kalyan]

(In reply to comment #1)
> (In reply to comment #0)
> > After https://bugs.webkit.org/show_bug.cgi?id=105136, we have GLX dependencies in X11WindowResources class. This is wrong and we should try to avoid any GL calls here( We could be using either GLX or EGL). 

> As I wrote in 105136, glViewport doesn't help.

K, I have a patch which does call swapbuffers in the platformLayer. This should handle all the cases i.e when surface is single buffered or double buffered and leave it to the surface to handle it as necessary.

I haven't checked the issue with EGL but I assume the issue should be present(if the buffers are not being resized automatically). While using EGL, we are currently dependent on native window/pixmap (X11 in this case) to share content with UI process.

[Kalyan]

Created attachment 181824 [details]
patch

[Kalyan]

Created attachment 182047 [details]
patch v2

[Viatcheslav Ostapenko]

Comment on attachment 182047 [details]
patch v2

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

> Source/WebCore/platform/graphics/surfaces/glx/X11WindowResources.cpp:53
> +    XFlush(m_sharedResources->x11Display());

For me XFlush didn't do anything good here. 
And glXSwapBuffers does it itself. At least mesa does (check mesa source at src/glx/glxcmds.c).

[Kalyan]

This patch handles only the GLX changes. EGL does seem to have the issue which needs to be handled separately. It would be good to have a layout test for this, to avoid breaking it.

[Kalyan]

(In reply to comment #5)
> (From update of attachment 182047 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182047&action=review
> 
> > Source/WebCore/platform/graphics/surfaces/glx/X11WindowResources.cpp:53
> > +    XFlush(m_sharedResources->x11Display());
> 
> For me XFlush didn't do anything good here. 
> And glXSwapBuffers does it itself. At least mesa does (check mesa source at src/glx/glxcmds.c).

That is true. With EGL we would need this though or use EGLWAITCLIENT for example. This just ensures that the buffers on X are updated appropriately. This doesn't involve a round trip to XServer like XSync does, so shouldn't be that costly and doesn't add a overhead on glx side.

[WebKit Review Bot]

Comment on attachment 182047 [details]
patch v2

Clearing flags on attachment: 182047

Committed r139276: <http://trac.webkit.org/changeset/139276>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.