|
slow detection of GL extensions on some hardware |
tobing
Member #5,213
November 2004
|
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: 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? |
SiegeLord
Member #7,827
October 2006
|
Looks good to me, let's ship it (https://github.com/liballeg/allegro5/issues/1125). "For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18 |
tobing
Member #5,213
November 2004
|
Here's a patch, I haven't forked allegro (yet), so I think I can't just create a pull request. Thanks in advance! 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
|
Elias
Member #358
May 2000
|
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. -- |
tobing
Member #5,213
November 2004
|
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. 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
|
|