Bug 106319 - [EFL] [WebGL] Remove any GLX dependencies from X11WindowResources.
Summary: [EFL] [WebGL] Remove any GLX dependencies from X11WindowResources.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Kalyan
URL:
Keywords:
Depends on: 105136
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-08 03:52 PST by Kalyan
Modified: 2013-01-09 19:44 PST (History)
6 users (show)

See Also:


Attachments
patch (2.67 KB, patch)
2013-01-08 19:35 PST, Kalyan
no flags Details | Formatted Diff | Diff
patch v2 (2.22 KB, patch)
2013-01-09 19:21 PST, Kalyan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kalyan 2013-01-08 03:52:37 PST
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).
Comment 1 Viatcheslav Ostapenko 2013-01-08 08:44:23 PST
(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.
Comment 2 Kalyan 2013-01-08 09:03:29 PST
(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.
Comment 3 Kalyan 2013-01-08 19:35:23 PST
Created attachment 181824 [details]
patch
Comment 4 Kalyan 2013-01-09 19:21:23 PST
Created attachment 182047 [details]
patch v2
Comment 5 Viatcheslav Ostapenko 2013-01-09 19:25:02 PST
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).
Comment 6 Kalyan 2013-01-09 19:29:04 PST
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.
Comment 7 Kalyan 2013-01-09 19:33:09 PST
(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.
Comment 8 WebKit Review Bot 2013-01-09 19:44:21 PST
Comment on attachment 182047 [details]
patch v2

Clearing flags on attachment: 182047

Committed r139276: <http://trac.webkit.org/changeset/139276>
Comment 9 WebKit Review Bot 2013-01-09 19:44:25 PST
All reviewed patches have been landed.  Closing bug.