[AD] Working on ex_threads, d3d_display_format.cpp, tls.c

[Edgar Reynaldo <edgarreynaldo@xxxxxxxxxx>]

Hello fellas,

ex_threads crashes with the D3D driver due to multiple threads accessing 
the same ALLEGRO_EXTRA_DISPLAY_SETTINGS list at the same time. I am 
working on a patch for this.

I'd like to add the display format list to thread local storage to avoid 
the multiple access problem. This is not hard.

I've also altered d3d_display_formats.cpp to use the thread local 
storage state as well.

I have a few questions though.

1. What is the proper way to access the tls_get function and the 
thread_local_state? There is no public api available to access tls_get, 
nor is there a private one. In tls.c, getter and setter functions are 
used to interact with the thread_local_state. Is this the preferred way 
to do this? Alternatively, there is the tls_get function, which seems 
the most appropriate to use in this case (for d3d_display_formats.cpp). 
It is accessible through one of three different headers, (tls_dll.inc , 
tls_pthread.inc , or tls_native.inc).

2. Should there be a public (at least public to the library) header 
available for the tls_get function? I propose moving the includes for 
tls_dll.inc, tls_pthread.inc, and tls_native.inc into their own tls 
header. Perhaps into tls.h, or into aintern_tls.h?

I have made a preliminary patch for this, available as an attachment. 
It's not complete though. There are some specific things that need to be 
completed first, such as solving #1 (how to import the tls_get function 
into d3d_display_formats.cpp), and what to do when tls_get returns NULL.

This was first discussed on the a.cc forum, but I haven't had time to do 
much until recently. See the associated thread here :
https://www.allegro.cc/forums/thread/616214/1021962#target

Let me know what you want me to do.

Thanks,
Edgar

 src/tls.c                       |  9 ++++++
 src/win/d3d_display_formats.cpp | 65 ++++++++++++++++++++++++-----------------
 2 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/src/tls.c b/src/tls.c
index 7dbc8bd..321f085 100644
--- a/src/tls.c
+++ b/src/tls.c
@@ -105,6 +105,11 @@ typedef struct thread_local_state {
    JNIEnv *jnienv;
 #endif
 
+   /* Per thread storage for available display settings list */
+   _AL_VECTOR eds_list;
+   bool eds_list_fullscreen;
+   int eds_list_adapter;
+
    /* Destructor ownership count */
    int dtor_owner_count;
 } thread_local_state;
@@ -149,6 +154,10 @@ static void initialize_tls_values(thread_local_state *tls)
    memset(tls->new_window_title, 0, ALLEGRO_NEW_WINDOW_TITLE_MAX_SIZE + 1);
 
    _al_fill_display_settings(&tls->new_display_settings);
+   
+   _al_vector_init(&(tls->eds_list) , sizeof(ALLEGRO_EXTRA_DISPLAY_SETTINGS*));
+   tls->eds_list_fullscreen = false;
+   tls->eds_list_adapter = 0;
 }
 
 
