slow detection of GL extensions on some hardware http://www.allegro.cc/forums/view/618076 Allegro.cc Forum Thread matthew@allegro.cc (Matthew Leverton) Wed, 08 Apr 2020 01:05:36 +0000

Hi, recently I switched my setup to use a 4k monitor next to the laptop, desktop is extended to two screens. You know, home office and such things.

I found that OpenGL usage becomes really slow during startup, so I did some debugging. The slow function is 

is_wgl_extension_supported()
{
   // some code
   _wglGetExtensionsStringARB = (_ALLEGRO_wglGetExtensionsStringARB_t)
      wglGetProcAddress("wglGetExtensionsStringARB");
   // more code
}

and some google search revealed, that it seems to be a known problem for some graphics cards and some drivers, but unknown reasons, that the detection of gl extensions is really slow. Nonetheless, somebody hinted to not call this in a loop anyways, but that's what allegro is doing in 

read_pixel_format_ext

called by 

get_available_pixel_formats_ext()
{
   // some code
   for (j = i = 0; i < maxindex; i++) {
      ALLEGRO_INFO("-- \n");
      ALLEGRO_INFO("Decoding visual no. %i...\n", i+1);
      eds_list[j] = read_pixel_format_ext(i, testdc);
      // more code
   }
   // more code
}

so I changed is_wgl_extension_supported to get the proc address only once and reuse it:

#SelectExpand
1static bool is_wgl_extension_supported(const char *extension, HDC dc) 2{ 3 static _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB; 4 static ext_is_initialized = false; // this is a simple blocker, would not be safe for multi-threading 5 int ret; 6 7 /* XXX deprecated in OpenGL 3.0 */ 8 if (!glGetString(GL_EXTENSIONS)) 9 return false; 10 11 if(!ext_is_initialized) 12 { 13 _wglGetExtensionsStringARB = (_ALLEGRO_wglGetExtensionsStringARB_t) 14 wglGetProcAddress("wglGetExtensionsStringARB"); 15 ext_is_initialized = true; 16 } 17 if (!_wglGetExtensionsStringARB) 18 return false; 19 20 ret = _al_ogl_look_for_an_extension(extension, 21 (const GLubyte*)_wglGetExtensionsStringARB(dc)); 22 23 return ret; 24}

but I'm not sure, iff that would work for all compilers, because it's not old-style-enough, I guess. I'm really much going modern C++ these days, so I forgot how to write it properly for allegro...

And it does the trick, now there's less than a second and allegro is up. As they said in the but report I found, that is still WAY too long, as long as it's not called in a loop, I can live with it.

So maybe one of the Windows / OpenGL experts can apply this or a similar change to wgl_disp.c? Or even other places with similar patterns?

]]> no-reply@allegro.cc (tobing) Wed, 25 Mar 2020 20:15:03 +0000

Looks good to me, let's ship it (https://github.com/liballeg/allegro5/issues/1125).

]]> no-reply@allegro.cc (SiegeLord) Sun, 05 Apr 2020 02:01:49 +0000

Here's a patch, I haven't forked allegro (yet), so I think I can't just create a pull request.

Thanks in advance!

#SelectExpand
1From 4d45eb36f0873f48672f02328693872c86a55815 Mon Sep 17 00:00:00 2001 2From: Tobias Scheuer <tobias@scheuer42.de> 3Date: Sun, 29 Mar 2020 12:24:59 +0200 4Subject: [PATCH] initialize wgl extension string only once 5 6--- 7 src/win/wgl_disp.c | 11 ++++++++--- 8 1 file changed, 8 insertions(+), 3 deletions(-) 9 10diff --git a/src/win/wgl_disp.c b/src/win/wgl_disp.c 11index b8e0878fc..893d56041 100644 12--- a/src/win/wgl_disp.c 13+++ b/src/win/wgl_disp.c 14@@ -68,15 +68,20 @@ typedef struct WGL_DISPLAY_PARAMETERS { 15 16 static bool is_wgl_extension_supported(const char *extension, HDC dc) 17 { 18- _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB; 19+ static _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB; 20+ static ext_is_initialized = false; // this is a simple blocker, would not be safe for multi-threading 21 int ret; 22 23 /* XXX deprecated in OpenGL 3.0 */ 24 if (!glGetString(GL_EXTENSIONS)) 25 return false; 26 27- _wglGetExtensionsStringARB = (_ALLEGRO_wglGetExtensionsStringARB_t) 28- wglGetProcAddress("wglGetExtensionsStringARB"); 29+ if(!ext_is_initialized) 30+ { 31+ _wglGetExtensionsStringARB = (_ALLEGRO_wglGetExtensionsStringARB_t) 32+ wglGetProcAddress("wglGetExtensionsStringARB"); 33+ ext_is_initialized = true; 34+ } 35 if (!_wglGetExtensionsStringARB) 36 return false; 37 38-- 392.24.1.windows.2

]]> no-reply@allegro.cc (tobing) Tue, 07 Apr 2020 20:25:54 +0000

