Re: OpenGL child windows (SketchUp)

[Roderick Colenbrander]

Hi,

I have had a quick look. Although I'm not a big fan of the old OpenML
extension. It would be really nice if DRI drivers could be fixed to
get a proper glXWaitGL and friends. It still feels like the OpenML
extension does a little more than we really need. Since all DRI2
drivers seem to support it, we could just use it. I think your code
looks reasonable. In the fallback path we could use glXWaitGL instead
of glFinish, but glFinish is nearly the same anyway. Have you done any
benchmarks?

Single buffered applications are an issue, but from what I remember
Windows Vista and up have the same problem. There were some articles
on what happens on recent Windows versions and I think it was similar
to what we are doing with glFlush/glFinish. I'm not sure if we should
complicate the code too much for those usually old applications.

Roderick

On Thu, Feb 16, 2012 at 6:45 AM, Brian Bloniarz
 wrote:
> Below is a patch which fixes the visuals for me, it simply uses
> glXWaitForSbcOML, falling back to glFinish if that's not supported.
>
> Partway through coding this up, I was thinking that maybe listening
> for damage events would be more natural. One reason: the current
> code is probably imperfect for single-buffered contexts. The window will get
> redrawn any time glFlush or glFinish was called (flush_gl_drawable is
> called inside both), but really apps might expect that a single-buffered
> window get updated whenever rendering finishes. Damage is naturally
> asynchronous (which might avoid a performance hit). I also wonder whether
> vsync is easier to make work with that kind of flow (how do compositing
> window managers accomplish this? they've got the same problem).
>
> All the same, that might be a more invasive thing (i.e. can you do it
> asynchronously? hook into the event loop and intercept damage events).
>
> diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c
> index e8d0f89..c541044 100644
> --- a/dlls/winex11.drv/opengl.c
> +++ b/dlls/winex11.drv/opengl.c
> @@ -256,6 +256,9 @@ MAKE_FUNCPTR(glXGetCurrentReadDrawable)
>  static GLXContext (*pglXCreateContextAttribsARB)(Display *dpy, GLXFBConfig config, GLXContext share_context, Bool direct, const int *attrib_list);
>  static void* (*pglXGetProcAddressARB)(const GLubyte *);
>  static int   (*pglXSwapIntervalSGI)(int);
> +static int   (*pglXSwapIntervalSGI)(int);
> +static BOOL  (*pglXWaitForSbcOML)(Display *dpy, GLXDrawable drawable, int64_t target_sbc, int64_t *ust, int64_t *msc, int64_t *sbc);
> +static BOOL  (*pglXGetSyncValuesOML)(Display *dpy, GLXDrawable drawable, int64_t *ust, int64_t *msc, int64_t *sbc);
>
> /* ATI GLX Extensions */
>  static BOOL  (*pglXBindTexImageATI)(Display *dpy, GLXPbuffer pbuffer, int buffer);
> @@ -602,6 +605,11 @@ static BOOL has_opengl(void)
>         pglXCopySubBufferMESA = pglXGetProcAddressARB((const GLubyte *) "glXCopySubBufferMESA");
>     }
> - if(glxRequireExtension("GLX_OML_sync_control")) {
> - pglXWaitForSbcOML = pglXGetProcAddressARB((const GLubyte *) "glXWaitForSbcOML");
> - pglXGetSyncValuesOML = pglXGetProcAddressARB((const GLubyte *) "glXGetSyncValuesOML");
> - }
> -
>
> X11DRV_WineGL_LoadExtensions();
>
> wine_tsx11_unlock();
> @@ -2281,6 +2289,11 @@ static void WINAPI X11DRV_wglGetIntegerv(GLenum pname, GLint* params)
>     wine_tsx11_unlock();
>  }
>
> +/* For offscreen-rendered (child) windows, this copies the window
> -
> - content into to the onscreen window. The caller must have
> -
> - performed any OpenGL flushing to ensure the gl_drawable is
> -
> - up-to-date
> - */
>
> void flush_gl_drawable(X11DRV_PDEVICE *physDev)
>  {
>     int w, h;
> @@ -2295,10 +2308,7 @@ void flush_gl_drawable(X11DRV_PDEVICE *physDev)
>         Drawable src = physDev->pixmap;
>         if(!src) src = physDev->gl_drawable;
> - /* The GL drawable may be lagged behind if we don't flush first, so
> - * flush the display make sure we copy up-to-date data */
>
> wine_tsx11_lock();
> - XFlush(gdi_display);
>
> XSetFunction(gdi_display, physDev->gc, GXcopy);
>         XCopyArea(gdi_display, src, physDev->drawable, physDev->gc, 0, 0, w, h,
>                   physDev->dc_rect.left, physDev->dc_rect.top);
> @@ -3857,6 +3867,8 @@ BOOL X11DRV_SwapBuffers(PHYSDEV dev)
>   wine_tsx11_lock();
>   sync_context(ctx);
>   if(physDev->pixmap) {
> - /* Child window w/ pixmap rendering
> - * (composite extension unavailable) */
>
> if(pglXCopySubBufferMESA) {
>           int w = physDev->dc_rect.right - physDev->dc_rect.left;
>           int h = physDev->dc_rect.bottom - physDev->dc_rect.top;
> @@ -3867,14 +3879,33 @@ BOOL X11DRV_SwapBuffers(PHYSDEV dev)
>           pglFlush();
>           if(w > 0 && h > 0)
>               pglXCopySubBufferMESA(gdi_display, drawable, 0, 0, w, h);
> - } else {
> - pglXSwapBuffers(gdi_display, drawable);
>
> }
> - else
> - flush_gl_drawable(physDev);
> - } else if(physDev->gl_drawable) {
> - /* Composite-redirected child window. We must wait for the buffer swap
> - * since flush_gl_drawable will be copying out the window contents. */
> - if(pglXWaitForSbcOML) {
> - /* Wait for swap buffer with GLX_OML_sync_control extension,
> - * if it's supported. Note that DRI-based renderers like r600g
> - * support this extension and wait for swap on glFinish(). */
> - int64_t ust, msc, sbc;
> - pglXGetSyncValuesOML(gdi_display, drawable, &ust, &msc, &sbc);
>
> pglXSwapBuffers(gdi_display, drawable);
> - }
> - else
> - pglXWaitForSbcOML(gdi_display, drawable, sbc+1, &ust, &msc, &sbc);
> - } else {
> - /* No GLX_OML_swap_method support, just assume that glFinish() will
> - * wait for the swap. This works on nv & fglrx. */
> - pglXSwapBuffers(gdi_display, drawable);
> - pglFinish();
> - }
> - flush_gl_drawable(physDev);
> - } else {
> - /* normal fullscreen window */
>
> pglXSwapBuffers(gdi_display, drawable);
> - }
> - flush_gl_drawable(physDev);
>
> wine_tsx11_unlock();
>
> /* FPS support */