[opengl] Another possible fix for the BadMatch error

[Leon Freitag]

Someone mentioned earlier that the code in wgl.c in DescribeDrawable() 
violates the GLX specification and therefore causes bogus return values for 
the visualid. However this code has been already present before the 
regression, but worked somehow. So the regression can't be caused by this 
violation.
I tried to correct this violation by calling glXChooseFBConfig and then 
glXGetChooseConfigAttrib, as suggested in the spec, and glXChooseFBConfig 
returns NULL for the appropriate FBCONFIG_ID.

However the problem seems to be in getting Drawable which is then passed to 
DescribeDrawable. The VisualID of the Drawable isn't needed at all, since 
before calling the glXMakeCurrent, we just want to know if the context 
visualid differs from visualid of Wine desktop. This one-liner should fix the 
BadMatch errors. Tested on Warcraft 3.

http://winehq.org/pipermail/wine-devel/2006-March/thread.html
------

diff --git a/dlls/opengl32/wgl.c b/dlls/opengl32/wgl.c
index c7d3147..e9046ca 100644
--- a/dlls/opengl32/wgl.c
+++ b/dlls/opengl32/wgl.c
@@ -551,7 +551,7 @@ BOOL WINAPI wglMakeCurrent(HDC hdc,
        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)) {
+       if (visualid != ctx_vis_id) {
          /**
           * Inherits from root window so reuse desktop visual
           */

Please test this patch and report if it works. 
If it works fine (and doesn't introduce any regressions) then I'll submit it 
to wine-patches.

Leon

[Willie Sippel]

Am Sonntag, 2. April 2006 16:44 schrieb Leon Freitag:
> Someone mentioned earlier that the code in wgl.c in DescribeDrawable()
> violates the GLX specification and therefore causes bogus return values for
> the visualid. However this code has been already present before the
> regression, but worked somehow. So the regression can't be caused by this
> violation.
> I tried to correct this violation by calling glXChooseFBConfig and then
> glXGetChooseConfigAttrib, as suggested in the spec, and glXChooseFBConfig
> returns NULL for the appropriate FBCONFIG_ID.
>
> However the problem seems to be in getting Drawable which is then passed to
> DescribeDrawable. The VisualID of the Drawable isn't needed at all, since
> before calling the glXMakeCurrent, we just want to know if the context
> visualid differs from visualid of Wine desktop. This one-liner should fix
> the BadMatch errors. Tested on Warcraft 3.
>
> http://winehq.org/pipermail/wine-devel/2006-March/thread.html
> ------
>
> diff --git a/dlls/opengl32/wgl.c b/dlls/opengl32/wgl.c
> index c7d3147..e9046ca 100644
> --- a/dlls/opengl32/wgl.c
> +++ b/dlls/opengl32/wgl.c
> @@ -551,7 +551,7 @@ BOOL WINAPI wglMakeCurrent(HDC hdc,
>         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)) {
> +       if (visualid != ctx_vis_id) {
>           /**
>            * Inherits from root window so reuse desktop visual
>            */
>
> Please test this patch and report if it works.
> If it works fine (and doesn't introduce any regressions) then I'll submit
> it to wine-patches.
>
> Leon

Doesn't seem to work, at least not for the applications I tried. Just tested 
Iconoclast by ASD [1] and Don't Stop by Portal Process [2], same results 
(BadMatch, X_GLXCreateGLXPixmap).

[1]: http://www.pouet.net/prod.php?which=18350
[2]: http://www.pouet.net/prod.php?which=18351
-- 
Willie Sippel

  ////////  |  Tritium Studios
 //         |  ______________________________
//// ///    |  http://www.tritium-studios.com

<willie(a)froq.net>

[Tomas Carnecky]

Willie Sippel wrote:
> Am Sonntag, 2. April 2006 16:44 schrieb Leon Freitag:
> > Someone mentioned earlier that the code in wgl.c in DescribeDrawable()
> > violates the GLX specification and therefore causes bogus return values for
> > the visualid. However this code has been already present before the
> > regression, but worked somehow. So the regression can't be caused by this
> > violation.
> > I tried to correct this violation by calling glXChooseFBConfig and then
> > glXGetChooseConfigAttrib, as suggested in the spec, and glXChooseFBConfig
> > returns NULL for the appropriate FBCONFIG_ID.

To describe what I've seen: because the window isn't created using
glXCreateWindow() it has no GLXFBConfig associated (at first), thus no
valid FBCONFIG_ID (and because of that you can't find out the
VISUAL_ID!), that seems reasonable because no GLX function has touched
the drawable before. But that's only up to the first 'activation' of the
drawable, eg. call to glXMakeCurrent(), in that function, my driver
(nvidia closed source) adds a valid GLXFBConfig to the drawable and
after that, describeDrawable() prints the correct IDs.
The variable 'draw_vis_id' should not be used in any checks, because it
may contain an incorrect VISUAL_ID.
> Doesn't seem to work, at least not for the applications I tried. Just tested 
> Iconoclast by ASD [1] and Don't Stop by Portal Process [2], same results 
> (BadMatch, X_GLXCreateGLXPixmap).

The BadMatch in X_GLXCreateGLXPixmap is another problem.. here you
should get a BadMatch in X_GLXMakeCurrent.

BadMatch in X_GLXCreateGLXPixmap is a known problem, I've submitted a
patch but it was rejected. Despite the recent x11drv rewrite (or was it
only the windowing code), the OpenGL handling seems very buggy :(

Maybe I should resend the describeDrawable() patch. Don't know why it
was rejected.

tom