Bug on WGL Display

[VirtuallTek]

trafficstars

There's a bug in wgl_disp.c where wglGetProcAddress gets called without a valid current OpenGL context, in wgl_create_display function. This is highly noticeable if you try to create a display for some desired OpenGL version (al_set_new_display_option with ALLEGRO_OPENGL_MAJOR_VERSION / ALLEGRO_OPENGL_MINOR_VERSION).

Fixed that by changing:

_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB
= (_ALLEGRO_wglGetExtensionsStringARB_t)wglGetProcAddress("wglGetExtensionsStringARB");

from being an argument to a local variable inside select_pixel_format and init_ogl_context_ex. select_pixel_format needed a temporary context creation.

Fixed code: wgl_disp.zip

[pedro-w]

I can't test, but the principle seems sound, good work! In the diff, are the changes to is_wgl_extension_supported and also the line with vt.set_wrap_mode part of the same issue or are they unrelated improvements?

diff --git a/src/win/wgl_disp.c b/src/win/wgl_disp.c
index 3331b5f46..490b04dfa 100644
--- a/src/win/wgl_disp.c
+++ b/src/win/wgl_disp.c
@@ -68,20 +68,16 @@ typedef struct WGL_DISPLAY_PARAMETERS {
 
 static bool is_wgl_extension_supported(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, const char *extension, HDC dc)
 {
-   bool ret = false;
-   const GLubyte* extensions;
+   int ret;
 
-   if (_wglGetExtensionsStringARB) {
-      extensions = (const GLubyte*)_wglGetExtensionsStringARB(dc);
-   }
-   else {
-      /* XXX deprecated in OpenGL 3.0 */
-      extensions = glGetString(GL_EXTENSIONS);
-   }
+   /* XXX deprecated in OpenGL 3.0 */
+   if (!glGetString(GL_EXTENSIONS))
+      return false;
+   if (!_wglGetExtensionsStringARB)
+      return false;
 
-   if (extensions) {
-      ret = _al_ogl_look_for_an_extension(extension, extensions);
-   }
+   ret = _al_ogl_look_for_an_extension(extension,
+         (const GLubyte*)_wglGetExtensionsStringARB(dc));
 
    return ret;
 }
@@ -647,8 +643,7 @@ static HGLRC init_ogl_context_ex(HDC dc, bool fc, int major, int minor)
       goto bail;
 
    _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB
-      = (_ALLEGRO_wglGetExtensionsStringARB_t)wglGetProcAddress("wglGetExtensionsStringARB");
-   ALLEGRO_INFO("_wglGetExtensionsStringARB %p\n", _wglGetExtensionsStringARB);
+	   = (_ALLEGRO_wglGetExtensionsStringARB_t)wglGetProcAddress("wglGetExtensionsStringARB");
 
    if (is_wgl_extension_supported(_wglGetExtensionsStringARB, "WGL_ARB_create_context", testdc)) {
       int attrib[] = {WGL_CONTEXT_MAJOR_VERSION_ARB, major,
@@ -684,7 +679,7 @@ bail:
 }
 
 
-static ALLEGRO_EXTRA_DISPLAY_SETTINGS** get_available_pixel_formats_ext(int *count)
+static ALLEGRO_EXTRA_DISPLAY_SETTINGS** get_available_pixel_formats_ext(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, int *count)
 {
    HWND testwnd = NULL;
    HDC testdc   = NULL;
@@ -714,15 +709,11 @@ static ALLEGRO_EXTRA_DISPLAY_SETTINGS** get_available_pixel_formats_ext(int *cou
    if (!testrc)
       goto bail;
 
-   _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB
-      = (_ALLEGRO_wglGetExtensionsStringARB_t)wglGetProcAddress("wglGetExtensionsStringARB");
-   ALLEGRO_INFO("_wglGetExtensionsStringARB %p\n", _wglGetExtensionsStringARB);
-
    if (!is_wgl_extension_supported(_wglGetExtensionsStringARB, "WGL_ARB_pixel_format", testdc) &&
        !is_wgl_extension_supported(_wglGetExtensionsStringARB, "WGL_EXT_pixel_format", testdc)) {
       ALLEGRO_ERROR("WGL_ARB/EXT_pf not supported.\n");
       goto bail;
-   }
+  }
 
    if (!init_pixel_format_extensions())
       goto bail;
@@ -848,7 +839,26 @@ static bool select_pixel_format(ALLEGRO_DISPLAY_WGL *d, HDC dc)
    }
 
    if (!force_old)
-      eds = get_available_pixel_formats_ext(&eds_count);
+   {
+	   HDC oldDC = wglGetCurrentDC();
+	   HGLRC oldCtx = wglGetCurrentContext();
+
+	   HGLRC ctx = wglCreateContext(dc);
+	   if (ctx)
+	   {
+		   if (wglMakeCurrent(dc, ctx))
+		   {
+			   _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB
+				   = (_ALLEGRO_wglGetExtensionsStringARB_t)wglGetProcAddress("wglGetExtensionsStringARB");
+
+			   eds = get_available_pixel_formats_ext(_wglGetExtensionsStringARB, &eds_count);
+
+			   wglMakeCurrent(NULL, NULL);
+			   wglMakeCurrent(oldDC, oldCtx);
+		   }
+		   wglDeleteContext(ctx);
+	   }
+   }
    if (!eds)
       eds = get_available_pixel_formats_old(&eds_count, dc);
 
@@ -1568,6 +1578,8 @@ ALLEGRO_DISPLAY_INTERFACE *_al_display_wgl_driver(void)
    vt.set_display_flag = _al_win_set_display_flag;
    vt.set_window_title = _al_win_set_window_title;
 
+   vt.set_wrap_mode = _al_ogl_set_wrap_mode;
+
    vt.update_render_state = _al_ogl_update_render_state;
    _al_ogl_add_drawing_functions(&vt);
    _al_win_add_clipboard_functions(&vt);

[VirtuallTek]

Oh, sorry. The line "vt.set_wrap_mode" is a change I've made to add support for setting the sampler texture addressing mode. It's not part of the issue, but the other one is.

[VirtuallTek]

The same issue happens in select_pixel_format as well. Seems totally fixed now wgl_disp.zip.

[EdgarReynaldo]

Could you fork A5 and apply your patch to a clean checkout of Allegro? Then make a pull request. It is better to use a pull request so the change is properly tracked.

[EdgarReynaldo]

Does anyone know if this has been applied yet? If not I will fork myself and make a pull request.

[SiegeLord]

It hasn't. I think because I couldn't reproduce the original issue.