Chris Robinson wrote:
This greatly simplifies some of the wgl code by keeping around a single customized fbconfig array, instead of holding a static, custom-typedef array which left many functions to query glx for the fbconfig list anyway (which in turn caused many needless, potentially problematic allocations and deallocations of memory).
This doesn't change the behavior of the code, but it makes it simpler for future enhancements (such as supporting extra fbconfigs for on-screen or off-screen use), and easier maintanence by removing a lot of duplicate code.
Please don't send compressed patches. It's not that huge to gzip it.
WineGLFBConfigsListSize = 1;WineGLDisplayablePixelFormatListSize = 1;
If you hard-coding the list size to 1 it's not a list anymore, but a single element.
@@ -670,7 +718,7 @@ static int ConvertAttribWGLtoGLX(const i switch (pop) { case WGL_TYPE_COLORINDEX_ARB: pop = GLX_COLOR_INDEX_BIT; isColor = 1; break ; case WGL_TYPE_RGBA_ARB: pop = GLX_RGBA_BIT; break ;
case WGL_TYPE_RGBA_FLOAT_ATI: pop = GLX_RGBA_FLOAT_ATI_BIT; break ;
case WGL_TYPE_RGBA_FLOAT_ARB: pop = GLX_RGBA_FLOAT_BIT; break ; default: ERR("unexpected PixelType(%x)\n", pop); pop = 0;
This code seems like unrelated to the description. Please send it as a separate patch.
@@ -681,12 +729,7 @@ static int ConvertAttribWGLtoGLX(const i
case WGL_SUPPORT_GDI_ARB: pop = iWGLAttr[++cur];
/* We only support a limited number of formats which are all renderable by X (similar to GDI).* Ignore this attribute to prevent us from not finding a match due to the limited* amount of formats supported right now. This option could be matched to GLX_X_RENDERABLE* but the issue is that when a program asks for no GDI support, there's no format we can return* as all our supported formats are renderable by X.*/
PUSH2(oGLXAttr, GLX_X_RENDERABLE, pop); TRACE("pAttr[%d] = WGL_SUPPORT_GDI_ARB: %d\n", cur, pop); break;
Your description said nothing about altering this behavior stated in comments. Same for the PUSH, and lots of other changes. It seems to me that you have at least 3-5 separate patches here. Please split them and send them separately.
@@ -1037,13 +987,13 @@ int X11DRV_ChoosePixelFormat(X11DRV_PDEV /* When we pass all the checks we have found a matching format :) */ ret = 1; TRACE("Successfully found a matching mode, returning index: %d\n", ret); +choose_exit:
- ; }
-choose_exit: if(!ret) TRACE("No matching mode was found returning 0\n");
Why do you need this change?
@@ -1519,11 +1458,13 @@ BOOL X11DRV_wglMakeCurrent(X11DRV_PDEVIC * We are certain that the drawable and context are compatible as we only allow compatible formats. */ TRACE(" Creating GLX Context\n");
ctx->ctx = pglXCreateContext(gdi_display, ctx->vis, NULL, type == OBJ_MEMDC ? False : True);
create_glx_context(physDev, ctx, NULL); TRACE(" created a delayed OpenGL context (%p)\n", ctx->ctx); } TRACE(" make current for dis %p, drawable %p, ctx %p\n", gdi_display, (void*) drawable, ctx->ctx);X11DRV_expect_error(gdi_display, error_catcher, NULL); ret = pglXMakeCurrent(gdi_display, drawable, ctx->ctx);X11DRV_expect_error(gdi_display, NULL, NULL); NtCurrentTeb()->glContext = ctx; if(ret) {
Can you explain this one a bit? Your error_catcher() does nothing, except ignoring the error. Yet here you assuming that calls succeeded and continue on, assigning the context to a thread which won't have the context assigned.
Vitaliy.
-------- Original-Nachricht --------
TRACE(" make current for dis %p, drawable %p, ctx %p\n",gdi_display, (void*) drawable, ctx->ctx);
X11DRV_expect_error(gdi_display, error_catcher, NULL); ret = pglXMakeCurrent(gdi_display, drawable, ctx->ctx);X11DRV_expect_error(gdi_display, NULL, NULL); NtCurrentTeb()->glContext = ctx; if(ret) {Can you explain this one a bit? Your error_catcher() does nothing, except ignoring the error. Yet here you assuming that calls succeeded and continue on, assigning the context to a thread which won't have the context assigned.
Vitaliy.
Please don't try to catch any error like a BadMatch which can appear there. It is easier if it really appears there to warn us instead of failing just to the windows app.
Roderick
On Monday 01 January 2007 00:47, Vitaliy Margolen wrote:
Please don't send compressed patches. It's not that huge to gzip it.
Sorry.
WineGLFBConfigsListSize = 1;WineGLDisplayablePixelFormatListSize = 1;If you hard-coding the list size to 1 it's not a list anymore, but a single element.
Originally, the list contained the full FBConfig list and dealt with it accordingly. But Thunderbird/Roderick said he'd prefer if the patch didn't do that and just retained the single main visual for now. Unless I misunderstood him..
@@ -1037,13 +987,13 @@ int X11DRV_ChoosePixelFormat(X11DRV_PDEV /* When we pass all the checks we have found a matching format :) */ ret = 1; TRACE("Successfully found a matching mode, returning index: %d\n", ret); +choose_exit:
- ; }
-choose_exit: if(!ret) TRACE("No matching mode was found returning 0\n");
Why do you need this change?
Originally that section was turned into a loop to deal with the list, and the choose_exit would be used to skip to the next iteration. But Roderick said he didn't want that turned into a loop yet so I removed the for() bit, and that got left.
Can you explain this one a bit? Your error_catcher() does nothing, except ignoring the error. Yet here you assuming that calls succeeded and continue on, assigning the context to a thread which won't have the context assigned.
According to the docs, "glXMakeCurrent returns True if it is successful, False otherwise."
And since the function returns what glXMakeCurrent returns, the call is not assumed to succeed. Without it, glXMakeCurrent may cause the program to abort before the rest of the program knows it failed. But I suppose that too should be sent seperately..
Apologies. I'll split up the patch and send the relavant parts.
-
Chris Robinson -
Roderick Colenbrander -
Vitaliy Margolen