diff --git a/src/win/d3d_display_formats.cpp b/src/win/d3d_display_formats.cpp
index 14f749a..0f74540 100644
--- a/src/win/d3d_display_formats.cpp
+++ b/src/win/d3d_display_formats.cpp
@@ -32,39 +32,40 @@ static BOOL IsDepthFormatExisting(D3DFORMAT DepthFormat, D3DFORMAT AdapterFormat
 
 static const int D3D_DEPTH_FORMATS = sizeof(depth_stencil_formats) / sizeof(*depth_stencil_formats);
 
-static _AL_VECTOR eds_list;
+///static _AL_VECTOR eds_list;
 
-void _al_d3d_destroy_display_format_list(void)
+void _al_d3d_destroy_display_format_list(_AL_VECTOR* eds_list)
 {
    /* Free the display format list */
-   for (int j = 0; j < (int)_al_vector_size(&eds_list); j++) {
-      void **eds = (void **)_al_vector_ref(&eds_list, j);
+   for (int j = 0; j < (int)_al_vector_size(eds_list); j++) {
+      void **eds = (void **)_al_vector_ref(eds_list, j);
       al_free(*eds);
    }
-   _al_vector_free(&eds_list);
+   _al_vector_free(eds_list);
 }
 
 void _al_d3d_generate_display_format_list(void)
 {
-   static bool fullscreen = !(al_get_new_display_flags() & ALLEGRO_FULLSCREEN); /* stop warning */
-   static int adapter = ~al_get_new_display_adapter(); /* stop warning */
-   int i;
 
-   if (!_al_vector_is_empty(&eds_list) && (fullscreen == (bool)(al_get_new_display_flags() & ALLEGRO_FULLSCREEN))
-         && (adapter == al_get_new_display_adapter())) {
-      return;
-   }
-   else if (!_al_vector_is_empty(&eds_list)) {
-     _al_d3d_destroy_display_format_list();
+   thread_local_state* tls = tls_get();
+   int i = 0;
+   
+   _AL_VECTOR* eds_list = &(tls->eds_list);
+   
+   if (!_al_vector_is_empty(eds_list)) {
+      if (  (tls->eds_list_fullscreen == (bool)(al_get_new_display_flags() & ALLEGRO_FULLSCREEN)) &&
+            (tls->eds_list_adapter == al_get_new_display_adapter())  ) {
+         /// Extra display settings list already current
+         return;
+      }
+      _al_d3d_destroy_display_format_list(eds_list);
    }
-
-   fullscreen = (al_get_new_display_flags() & ALLEGRO_FULLSCREEN) != 0;
-   adapter = al_get_new_display_adapter();
-   if (adapter < 0)
-      adapter = 0;
+   
+   tls->eds_list_fullscreen = (al_get_new_display_flags() & ALLEGRO_FULLSCREEN) != 0;
+   tls->eds_list_adapter = (al_get_new_display_adapter() >= 0)?al_get_new_display_adapter():0;
 
    _al_vector_init(&eds_list, sizeof(ALLEGRO_EXTRA_DISPLAY_SETTINGS *));
-
+   
    /* Loop through each bit combination of:
     * bit 0: 16/32 bit
     * bit 1: single-buffer
@@ -94,7 +95,7 @@ void _al_d3d_generate_display_format_list(void)
             DEPTH_STENCIL_DESC *ds = depth_stencil_formats + j;
             for (int k = 0; k < (int)quality_levels + 1; k++) {
                ALLEGRO_EXTRA_DISPLAY_SETTINGS *eds, **peds;
-               peds = (ALLEGRO_EXTRA_DISPLAY_SETTINGS **)_al_vector_alloc_back(&eds_list);
+               peds = (ALLEGRO_EXTRA_DISPLAY_SETTINGS **)_al_vector_alloc_back(eds_list);
                eds = *peds = (ALLEGRO_EXTRA_DISPLAY_SETTINGS *)al_malloc(sizeof *eds);               
                memset(eds->settings, 0, sizeof(int) * ALLEGRO_DISPLAY_OPTIONS_COUNT);
 
@@ -146,15 +147,19 @@ void _al_d3d_generate_display_format_list(void)
 
 void _al_d3d_score_display_settings(ALLEGRO_EXTRA_DISPLAY_SETTINGS *ref)
 {
-   for (int i = 0; i < (int)_al_vector_size(&eds_list); i++) {
+   thread_local_state* tls = tls_get();
+
+   _AL_VECTOR* eds_list = &(tls->eds_list);
+
+   for (int i = 0; i < (int)_al_vector_size(eds_list); i++) {
       ALLEGRO_EXTRA_DISPLAY_SETTINGS *eds, **peds;
-      peds = (ALLEGRO_EXTRA_DISPLAY_SETTINGS **)_al_vector_ref(&eds_list, i);
+      peds = (ALLEGRO_EXTRA_DISPLAY_SETTINGS **)_al_vector_ref(eds_list, i);
       eds = *peds;
       eds->score = _al_score_display_settings(eds, ref);
       eds->index = i;
    }
 
-   qsort(eds_list._items, eds_list._size, eds_list._itemsize, _al_display_settings_sorter);
+   qsort(eds_list->_items, eds_list->_size, eds_list->_itemsize, _al_display_settings_sorter);
 }
 
 /* Helper function for sorting pixel formats by index */
@@ -180,12 +185,20 @@ static int d3d_display_list_resorter(const void *p0, const void *p1)
 
 void _al_d3d_resort_display_settings(void)
 {
-   qsort(eds_list._items, eds_list._size, eds_list._itemsize, d3d_display_list_resorter);
+   thread_local_state* tls = tls_get();
+
+   _AL_VECTOR* eds_list = &(tls->eds_list);
+
+   qsort(eds_list->_items, eds_list->_size, eds_list->_itemsize, d3d_display_list_resorter);
 }
 
 ALLEGRO_EXTRA_DISPLAY_SETTINGS *_al_d3d_get_display_settings(int i)
 {
+   thread_local_state* tls = tls_get();
+
+   _AL_VECTOR* eds_list = &(tls->eds_list);
+
    if (i < (int)_al_vector_size(&eds_list))
-      return *(ALLEGRO_EXTRA_DISPLAY_SETTINGS **)_al_vector_ref(&eds_list, i);
+      return *(ALLEGRO_EXTRA_DISPLAY_SETTINGS **)_al_vector_ref(eds_list, i);
    return NULL;
 }