Skip to content

Commit d49a47f

Browse files
SiegeLordExSiegeLord
authored andcommitted
Rework how rate-limiting of resize events works on Windows.
Before, we'd spawn bespoke threads every 50ms. One issue with this design was that destroying a display before that 50ms elapsed caused crashes, which happened in the menu code. The new code has a persistent thread that periodically sends those resize events. More importantly, we make sure that thread is aware of displays being destroyed. Fixes #1381
1 parent 884e5fc commit d49a47f

File tree

5 files changed

+93
-50
lines changed

5 files changed

+93
-50
lines changed

include/allegro5/platform/aintwin.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,6 @@ struct ALLEGRO_DISPLAY_WIN
8585
volatile bool end_thread; /* The display thread should end */
8686
volatile bool thread_ended; /* The display thread has ended */
8787

88-
/* For internal use by drivers, when this has been set to true
89-
* after al_resize_display called you can call acknowledge_resize
90-
*/
91-
bool can_acknowledge;
92-
9388
/* For internal use by the windows driver. When this is set and a Windows
9489
* window resize event is received by the window procedure, the event is
9590
* ignored and this value is set to false.
@@ -179,6 +174,7 @@ HWND _al_win_create_faux_fullscreen_window(LPCTSTR devname, ALLEGRO_DISPLAY *dis
179174
int x1, int y1, int width, int height,
180175
int refresh_rate, int flags);
181176
int _al_win_init_window(void);
177+
void _al_win_shutdown_window(void);
182178
HWND _al_win_create_hidden_window(void);
183179
void _al_win_post_create_window(ALLEGRO_DISPLAY *display);
184180

src/win/d3d_disp.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1818,7 +1818,6 @@ static ALLEGRO_DISPLAY *d3d_create_display_locked(int w, int h)
18181818
win_display->mouse_selected_hcursor = 0;
18191819
win_display->mouse_cursor_shown = false;
18201820
win_display->hide_mouse_on_move = false;
1821-
win_display->can_acknowledge = false;
18221821

18231822
SetForegroundWindow(win_display->window);
18241823
_al_win_grab_input(win_display);
@@ -2238,8 +2237,6 @@ static bool d3d_resize_helper(ALLEGRO_DISPLAY *d, int width, int height)
22382237
return true;
22392238
}
22402239

2241-
win_display->can_acknowledge = false;
2242-
22432240
if (d->flags & ALLEGRO_FULLSCREEN) {
22442241
/* Don't generate ALLEGRO_EVENT_DISPLAY_LOST events when destroying a
22452242
* display for resizing.

src/win/wgl_disp.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,6 @@ static bool create_display_internals(ALLEGRO_DISPLAY_WGL *wgl_disp)
999999
win_disp->mouse_selected_hcursor = 0;
10001000
win_disp->mouse_cursor_shown = false;
10011001
win_disp->hide_mouse_on_move = false;
1002-
win_disp->can_acknowledge = false;
10031002

10041003
_al_win_grab_input(win_disp);
10051004

@@ -1371,8 +1370,6 @@ static bool wgl_resize_helper(ALLEGRO_DISPLAY *d, int width, int height)
13711370
return true;
13721371
}
13731372

1374-
win_disp->can_acknowledge = false;
1375-
13761373
if (d->flags & ALLEGRO_FULLSCREEN) {
13771374
ALLEGRO_BITMAP *target_bmp;
13781375
_AL_VECTOR disp_bmps;

src/win/wsystem.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ static void win_shutdown(void)
230230
_al_d3d_shutdown_display();
231231
#endif
232232

233+
_al_win_shutdown_window();
234+
233235
_al_win_shutdown_time();
234236

235237
if (using_higher_res_timer) {

src/win/wwindow.c

Lines changed: 90 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ ALLEGRO_DEBUG_CHANNEL("wwindow")
4949

5050
static WNDCLASS window_class;
5151

52-
static bool resize_postponed = false;
52+
static _AL_VECTOR resizing_displays;
53+
static ALLEGRO_MUTEX *resize_event_thread_mutex;
54+
static bool end_resize_event_thread;
55+
static bool resize_event_thread_ended;
5356

5457

5558
UINT _al_win_msg_call_proc = 0;
@@ -308,6 +311,10 @@ static void win_generate_resize_event(ALLEGRO_DISPLAY_WIN *win_display)
308311
WINDOWINFO wi;
309312
int x, y, w, h;
310313

314+
if (win_display->ignore_resize) {
315+
return;
316+
}
317+
311318
wi.cbSize = sizeof(WINDOWINFO);
312319
GetWindowInfo(win_display->window, &wi);
313320
x = wi.rcClient.left;
@@ -347,25 +354,6 @@ static void win_generate_resize_event(ALLEGRO_DISPLAY_WIN *win_display)
347354
}
348355
}
349356

350-
static void postpone_thread_proc(void *arg)
351-
{
352-
ALLEGRO_DISPLAY *display = (ALLEGRO_DISPLAY *)arg;
353-
ALLEGRO_DISPLAY_WIN *win_display = (ALLEGRO_DISPLAY_WIN *)display;
354-
355-
Sleep(50);
356-
357-
if (win_display->ignore_resize) {
358-
win_display->ignore_resize = false;
359-
}
360-
else {
361-
win_generate_resize_event(win_display);
362-
}
363-
364-
resize_postponed = false;
365-
win_display->can_acknowledge = true;
366-
}
367-
368-
369357
static void handle_mouse_capture(bool down, HWND hWnd)
370358
{
371359
int i;
@@ -412,6 +400,32 @@ static bool accept_mouse_event(void)
412400
return !((GetMessageExtraInfo() & _AL_MOUSEEVENTF_FROMTOUCH) == _AL_MOUSEEVENTF_FROMTOUCH);
413401
}
414402

403+
/* We want to rate-limit the ALLEGRO_EVENT_DISPLAY_RESIZE events while
404+
* live-resizing via mouse. To this end, we detect start/end of this
405+
* via WM_ENTERSIZEMOVE/WM_EXITSIZEMOVE and then enable this code to
406+
* periodically send those events.
407+
*/
408+
static void resize_event_thread_proc(void *arg)
409+
{
410+
(void)arg;
411+
while (true) {
412+
al_lock_mutex(resize_event_thread_mutex);
413+
if (end_resize_event_thread) {
414+
al_unlock_mutex(resize_event_thread_mutex);
415+
break;
416+
}
417+
418+
for (int j = 0; j < (int)_al_vector_size(&resizing_displays); j++) {
419+
ALLEGRO_DISPLAY_WIN **win_display = (ALLEGRO_DISPLAY_WIN **)_al_vector_ref(&resizing_displays, j);
420+
win_generate_resize_event(*win_display);
421+
}
422+
423+
al_unlock_mutex(resize_event_thread_mutex);
424+
Sleep(50);
425+
}
426+
resize_event_thread_ended = true;
427+
}
428+
415429
static LRESULT CALLBACK window_callback(HWND hWnd, UINT message,
416430
WPARAM wParam, LPARAM lParam)
417431
{
@@ -438,6 +452,9 @@ static LRESULT CALLBACK window_callback(HWND hWnd, UINT message,
438452
break_window_message_pump(win_display, hWnd);
439453
if (_al_win_unregister_touch_window)
440454
_al_win_unregister_touch_window(hWnd);
455+
al_lock_mutex(resize_event_thread_mutex);
456+
_al_vector_find_and_delete(&resizing_displays, &win_display);
457+
al_unlock_mutex(resize_event_thread_mutex);
441458
DestroyWindow(hWnd);
442459
return 0;
443460
}
@@ -459,6 +476,9 @@ static LRESULT CALLBACK window_callback(HWND hWnd, UINT message,
459476
break_window_message_pump(win_display, hWnd);
460477
if (_al_win_unregister_touch_window)
461478
_al_win_unregister_touch_window(hWnd);
479+
al_lock_mutex(resize_event_thread_mutex);
480+
_al_vector_find_and_delete(&resizing_displays, &win_display);
481+
al_unlock_mutex(resize_event_thread_mutex);
462482
DestroyWindow(hWnd);
463483
return 0;
464484
}
@@ -939,15 +959,10 @@ static LRESULT CALLBACK window_callback(HWND hWnd, UINT message,
939959
}
940960
}
941961
break;
942-
case WM_SIZE:
962+
case WM_SIZE: {
963+
bool generate = false;
943964
if (wParam == SIZE_RESTORED || wParam == SIZE_MAXIMIZED || wParam == SIZE_MINIMIZED) {
944-
/*
945-
* Delay the resize event so we don't get bogged down with them
946-
*/
947-
if (!resize_postponed) {
948-
resize_postponed = true;
949-
_beginthread(postpone_thread_proc, 0, (void *)d);
950-
}
965+
generate = true;
951966
d->flags &= ~ALLEGRO_MAXIMIZED;
952967
if (wParam == SIZE_MAXIMIZED) {
953968
d->flags |= ALLEGRO_MAXIMIZED;
@@ -959,31 +974,43 @@ static LRESULT CALLBACK window_callback(HWND hWnd, UINT message,
959974
* We have to create the resize event, so the application could
960975
* redraw its content.
961976
*/
962-
if (!resize_postponed) {
963-
resize_postponed = true;
964-
_beginthread(postpone_thread_proc, 0, (void *)d);
965-
}
977+
generate = true;
978+
}
979+
al_lock_mutex(resize_event_thread_mutex);
980+
if (_al_vector_contains(&resizing_displays, &win_display)) {
981+
generate = false;
982+
}
983+
al_unlock_mutex(resize_event_thread_mutex);
984+
if (generate) {
985+
win_generate_resize_event(win_display);
966986
}
967987
return 0;
968-
case WM_ENTERSIZEMOVE:
988+
}
989+
case WM_ENTERSIZEMOVE: {
969990
/* DefWindowProc for WM_ENTERSIZEMOVE enters a modal loop, which also
970991
* ends up blocking the loop in d3d_display_thread_proc (which is
971992
* where we are called from, if using D3D). Rather than batching up
972993
* intermediate resize events which the user cannot acknowledge in the
973994
* meantime anyway, make it so only a single resize event is generated
974995
* at WM_EXITSIZEMOVE.
975996
*/
997+
al_lock_mutex(resize_event_thread_mutex);
976998
if (d->flags & ALLEGRO_DIRECT3D_INTERNAL) {
977-
resize_postponed = true;
999+
win_display->ignore_resize = true;
9781000
}
1001+
ALLEGRO_DISPLAY_WIN **add = (ALLEGRO_DISPLAY_WIN **)_al_vector_alloc_back(&resizing_displays);
1002+
*add = win_display;
1003+
al_unlock_mutex(resize_event_thread_mutex);
9791004
break;
1005+
}
9801006
case WM_EXITSIZEMOVE:
981-
if (resize_postponed) {
982-
win_generate_resize_event(win_display);
1007+
al_lock_mutex(resize_event_thread_mutex);
1008+
if (d->flags & ALLEGRO_DIRECT3D_INTERNAL) {
9831009
win_display->ignore_resize = false;
984-
resize_postponed = false;
985-
win_display->can_acknowledge = true;
9861010
}
1011+
_al_vector_find_and_delete(&resizing_displays, &win_display);
1012+
al_unlock_mutex(resize_event_thread_mutex);
1013+
win_generate_resize_event(win_display);
9871014
break;
9881015
case WM_DPICHANGED:
9891016
if ((d->flags & ALLEGRO_RESIZABLE) && !(d->flags & ALLEGRO_FULLSCREEN)) {
@@ -1008,7 +1035,7 @@ static LRESULT CALLBACK window_callback(HWND hWnd, UINT message,
10081035
return DefWindowProc(hWnd,message,wParam,lParam);
10091036
}
10101037

1011-
int _al_win_init_window()
1038+
int _al_win_init_window(void)
10121039
{
10131040
// Create A Window Class Structure
10141041
window_class.cbClsExtra = 0;
@@ -1029,9 +1056,33 @@ int _al_win_init_window()
10291056
_al_win_msg_suicide = RegisterWindowMessage(TEXT("Allegro window suicide"));
10301057
}
10311058

1059+
resize_event_thread_mutex = al_create_mutex();
1060+
_al_vector_init(&resizing_displays, sizeof(ALLEGRO_DISPLAY_WIN *));
1061+
_beginthread(resize_event_thread_proc, 0, 0);
1062+
10321063
return true;
10331064
}
10341065

1066+
void _al_win_shutdown_window(void)
1067+
{
1068+
al_lock_mutex(resize_event_thread_mutex);
1069+
end_resize_event_thread = true;
1070+
al_unlock_mutex(resize_event_thread_mutex);
1071+
1072+
while (true) {
1073+
al_lock_mutex(resize_event_thread_mutex);
1074+
if (resize_event_thread_ended) {
1075+
al_unlock_mutex(resize_event_thread_mutex);
1076+
break;
1077+
}
1078+
al_unlock_mutex(resize_event_thread_mutex);
1079+
Sleep(10);
1080+
}
1081+
_al_vector_free(&resizing_displays);
1082+
al_destroy_mutex(resize_event_thread_mutex);
1083+
resize_event_thread_mutex = NULL;
1084+
}
1085+
10351086

10361087
static int win_choose_icon_bitmap(const int sys_w, const int sys_h,
10371088
const int num_icons, ALLEGRO_BITMAP *bmps[])

0 commit comments

Comments
 (0)