Tomas Carnecky wrote: > Changelog: > - fail if the drawable and context Visual IDs don't match. > > why? because this will produce a BadMatch and crash the application so > instead of the crash rather return GL_FALSE and let the application > handle it.
Please don't apply this patch yet. It fixes one case but there is more to be aware of. This BadMatch bug is triggered if an application creates a window (with a default Visual ID) and a custom (non-default) pixel format, the IDs were 0x21 and 0x23 in my case (glxinfo/xdpyinfo report Visual ID in the range from 0x21 to 0x70). But! I've seen that World of Warcraft calls wglMakeCurrent() with a darawable that has Visual ID 0x71 and a context with Visual ID 0x21. Now 0x71 is not defined (according to the glxino output) but it works fine, eg. glXMakeCurrent() doesn't produce the X Error in that case. So I went up the backtrace, into create_glxpixmap(). I let this function fail if it would produce a X Error, eg. if "depth of pixmap does not match the GLX_BUFFER_SIZE value of vis". Then I had to modify wglMakeCurrent() to respect the create_glxpixmap() failure and return FALSE. Works good so far, framerate in WoW went up from ~20 to up to 70 fps, average is somewhere between 40-50 !!! This is incredible. So.. in this attachment you'll find a patch that does what I've just described. I can't test it on anything else than WoW, so if someone would please review it and test with outher opengl/d3d applications it would be great. Also, it would be great if we could put the *Swap*Buffers() into their own log domain, something like 'swapbuffers', because the trace is usually useless, only when you explicitly look whether these functions are called or not, otherwise they only fill the log with garbage. thanks tom
diff --git a/dlls/opengl32/wgl.c b/dlls/opengl32/wgl.c index 71f7511..0df0e46 100644 --- a/dlls/opengl32/wgl.c +++ b/dlls/opengl32/wgl.c @@ -543,13 +543,15 @@ BOOL WINAPI wglMakeCurrent(HDC hdc, } else { Wine_GLContext *ctx = (Wine_GLContext *) hglrc; Drawable drawable = get_drawable( hdc ); + if (!drawable) { + TRACE(" couldn't get the drawable\n"); + return FALSE; + } + int draw_vis_id = describeDrawable(ctx, drawable); + int ctx_vis_id = describeContext(ctx); if (ctx->ctx == NULL) { - int draw_vis_id, ctx_vis_id; VisualID visualid = (VisualID)GetPropA( GetDesktopWindow(), "__wine_x11_visual_id" ); TRACE(" Wine desktop VISUAL_ID is 0x%x\n", (unsigned int) visualid); - draw_vis_id = describeDrawable(ctx, drawable); - ctx_vis_id = describeContext(ctx); - if (-1 == draw_vis_id || (draw_vis_id == visualid && draw_vis_id != ctx_vis_id)) { /** * Inherits from root window so reuse desktop visual @@ -560,14 +562,20 @@ BOOL WINAPI wglMakeCurrent(HDC hdc, template.visualid = visualid; vis = XGetVisualInfo(ctx->display, VisualIDMask, &template, &num); - TRACE(" Creating GLX Context\n"); + TRACE(" Creating GLX Context with VisualID: %x\n", (int)vis->visualid); ctx->ctx = glXCreateContext(ctx->display, vis, NULL, type == OBJ_MEMDC ? False : True); } else { - TRACE(" Creating GLX Context\n"); + TRACE(" Creating GLX Context with VisualID: %x\n", (int)ctx->vis->visualid); ctx->ctx = glXCreateContext(ctx->display, ctx->vis, NULL, type == OBJ_MEMDC ? False : True); } TRACE(" created a delayed OpenGL context (%p)\n", ctx->ctx); } + if (draw_vis_id != ctx_vis_id) { + TRACE(" VisualIDs for dpy: %p, drawable: %p, ctx: %p don't match: %x - %x\n", ctx->display, (void*) drawable, ctx->ctx, draw_vis_id, ctx_vis_id); + // LEAVE_GL(); + // return GL_FALSE; + } + TRACE(" make current for dis %p, drawable %p, ctx %p\n", ctx->display, (void*) drawable, ctx->ctx); ret = glXMakeCurrent(ctx->display, drawable, ctx->ctx); NtCurrentTeb()->glContext = ctx; @@ -1212,6 +1220,7 @@ static void wgl_initialize_glx(Display * /*wine_glx.p_glXGetFBConfigs = proc( (const GLubyte *) "glXGetFBConfigs");*/ wine_glx.p_glXQueryDrawable = proc( (const GLubyte *) "glXQueryDrawable"); + wine_glx.p_glXQueryContext = proc( (const GLubyte *) "glXQueryContext"); } else { if (NULL != strstr(server_glx_extensions, "GLX_SGIX_fbconfig")) { wine_glx.p_glXChooseFBConfig = proc( (const GLubyte *) "glXChooseFBConfigSGIX"); diff --git a/dlls/opengl32/wgl_ext.h b/dlls/opengl32/wgl_ext.h index 0841aa0..672ace9 100644 --- a/dlls/opengl32/wgl_ext.h +++ b/dlls/opengl32/wgl_ext.h @@ -48,6 +48,7 @@ typedef struct wine_glx_s { /** 1.3 */ GLXFBConfig* (*p_glXGetFBConfigs) (Display *dpy, int screen, int *nelements); void (*p_glXQueryDrawable) (Display *dpy, GLXDrawable draw, int attribute, unsigned int *value); + void (*p_glXQueryContext) (Display *dpy, GLXContext ctx, int attribute, int *value); Bool (*p_glXMakeContextCurrent) (Display *, GLXDrawable, GLXDrawable, GLXContext); } wine_glx_t; extern wine_glx_t wine_glx; diff --git a/dlls/x11drv/init.c b/dlls/x11drv/init.c index 9774c6f..25548e8 100644 --- a/dlls/x11drv/init.c +++ b/dlls/x11drv/init.c @@ -414,6 +414,11 @@ INT X11DRV_ExtEscape( X11DRV_PDEVICE *ph { if(!physDev->bitmap->glxpixmap) physDev->bitmap->glxpixmap = create_glxpixmap(physDev); + + if (!physDev->bitmap->glxpixmap) { + TRACE(" create_glxpixmap() failed\n"); + return FALSE; + } *(Drawable *)out_data = physDev->bitmap->glxpixmap; } else diff --git a/dlls/x11drv/opengl.c b/dlls/x11drv/opengl.c index 3fc0231..8b85c09 100644 --- a/dlls/x11drv/opengl.c +++ b/dlls/x11drv/opengl.c @@ -579,7 +579,7 @@ XVisualInfo *X11DRV_setup_opengl_visual( XID create_glxpixmap(X11DRV_PDEVICE *physDev) { - GLXPixmap ret; + GLXPixmap ret = 0; XVisualInfo *vis; XVisualInfo template; int num; @@ -591,7 +591,13 @@ XID create_glxpixmap(X11DRV_PDEVICE *phy vis = XGetVisualInfo(gdi_display, VisualIDMask, &template, &num); - ret = pglXCreateGLXPixmap(gdi_display, vis, physDev->bitmap->pixmap); + int visual_buffer_size; + pglXGetConfig(gdi_display, vis, GLX_BUFFER_SIZE, &visual_buffer_size); + if (visual_buffer_size != physDev->bitmap->pixmap_depth) { + TRACE(" depth of pixmap does not match the GLX_BUFFER_SIZE value of vis\n"); + } else { + ret = pglXCreateGLXPixmap(gdi_display, vis, physDev->bitmap->pixmap); + } XFree(vis); XFree(cfgs); wine_tsx11_unlock();