I think in theory a different OpenGL context could return a different address - so we'd need a separate _wglGetExtensionsStringARB for each thread and clear it when the context gets changed on that thread.

If it doesn't break any of the examples it's probably good enough though for now - but maybe at least should have a // TODO/FIXME comment about what it does.

]]> no-reply@allegro.cc (Elias) Wed, 08 Apr 2020 00:30:08 +0000

This is called in a loop, so maybe the context could be a parameter... just a thought for now, I'll have a look at the code tomorrow and maybe come up with a better patch.

Edit: I have added a parameter to some of the functions, like in attached patch. Now there are more changed lines, but no assumptions about threads and such. I guess that this is better, and performance seems to be quite good as well, maybe even more than the first change.

#SelectExpand
1From 276d7889c9b3bebdcd538a694154bde9c3600000 Mon Sep 17 00:00:00 2001 2From: Tobias Scheuer <tobias@scheuer42.de> 3Date: Sun, 29 Mar 2020 12:24:59 +0200 4Subject: [PATCH] avoid multiple wgl extension string initialization 5 6--- 7 src/win/wgl_disp.c | 48 +++++++++++++++++++++++----------------------- 8 1 file changed, 24 insertions(+), 24 deletions(-) 9 10diff --git a/src/win/wgl_disp.c b/src/win/wgl_disp.c 11index b8e0878fc..d6972265f 100644 12--- a/src/win/wgl_disp.c 13+++ b/src/win/wgl_disp.c 14@@ -66,17 +66,13 @@ typedef struct WGL_DISPLAY_PARAMETERS { 15 const char* window_title; 16 } WGL_DISPLAY_PARAMETERS; 17 18-static bool is_wgl_extension_supported(const char *extension, HDC dc) 19+static bool is_wgl_extension_supported(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, const char *extension, HDC dc) 20 { 21- _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB; 22 int ret; 23 24 /* XXX deprecated in OpenGL 3.0 */ 25 if (!glGetString(GL_EXTENSIONS)) 26 return false; 27- 28- _wglGetExtensionsStringARB = (_ALLEGRO_wglGetExtensionsStringARB_t) 29- wglGetProcAddress("wglGetExtensionsStringARB"); 30 if (!_wglGetExtensionsStringARB) 31 return false; 32 33@@ -441,7 +437,7 @@ static ALLEGRO_EXTRA_DISPLAY_SETTINGS* read_pixel_format_old(int fmt, HDC dc) 34 } 35 36 37-static ALLEGRO_EXTRA_DISPLAY_SETTINGS* read_pixel_format_ext(int fmt, HDC dc) 38+static ALLEGRO_EXTRA_DISPLAY_SETTINGS* read_pixel_format_ext(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, int fmt, HDC dc) 39 { 40 ALLEGRO_EXTRA_DISPLAY_SETTINGS *eds = NULL; 41 42@@ -493,11 +489,11 @@ static ALLEGRO_EXTRA_DISPLAY_SETTINGS* read_pixel_format_ext(int fmt, HDC dc) 43 return NULL; 44 45 /* If multisampling is supported, query for it. */ 46- if (is_wgl_extension_supported("WGL_ARB_multisample", dc)) { 47+ if (is_wgl_extension_supported(_wglGetExtensionsStringARB, "WGL_ARB_multisample", dc)) { 48 attrib[num_attribs - 3] = WGL_SAMPLE_BUFFERS_ARB; 49 attrib[num_attribs - 2] = WGL_SAMPLES_ARB; 50 } 51- if (is_wgl_extension_supported("WGL_EXT_depth_float", dc)) { 52+ if (is_wgl_extension_supported(_wglGetExtensionsStringARB, "WGL_EXT_depth_float", dc)) { 53 attrib[num_attribs - 1] = WGL_DEPTH_FLOAT_EXT; 54 } 55 56@@ -623,7 +619,7 @@ static bool change_display_mode(ALLEGRO_DISPLAY *d) 57 } 58 59 60-static HGLRC init_ogl_context_ex(HDC dc, bool fc, int major, int minor) 61+static HGLRC init_ogl_context_ex(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, HDC dc, bool fc, int major, int minor) 62 { 63 HWND testwnd = NULL; 64 HDC testdc = NULL; 65@@ -644,7 +640,7 @@ static HGLRC init_ogl_context_ex(HDC dc, bool fc, int major, int minor) 66 if (!testrc) 67 goto bail; 68 69- if (is_wgl_extension_supported("WGL_ARB_create_context", testdc)) { 70+ if(is_wgl_extension_supported(_wglGetExtensionsStringARB, "WGL_ARB_create_context", testdc)) { 71 int attrib[] = {WGL_CONTEXT_MAJOR_VERSION_ARB, major, 72 WGL_CONTEXT_MINOR_VERSION_ARB, minor, 73 WGL_CONTEXT_FLAGS_ARB, 0, 74@@ -678,7 +674,7 @@ bail: 75 } 76 77 78-static ALLEGRO_EXTRA_DISPLAY_SETTINGS** get_available_pixel_formats_ext(int *count) 79+static ALLEGRO_EXTRA_DISPLAY_SETTINGS** get_available_pixel_formats_ext(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, int *count) 80 { 81 HWND testwnd = NULL; 82 HDC testdc = NULL; 83@@ -708,8 +704,8 @@ static ALLEGRO_EXTRA_DISPLAY_SETTINGS** get_available_pixel_formats_ext(int *cou 84 if (!testrc) 85 goto bail; 86 87- if (!is_wgl_extension_supported("WGL_ARB_pixel_format", testdc) && 88- !is_wgl_extension_supported("WGL_EXT_pixel_format", testdc)) { 89+ if(!is_wgl_extension_supported(_wglGetExtensionsStringARB, "WGL_ARB_pixel_format", testdc) && 90+ !is_wgl_extension_supported(_wglGetExtensionsStringARB, "WGL_EXT_pixel_format", testdc)) { 91 ALLEGRO_ERROR("WGL_ARB/EXT_pf not supported.\n"); 92 goto bail; 93 } 94@@ -730,7 +726,7 @@ static ALLEGRO_EXTRA_DISPLAY_SETTINGS** get_available_pixel_formats_ext(int *cou 95 for (j = i = 0; i < maxindex; i++) { 96 ALLEGRO_INFO("-- \n"); 97 ALLEGRO_INFO("Decoding visual no. %i...\n", i+1); 98- eds_list[j] = read_pixel_format_ext(i, testdc); 99+ eds_list[j] = read_pixel_format_ext(_wglGetExtensionsStringARB, i, testdc); 100 if (!eds_list[j]) 101 continue; 102 // Fill vsync setting here and enable/disable it after display creation 103@@ -817,7 +813,7 @@ static ALLEGRO_EXTRA_DISPLAY_SETTINGS** get_available_pixel_formats_old(int *cou 104 } 105 106 107-static bool select_pixel_format(ALLEGRO_DISPLAY_WGL *d, HDC dc) 108+static bool select_pixel_format(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, ALLEGRO_DISPLAY_WGL *d, HDC dc) 109 { 110 ALLEGRO_EXTRA_DISPLAY_SETTINGS **eds = NULL; 111 ALLEGRO_CONFIG *sys_cfg = al_get_system_config(); 112@@ -838,7 +834,7 @@ static bool select_pixel_format(ALLEGRO_DISPLAY_WGL *d, HDC dc) 113 } 114 115 if (!force_old) 116- eds = get_available_pixel_formats_ext(&eds_count); 117+ eds = get_available_pixel_formats_ext(_wglGetExtensionsStringARB, &eds_count); 118 if (!eds) 119 eds = get_available_pixel_formats_old(&eds_count, dc); 120 121@@ -879,7 +875,7 @@ static bool select_pixel_format(ALLEGRO_DISPLAY_WGL *d, HDC dc) 122 return true; 123 } 124 125-static bool create_display_internals(ALLEGRO_DISPLAY_WGL *wgl_disp) 126+static bool create_display_internals(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, ALLEGRO_DISPLAY_WGL *wgl_disp) 127 { 128 ALLEGRO_DISPLAY *disp = (void*)wgl_disp; 129 ALLEGRO_DISPLAY_WIN *win_disp = (void*)wgl_disp; 130@@ -921,7 +917,7 @@ static bool create_display_internals(ALLEGRO_DISPLAY_WGL *wgl_disp) 131 /* WGL display lists cannot be shared with the API currently in use. */ 132 disp->ogl_extras->is_shared = false; 133 134- if (!select_pixel_format(wgl_disp, wgl_disp->dc)) { 135+ if(!select_pixel_format(_wglGetExtensionsStringARB, wgl_disp, wgl_disp->dc)) { 136 destroy_display_internals(wgl_disp); 137 return false; 138 } 139@@ -934,7 +930,7 @@ static bool create_display_internals(ALLEGRO_DISPLAY_WGL *wgl_disp) 140 if (major == 0) 141 major = 3; 142 bool fc = (disp->flags & ALLEGRO_OPENGL_FORWARD_COMPATIBLE) != 0; 143- wgl_disp->glrc = init_ogl_context_ex(wgl_disp->dc, fc, major, 144+ wgl_disp->glrc = init_ogl_context_ex(_wglGetExtensionsStringARB, wgl_disp->dc, fc, major, 145 minor); 146 } 147 else { 148@@ -1024,7 +1020,9 @@ static ALLEGRO_DISPLAY* wgl_create_display(int w, int h) 149 150 display->ogl_extras = al_calloc(1, sizeof(ALLEGRO_OGL_EXTRAS)); 151 152- if (!create_display_internals(wgl_display)) { 153+ _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB 154+ = (_ALLEGRO_wglGetExtensionsStringARB_t)wglGetProcAddress("wglGetExtensionsStringARB"); 155+ if(!create_display_internals(_wglGetExtensionsStringARB, wgl_display)) { 156 al_free(display->ogl_extras); 157 al_free(display); 158 return NULL; 159@@ -1331,7 +1329,7 @@ static void wgl_update_display_region(ALLEGRO_DISPLAY *d, 160 } 161 162 163-static bool wgl_resize_helper(ALLEGRO_DISPLAY *d, int width, int height) 164+static bool wgl_resize_helper(_ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB, ALLEGRO_DISPLAY *d, int width, int height) 165 { 166 ALLEGRO_DISPLAY_WGL *wgl_disp = (ALLEGRO_DISPLAY_WGL *)d; 167 ALLEGRO_DISPLAY_WIN *win_disp = (ALLEGRO_DISPLAY_WIN *)d; 168@@ -1380,7 +1378,7 @@ static bool wgl_resize_helper(ALLEGRO_DISPLAY *d, int width, int height) 169 170 d->w = width; 171 d->h = height; 172- if (!create_display_internals(wgl_disp)) 173+ if(!create_display_internals(_wglGetExtensionsStringARB, wgl_disp)) 174 return false; 175 176 /* We have a new backbuffer now. */ 177@@ -1435,8 +1433,10 @@ static bool wgl_resize_display(ALLEGRO_DISPLAY *d, int width, int height) 178 179 win_display->ignore_resize = true; 180 181- if (!wgl_resize_helper(d, width, height)) { 182- wgl_resize_helper(d, orig_w, orig_h); 183+ _ALLEGRO_wglGetExtensionsStringARB_t _wglGetExtensionsStringARB 184+ = (_ALLEGRO_wglGetExtensionsStringARB_t)wglGetProcAddress("wglGetExtensionsStringARB"); 185+ if(!wgl_resize_helper(_wglGetExtensionsStringARB, d, width, height)) { 186+ wgl_resize_helper(_wglGetExtensionsStringARB, d, orig_w, orig_h); 187 ret = false; 188 } else { 189 ret = true; 190-- 1912.24.1.windows.2

]]> no-reply@allegro.cc (tobing) Wed, 08 Apr 2020 01:05:36 +0000