diff --git a/src/SDL_hashtable.c b/src/SDL_hashtable.c index 571e376974..3124b41a8a 100644 --- a/src/SDL_hashtable.c +++ b/src/SDL_hashtable.c @@ -19,34 +19,6 @@ 3. This notice may not be removed or altered from any source distribution. */ #include "SDL_internal.h" -#include "SDL_hashtable.h" - -// XXX: We can't use SDL_assert here because it's going to call into hashtable code -#ifdef NDEBUG -#define HT_ASSERT(x) (void)(0) -#else -#if (defined(_WIN32) || defined(SDL_PLATFORM_CYGWIN)) && !defined(SDL_PLATFORM_XBOXONE) && !defined(SDL_PLATFORM_XBOXSERIES) -#include -#endif -/* This is not declared in any header, although it is shared between some - parts of SDL, because we don't want anything calling it without an - extremely good reason. */ -extern SDL_NORETURN void SDL_ExitProcess(int exitcode); -static void HT_ASSERT_FAIL(const char *msg) -{ - const char *caption = "SDL_HashTable Assertion Failure!"; - (void)caption; -#if (defined(_WIN32) || defined(SDL_PLATFORM_CYGWIN)) && !defined(SDL_PLATFORM_XBOXONE) && !defined(SDL_PLATFORM_XBOXSERIES) - MessageBoxA(NULL, msg, caption, MB_OK | MB_ICONERROR); -#elif defined(HAVE_STDIO_H) - fprintf(stderr, "\n\n%s\n%s\n\n", caption, msg); - fflush(stderr); -#endif - SDL_TriggerBreakpoint(); - SDL_ExitProcess(-1); -} -#define HT_ASSERT(x) if (!(x)) HT_ASSERT_FAIL("SDL_HashTable Assertion Failure: " #x) -#endif typedef struct SDL_HashItem { @@ -67,47 +39,49 @@ SDL_COMPILE_TIME_ASSERT(sizeof_SDL_HashItem, sizeof(SDL_HashItem) <= MAX_HASHITE struct SDL_HashTable { - SDL_RWLock *lock; + SDL_RWLock *lock; // NULL if not created threadsafe SDL_HashItem *table; - SDL_HashTable_HashFn hash; - SDL_HashTable_KeyMatchFn keymatch; - SDL_HashTable_NukeFn nuke; - void *data; + SDL_HashCallback hash; + SDL_HashKeyMatchCallback keymatch; + SDL_HashDestroyCallback destroy; + void *userdata; Uint32 hash_mask; Uint32 max_probe_len; Uint32 num_occupied_slots; - bool stackable; }; -SDL_HashTable *SDL_CreateHashTable(void *data, - Uint32 num_buckets, - SDL_HashTable_HashFn hashfn, - SDL_HashTable_KeyMatchFn keymatchfn, - SDL_HashTable_NukeFn nukefn, - bool threadsafe, - bool stackable) + +static Uint32 CalculateHashBucketsFromEstimate(int estimated_capacity) { - SDL_HashTable *table; - - // num_buckets must be a power of two so we can derive the bucket index with just a bit-and. - if ((num_buckets < 1) || !SDL_HasExactlyOneBitSet32(num_buckets)) { - SDL_SetError("num_buckets must be a power of two"); - return NULL; + if (estimated_capacity <= 0) { + return 4; // start small, grow as necessary. } - if (num_buckets > MAX_HASHTABLE_SIZE) { - SDL_SetError("num_buckets is too large"); - return NULL; + const Uint32 estimated32 = (Uint32) estimated_capacity; + Uint32 buckets = ((Uint32) 1) << SDL_MostSignificantBitIndex32(estimated32); + if (!SDL_HasExactlyOneBitSet32(estimated32)) { + buckets <<= 1; // need next power of two up to fit overflow capacity bits. } - table = (SDL_HashTable *)SDL_calloc(1, sizeof(SDL_HashTable)); + return SDL_min(buckets, MAX_HASHTABLE_SIZE); +} + +SDL_HashTable *SDL_CreateHashTable(int estimated_capacity, bool threadsafe, SDL_HashCallback hash, + SDL_HashKeyMatchCallback keymatch, + SDL_HashDestroyCallback destroy, void *userdata) +{ + const Uint32 num_buckets = CalculateHashBucketsFromEstimate(estimated_capacity); + SDL_HashTable *table = (SDL_HashTable *)SDL_calloc(1, sizeof(SDL_HashTable)); if (!table) { return NULL; } if (threadsafe) { - // Don't fail if we can't create a lock (single threaded environment?) table->lock = SDL_CreateRWLock(); + if (!table->lock) { + SDL_DestroyHashTable(table); + return NULL; + } } table->table = (SDL_HashItem *)SDL_calloc(num_buckets, sizeof(SDL_HashItem)); @@ -117,24 +91,22 @@ SDL_HashTable *SDL_CreateHashTable(void *data, } table->hash_mask = num_buckets - 1; - table->stackable = stackable; - table->data = data; - table->hash = hashfn; - table->keymatch = keymatchfn; - table->nuke = nukefn; + table->userdata = userdata; + table->hash = hash; + table->keymatch = keymatch; + table->destroy = destroy; return table; } static SDL_INLINE Uint32 calc_hash(const SDL_HashTable *table, const void *key) { const Uint32 BitMixer = 0x9E3779B1u; - return table->hash(key, table->data) * BitMixer; + return table->hash(table->userdata, key) * BitMixer; } static SDL_INLINE Uint32 get_probe_length(Uint32 zero_idx, Uint32 actual_idx, Uint32 num_buckets) { // returns the probe sequence length from zero_idx to actual_idx - if (actual_idx < zero_idx) { return num_buckets - zero_idx + actual_idx; } @@ -149,7 +121,7 @@ static SDL_HashItem *find_item(const SDL_HashTable *ht, const void *key, Uint32 SDL_HashItem *table = ht->table; - for (;;) { + while (true) { SDL_HashItem *item = table + *i; Uint32 item_hash = item->hash; @@ -157,12 +129,12 @@ static SDL_HashItem *find_item(const SDL_HashTable *ht, const void *key, Uint32 return NULL; } - if (item_hash == hash && ht->keymatch(item->key, key, ht->data)) { + if (item_hash == hash && ht->keymatch(ht->userdata, item->key, key)) { return item; } Uint32 item_probe_len = item->probe_len; - HT_ASSERT(item_probe_len == get_probe_length(item_hash & hash_mask, (Uint32)(item - table), hash_mask + 1)); + SDL_assert(item_probe_len == get_probe_length(item_hash & hash_mask, (Uint32)(item - table), hash_mask + 1)); if (*probe_len > item_probe_len) { return NULL; @@ -185,23 +157,23 @@ static SDL_HashItem *find_first_item(const SDL_HashTable *ht, const void *key, U static SDL_HashItem *insert_item(SDL_HashItem *item_to_insert, SDL_HashItem *table, Uint32 hash_mask, Uint32 *max_probe_len_ptr) { + const Uint32 num_buckets = hash_mask + 1; Uint32 idx = item_to_insert->hash & hash_mask; - SDL_HashItem temp_item, *target = NULL; - Uint32 num_buckets = hash_mask + 1; + SDL_HashItem *target = NULL; + SDL_HashItem temp_item; - for (;;) { + while (true) { SDL_HashItem *candidate = table + idx; if (!candidate->live) { // Found an empty slot. Put it here and we're done. - *candidate = *item_to_insert; if (target == NULL) { target = candidate; } - Uint32 probe_len = get_probe_length(candidate->hash & hash_mask, idx, num_buckets); + const Uint32 probe_len = get_probe_length(candidate->hash & hash_mask, idx, num_buckets); candidate->probe_len = probe_len; if (*max_probe_len_ptr < probe_len) { @@ -211,9 +183,9 @@ static SDL_HashItem *insert_item(SDL_HashItem *item_to_insert, SDL_HashItem *tab break; } - Uint32 candidate_probe_len = candidate->probe_len; - HT_ASSERT(candidate_probe_len == get_probe_length(candidate->hash & hash_mask, idx, num_buckets)); - Uint32 new_probe_len = get_probe_length(item_to_insert->hash & hash_mask, idx, num_buckets); + const Uint32 candidate_probe_len = candidate->probe_len; + SDL_assert(candidate_probe_len == get_probe_length(candidate->hash & hash_mask, idx, num_buckets)); + const Uint32 new_probe_len = get_probe_length(item_to_insert->hash & hash_mask, idx, num_buckets); if (candidate_probe_len < new_probe_len) { // Robin Hood hashing: the item at idx has a better probe length than our item would at this position. @@ -229,7 +201,7 @@ static SDL_HashItem *insert_item(SDL_HashItem *item_to_insert, SDL_HashItem *tab *item_to_insert = temp_item; - HT_ASSERT(new_probe_len == get_probe_length(candidate->hash & hash_mask, idx, num_buckets)); + SDL_assert(new_probe_len == get_probe_length(candidate->hash & hash_mask, idx, num_buckets)); candidate->probe_len = new_probe_len; if (*max_probe_len_ptr < new_probe_len) { @@ -245,17 +217,19 @@ static SDL_HashItem *insert_item(SDL_HashItem *item_to_insert, SDL_HashItem *tab static void delete_item(SDL_HashTable *ht, SDL_HashItem *item) { - Uint32 hash_mask = ht->hash_mask; + const Uint32 hash_mask = ht->hash_mask; SDL_HashItem *table = ht->table; - if (ht->nuke) { - ht->nuke(item->key, item->value, ht->data); + if (ht->destroy) { + ht->destroy(ht->userdata, item->key, item->value); } + + SDL_assert(ht->num_occupied_slots > 0); ht->num_occupied_slots--; Uint32 idx = (Uint32)(item - ht->table); - for (;;) { + while (true) { idx = (idx + 1) & hash_mask; SDL_HashItem *next_item = table + idx; @@ -266,22 +240,23 @@ static void delete_item(SDL_HashTable *ht, SDL_HashItem *item) *item = *next_item; item->probe_len -= 1; - HT_ASSERT(item->probe_len < ht->max_probe_len); + SDL_assert(item->probe_len < ht->max_probe_len); item = next_item; } } static bool resize(SDL_HashTable *ht, Uint32 new_size) { - SDL_HashItem *old_table = ht->table; - Uint32 old_size = ht->hash_mask + 1; - Uint32 new_hash_mask = new_size - 1; + const Uint32 new_hash_mask = new_size - 1; SDL_HashItem *new_table = SDL_calloc(new_size, sizeof(*new_table)); if (!new_table) { return false; } + SDL_HashItem *old_table = ht->table; + const Uint32 old_size = ht->hash_mask + 1; + ht->max_probe_len = 0; ht->hash_mask = new_hash_mask; ht->table = new_table; @@ -299,14 +274,14 @@ static bool resize(SDL_HashTable *ht, Uint32 new_size) static bool maybe_resize(SDL_HashTable *ht) { - Uint32 capacity = ht->hash_mask + 1; + const Uint32 capacity = ht->hash_mask + 1; if (capacity >= MAX_HASHTABLE_SIZE) { return false; } - Uint32 max_load_factor = 217; // range: 0-255; 217 is roughly 85% - Uint32 resize_threshold = (Uint32)((max_load_factor * (Uint64)capacity) >> 8); + const Uint32 max_load_factor = 217; // range: 0-255; 217 is roughly 85% + const Uint32 resize_threshold = (Uint32)((max_load_factor * (Uint64)capacity) >> 8); if (ht->num_occupied_slots > resize_threshold) { return resize(ht, capacity * 2); @@ -315,72 +290,66 @@ static bool maybe_resize(SDL_HashTable *ht) return true; } -bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *value) +bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *value, bool replace) { - SDL_HashItem *item; - Uint32 hash; + if (!table) { + return SDL_InvalidParamError("table"); + } + bool result = false; - if (!table) { - return false; + SDL_LockRWLockForWriting(table->lock); + + const Uint32 hash = calc_hash(table, key); + SDL_HashItem *item = find_first_item(table, key, hash); + bool do_insert = true; + + if (item) { + if (replace) { + delete_item(table, item); + } else { + SDL_SetError("key already exists and replace is disabled"); + do_insert = false; + } } - if (table->lock) { - SDL_LockRWLockForWriting(table->lock); + if (do_insert) { + SDL_HashItem new_item; + new_item.key = key; + new_item.value = value; + new_item.hash = hash; + new_item.live = true; + new_item.probe_len = 0; + + table->num_occupied_slots++; + + if (!maybe_resize(table)) { + table->num_occupied_slots--; + } else { + // This never returns NULL + insert_item(&new_item, table->table, table->hash_mask, &table->max_probe_len); + result = true; + } } - hash = calc_hash(table, key); - item = find_first_item(table, key, hash); - - if (item && !table->stackable) { - // Allow overwrites, this might have been inserted on another thread - delete_item(table, item); - } - - SDL_HashItem new_item; - new_item.key = key; - new_item.value = value; - new_item.hash = hash; - new_item.live = true; - new_item.probe_len = 0; - - table->num_occupied_slots++; - - if (!maybe_resize(table)) { - table->num_occupied_slots--; - goto done; - } - - // This never returns NULL - insert_item(&new_item, table->table, table->hash_mask, &table->max_probe_len); - result = true; - -done: - if (table->lock) { - SDL_UnlockRWLock(table->lock); - } + SDL_UnlockRWLock(table->lock); return result; } bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void **value) { - Uint32 hash; - SDL_HashItem *i; - bool result = false; - if (!table) { if (value) { *value = NULL; } - return false; + return SDL_InvalidParamError("table"); } - if (table->lock) { - SDL_LockRWLockForReading(table->lock); - } + SDL_LockRWLockForReading(table->lock); - hash = calc_hash(table, key); - i = find_first_item(table, key, hash); + bool result = false; + const Uint32 hash = calc_hash(table, key); + SDL_HashItem *i = find_first_item(table, key, hash); if (i) { if (value) { *value = i->value; @@ -388,156 +357,91 @@ bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void result = true; } - if (table->lock) { - SDL_UnlockRWLock(table->lock); - } + SDL_UnlockRWLock(table->lock); + return result; } bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key) { - Uint32 hash; - SDL_HashItem *item; - bool result = false; - if (!table) { - return false; + return SDL_InvalidParamError("table"); } - if (table->lock) { - SDL_LockRWLockForWriting(table->lock); + SDL_LockRWLockForWriting(table->lock); + + bool result = false; + const Uint32 hash = calc_hash(table, key); + SDL_HashItem *item = find_first_item(table, key, hash); + if (item) { + delete_item(table, item); + result = true; } - // FIXME: what to do for stacking hashtables? - // The original code removes just one item. - // This hashtable happens to preserve the insertion order of multi-value keys, - // so deleting the first one will always delete the least-recently inserted one. - // But maybe it makes more sense to remove all matching items? - - hash = calc_hash(table, key); - item = find_first_item(table, key, hash); - if (!item) { - goto done; - } - - delete_item(table, item); - result = true; - -done: - if (table->lock) { - SDL_UnlockRWLock(table->lock); - } + SDL_UnlockRWLock(table->lock); return result; } -bool SDL_IterateHashTableKey(const SDL_HashTable *table, const void *key, const void **_value, void **iter) +bool SDL_IterateHashTable(const SDL_HashTable *table, SDL_HashTableIterateCallback callback, void *userdata) { - SDL_HashItem *item = (SDL_HashItem *)*iter; - if (!table) { - return false; + return SDL_InvalidParamError("table"); + } else if (!callback) { + return SDL_InvalidParamError("callback"); } - Uint32 i, probe_len, hash; - - if (item) { - HT_ASSERT(item >= table->table); - HT_ASSERT(item < table->table + (table->hash_mask + 1)); - - hash = item->hash; - probe_len = item->probe_len + 1; - i = ((Uint32)(item - table->table) + 1) & table->hash_mask; - item = table->table + i; - } else { - hash = calc_hash(table, key); - i = hash & table->hash_mask; - probe_len = 0; - } - - item = find_item(table, key, hash, &i, &probe_len); - - if (!item) { - *_value = NULL; - return false; - } - - *_value = item->value; - *iter = item; - - return true; -} - -bool SDL_IterateHashTable(const SDL_HashTable *table, const void **_key, const void **_value, void **iter) -{ - SDL_HashItem *item = (SDL_HashItem *)*iter; - - if (!table) { - return false; - } - - if (!item) { - item = table->table; - } else { - item++; - } - - HT_ASSERT(item >= table->table); + SDL_LockRWLockForReading(table->lock); SDL_HashItem *end = table->table + (table->hash_mask + 1); + Uint32 num_iterated = 0; - while (item < end && !item->live) { - ++item; - } - - HT_ASSERT(item <= end); - - if (item == end) { - if (_key) { - *_key = NULL; + for (SDL_HashItem *item = table->table; item < end; item++) { + if (item->live) { + if (!callback(userdata, table, item->key, item->value)) { + break; // callback requested iteration stop. + } else if (++num_iterated >= table->num_occupied_slots) { + break; // we can drop out early because we've seen all the live items. + } } - if (_value) { - *_value = NULL; - } - return false; } - if (_key) { - *_key = item->key; - } - if (_value) { - *_value = item->value; - } - *iter = item; - + SDL_UnlockRWLock(table->lock); return true; } bool SDL_HashTableEmpty(SDL_HashTable *table) { - return !(table && table->num_occupied_slots); + if (!table) { + return SDL_InvalidParamError("table"); + } + + SDL_LockRWLockForReading(table->lock); + const bool retval = (table->num_occupied_slots == 0); + SDL_UnlockRWLock(table->lock); + return retval; } -static void nuke_all(SDL_HashTable *table) -{ - void *data = table->data; - SDL_HashItem *end = table->table + (table->hash_mask + 1); - SDL_HashItem *i; - for (i = table->table; i < end; ++i) { - if (i->live) { - table->nuke(i->key, i->value, data); +static void destroy_all(SDL_HashTable *table) +{ + SDL_HashDestroyCallback destroy = table->destroy; + if (destroy) { + void *userdata = table->userdata; + SDL_HashItem *end = table->table + (table->hash_mask + 1); + for (SDL_HashItem *i = table->table; i < end; ++i) { + if (i->live) { + i->live = false; + destroy(userdata, i->key, i->value); + } } } } -void SDL_EmptyHashTable(SDL_HashTable *table) +void SDL_ClearHashTable(SDL_HashTable *table) { if (table) { SDL_LockRWLockForWriting(table->lock); { - if (table->nuke) { - nuke_all(table); - } - + destroy_all(table); SDL_memset(table->table, 0, sizeof(*table->table) * (table->hash_mask + 1)); table->num_occupied_slots = 0; } @@ -548,9 +452,10 @@ void SDL_EmptyHashTable(SDL_HashTable *table) void SDL_DestroyHashTable(SDL_HashTable *table) { if (table) { - SDL_EmptyHashTable(table); - - SDL_DestroyRWLock(table->lock); + destroy_all(table); + if (table->lock) { + SDL_DestroyRWLock(table->lock); + } SDL_free(table->table); SDL_free(table); } @@ -566,26 +471,26 @@ static SDL_INLINE Uint32 hash_string_djbxor(const char *str, size_t len) return hash; } -Uint32 SDL_HashPointer(const void *key, void *unused) +Uint32 SDL_HashPointer(void *unused, const void *key) { (void)unused; return SDL_murmur3_32(&key, sizeof(key), 0); } -bool SDL_KeyMatchPointer(const void *a, const void *b, void *unused) +bool SDL_KeyMatchPointer(void *unused, const void *a, const void *b) { (void)unused; return (a == b); } -Uint32 SDL_HashString(const void *key, void *unused) +Uint32 SDL_HashString(void *unused, const void *key) { (void)unused; const char *str = (const char *)key; return hash_string_djbxor(str, SDL_strlen(str)); } -bool SDL_KeyMatchString(const void *a, const void *b, void *unused) +bool SDL_KeyMatchString(void *unused, const void *a, const void *b) { const char *a_string = (const char *)a; const char *b_string = (const char *)b; @@ -604,26 +509,33 @@ bool SDL_KeyMatchString(const void *a, const void *b, void *unused) // We assume we can fit the ID in the key directly SDL_COMPILE_TIME_ASSERT(SDL_HashID_KeySize, sizeof(Uint32) <= sizeof(const void *)); -Uint32 SDL_HashID(const void *key, void *unused) +Uint32 SDL_HashID(void *unused, const void *key) { (void)unused; return (Uint32)(uintptr_t)key; } -bool SDL_KeyMatchID(const void *a, const void *b, void *unused) +bool SDL_KeyMatchID(void *unused, const void *a, const void *b) { (void)unused; return (a == b); } -void SDL_NukeFreeKey(const void *key, const void *value, void *unused) +void SDL_DestroyHashKeyAndValue(void *unused, const void *key, const void *value) +{ + (void)unused; + SDL_free((void *)key); + SDL_free((void *)value); +} + +void SDL_DestroyHashKey(void *unused, const void *key, const void *value) { (void)value; (void)unused; SDL_free((void *)key); } -void SDL_NukeFreeValue(const void *key, const void *value, void *unused) +void SDL_DestroyHashValue(void *unused, const void *key, const void *value) { (void)key; (void)unused; diff --git a/src/SDL_hashtable.h b/src/SDL_hashtable.h index 5983b8d391..598a6d6be1 100644 --- a/src/SDL_hashtable.h +++ b/src/SDL_hashtable.h @@ -18,61 +18,616 @@ misrepresented as being the original software. 3. This notice may not be removed or altered from any source distribution. */ + +/* this is over-documented because it was almost a public API. Leaving the + full docs here in case it _does_ become public some day. */ + +/* WIKI CATEGORY: HashTable */ + +/** + * # CategoryHashTable + * + * SDL offers a hash table implementation, as a convenience for C code that + * needs efficient organization and access of arbitrary data. + * + * Hash tables are a popular data structure, designed to make it quick to + * store and look up arbitrary data. Data is stored with an associated "key." + * While one would look up an element of an array with an index, a hash table + * uses a unique key to find an element later. + * + * A key can be anything, as long as its unique and in a format that the table + * understands. For example, it's popular to use strings as keys: the key + * might be a username, and it is used to lookup account information for that + * user, etc. + * + * Hash tables are named because they "hash" their keys down into simple + * integers that can be used to efficiently organize and access the associated + * data. + * + * As this is a C API, there is one generic interface that is intended to work + * with different data types. This can be a little awkward to set up, but is + * easy to use after that. + * + * Hashtables are generated by a call to SDL_CreateHashTable(). This function + * requires several callbacks to be provided (for hashing keys, comparing + * entries, and cleaning up entries when removed). These are necessary to + * allow the hash to manage any arbitrary data type. + * + * Once a hash table is created, the common tasks are inserting data into the + * table, (SDL_InsertIntoHashTable), looking up previously inserted data + * (SDL_FindInHashTable), and removing data (SDL_RemoveFromHashTable and + * SDL_ClearHashTable). Less common but still useful is the ability to + * iterate through all the items in the table (SDL_IterateHashTable). + * + * The underlying hash table implementation is always subject to change, but + * at the time of writing, it uses open addressing and Robin Hood hashing. + * The technical details are explained [here](https://github.com/libsdl-org/SDL/pull/10897). + * + * Hashtables keep an SDL_RWLock internally, so multiple threads can perform + * hash lookups in parallel, while changes to the table will safely serialize + * access between threads. + * + * SDL provides a layer on top of this hash table implementation that might be + * more pleasant to use. SDL_PropertiesID maps a string to arbitrary data of + * various types in the same table, which could be both easier to use and more + * flexible. Refer to [CategoryProperties](CategoryProperties) for details. + */ + #ifndef SDL_hashtable_h_ #define SDL_hashtable_h_ -// this is not (currently) a public API. But maybe it should be! +#include -struct SDL_HashTable; +#include +/* Set up for C function definitions, even when using C++ */ +#ifdef __cplusplus +extern "C" { +#endif + +/** + * The opaque type that represents a hash table. + * + * This is hidden behind an opaque pointer because not only does the table + * need to store arbitrary data types, but the hash table implementation may + * change in the future. + * + * \since This struct is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ typedef struct SDL_HashTable SDL_HashTable; -typedef Uint32 (*SDL_HashTable_HashFn)(const void *key, void *data); -typedef bool (*SDL_HashTable_KeyMatchFn)(const void *a, const void *b, void *data); -typedef void (*SDL_HashTable_NukeFn)(const void *key, const void *value, void *data); -extern SDL_HashTable *SDL_CreateHashTable(void *data, - Uint32 num_buckets, - SDL_HashTable_HashFn hashfn, - SDL_HashTable_KeyMatchFn keymatchfn, - SDL_HashTable_NukeFn nukefn, - bool threadsafe, - bool stackable); +/** + * A function pointer representing a hash table hashing callback. + * + * This is called by SDL_HashTable when it needs to look up a key in + * its dataset. It generates a hash value from that key, and then uses that + * value as a basis for an index into an internal array. + * + * There are no rules on what hashing algorithm is used, so long as it + * can produce a reliable 32-bit value from `key`, and ideally distributes + * those values well across the 32-bit value space. The quality of a + * hashing algorithm is directly related to how well a hash table performs. + * + * Hashing can be a complicated subject, and often times what works best + * for one dataset will be suboptimal for another. There is a good discussion + * of the field [on Wikipedia](https://en.wikipedia.org/wiki/Hash_function). + * + * Also: do you _need_ to write a hashing function? SDL provides generic + * functions for strings (SDL_HashString), generic integer IDs (SDL_HashID), + * and generic pointers (SDL_HashPointer). Often you should use one of these + * before writing your own. + * + * \param userdata what was passed as `userdata` to SDL_CreateHashTable(). + * \param key the key to be hashed. + * \returns a 32-bit value that represents a hash of `key`. + * + * \threadsafety This function must be thread safe if the hash table is used + * from multiple threads at the same time. + * + * \since This datatype is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + * \sa SDL_HashString + * \sa SDL_HashID + * \sa SDL_HashPointer + */ +typedef Uint32 (SDLCALL *SDL_HashCallback)(void *userdata, const void *key); -// This function is thread-safe if the hashtable was created with threadsafe = true -extern void SDL_EmptyHashTable(SDL_HashTable *table); -// This function is not thread-safe. +/** + * A function pointer representing a hash table matching callback. + * + * This is called by SDL_HashTable when it needs to look up a key in its + * dataset. After hashing the key, it looks for items stored in relation to + * that hash value. Since there can be more than one item found through the + * same hash value, this function verifies a specific value is actually + * correct before choosing it. + * + * So this function needs to compare the keys at `a` and `b` and decide if + * they are actually the same. + * + * For example, if the keys are C strings, this function might just be: + * + * ```c + * return (SDL_strcmp((const char *) a, const char *b) == 0);` + * ``` + * + * Also: do you _need_ to write a matching function? SDL provides generic + * functions for strings (SDL_KeyMatchString), generic integer IDs + * (SDL_KeyMatchID), and generic pointers (SDL_KeyMatchPointer). Often you + * should use one of these before writing your own. + * + * \param userdata what was passed as `userdata` to SDL_CreateHashTable(). + * \param a the first key to be compared. + * \param b the second key to be compared. + * \returns true if two keys are identical, false otherwise. + * + * \threadsafety This function must be thread safe if the hash table is used + * from multiple threads at the same time. + * + * \since This datatype is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +typedef bool (SDLCALL *SDL_HashKeyMatchCallback)(void *userdata, const void *a, const void *b); + + +/** + * A function pointer representing a hash table cleanup callback. + * + * This is called by SDL_HashTable when removing items from the hash, or + * destroying the hash table. It is used to optionally deallocate the + * key/value pairs. + * + * This is not required to do anything, if all the data in the table is + * static or POD data, but it can also do more than a simple free: for + * example, if the hash table is storing open files, it can close them here. + * It can also free only the key or only the value; it depends on what the + * hash table contains. + * + * \param userdata what was passed as `userdata` to SDL_CreateHashTable(). + * \param key the key to deallocate. + * \param value the value to deallocate. + * + * \threadsafety This function must be thread safe if the hash table is used + * from multiple threads at the same time. + * + * \since This datatype is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +typedef void (SDLCALL *SDL_HashDestroyCallback)(void *userdata, const void *key, const void *value); + + +/** + * A function pointer representing a hash table iterator callback. + * + * This function is called once for each key/value pair to be considered + * when iterating a hash table. + * + * Iteration continues as long as there are more items to examine and this + * callback continues to return true. + * + * Do not attempt to modify the hash table during this callback, as it will + * cause incorrect behavior and possibly crashes. + * + * \param userdata what was passed as `userdata` to an iterator function. + * \param table the hash table being iterated. + * \param key the current key being iterated. + * \param value the current value being iterated. + * \returns true to keep iterating, false to stop iteration. + * + * \threadsafety A read lock is held during iteration, so other threads can + * still access the the hash table, but threads attempting to + * make changes will be blocked until iteration completes. If + * this is a concern, do as little in the callback as possible + * and finish iteration quickly. + * + * \since This datatype is available since SDL 3.4.0. + * + * \sa SDL_IterateHashTable + */ +typedef bool (SDLCALL *SDL_HashTableIterateCallback)(void *userdata, const SDL_HashTable *table, const void *key, const void *value); + + +/** + * Create a new hash table. + * + * To deal with different datatypes and needs of the caller, hash tables + * require several callbacks that deal with some specifics: how to hash a key, + * how to compare a key for equality, and how to clean up keys and values. + * SDL provides a few generic functions that can be used for these callbacks: + * + * - SDL_HashString and SDL_KeyMatchString for C strings. + * - SDL_HashPointer and SDL_KeyMatchPointer for generic pointers. + * - SDL_HashID and SDL_KeyMatchID for generic (possibly small) integers. + * + * Oftentimes, these are all you need for any hash table, but depending on + * your dataset, custom implementations might make more sense. + * + * You can specify an estimate of the number of items expected to be stored + * in the table, which can help make the table run more efficiently. The table + * will preallocate resources to accomodate this number of items, which is + * most useful if you intend to fill the table with a lot of data right after + * creating it. Otherwise, it might make more sense to specify the _minimum_ + * you expect the table to hold and let it grow as necessary from there. This + * number is only a hint, and the table will be able to handle any amount of + * data--as long as the system doesn't run out of resources--so a perfect + * answer is not required. A value of 0 signifies no guess at all, and the + * table will start small and reallocate as necessary; often this is the + * correct thing to do. + * + * !!! FIXME: add note about `threadsafe` here. And update `threadsafety` tags. + * !!! FIXME: note that `threadsafe` tables can't be recursively locked, so + * !!! FIXME: you can't use `destroy` callbacks that might end up relocking. + * + * Note that SDL provides a higher-level option built on its hash tables: + * SDL_PropertiesID lets you map strings to various datatypes, and this + * might be easier to use. It only allows strings for keys, however. Those are + * created with SDL_CreateProperties(). + * + * The returned hash table should be destroyed with SDL_DestroyHashTable() + * when no longer needed. + * + * \param estimated_capacity the approximate maximum number of items to be held + * in the hash table, or 0 for no estimate. + * \param threadsafe true to create an internal rwlock for this table. + * \param hash the function to use to hash keys. + * \param keymatch the function to use to compare keys. + * \param destroy the function to use to clean up keys and values, may be NULL. + * \param userdata a pointer that is passed to the callbacks. + * \returns a newly-created hash table, or NULL if there was an error; call + * SDL_GetError() for more information. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_DestroyHashTable + */ +extern SDL_HashTable * SDL_CreateHashTable(int estimated_capacity, + bool threadsafe, + SDL_HashCallback hash, + SDL_HashKeyMatchCallback keymatch, + SDL_HashDestroyCallback destroy, + void *userdata); + + +/** + * Destroy a hash table. + * + * This will call the hash table's SDL_HashDestroyCallback for each item in + * the table, removing all inserted items, before deallocating the table + * itself. + * + * The table becomes invalid once this function is called, and no other thread + * should be accessing this table once this function has started. + * + * \param table the hash table to destroy. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + */ extern void SDL_DestroyHashTable(SDL_HashTable *table); -// This function is thread-safe if the hashtable was created with threadsafe = true -extern bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *value); +/** + * Add an item to a hash table. + * + * All keys in the table must be unique. If attempting to insert a key that + * already exists in the hash table, what will be done depends on the + * `replace` value: + * + * - If `replace` is false, this function will return false without modifying + * the table. + * - If `replace` is true, SDL will remove the previous item first, so the new + * value is the only one associated with that key. This will call the hash + * table's SDL_HashDestroyCallback for the previous item. + * + * \param table the hash table to insert into. + * \param key the key of the new item to insert. + * \param value the value of the new item to insert. + * \param replace true if a duplicate key should replace the previous value. + * \returns true if the new item was inserted, false otherwise. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + */ +extern bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *value, bool replace); -// This function is thread-safe if the hashtable was created with threadsafe = true +/** + * Look up an item in a hash table. + * + * On return, the value associated with `key` is stored to `*value`. + * If the key does not exist in the table, `*value` will be set to NULL. + * + * It is legal for `value` to be NULL, to not retrieve the key's value. In + * this case, the return value is still useful for reporting if the key exists + * in the table at all. + * + * \param table the hash table to search. + * \param key the key to search for in the table. + * \param value the found value will be stored here. Can be NULL. + * \returns true if key exists in the table, false otherwise. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_InsertIntoHashTable + */ +extern bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void **value); + +/** + * Remove an item from a hash table. + * + * If there is an item that matches `key`, it is removed from the table. This + * will call the hash table's SDL_HashDestroyCallback for the item to be + * removed. + * + * \param table the hash table to remove from. + * \param key the key of the item to remove from the table. + * \returns true if a key was removed, false if the key was not found. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + */ extern bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key); -// This function is thread-safe if the hashtable was created with threadsafe = true -extern bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void **_value); +/** + * Remove all items in a hash table. + * + * This will call the hash table's SDL_HashDestroyCallback for each item in + * the table, removing all inserted items. + * + * When this function returns, the hash table will be empty. + * + * \param table the hash table to clear. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + */ +extern void SDL_ClearHashTable(SDL_HashTable *table); -// This function is thread-safe if the hashtable was created with threadsafe = true +/** + * Check if any items are currently stored in a hash table. + * + * If there are no items stored (the table is completely empty), this will + * return true. + * + * \param table the hash table to check. + * \returns true if the table is completely empty, false otherwise. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_ClearHashTable + */ extern bool SDL_HashTableEmpty(SDL_HashTable *table); -// iterate all values for a specific key. This only makes sense if the hash is stackable. If not-stackable, just use SDL_FindInHashTable(). -// This function is not thread-safe, you should use external locking if you use this function -extern bool SDL_IterateHashTableKey(const SDL_HashTable *table, const void *key, const void **_value, void **iter); +/** + * Iterate all key/value pairs in a hash table. + * + * This function will call `callback` once for each key/value pair in the + * table, until either all pairs have been presented to the callback, or the + * callback has returned false to signal it is done. + * + * There is no guarantee what order results will be returned in. + * + * \param table the hash table to iterate. + * \param callback the function pointer to call for each value. + * \param userdata a pointer that is passed to `callback`. + * \returns true if iteration happened, false if not (bogus parameter, etc). + * + * \since This function is available since SDL 3.4.0. + */ +extern bool SDL_IterateHashTable(const SDL_HashTable *table, SDL_HashTableIterateCallback callback, void *userdata); -// iterate all key/value pairs in a hash (stackable hashes can have duplicate keys with multiple values). -// This function is not thread-safe, you should use external locking if you use this function -extern bool SDL_IterateHashTable(const SDL_HashTable *table, const void **_key, const void **_value, void **iter); -extern Uint32 SDL_HashPointer(const void *key, void *unused); -extern bool SDL_KeyMatchPointer(const void *a, const void *b, void *unused); +/* Helper functions for SDL_CreateHashTable callbacks... */ -extern Uint32 SDL_HashString(const void *key, void *unused); -extern bool SDL_KeyMatchString(const void *a, const void *b, void *unused); +/** + * Generate a hash from a generic pointer. + * + * The key is intended to be a unique pointer to any datatype. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of keys to be used with the hash table. + * + * Note that the implementation may change in the future; do not expect + * the results to be stable vs future SDL releases. Use this in a hash table + * in the current process and don't store them to disk for the future. + * + * \param unused this parameter is ignored. + * \param key the key to hash as a generic pointer. + * \returns a 32-bit hash of the key. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern Uint32 SDL_HashPointer(void *unused, const void *key); -extern Uint32 SDL_HashID(const void *key, void *unused); -extern bool SDL_KeyMatchID(const void *a, const void *b, void *unused); +/** + * Compare two generic pointers as hash table keys. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of keys to be used with the hash table. + * + * \param unused this parameter is ignored. + * \param a the first generic pointer to compare. + * \param b the second generic pointer to compare. + * \returns true if the pointers are the same, false otherwise. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern bool SDL_KeyMatchPointer(void *unused, const void *a, const void *b); -extern void SDL_NukeFreeKey(const void *key, const void *value, void *unused); -extern void SDL_NukeFreeValue(const void *key, const void *value, void *unused); +/** + * Generate a hash from a C string. + * + * The key is intended to be a NULL-terminated string, in UTF-8 format. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of keys to be used with the hash table. + * + * Note that the implementation may change in the future; do not expect + * the results to be stable vs future SDL releases. Use this in a hash table + * in the current process and don't store them to disk for the future. + * + * \param unused this parameter is ignored. + * \param key the key to hash as a generic pointer. + * \returns a 32-bit hash of the key. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern Uint32 SDL_HashString(void *unused, const void *key); -#endif // SDL_hashtable_h_ +/** + * Compare two C strings as hash table keys. + * + * Strings will be compared in a case-sensitive manner. More specifically, + * they'll be compared as NULL-terminated arrays of bytes. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of keys to be used with the hash table. + * + * \param unused this parameter is ignored. + * \param a the first string to compare. + * \param b the second string to compare. + * \returns true if the strings are the same, false otherwise. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern bool SDL_KeyMatchString(void *unused, const void *a, const void *b); + +/** + * Generate a hash from an integer ID. + * + * The key is intended to a unique integer, possibly within a small range. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of keys to be used with the hash table. + * + * Note that the implementation may change in the future; do not expect + * the results to be stable vs future SDL releases. Use this in a hash table + * in the current process and don't store them to disk for the future. + * + * \param unused this parameter is ignored. + * \param key the key to hash as a generic pointer. + * \returns a 32-bit hash of the key. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern Uint32 SDL_HashID(void *unused, const void *key); + +/** + * Compare two integer IDs as hash table keys. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of keys to be used with the hash table. + * + * \param unused this parameter is ignored. + * \param a the first ID to compare. + * \param b the second ID to compare. + * \returns true if the IDs are the same, false otherwise. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern bool SDL_KeyMatchID(void *unused, const void *a, const void *b); + +/** + * Free both the key and value pointers of a hash table item. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of data to be used with the hash table. + * + * This literally calls `SDL_free(key);` and `SDL_free(value);`. + * + * \param unused this parameter is ignored. + * \param key the key to be destroyed. + * \param value the value to be destroyed. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern void SDL_DestroyHashKeyAndValue(void *unused, const void *key, const void *value); + +/** + * Free just the value pointer of a hash table item. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of data to be used with the hash table. + * + * This literally calls `SDL_free(key);` and leaves `value` alone. + * + * \param unused this parameter is ignored. + * \param key the key to be destroyed. + * \param value the value to be destroyed. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern void SDL_DestroyHashKey(void *unused, const void *key, const void *value); + +/** + * Free just the value pointer of a hash table item. + * + * This is intended to be used as one of the callbacks to SDL_CreateHashTable, + * if this is useful to the type of data to be used with the hash table. + * + * This literally calls `SDL_free(value);` and leaves `key` alone. + * + * \param unused this parameter is ignored. + * \param key the key to be destroyed. + * \param value the value to be destroyed. + * + * \threadsafety It is safe to call this function from any thread. + * + * \since This function is available since SDL 3.4.0. + * + * \sa SDL_CreateHashTable + */ +extern void SDL_DestroyHashValue(void *unused, const void *key, const void *value); + + +/* Ends C function definitions when using C++ */ +#ifdef __cplusplus +} +#endif +#include + +#endif /* SDL_hashtable_h_ */ diff --git a/src/SDL_properties.c b/src/SDL_properties.c index 41c81e9e59..caadaeb820 100644 --- a/src/SDL_properties.c +++ b/src/SDL_properties.c @@ -76,7 +76,7 @@ static void SDL_FreePropertyWithCleanup(const void *key, const void *value, void SDL_free((void *)value); } -static void SDL_FreeProperty(const void *key, const void *value, void *data) +static void SDLCALL SDL_FreeProperty(void *data, const void *key, const void *value) { SDL_FreePropertyWithCleanup(key, value, data, true); } @@ -84,14 +84,8 @@ static void SDL_FreeProperty(const void *key, const void *value, void *data) static void SDL_FreeProperties(SDL_Properties *properties) { if (properties) { - if (properties->props) { - SDL_DestroyHashTable(properties->props); - properties->props = NULL; - } - if (properties->lock) { - SDL_DestroyMutex(properties->lock); - properties->lock = NULL; - } + SDL_DestroyHashTable(properties->props); + SDL_DestroyMutex(properties->lock); SDL_free(properties); } } @@ -102,18 +96,16 @@ bool SDL_InitProperties(void) return true; } - SDL_properties = SDL_CreateHashTable(NULL, 16, SDL_HashID, SDL_KeyMatchID, NULL, true, false); - if (!SDL_properties) { - goto error; - } + SDL_properties = SDL_CreateHashTable(0, true, SDL_HashID, SDL_KeyMatchID, NULL, NULL); + const bool initialized = (SDL_properties != NULL); + SDL_SetInitialized(&SDL_properties_init, initialized); + return initialized; +} - SDL_SetInitialized(&SDL_properties_init, true); - return true; - -error: - SDL_SetInitialized(&SDL_properties_init, true); - SDL_QuitProperties(); - return false; +static bool SDLCALL FreeOneProperties(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + SDL_FreeProperties((SDL_Properties *)value); + return true; // keep iterating. } void SDL_QuitProperties(void) @@ -131,17 +123,12 @@ void SDL_QuitProperties(void) SDL_DestroyProperties(props); } - if (SDL_properties) { - void *iter; - const void *key, *value; - - iter = NULL; - while (SDL_IterateHashTable(SDL_properties, &key, &value, &iter)) { - SDL_FreeProperties((SDL_Properties *)value); - } - SDL_DestroyHashTable(SDL_properties); - SDL_properties = NULL; - } + // this can't just DestroyHashTable with SDL_FreeProperties as the destructor, because + // other destructors under this might cause use to attempt a recursive lock on SDL_properties, + // which isn't allowed with rwlocks. So manually iterate and free everything. + SDL_IterateHashTable(SDL_properties, FreeOneProperties, NULL); + SDL_DestroyHashTable(SDL_properties); + SDL_properties = NULL; SDL_SetInitialized(&SDL_properties_init, false); } @@ -167,55 +154,101 @@ SDL_PropertiesID SDL_GetGlobalProperties(void) SDL_PropertiesID SDL_CreateProperties(void) { - SDL_PropertiesID props = 0; - SDL_Properties *properties = NULL; - bool inserted = false; - if (!SDL_CheckInitProperties()) { return 0; } - properties = (SDL_Properties *)SDL_calloc(1, sizeof(*properties)); + SDL_Properties *properties = (SDL_Properties *)SDL_calloc(1, sizeof(*properties)); if (!properties) { - goto error; - } - properties->props = SDL_CreateHashTable(NULL, 4, SDL_HashString, SDL_KeyMatchString, SDL_FreeProperty, false, false); - if (!properties->props) { - goto error; + return 0; } - // If this fails we'll continue without it. properties->lock = SDL_CreateMutex(); + if (!properties->lock) { + SDL_free(properties); + return 0; + } - for ( ; ; ) { + properties->props = SDL_CreateHashTable(0, false, SDL_HashString, SDL_KeyMatchString, SDL_FreeProperty, NULL); + if (!properties->props) { + SDL_DestroyMutex(properties->lock); + SDL_free(properties); + return 0; + } + + SDL_PropertiesID props = 0; + while (true) { props = (SDL_GetAtomicU32(&SDL_last_properties_id) + 1); if (props == 0) { continue; - } - if (SDL_CompareAndSwapAtomicU32(&SDL_last_properties_id, props - 1, props)) { + } else if (SDL_CompareAndSwapAtomicU32(&SDL_last_properties_id, props - 1, props)) { break; } } - if (SDL_InsertIntoHashTable(SDL_properties, (const void *)(uintptr_t)props, properties)) { - inserted = true; + + SDL_assert(!SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, NULL)); // should NOT be in the hash table already. + + if (!SDL_InsertIntoHashTable(SDL_properties, (const void *)(uintptr_t)props, properties, false)) { + SDL_FreeProperties(properties); + return 0; } - if (inserted) { - // All done! - return props; + return props; // All done! +} + +typedef struct CopyOnePropertyData +{ + SDL_Properties *dst_properties; + bool result; +} CopyOnePropertyData; + +static bool SDLCALL CopyOneProperty(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + const SDL_Property *src_property = (const SDL_Property *)value; + if (src_property->cleanup) { + // Can't copy properties with cleanup functions, we don't know how to duplicate the data + return true; // keep iterating. } -error: - SDL_FreeProperties(properties); - return 0; + CopyOnePropertyData *data = (CopyOnePropertyData *) userdata; + SDL_Properties *dst_properties = data->dst_properties; + const char *src_name = (const char *)key; + SDL_Property *dst_property; + + char *dst_name = SDL_strdup(src_name); + if (!dst_name) { + data->result = false; + return true; // keep iterating (I guess...?) + } + + dst_property = (SDL_Property *)SDL_malloc(sizeof(*dst_property)); + if (!dst_property) { + SDL_free(dst_name); + data->result = false; + return true; // keep iterating (I guess...?) + } + + SDL_copyp(dst_property, src_property); + if (src_property->type == SDL_PROPERTY_TYPE_STRING) { + dst_property->value.string_value = SDL_strdup(src_property->value.string_value); + if (!dst_property->value.string_value) { + SDL_free(dst_name); + SDL_free(dst_property); + data->result = false; + return true; // keep iterating (I guess...?) + } + } + + if (!SDL_InsertIntoHashTable(dst_properties->props, dst_name, dst_property, true)) { + SDL_FreePropertyWithCleanup(dst_name, dst_property, NULL, false); + data->result = false; + } + + return true; // keep iterating. } bool SDL_CopyProperties(SDL_PropertiesID src, SDL_PropertiesID dst) { - SDL_Properties *src_properties = NULL; - SDL_Properties *dst_properties = NULL; - bool result = true; - if (!src) { return SDL_InvalidParamError("src"); } @@ -223,55 +256,25 @@ bool SDL_CopyProperties(SDL_PropertiesID src, SDL_PropertiesID dst) return SDL_InvalidParamError("dst"); } + SDL_Properties *src_properties = NULL; + SDL_Properties *dst_properties = NULL; + SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)src, (const void **)&src_properties); - SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)dst, (const void **)&dst_properties); if (!src_properties) { return SDL_InvalidParamError("src"); } + SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)dst, (const void **)&dst_properties); if (!dst_properties) { return SDL_InvalidParamError("dst"); } + bool result = true; SDL_LockMutex(src_properties->lock); SDL_LockMutex(dst_properties->lock); { - void *iter; - const void *key, *value; - - iter = NULL; - while (SDL_IterateHashTable(src_properties->props, &key, &value, &iter)) { - const char *src_name = (const char *)key; - const SDL_Property *src_property = (const SDL_Property *)value; - char *dst_name; - SDL_Property *dst_property; - - if (src_property->cleanup) { - // Can't copy properties with cleanup functions, we don't know how to duplicate the data - continue; - } - - SDL_RemoveFromHashTable(dst_properties->props, src_name); - - dst_name = SDL_strdup(src_name); - if (!dst_name) { - result = false; - continue; - } - dst_property = (SDL_Property *)SDL_malloc(sizeof(*dst_property)); - if (!dst_property) { - SDL_free(dst_name); - result = false; - continue; - } - SDL_copyp(dst_property, src_property); - if (src_property->type == SDL_PROPERTY_TYPE_STRING) { - dst_property->value.string_value = SDL_strdup(src_property->value.string_value); - } - if (!SDL_InsertIntoHashTable(dst_properties->props, dst_name, dst_property)) { - SDL_FreePropertyWithCleanup(dst_name, dst_property, NULL, false); - result = false; - } - } + CopyOnePropertyData data = { dst_properties, true }; + SDL_IterateHashTable(src_properties->props, CopyOneProperty, &data); + result = data.result; } SDL_UnlockMutex(dst_properties->lock); SDL_UnlockMutex(src_properties->lock); @@ -337,7 +340,7 @@ static bool SDL_PrivateSetProperty(SDL_PropertiesID props, const char *name, SDL SDL_RemoveFromHashTable(properties->props, name); if (property) { char *key = SDL_strdup(name); - if (!SDL_InsertIntoHashTable(properties->props, key, property)) { + if (!key || !SDL_InsertIntoHashTable(properties->props, key, property, false)) { SDL_FreePropertyWithCleanup(key, property, NULL, true); result = false; } @@ -518,10 +521,9 @@ void *SDL_GetPointerProperty(SDL_PropertiesID props, const char *name, void *def return value; } - /* Note that taking the lock here only guarantees that we won't read the - * hashtable while it's being modified. The value itself can easily be - * freed from another thread after it is returned here. - */ + // Note that taking the lock here only guarantees that we won't read the + // hashtable while it's being modified. The value itself can easily be + // freed from another thread after it is returned here. SDL_LockMutex(properties->lock); { SDL_Property *property = NULL; @@ -731,6 +733,23 @@ bool SDL_ClearProperty(SDL_PropertiesID props, const char *name) return SDL_PrivateSetProperty(props, name, NULL); } +typedef struct EnumerateOnePropertyData +{ + SDL_EnumeratePropertiesCallback callback; + void *userdata; + SDL_PropertiesID props; +} EnumerateOnePropertyData; + + +static bool SDLCALL EnumerateOneProperty(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + (void) table; + (void) value; + const EnumerateOnePropertyData *data = (const EnumerateOnePropertyData *) userdata; + data->callback(data->userdata, data->props, (const char *)key); + return true; // keep iterating. +} + bool SDL_EnumerateProperties(SDL_PropertiesID props, SDL_EnumeratePropertiesCallback callback, void *userdata) { SDL_Properties *properties = NULL; @@ -749,13 +768,8 @@ bool SDL_EnumerateProperties(SDL_PropertiesID props, SDL_EnumeratePropertiesCall SDL_LockMutex(properties->lock); { - void *iter; - const void *key, *value; - - iter = NULL; - while (SDL_IterateHashTable(properties->props, &key, &value, &iter)) { - callback(userdata, props, (const char *)key); - } + EnumerateOnePropertyData data = { callback, userdata, props }; + SDL_IterateHashTable(properties->props, EnumerateOneProperty, &data); } SDL_UnlockMutex(properties->lock); @@ -796,14 +810,14 @@ bool SDL_DumpProperties(SDL_PropertiesID props) void SDL_DestroyProperties(SDL_PropertiesID props) { - SDL_Properties *properties = NULL; - - if (!props) { - return; - } - - if (SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties)) { - SDL_FreeProperties(properties); - SDL_RemoveFromHashTable(SDL_properties, (const void *)(uintptr_t)props); + if (props) { + // this can't just use RemoveFromHashTable with SDL_FreeProperties as the destructor, because + // other destructors under this might cause use to attempt a recursive lock on SDL_properties, + // which isn't allowed with rwlocks. So manually look it up and remove/free it. + SDL_Properties *properties = NULL; + if (SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties)) { + SDL_FreeProperties(properties); + SDL_RemoveFromHashTable(SDL_properties, (const void *)(uintptr_t)props); + } } } diff --git a/src/SDL_utils.c b/src/SDL_utils.c index fb82aac14b..b9a42e2370 100644 --- a/src/SDL_utils.c +++ b/src/SDL_utils.c @@ -135,12 +135,12 @@ Uint32 SDL_GetNextObjectID(void) static SDL_InitState SDL_objects_init; static SDL_HashTable *SDL_objects; -static Uint32 SDL_HashObject(const void *key, void *unused) +static Uint32 SDLCALL SDL_HashObject(void *unused, const void *key) { return (Uint32)(uintptr_t)key; } -static bool SDL_KeyMatchObject(const void *a, const void *b, void *unused) +static bool SDL_KeyMatchObject(void *unused, const void *a, const void *b) { return (a == b); } @@ -149,16 +149,17 @@ void SDL_SetObjectValid(void *object, SDL_ObjectType type, bool valid) { SDL_assert(object != NULL); - if (valid && SDL_ShouldInit(&SDL_objects_init)) { - SDL_objects = SDL_CreateHashTable(NULL, 32, SDL_HashObject, SDL_KeyMatchObject, NULL, true, false); - if (!SDL_objects) { - SDL_SetInitialized(&SDL_objects_init, false); + if (SDL_ShouldInit(&SDL_objects_init)) { + SDL_objects = SDL_CreateHashTable(0, true, SDL_HashObject, SDL_KeyMatchObject, NULL, NULL); + const bool initialized = (SDL_objects != NULL); + SDL_SetInitialized(&SDL_objects_init, initialized); + if (!initialized) { + return; } - SDL_SetInitialized(&SDL_objects_init, true); } if (valid) { - SDL_InsertIntoHashTable(SDL_objects, object, (void *)(uintptr_t)type); + SDL_InsertIntoHashTable(SDL_objects, object, (void *)(uintptr_t)type, true); } else { SDL_RemoveFromHashTable(SDL_objects, object); } @@ -178,75 +179,65 @@ bool SDL_ObjectValid(void *object, SDL_ObjectType type) return (((SDL_ObjectType)(uintptr_t)object_type) == type); } +typedef struct GetOneObjectData +{ + const SDL_ObjectType type; + void **objects; + const int count; + int num_objects; +} GetOneObjectData; + +static bool SDLCALL GetOneObject(void *userdata, const SDL_HashTable *table, const void *object, const void *object_type) +{ + GetOneObjectData *data = (GetOneObjectData *) userdata; + if ((SDL_ObjectType)(uintptr_t)object_type == data->type) { + if (data->num_objects < data->count) { + data->objects[data->num_objects] = (void *)object; + } + ++data->num_objects; + } + return true; // keep iterating. +} + + int SDL_GetObjects(SDL_ObjectType type, void **objects, int count) { - const void *object, *object_type; - void *iter = NULL; - int num_objects = 0; - while (SDL_IterateHashTable(SDL_objects, &object, &object_type, &iter)) { - if ((SDL_ObjectType)(uintptr_t)object_type == type) { - if (num_objects < count) { - objects[num_objects] = (void *)object; - } - ++num_objects; - } + GetOneObjectData data = { type, objects, count, 0 }; + SDL_IterateHashTable(SDL_objects, GetOneObject, &data); + return data.num_objects; +} + +static bool SDLCALL LogOneLeakedObject(void *userdata, const SDL_HashTable *table, const void *object, const void *object_type) +{ + const char *type = "unknown object"; + switch ((SDL_ObjectType)(uintptr_t)object_type) { + #define SDLOBJTYPECASE(typ, name) case SDL_OBJECT_TYPE_##typ: type = name; break + SDLOBJTYPECASE(WINDOW, "SDL_Window"); + SDLOBJTYPECASE(RENDERER, "SDL_Renderer"); + SDLOBJTYPECASE(TEXTURE, "SDL_Texture"); + SDLOBJTYPECASE(JOYSTICK, "SDL_Joystick"); + SDLOBJTYPECASE(GAMEPAD, "SDL_Gamepad"); + SDLOBJTYPECASE(HAPTIC, "SDL_Haptic"); + SDLOBJTYPECASE(SENSOR, "SDL_Sensor"); + SDLOBJTYPECASE(HIDAPI_DEVICE, "hidapi device"); + SDLOBJTYPECASE(HIDAPI_JOYSTICK, "hidapi joystick"); + SDLOBJTYPECASE(THREAD, "thread"); + SDLOBJTYPECASE(TRAY, "SDL_Tray"); + #undef SDLOBJTYPECASE + default: break; } - return num_objects; + SDL_Log("Leaked %s (%p)", type, object); + return true; // keep iterating. } void SDL_SetObjectsInvalid(void) { if (SDL_ShouldQuit(&SDL_objects_init)) { // Log any leaked objects - const void *object, *object_type; - void *iter = NULL; - while (SDL_IterateHashTable(SDL_objects, &object, &object_type, &iter)) { - const char *type; - switch ((SDL_ObjectType)(uintptr_t)object_type) { - case SDL_OBJECT_TYPE_WINDOW: - type = "SDL_Window"; - break; - case SDL_OBJECT_TYPE_RENDERER: - type = "SDL_Renderer"; - break; - case SDL_OBJECT_TYPE_TEXTURE: - type = "SDL_Texture"; - break; - case SDL_OBJECT_TYPE_JOYSTICK: - type = "SDL_Joystick"; - break; - case SDL_OBJECT_TYPE_GAMEPAD: - type = "SDL_Gamepad"; - break; - case SDL_OBJECT_TYPE_HAPTIC: - type = "SDL_Haptic"; - break; - case SDL_OBJECT_TYPE_SENSOR: - type = "SDL_Sensor"; - break; - case SDL_OBJECT_TYPE_HIDAPI_DEVICE: - type = "hidapi device"; - break; - case SDL_OBJECT_TYPE_HIDAPI_JOYSTICK: - type = "hidapi joystick"; - break; - case SDL_OBJECT_TYPE_THREAD: - type = "thread"; - break; - case SDL_OBJECT_TYPE_TRAY: - type = "SDL_Tray"; - break; - default: - type = "unknown object"; - break; - } - SDL_Log("Leaked %s (%p)", type, object); - } + SDL_IterateHashTable(SDL_objects, LogOneLeakedObject, NULL); SDL_assert(SDL_HashTableEmpty(SDL_objects)); - SDL_DestroyHashTable(SDL_objects); SDL_objects = NULL; - SDL_SetInitialized(&SDL_objects_init, false); } } @@ -384,7 +375,7 @@ const char *SDL_GetPersistentString(const char *string) SDL_HashTable *strings = (SDL_HashTable *)SDL_GetTLS(&SDL_string_storage); if (!strings) { - strings = SDL_CreateHashTable(NULL, 32, SDL_HashString, SDL_KeyMatchString, SDL_NukeFreeValue, false, false); + strings = SDL_CreateHashTable(0, false, SDL_HashString, SDL_KeyMatchString, SDL_DestroyHashValue, NULL); if (!strings) { return NULL; } @@ -400,7 +391,7 @@ const char *SDL_GetPersistentString(const char *string) } // If the hash table insert fails, at least we can return the string we allocated - SDL_InsertIntoHashTable(strings, new_string, new_string); + SDL_InsertIntoHashTable(strings, new_string, new_string, false); result = new_string; } return result; diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c index 38b7e16ca1..f9d80da5d0 100644 --- a/src/audio/SDL_audio.c +++ b/src/audio/SDL_audio.c @@ -651,7 +651,7 @@ static SDL_AudioDevice *CreatePhysicalAudioDevice(const char *name, bool recordi device->instance_id = AssignAudioDeviceInstanceId(recording, /*islogical=*/false); SDL_LockRWLockForWriting(current_audio.device_hash_lock); - if (SDL_InsertIntoHashTable(current_audio.device_hash, (const void *) (uintptr_t) device->instance_id, device)) { + if (SDL_InsertIntoHashTable(current_audio.device_hash, (const void *) (uintptr_t) device->instance_id, device, false)) { SDL_AddAtomicInt(device_count, 1); } else { SDL_DestroyCondition(device->close_cond); @@ -865,50 +865,47 @@ static void CompleteAudioEntryPoints(void) #undef FILL_STUB } -static SDL_AudioDevice *GetFirstAddedAudioDevice(const bool recording) +typedef struct FindLowestDeviceIDData { - SDL_AudioDeviceID highest = (SDL_AudioDeviceID) SDL_AUDIO_DEVICE_DEFAULT_PLAYBACK; // According to AssignAudioDeviceInstanceId, nothing can have a value this large. - SDL_AudioDevice *result = NULL; + const bool recording; + SDL_AudioDeviceID highest; + SDL_AudioDevice *result; +} FindLowestDeviceIDData; - // (Device IDs increase as new devices are added, so the first device added has the lowest SDL_AudioDeviceID value.) - SDL_LockRWLockForReading(current_audio.device_hash_lock); - - const void *key; - const void *value; - void *iter = NULL; - while (SDL_IterateHashTable(current_audio.device_hash, &key, &value, &iter)) { - const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key; - // bit #0 of devid is set for playback devices and unset for recording. - // bit #1 of devid is set for physical devices and unset for logical. - const bool devid_recording = !(devid & (1 << 0)); - const bool isphysical = !!(devid & (1 << 1)); - if (isphysical && (devid_recording == recording) && (devid < highest)) { - highest = devid; - result = (SDL_AudioDevice *) value; - } +static bool SDLCALL FindLowestDeviceID(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + FindLowestDeviceIDData *data = (FindLowestDeviceIDData *) userdata; + const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key; + // bit #0 of devid is set for playback devices and unset for recording. + // bit #1 of devid is set for physical devices and unset for logical. + const bool devid_recording = !(devid & (1 << 0)); + const bool isphysical = !!(devid & (1 << 1)); + if (isphysical && (devid_recording == data->recording) && (devid < data->highest)) { + data->highest = devid; + data->result = (SDL_AudioDevice *) value; } - - SDL_UnlockRWLock(current_audio.device_hash_lock); - return result; + return true; // keep iterating. } -static Uint32 HashAudioDeviceID(const void *key, void *data) +static SDL_AudioDevice *GetFirstAddedAudioDevice(const bool recording) +{ + const SDL_AudioDeviceID highest = (SDL_AudioDeviceID) SDL_AUDIO_DEVICE_DEFAULT_PLAYBACK; // According to AssignAudioDeviceInstanceId, nothing can have a value this large. + + // (Device IDs increase as new devices are added, so the first device added has the lowest SDL_AudioDeviceID value.) + FindLowestDeviceIDData data = { recording, highest, NULL }; + SDL_LockRWLockForReading(current_audio.device_hash_lock); + SDL_IterateHashTable(current_audio.device_hash, FindLowestDeviceID, &data); + SDL_UnlockRWLock(current_audio.device_hash_lock); + return data.result; +} + +static Uint32 SDLCALL HashAudioDeviceID(void *userdata, const void *key) { // shift right 2, to dump the first two bits, since these are flags // (recording vs playback, logical vs physical) and the rest are unique incrementing integers. return ((Uint32) ((uintptr_t) key)) >> 2; } -static bool MatchAudioDeviceID(const void *a, const void *b, void *data) -{ - return (a == b); -} - -static void NukeAudioDeviceHashItem(const void *key, const void *value, void *data) -{ - // no-op, keys and values in this hashtable are treated as Plain Old Data and don't get freed here. -} - // !!! FIXME: the video subsystem does SDL_VideoInit, not SDL_InitVideo. Make this match. bool SDL_InitAudio(const char *driver_name) { @@ -927,7 +924,7 @@ bool SDL_InitAudio(const char *driver_name) return false; } - SDL_HashTable *device_hash = SDL_CreateHashTable(NULL, 8, HashAudioDeviceID, MatchAudioDeviceID, NukeAudioDeviceHashItem, false, false); + SDL_HashTable *device_hash = SDL_CreateHashTable(0, false, HashAudioDeviceID, SDL_KeyMatchID, NULL, NULL); if (!device_hash) { SDL_DestroyRWLock(device_hash_lock); return false; @@ -1048,6 +1045,17 @@ bool SDL_InitAudio(const char *driver_name) return true; } +static bool SDLCALL DestroyOnePhysicalAudioDevice(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + // bit #1 of devid is set for physical devices and unset for logical. + const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key; + const bool isphysical = !!(devid & (1<<1)); + if (isphysical) { + DestroyPhysicalAudioDevice((SDL_AudioDevice *) value); + } + return true; // keep iterating. +} + void SDL_QuitAudio(void) { if (!current_audio.name) { // not initialized?! @@ -1077,17 +1085,7 @@ void SDL_QuitAudio(void) SDL_free(i); } - const void *key; - const void *value; - void *iter = NULL; - while (SDL_IterateHashTable(device_hash, &key, &value, &iter)) { - // bit #1 of devid is set for physical devices and unset for logical. - const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key; - const bool isphysical = !!(devid & (1<<1)); - if (isphysical) { - DestroyPhysicalAudioDevice((SDL_AudioDevice *) value); - } - } + SDL_IterateHashTable(device_hash, DestroyOnePhysicalAudioDevice, NULL); // Free the driver data current_audio.impl.Deinitialize(); @@ -1384,6 +1382,28 @@ static int SDLCALL RecordingAudioThread(void *devicep) // thread entry point return 0; } +typedef struct CountAudioDevicesData +{ + int devs_seen; + const int num_devices; + SDL_AudioDeviceID *result; + const bool recording; +} CountAudioDevicesData; + +static bool SDLCALL CountAudioDevices(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + CountAudioDevicesData *data = (CountAudioDevicesData *) userdata; + const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key; + // bit #0 of devid is set for playback devices and unset for recording. + // bit #1 of devid is set for physical devices and unset for logical. + const bool devid_recording = !(devid & (1<<0)); + const bool isphysical = !!(devid & (1<<1)); + if (isphysical && (devid_recording == data->recording)) { + SDL_assert(data->devs_seen < data->num_devices); + data->result[data->devs_seen++] = devid; + } + return true; // keep iterating. +} static SDL_AudioDeviceID *GetAudioDevices(int *count, bool recording) { @@ -1396,24 +1416,10 @@ static SDL_AudioDeviceID *GetAudioDevices(int *count, bool recording) num_devices = SDL_GetAtomicInt(recording ? ¤t_audio.recording_device_count : ¤t_audio.playback_device_count); result = (SDL_AudioDeviceID *) SDL_malloc((num_devices + 1) * sizeof (SDL_AudioDeviceID)); if (result) { - int devs_seen = 0; - const void *key; - const void *value; - void *iter = NULL; - while (SDL_IterateHashTable(current_audio.device_hash, &key, &value, &iter)) { - const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key; - // bit #0 of devid is set for playback devices and unset for recording. - // bit #1 of devid is set for physical devices and unset for logical. - const bool devid_recording = !(devid & (1<<0)); - const bool isphysical = !!(devid & (1<<1)); - if (isphysical && (devid_recording == recording)) { - SDL_assert(devs_seen < num_devices); - result[devs_seen++] = devid; - } - } - - SDL_assert(devs_seen == num_devices); - result[devs_seen] = 0; // null-terminated. + CountAudioDevicesData data = { 0, num_devices, result, recording }; + SDL_IterateHashTable(current_audio.device_hash, CountAudioDevices, &data); + SDL_assert(data.devs_seen == num_devices); + result[data.devs_seen] = 0; // null-terminated. } } SDL_UnlockRWLock(current_audio.device_hash_lock); @@ -1441,7 +1447,30 @@ SDL_AudioDeviceID *SDL_GetAudioRecordingDevices(int *count) return GetAudioDevices(count, true); } +typedef struct FindAudioDeviceByCallbackData +{ + bool (*callback)(SDL_AudioDevice *device, void *userdata); + void *userdata; + SDL_AudioDevice *retval; +} FindAudioDeviceByCallbackData; +static bool SDLCALL FindAudioDeviceByCallback(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + FindAudioDeviceByCallbackData *data = (FindAudioDeviceByCallbackData *) userdata; + const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key; + // bit #1 of devid is set for physical devices and unset for logical. + const bool isphysical = !!(devid & (1<<1)); + if (isphysical) { + SDL_AudioDevice *device = (SDL_AudioDevice *) value; + if (data->callback(device, data->userdata)) { // found it? + data->retval = device; + return false; // stop iterating, we found it. + } + } + return true; // keep iterating. +} + +// !!! FIXME: SDL convention is for userdata to come first in the callback's params. Fix this at some point. SDL_AudioDevice *SDL_FindPhysicalAudioDeviceByCallback(bool (*callback)(SDL_AudioDevice *device, void *userdata), void *userdata) { if (!SDL_GetCurrentAudioDriver()) { @@ -1449,27 +1478,16 @@ SDL_AudioDevice *SDL_FindPhysicalAudioDeviceByCallback(bool (*callback)(SDL_Audi return NULL; } - const void *key; - const void *value; - void *iter = NULL; - + FindAudioDeviceByCallbackData data = { callback, userdata, NULL }; SDL_LockRWLockForReading(current_audio.device_hash_lock); - while (SDL_IterateHashTable(current_audio.device_hash, &key, &value, &iter)) { - const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key; - // bit #1 of devid is set for physical devices and unset for logical. - const bool isphysical = !!(devid & (1<<1)); - if (isphysical) { - SDL_AudioDevice *device = (SDL_AudioDevice *) value; - if (callback(device, userdata)) { // found it? - SDL_UnlockRWLock(current_audio.device_hash_lock); - return device; - } - } - } + SDL_IterateHashTable(current_audio.device_hash, FindAudioDeviceByCallback, &data); SDL_UnlockRWLock(current_audio.device_hash_lock); - SDL_SetError("Device not found"); - return NULL; + if (!data.retval) { + SDL_SetError("Device not found"); + } + + return data.retval; } static bool TestDeviceHandleCallback(SDL_AudioDevice *device, void *handle) @@ -1795,7 +1813,7 @@ SDL_AudioDeviceID SDL_OpenAudioDevice(SDL_AudioDeviceID devid, const SDL_AudioSp if (result) { SDL_LockRWLockForWriting(current_audio.device_hash_lock); - const bool inserted = SDL_InsertIntoHashTable(current_audio.device_hash, (const void *) (uintptr_t) result, logdev); + const bool inserted = SDL_InsertIntoHashTable(current_audio.device_hash, (const void *) (uintptr_t) result, logdev, false); SDL_UnlockRWLock(current_audio.device_hash_lock); if (!inserted) { SDL_CloseAudioDevice(result); diff --git a/src/camera/SDL_camera.c b/src/camera/SDL_camera.c index 2a07bec76e..98ecda614c 100644 --- a/src/camera/SDL_camera.c +++ b/src/camera/SDL_camera.c @@ -282,21 +282,6 @@ static void ClosePhysicalCamera(SDL_Camera *device) device->adjust_timestamp = 0; } -// this must not be called while `device` is still in a device list, or while a device's camera thread is still running. -static void DestroyPhysicalCamera(SDL_Camera *device) -{ - if (device) { - // Destroy any logical devices that still exist... - ClosePhysicalCamera(device); - camera_driver.impl.FreeDeviceHandle(device); - SDL_DestroyMutex(device->lock); - SDL_free(device->all_specs); - SDL_free(device->name); - SDL_free(device); - } -} - - // Don't hold the device lock when calling this, as we may destroy the device! void UnrefPhysicalCamera(SDL_Camera *device) { @@ -307,7 +292,6 @@ void UnrefPhysicalCamera(SDL_Camera *device) SDL_AddAtomicInt(&camera_driver.device_count, -1); } SDL_UnlockRWLock(camera_driver.device_hash_lock); - DestroyPhysicalCamera(device); // ...and nuke it. } } @@ -500,7 +484,7 @@ SDL_Camera *SDL_AddCamera(const char *name, SDL_CameraPosition position, int num RefPhysicalCamera(device); SDL_LockRWLockForWriting(camera_driver.device_hash_lock); - if (SDL_InsertIntoHashTable(camera_driver.device_hash, (const void *) (uintptr_t) device->instance_id, device)) { + if (SDL_InsertIntoHashTable(camera_driver.device_hash, (const void *) (uintptr_t) device->instance_id, device, false)) { SDL_AddAtomicInt(&camera_driver.device_count, 1); } else { SDL_DestroyMutex(device->lock); @@ -624,7 +608,25 @@ void SDL_CameraPermissionOutcome(SDL_Camera *device, bool approved) } } +typedef struct FindOnePhysicalCameraByCallbackData +{ + bool (*callback)(SDL_Camera *device, void *userdata); + void *userdata; + SDL_Camera *device; +} FindOnePhysicalCameraByCallbackData; +static bool SDLCALL FindOnePhysicalCameraByCallback(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + FindOnePhysicalCameraByCallbackData *data = (FindOnePhysicalCameraByCallbackData *) userdata; + SDL_Camera *device = (SDL_Camera *) value; + if (data->callback(device, data->userdata)) { + data->device = device; + return false; // stop iterating. + } + return true; // keep iterating. +} + +// !!! FIXME: this doesn't follow SDL convention of `userdata` being the first param of the callback. SDL_Camera *SDL_FindPhysicalCameraByCallback(bool (*callback)(SDL_Camera *device, void *userdata), void *userdata) { if (!SDL_GetCurrentCameraDriver()) { @@ -632,23 +634,17 @@ SDL_Camera *SDL_FindPhysicalCameraByCallback(bool (*callback)(SDL_Camera *device return NULL; } - const void *key; - const void *value; - void *iter = NULL; + FindOnePhysicalCameraByCallbackData data = { callback, userdata, NULL }; SDL_LockRWLockForReading(camera_driver.device_hash_lock); - while (SDL_IterateHashTable(camera_driver.device_hash, &key, &value, &iter)) { - SDL_Camera *device = (SDL_Camera *) value; - if (callback(device, userdata)) { // found it? - SDL_UnlockRWLock(camera_driver.device_hash_lock); - return device; - } - } - + SDL_IterateHashTable(camera_driver.device_hash, FindOnePhysicalCameraByCallback, &data); SDL_UnlockRWLock(camera_driver.device_hash_lock); - SDL_SetError("Device not found"); - return NULL; + if (!data.device) { + SDL_SetError("Device not found"); + } + + return data.device; } void SDL_CloseCamera(SDL_Camera *camera) @@ -704,6 +700,19 @@ SDL_CameraPosition SDL_GetCameraPosition(SDL_CameraID instance_id) } +typedef struct GetOneCameraData +{ + SDL_CameraID *result; + int devs_seen; +} GetOneCameraData; + +static bool SDLCALL GetOneCamera(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + GetOneCameraData *data = (GetOneCameraData *) userdata; + data->result[data->devs_seen++] = (SDL_CameraID) (uintptr_t) key; + return true; // keep iterating. +} + SDL_CameraID *SDL_GetCameras(int *count) { int dummy_count; @@ -725,16 +734,10 @@ SDL_CameraID *SDL_GetCameras(int *count) if (!result) { num_devices = 0; } else { - int devs_seen = 0; - const void *key; - const void *value; - void *iter = NULL; - while (SDL_IterateHashTable(camera_driver.device_hash, &key, &value, &iter)) { - result[devs_seen++] = (SDL_CameraID) (uintptr_t) key; - } - - SDL_assert(devs_seen == num_devices); - result[devs_seen] = 0; // null-terminated. + GetOneCameraData data = { result, 0 }; + SDL_IterateHashTable(camera_driver.device_hash, GetOneCamera, &data); + SDL_assert(data.devs_seen == num_devices); + result[num_devices] = 0; // null-terminated. } SDL_UnlockRWLock(camera_driver.device_hash_lock); @@ -1381,37 +1384,26 @@ void SDL_QuitCamera(void) SDL_free(i); } - const void *key; - const void *value; - void *iter = NULL; - while (SDL_IterateHashTable(device_hash, &key, &value, &iter)) { - DestroyPhysicalCamera((SDL_Camera *) value); - } + SDL_DestroyHashTable(device_hash); // Free the driver data camera_driver.impl.Deinitialize(); SDL_DestroyRWLock(camera_driver.device_hash_lock); - SDL_DestroyHashTable(device_hash); SDL_zero(camera_driver); } - -static Uint32 HashCameraID(const void *key, void *data) +// Physical camera objects are only destroyed when removed from the device hash. +static void SDLCALL DestroyCameraHashItem(void *userdata, const void *key, const void *value) { - // The values are unique incrementing integers, starting at 1, so just return minus 1 to start with bucket zero. - return ((Uint32) ((uintptr_t) key)) - 1; -} - -static bool MatchCameraID(const void *a, const void *b, void *data) -{ - return (a == b); // simple integers, just compare them as pointer values. -} - -static void NukeCameraHashItem(const void *key, const void *value, void *data) -{ - // no-op, keys and values in this hashtable are treated as Plain Old Data and don't get freed here. + SDL_Camera *device = (SDL_Camera *) userdata; + ClosePhysicalCamera(device); + camera_driver.impl.FreeDeviceHandle(device); + SDL_DestroyMutex(device->lock); + SDL_free(device->all_specs); + SDL_free(device->name); + SDL_free(device); } bool SDL_CameraInit(const char *driver_name) @@ -1425,7 +1417,7 @@ bool SDL_CameraInit(const char *driver_name) return false; } - SDL_HashTable *device_hash = SDL_CreateHashTable(NULL, 8, HashCameraID, MatchCameraID, NukeCameraHashItem, false, false); + SDL_HashTable *device_hash = SDL_CreateHashTable(0, false, SDL_HashID, SDL_KeyMatchID, DestroyCameraHashItem, NULL); if (!device_hash) { SDL_DestroyRWLock(device_hash_lock); return false; diff --git a/src/camera/SDL_syscamera.h b/src/camera/SDL_syscamera.h index 7af115ad79..7c485ddeca 100644 --- a/src/camera/SDL_syscamera.h +++ b/src/camera/SDL_syscamera.h @@ -190,7 +190,7 @@ typedef struct SDL_CameraDriver const char *desc; // The description of this camera driver SDL_CameraDriverImpl impl; // the backend's interface - SDL_RWLock *device_hash_lock; // A rwlock that protects `device_hash` + SDL_RWLock *device_hash_lock; // A rwlock that protects `device_hash` // !!! FIXME: device_hash _also_ has a rwlock, see if we still need this one. SDL_HashTable *device_hash; // the collection of currently-available camera devices SDL_PendingCameraEvent pending_events; SDL_PendingCameraEvent *pending_events_tail; diff --git a/src/events/SDL_keymap.c b/src/events/SDL_keymap.c index bc21c33103..a2a81f6c04 100644 --- a/src/events/SDL_keymap.c +++ b/src/events/SDL_keymap.c @@ -39,8 +39,8 @@ SDL_Keymap *SDL_CreateKeymap(void) return NULL; } - keymap->scancode_to_keycode = SDL_CreateHashTable(NULL, 64, SDL_HashID, SDL_KeyMatchID, NULL, false, false); - keymap->keycode_to_scancode = SDL_CreateHashTable(NULL, 64, SDL_HashID, SDL_KeyMatchID, NULL, false, false); + keymap->scancode_to_keycode = SDL_CreateHashTable(256, false, SDL_HashID, SDL_KeyMatchID, NULL, NULL); + keymap->keycode_to_scancode = SDL_CreateHashTable(256, false, SDL_HashID, SDL_KeyMatchID, NULL, NULL); if (!keymap->scancode_to_keycode || !keymap->keycode_to_scancode) { SDL_DestroyKeymap(keymap); return NULL; @@ -76,31 +76,27 @@ void SDL_SetKeymapEntry(SDL_Keymap *keymap, SDL_Scancode scancode, SDL_Keymod mo Uint32 key = ((Uint32)modstate << 16) | scancode; const void *value; if (SDL_FindInHashTable(keymap->scancode_to_keycode, (void *)(uintptr_t)key, &value)) { - SDL_Keycode existing_keycode = (SDL_Keycode)(uintptr_t)value; + const SDL_Keycode existing_keycode = (SDL_Keycode)(uintptr_t)value; if (existing_keycode == keycode) { // We already have this mapping return; } - - // Changing the mapping, need to remove the existing entry from the keymap - SDL_RemoveFromHashTable(keymap->scancode_to_keycode, (void *)(uintptr_t)key); + // InsertIntoHashTable will replace the existing entry in the keymap atomically. } - SDL_InsertIntoHashTable(keymap->scancode_to_keycode, (void *)(uintptr_t)key, (void *)(uintptr_t)keycode); + SDL_InsertIntoHashTable(keymap->scancode_to_keycode, (void *)(uintptr_t)key, (void *)(uintptr_t)keycode, true); bool update_keycode = true; if (SDL_FindInHashTable(keymap->keycode_to_scancode, (void *)(uintptr_t)keycode, &value)) { - Uint32 existing_value = (Uint32)(uintptr_t)value; - SDL_Keymod existing_modstate = (SDL_Keymod)(existing_value >> 16); + const Uint32 existing_value = (Uint32)(uintptr_t)value; + const SDL_Keymod existing_modstate = (SDL_Keymod)(existing_value >> 16); // Keep the simplest combination of scancode and modifiers to generate this keycode if (existing_modstate <= modstate) { update_keycode = false; - } else { - SDL_RemoveFromHashTable(keymap->keycode_to_scancode, (void *)(uintptr_t)keycode); } } if (update_keycode) { - SDL_InsertIntoHashTable(keymap->keycode_to_scancode, (void *)(uintptr_t)keycode, (void *)(uintptr_t)key); + SDL_InsertIntoHashTable(keymap->keycode_to_scancode, (void *)(uintptr_t)keycode, (void *)(uintptr_t)key, true); } } @@ -108,7 +104,7 @@ SDL_Keycode SDL_GetKeymapKeycode(SDL_Keymap *keymap, SDL_Scancode scancode, SDL_ { SDL_Keycode keycode; - Uint32 key = ((Uint32)NormalizeModifierStateForKeymap(modstate) << 16) | scancode; + const Uint32 key = ((Uint32)NormalizeModifierStateForKeymap(modstate) << 16) | scancode; const void *value; if (keymap && SDL_FindInHashTable(keymap->scancode_to_keycode, (void *)(uintptr_t)key, &value)) { keycode = (SDL_Keycode)(uintptr_t)value; diff --git a/src/gpu/vulkan/SDL_gpu_vulkan.c b/src/gpu/vulkan/SDL_gpu_vulkan.c index bad67db043..945175408d 100644 --- a/src/gpu/vulkan/SDL_gpu_vulkan.c +++ b/src/gpu/vulkan/SDL_gpu_vulkan.c @@ -2924,57 +2924,74 @@ static void VULKAN_INTERNAL_DestroyFramebuffer( SDL_free(framebuffer); } +typedef struct CheckOneFramebufferForRemovalData +{ + Uint32 keysToRemoveCapacity; + Uint32 keysToRemoveCount; + FramebufferHashTableKey **keysToRemove; + VkImageView view; +} CheckOneFramebufferForRemovalData; + +static bool SDLCALL CheckOneFramebufferForRemoval(void *userdata, const SDL_HashTable *table, const void *vkey, const void *vvalue) +{ + CheckOneFramebufferForRemovalData *data = (CheckOneFramebufferForRemovalData *) userdata; + FramebufferHashTableKey *key = (FramebufferHashTableKey *) vkey; + VkImageView view = data->view; + bool remove = false; + + for (Uint32 i = 0; i < key->numColorTargets; i += 1) { + if (key->colorAttachmentViews[i] == view) { + remove = true; + } + } + for (Uint32 i = 0; i < key->numResolveAttachments; i += 1) { + if (key->resolveAttachmentViews[i] == view) { + remove = true; + } + } + if (key->depthStencilAttachmentView == view) { + remove = true; + } + + if (remove) { + if (data->keysToRemoveCount == data->keysToRemoveCapacity) { + data->keysToRemoveCapacity *= 2; + void *ptr = SDL_realloc(data->keysToRemove, data->keysToRemoveCapacity * sizeof(FramebufferHashTableKey *)); + if (!ptr) { + return false; // ugh, stop iterating. We're in trouble. + } + data->keysToRemove = (FramebufferHashTableKey **) ptr; + } + data->keysToRemove[data->keysToRemoveCount] = key; + data->keysToRemoveCount++; + } + + return true; // keep iterating. +} + static void VULKAN_INTERNAL_RemoveFramebuffersContainingView( VulkanRenderer *renderer, VkImageView view) { - FramebufferHashTableKey *key; - VulkanFramebuffer *value; - void *iter = NULL; - // Can't remove while iterating! - Uint32 keysToRemoveCapacity = 8; - Uint32 keysToRemoveCount = 0; - FramebufferHashTableKey **keysToRemove = SDL_malloc(keysToRemoveCapacity * sizeof(FramebufferHashTableKey *)); + + CheckOneFramebufferForRemovalData data = { 8, 0, NULL, view }; + data.keysToRemove = (FramebufferHashTableKey **) SDL_malloc(data.keysToRemoveCapacity * sizeof(FramebufferHashTableKey *)); + if (!data.keysToRemove) { + return; // uhoh. + } SDL_LockMutex(renderer->framebufferFetchLock); - while (SDL_IterateHashTable(renderer->framebufferHashTable, (const void **)&key, (const void **)&value, &iter)) { - bool remove = false; - for (Uint32 i = 0; i < key->numColorTargets; i += 1) { - if (key->colorAttachmentViews[i] == view) { - remove = true; - } - } - for (Uint32 i = 0; i < key->numResolveAttachments; i += 1) { - if (key->resolveAttachmentViews[i] == view) { - remove = true; - } - } - if (key->depthStencilAttachmentView == view) { - remove = true; - } + SDL_IterateHashTable(renderer->framebufferHashTable, CheckOneFramebufferForRemoval, &data); - if (remove) { - if (keysToRemoveCount == keysToRemoveCapacity) { - keysToRemoveCapacity *= 2; - keysToRemove = SDL_realloc( - keysToRemove, - keysToRemoveCapacity * sizeof(FramebufferHashTableKey *)); - } - - keysToRemove[keysToRemoveCount] = key; - keysToRemoveCount += 1; - } - } - - for (Uint32 i = 0; i < keysToRemoveCount; i += 1) { - SDL_RemoveFromHashTable(renderer->framebufferHashTable, (void *)keysToRemove[i]); + for (Uint32 i = 0; i < data.keysToRemoveCount; i += 1) { + SDL_RemoveFromHashTable(renderer->framebufferHashTable, (void *)data.keysToRemove[i]); } SDL_UnlockMutex(renderer->framebufferFetchLock); - SDL_free(keysToRemove); + SDL_free(data.keysToRemove); } static void VULKAN_INTERNAL_DestroyTexture( @@ -3280,7 +3297,7 @@ static void VULKAN_INTERNAL_DestroyDescriptorSetCache( // Hashtable functions -static Uint32 VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashFunction(const void *key, void *data) +static Uint32 SDLCALL VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashFunction(void *userdata, const void *key) { GraphicsPipelineResourceLayoutHashTableKey *hashTableKey = (GraphicsPipelineResourceLayoutHashTableKey *)key; /* The algorithm for this hashing function @@ -3299,19 +3316,19 @@ static Uint32 VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashFunction(const v result = result * hashFactor + hashTableKey->fragmentUniformBufferCount; return result; } -static bool VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashKeyMatch(const void *aKey, const void *bKey, void *data) +static bool SDLCALL VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashKeyMatch(void *userdata, const void *aKey, const void *bKey) { return SDL_memcmp(aKey, bKey, sizeof(GraphicsPipelineResourceLayoutHashTableKey)) == 0; } -static void VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashNuke(const void *key, const void *value, void *data) +static void SDLCALL VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashDestroy(void *userdata, const void *key, const void *value) { - VulkanRenderer *renderer = (VulkanRenderer *)data; + VulkanRenderer *renderer = (VulkanRenderer *)userdata; VulkanGraphicsPipelineResourceLayout *resourceLayout = (VulkanGraphicsPipelineResourceLayout *)value; VULKAN_INTERNAL_DestroyGraphicsPipelineResourceLayout(renderer, resourceLayout); SDL_free((void*)key); } -static Uint32 VULKAN_INTERNAL_ComputePipelineResourceLayoutHashFunction(const void *key, void *data) +static Uint32 SDLCALL VULKAN_INTERNAL_ComputePipelineResourceLayoutHashFunction(void *userdata, const void *key) { ComputePipelineResourceLayoutHashTableKey *hashTableKey = (ComputePipelineResourceLayoutHashTableKey *)key; /* The algorithm for this hashing function @@ -3329,20 +3346,20 @@ static Uint32 VULKAN_INTERNAL_ComputePipelineResourceLayoutHashFunction(const vo return result; } -static bool VULKAN_INTERNAL_ComputePipelineResourceLayoutHashKeyMatch(const void *aKey, const void *bKey, void *data) +static bool SDLCALL VULKAN_INTERNAL_ComputePipelineResourceLayoutHashKeyMatch(void *userdata, const void *aKey, const void *bKey) { return SDL_memcmp(aKey, bKey, sizeof(ComputePipelineResourceLayoutHashTableKey)) == 0; } -static void VULKAN_INTERNAL_ComputePipelineResourceLayoutHashNuke(const void *key, const void *value, void *data) +static void SDLCALL VULKAN_INTERNAL_ComputePipelineResourceLayoutHashDestroy(void *userdata, const void *key, const void *value) { - VulkanRenderer *renderer = (VulkanRenderer *)data; + VulkanRenderer *renderer = (VulkanRenderer *)userdata; VulkanComputePipelineResourceLayout *resourceLayout = (VulkanComputePipelineResourceLayout *)value; VULKAN_INTERNAL_DestroyComputePipelineResourceLayout(renderer, resourceLayout); SDL_free((void*)key); } -static Uint32 VULKAN_INTERNAL_DescriptorSetLayoutHashFunction(const void *key, void *data) +static Uint32 SDLCALL VULKAN_INTERNAL_DescriptorSetLayoutHashFunction(void *userdata, const void *key) { DescriptorSetLayoutHashTableKey *hashTableKey = (DescriptorSetLayoutHashTableKey *)key; @@ -3362,42 +3379,40 @@ static Uint32 VULKAN_INTERNAL_DescriptorSetLayoutHashFunction(const void *key, v return result; } -static bool VULKAN_INTERNAL_DescriptorSetLayoutHashKeyMatch(const void *aKey, const void *bKey, void *data) +static bool SDLCALL VULKAN_INTERNAL_DescriptorSetLayoutHashKeyMatch(void *userdata, const void *aKey, const void *bKey) { return SDL_memcmp(aKey, bKey, sizeof(DescriptorSetLayoutHashTableKey)) == 0; } -static void VULKAN_INTERNAL_DescriptorSetLayoutHashNuke(const void *key, const void *value, void *data) +static void SDLCALL VULKAN_INTERNAL_DescriptorSetLayoutHashDestroy(void *userdata, const void *key, const void *value) { - VulkanRenderer *renderer = (VulkanRenderer *)data; + VulkanRenderer *renderer = (VulkanRenderer *)userdata; DescriptorSetLayout *layout = (DescriptorSetLayout *)value; VULKAN_INTERNAL_DestroyDescriptorSetLayout(renderer, layout); SDL_free((void*)key); } -static Uint32 VULKAN_INTERNAL_CommandPoolHashFunction(const void *key, void *data) +static Uint32 SDLCALL VULKAN_INTERNAL_CommandPoolHashFunction(void *userdata, const void *key) { return (Uint32)((CommandPoolHashTableKey *)key)->threadID; } -static bool VULKAN_INTERNAL_CommandPoolHashKeyMatch(const void *aKey, const void *bKey, void *data) +static bool SDLCALL VULKAN_INTERNAL_CommandPoolHashKeyMatch(void *userdata, const void *aKey, const void *bKey) { CommandPoolHashTableKey *a = (CommandPoolHashTableKey *)aKey; CommandPoolHashTableKey *b = (CommandPoolHashTableKey *)bKey; return a->threadID == b->threadID; } -static void VULKAN_INTERNAL_CommandPoolHashNuke(const void *key, const void *value, void *data) +static void SDLCALL VULKAN_INTERNAL_CommandPoolHashDestroy(void *userdata, const void *key, const void *value) { - VulkanRenderer *renderer = (VulkanRenderer *)data; + VulkanRenderer *renderer = (VulkanRenderer *)userdata; VulkanCommandPool *pool = (VulkanCommandPool *)value; VULKAN_INTERNAL_DestroyCommandPool(renderer, pool); SDL_free((void *)key); } -static Uint32 VULKAN_INTERNAL_RenderPassHashFunction( - const void *key, - void *data) +static Uint32 SDLCALL VULKAN_INTERNAL_RenderPassHashFunction(void *userdata, const void *key) { RenderPassHashTableKey *hashTableKey = (RenderPassHashTableKey *)key; @@ -3429,10 +3444,7 @@ static Uint32 VULKAN_INTERNAL_RenderPassHashFunction( return result; } -static bool VULKAN_INTERNAL_RenderPassHashKeyMatch( - const void *aKey, - const void *bKey, - void *data) +static bool SDLCALL VULKAN_INTERNAL_RenderPassHashKeyMatch(void *userdata, const void *aKey, const void *bKey) { RenderPassHashTableKey *a = (RenderPassHashTableKey *)aKey; RenderPassHashTableKey *b = (RenderPassHashTableKey *)bKey; @@ -3492,9 +3504,9 @@ static bool VULKAN_INTERNAL_RenderPassHashKeyMatch( return 1; } -static void VULKAN_INTERNAL_RenderPassHashNuke(const void *key, const void *value, void *data) +static void SDLCALL VULKAN_INTERNAL_RenderPassHashDestroy(void *userdata, const void *key, const void *value) { - VulkanRenderer *renderer = (VulkanRenderer *)data; + VulkanRenderer *renderer = (VulkanRenderer *)userdata; VulkanRenderPassHashTableValue *renderPassWrapper = (VulkanRenderPassHashTableValue *)value; renderer->vkDestroyRenderPass( renderer->logicalDevice, @@ -3504,9 +3516,7 @@ static void VULKAN_INTERNAL_RenderPassHashNuke(const void *key, const void *valu SDL_free((void *)key); } -static Uint32 VULKAN_INTERNAL_FramebufferHashFunction( - const void *key, - void *data) +static Uint32 SDLCALL VULKAN_INTERNAL_FramebufferHashFunction(void *userdata, const void *key) { FramebufferHashTableKey *hashTableKey = (FramebufferHashTableKey *)key; @@ -3531,10 +3541,7 @@ static Uint32 VULKAN_INTERNAL_FramebufferHashFunction( return result; } -static bool VULKAN_INTERNAL_FramebufferHashKeyMatch( - const void *aKey, - const void *bKey, - void *data) +static bool SDLCALL VULKAN_INTERNAL_FramebufferHashKeyMatch(void *userdata, const void *aKey, const void *bKey) { FramebufferHashTableKey *a = (FramebufferHashTableKey *)aKey; FramebufferHashTableKey *b = (FramebufferHashTableKey *)bKey; @@ -3574,9 +3581,9 @@ static bool VULKAN_INTERNAL_FramebufferHashKeyMatch( return 1; } -static void VULKAN_INTERNAL_FramebufferHashNuke(const void *key, const void *value, void *data) +static void SDLCALL VULKAN_INTERNAL_FramebufferHashDestroy(void *userdata, const void *key, const void *value) { - VulkanRenderer *renderer = (VulkanRenderer *)data; + VulkanRenderer *renderer = (VulkanRenderer *)userdata; VulkanFramebuffer *framebuffer = (VulkanFramebuffer *)value; VULKAN_INTERNAL_ReleaseFramebuffer(renderer, framebuffer); SDL_free((void *)key); @@ -3845,7 +3852,7 @@ static DescriptorSetLayout *VULKAN_INTERNAL_FetchDescriptorSetLayout( SDL_InsertIntoHashTable( renderer->descriptorSetLayoutHashTable, (const void *)allocedKey, - (const void *)layout); + (const void *)layout, true); return layout; } @@ -3962,7 +3969,7 @@ static VulkanGraphicsPipelineResourceLayout *VULKAN_INTERNAL_FetchGraphicsPipeli SDL_InsertIntoHashTable( renderer->graphicsPipelineResourceLayoutHashTable, (const void *)allocedKey, - (const void *)pipelineResourceLayout); + (const void *)pipelineResourceLayout, true); return pipelineResourceLayout; } @@ -4063,7 +4070,7 @@ static VulkanComputePipelineResourceLayout *VULKAN_INTERNAL_FetchComputePipeline SDL_InsertIntoHashTable( renderer->computePipelineResourceLayoutHashTable, (const void *)allocedKey, - (const void *)pipelineResourceLayout); + (const void *)pipelineResourceLayout, true); return pipelineResourceLayout; } @@ -7140,7 +7147,7 @@ static VkRenderPass VULKAN_INTERNAL_FetchRenderPass( SDL_InsertIntoHashTable( renderer->renderPassHashTable, (const void *)allocedKey, - (const void *)renderPassWrapper); + (const void *)renderPassWrapper, true); return renderPassHandle; } @@ -7285,7 +7292,7 @@ static VulkanFramebuffer *VULKAN_INTERNAL_FetchFramebuffer( SDL_InsertIntoHashTable( renderer->framebufferHashTable, (const void *)allocedKey, - (const void *)vulkanFramebuffer); + (const void *)vulkanFramebuffer, true); SDL_UnlockMutex(renderer->framebufferFetchLock); } else { @@ -9389,7 +9396,7 @@ static VulkanCommandPool *VULKAN_INTERNAL_FetchCommandPool( SDL_InsertIntoHashTable( renderer->commandPoolHashTable, (const void *)allocedKey, - (const void *)vulkanCommandPool); + (const void *)vulkanCommandPool, true); return vulkanCommandPool; } @@ -11739,59 +11746,53 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S // Initialize caches - // manually synchronized due to submission timing renderer->commandPoolHashTable = SDL_CreateHashTable( - (void *)renderer, - 64, + 0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful. + false, // manually synchronized due to submission timing VULKAN_INTERNAL_CommandPoolHashFunction, VULKAN_INTERNAL_CommandPoolHashKeyMatch, - VULKAN_INTERNAL_CommandPoolHashNuke, - false, false); + VULKAN_INTERNAL_CommandPoolHashDestroy, + (void *)renderer); - // thread-safe renderer->renderPassHashTable = SDL_CreateHashTable( - (void *)renderer, - 64, + 0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful. + true, // thread-safe VULKAN_INTERNAL_RenderPassHashFunction, VULKAN_INTERNAL_RenderPassHashKeyMatch, - VULKAN_INTERNAL_RenderPassHashNuke, - true, false); + VULKAN_INTERNAL_RenderPassHashDestroy, + (void *)renderer); - // manually synchronized due to iteration renderer->framebufferHashTable = SDL_CreateHashTable( - (void *)renderer, - 64, + 0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful. + false, // manually synchronized due to iteration VULKAN_INTERNAL_FramebufferHashFunction, VULKAN_INTERNAL_FramebufferHashKeyMatch, - VULKAN_INTERNAL_FramebufferHashNuke, - false, false); + VULKAN_INTERNAL_FramebufferHashDestroy, + (void *)renderer); - // thread-safe renderer->graphicsPipelineResourceLayoutHashTable = SDL_CreateHashTable( - (void *)renderer, - 64, + 0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful. + true, // thread-safe VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashFunction, VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashKeyMatch, - VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashNuke, - true, false); + VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashDestroy, + (void *)renderer); - // thread-safe renderer->computePipelineResourceLayoutHashTable = SDL_CreateHashTable( - (void *)renderer, - 64, + 0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful. + true, // thread-safe VULKAN_INTERNAL_ComputePipelineResourceLayoutHashFunction, VULKAN_INTERNAL_ComputePipelineResourceLayoutHashKeyMatch, - VULKAN_INTERNAL_ComputePipelineResourceLayoutHashNuke, - true, false); + VULKAN_INTERNAL_ComputePipelineResourceLayoutHashDestroy, + (void *)renderer); - // thread-safe renderer->descriptorSetLayoutHashTable = SDL_CreateHashTable( - (void *)renderer, - 64, + 0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful. + true, // thread-safe VULKAN_INTERNAL_DescriptorSetLayoutHashFunction, VULKAN_INTERNAL_DescriptorSetLayoutHashKeyMatch, - VULKAN_INTERNAL_DescriptorSetLayoutHashNuke, - true, false); + VULKAN_INTERNAL_DescriptorSetLayoutHashDestroy, + (void *)renderer); // Initialize fence pool diff --git a/src/joystick/SDL_gamepad.c b/src/joystick/SDL_gamepad.c index b1b88e8389..ba434eea45 100644 --- a/src/joystick/SDL_gamepad.c +++ b/src/joystick/SDL_gamepad.c @@ -562,12 +562,10 @@ static void PopMappingChangeTracking(void) GamepadMapping_t *old_mapping = gamepad ? gamepad->mapping : tracker->joystick_mappings[i]; if (new_mapping && !old_mapping) { - SDL_RemoveFromHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)joystick); - SDL_InsertIntoHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)joystick, (const void *)true); + SDL_InsertIntoHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)joystick, (const void *)true, true); SDL_PrivateGamepadAdded(joystick); } else if (old_mapping && !new_mapping) { - SDL_RemoveFromHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)joystick); - SDL_InsertIntoHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)joystick, (const void *)false); + SDL_InsertIntoHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)joystick, (const void *)false, true); SDL_PrivateGamepadRemoved(joystick); } else if (old_mapping != new_mapping || HasMappingChangeTracking(tracker, new_mapping)) { if (gamepad) { @@ -2637,9 +2635,9 @@ bool SDL_IsGamepad(SDL_JoystickID instance_id) } if (!s_gamepadInstanceIDs) { - s_gamepadInstanceIDs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NULL, false, false); + s_gamepadInstanceIDs = SDL_CreateHashTable(0, false, SDL_HashID, SDL_KeyMatchID, NULL, NULL); } - SDL_InsertIntoHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)instance_id, (void *)(uintptr_t)result); + SDL_InsertIntoHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)instance_id, (void *)(uintptr_t)result, true); } } SDL_UnlockJoysticks(); diff --git a/src/render/gpu/SDL_pipeline_gpu.c b/src/render/gpu/SDL_pipeline_gpu.c index 0f7fabfef6..078337401b 100644 --- a/src/render/gpu/SDL_pipeline_gpu.c +++ b/src/render/gpu/SDL_pipeline_gpu.c @@ -36,58 +36,50 @@ struct GPU_PipelineCacheKeyStruct Uint64 primitive_type : 3; }; -typedef union GPU_PipelineCacheKey +typedef union GPU_PipelineCacheKeyConverter { struct GPU_PipelineCacheKeyStruct as_struct; Uint64 as_uint64; -} GPU_PipelineCacheKey; +} GPU_PipelineCacheKeyConverter; -SDL_COMPILE_TIME_ASSERT(GPU_PipelineCacheKey_Size, sizeof(GPU_PipelineCacheKey) <= sizeof(Uint64)); +SDL_COMPILE_TIME_ASSERT(GPU_PipelineCacheKeyConverter_Size, sizeof(GPU_PipelineCacheKeyConverter) <= sizeof(Uint64)); -typedef struct GPU_PipelineCacheEntry +static Uint32 SDLCALL HashPipelineCacheKey(void *userdata, const void *key) { - GPU_PipelineCacheKey key; - SDL_GPUGraphicsPipeline *pipeline; -} GPU_PipelineCacheEntry; + const GPU_PipelineParameters *params = (const GPU_PipelineParameters *) key; + GPU_PipelineCacheKeyConverter cvt; + cvt.as_uint64 = 0; + cvt.as_struct.blend_mode = params->blend_mode; + cvt.as_struct.frag_shader = params->frag_shader; + cvt.as_struct.vert_shader = params->vert_shader; + cvt.as_struct.attachment_format = params->attachment_format; + cvt.as_struct.primitive_type = params->primitive_type; -static Uint32 HashPipelineCacheKey(const GPU_PipelineCacheKey *key) -{ - Uint64 x = key->as_uint64; // 64-bit uint hash function stolen from taisei (which stole it from somewhere else) + Uint64 x = cvt.as_uint64; x = (x ^ (x >> 30)) * UINT64_C(0xbf58476d1ce4e5b9); x = (x ^ (x >> 27)) * UINT64_C(0x94d049bb133111eb); x = x ^ (x >> 31); return (Uint32)(x & 0xffffffff); } -static Uint32 HashPassthrough(const void *key, void *data) +static bool SDLCALL MatchPipelineCacheKey(void *userdata, const void *a, const void *b) { - // double-cast to silence a clang warning - return (Uint32)(uintptr_t)key; + return (SDL_memcmp(a, b, sizeof (GPU_PipelineParameters)) == 0); } -static bool MatchPipelineCacheKey(const void *a, const void *b, void *data) +static void SDLCALL DestroyPipelineCacheHashItem(void *userdata, const void *key, const void *value) { - return a == b; -} - -static void NukePipelineCacheEntry(const void *key, const void *value, void *data) -{ - GPU_PipelineCacheEntry *entry = (GPU_PipelineCacheEntry *)value; - SDL_GPUDevice *device = data; - - SDL_ReleaseGPUGraphicsPipeline(device, entry->pipeline); - SDL_free(entry); + SDL_GPUGraphicsPipeline *pipeline = (SDL_GPUGraphicsPipeline *) value; + SDL_GPUDevice *device = (SDL_GPUDevice *) userdata; + SDL_ReleaseGPUGraphicsPipeline(device, pipeline); + SDL_free((GPU_PipelineParameters *) key); } bool GPU_InitPipelineCache(GPU_PipelineCache *cache, SDL_GPUDevice *device) { - // FIXME how many buckets do we need? - cache->table = SDL_CreateHashTable(device, 32, HashPassthrough, MatchPipelineCacheKey, NukePipelineCacheEntry, false, true); - if (!cache->table) { - return false; - } - return true; + cache->table = SDL_CreateHashTable(0, false, HashPipelineCacheKey, MatchPipelineCacheKey, DestroyPipelineCacheHashItem, device); + return (cache->table != NULL); } void GPU_DestroyPipelineCache(GPU_PipelineCache *cache) @@ -180,45 +172,30 @@ static SDL_GPUGraphicsPipeline *MakePipeline(SDL_GPUDevice *device, GPU_Shaders return SDL_CreateGPUGraphicsPipeline(device, &pci); } -static GPU_PipelineCacheKey MakePipelineCacheKey(const GPU_PipelineParameters *params) -{ - GPU_PipelineCacheKey key; - SDL_zero(key); - key.as_struct.blend_mode = params->blend_mode; - key.as_struct.frag_shader = params->frag_shader; - key.as_struct.vert_shader = params->vert_shader; - key.as_struct.attachment_format = params->attachment_format; - key.as_struct.primitive_type = params->primitive_type; - return key; -} - SDL_GPUGraphicsPipeline *GPU_GetPipeline(GPU_PipelineCache *cache, GPU_Shaders *shaders, SDL_GPUDevice *device, const GPU_PipelineParameters *params) { - GPU_PipelineCacheKey key = MakePipelineCacheKey(params); - void *keyval = (void *)(uintptr_t)HashPipelineCacheKey(&key); SDL_GPUGraphicsPipeline *pipeline = NULL; + if (!SDL_FindInHashTable(cache->table, params, (const void **) &pipeline)) { + bool inserted = false; + // !!! FIXME: why don't we have an SDL_alloc_copy function/macro? + GPU_PipelineParameters *paramscpy = (GPU_PipelineParameters *) SDL_malloc(sizeof (*paramscpy)); + if (paramscpy) { + SDL_copyp(paramscpy, params); + pipeline = MakePipeline(device, shaders, params); + if (pipeline) { + inserted = SDL_InsertIntoHashTable(cache->table, paramscpy, pipeline, false); + } + } - void *iter = NULL; - GPU_PipelineCacheEntry *entry = NULL; - - while (SDL_IterateHashTableKey(cache->table, keyval, (const void **)&entry, &iter)) { - if (entry->key.as_uint64 == key.as_uint64) { - return entry->pipeline; + if (!inserted) { + SDL_free(paramscpy); + if (pipeline) { + SDL_ReleaseGPUGraphicsPipeline(device, pipeline); + pipeline = NULL; + } } } - pipeline = MakePipeline(device, shaders, params); - - if (pipeline == NULL) { - return NULL; - } - - entry = SDL_malloc(sizeof(*entry)); - entry->key = key; - entry->pipeline = pipeline; - - SDL_InsertIntoHashTable(cache->table, keyval, entry); - return pipeline; } diff --git a/src/stdlib/SDL_getenv.c b/src/stdlib/SDL_getenv.c index 2659bd421d..b4a1922465 100644 --- a/src/stdlib/SDL_getenv.c +++ b/src/stdlib/SDL_getenv.c @@ -53,7 +53,7 @@ static char **environ; struct SDL_Environment { - SDL_Mutex *lock; + SDL_Mutex *lock; // !!! FIXME: reuse SDL_HashTable's lock. SDL_HashTable *strings; }; static SDL_Environment *SDL_environment; @@ -88,13 +88,13 @@ SDL_Environment *SDL_CreateEnvironment(bool populated) return NULL; } - env->strings = SDL_CreateHashTable(NULL, 16, SDL_HashString, SDL_KeyMatchString, SDL_NukeFreeKey, false, false); + env->strings = SDL_CreateHashTable(0, false, SDL_HashString, SDL_KeyMatchString, SDL_DestroyHashKey, NULL); if (!env->strings) { SDL_free(env); return NULL; } - // Don't fail if we can't create a mutex (e.g. on a single-thread environment) + // Don't fail if we can't create a mutex (e.g. on a single-thread environment) // !!! FIXME: single-threaded environments should still return a non-NULL, do-nothing object here. Check for failure! env->lock = SDL_CreateMutex(); if (populated) { @@ -114,7 +114,7 @@ SDL_Environment *SDL_CreateEnvironment(bool populated) } *value++ = '\0'; - SDL_InsertIntoHashTable(env->strings, variable, value); + SDL_InsertIntoHashTable(env->strings, variable, value, true); } FreeEnvironmentStringsW(strings); } @@ -138,7 +138,7 @@ SDL_Environment *SDL_CreateEnvironment(bool populated) } *value++ = '\0'; - SDL_InsertIntoHashTable(env->strings, variable, value); + SDL_InsertIntoHashTable(env->strings, variable, value, true); } } #endif // SDL_PLATFORM_WINDOWS @@ -170,6 +170,49 @@ const char *SDL_GetEnvironmentVariable(SDL_Environment *env, const char *name) return result; } +typedef struct CountEnvStringsData +{ + size_t count; + size_t length; +} CountEnvStringsData; + +static bool SDLCALL CountEnvStrings(void *userdata, const SDL_HashTable *table, const void *key, const void *value) +{ + CountEnvStringsData *data = (CountEnvStringsData *) userdata; + data->length += SDL_strlen((const char *) key) + 1 + SDL_strlen((const char *) value) + 1; + data->count++; + return true; // keep iterating. +} + +typedef struct CopyEnvStringsData +{ + char **result; + char *string; + size_t count; +} CopyEnvStringsData; + +static bool SDLCALL CopyEnvStrings(void *userdata, const SDL_HashTable *table, const void *vkey, const void *vvalue) +{ + CopyEnvStringsData *data = (CopyEnvStringsData *) userdata; + const char *key = (const char *) vkey; + const char *value = (const char *) vvalue; + size_t len; + + len = SDL_strlen(key); + data->result[data->count] = data->string; + SDL_memcpy(data->string, key, len); + data->string += len; + *(data->string++) = '='; + + len = SDL_strlen(value); + SDL_memcpy(data->string, value, len); + data->string += len; + *(data->string++) = '\0'; + data->count++; + + return true; // keep iterating. +} + char **SDL_GetEnvironmentVariables(SDL_Environment *env) { char **result = NULL; @@ -181,40 +224,20 @@ char **SDL_GetEnvironmentVariables(SDL_Environment *env) SDL_LockMutex(env->lock); { - size_t count, length = 0; - void *iter; - const char *key, *value; - // First pass, get the size we need for all the strings - count = 0; - iter = NULL; - while (SDL_IterateHashTable(env->strings, (const void **)&key, (const void **)&value, &iter)) { - length += SDL_strlen(key) + 1 + SDL_strlen(value) + 1; - ++count; - } + CountEnvStringsData countdata = { 0, 0 }; + SDL_IterateHashTable(env->strings, CountEnvStrings, &countdata); // Allocate memory for the strings - result = (char **)SDL_malloc((count + 1) * sizeof(*result) + length); - char *string = (char *)(result + count + 1); - - // Second pass, copy the strings - count = 0; - iter = NULL; - while (SDL_IterateHashTable(env->strings, (const void **)&key, (const void **)&value, &iter)) { - size_t len; - - result[count] = string; - len = SDL_strlen(key); - SDL_memcpy(string, key, len); - string += len; - *string++ = '='; - len = SDL_strlen(value); - SDL_memcpy(string, value, len); - string += len; - *string++ = '\0'; - ++count; + result = (char **)SDL_malloc((countdata.count + 1) * sizeof(*result) + countdata.length); + if (result) { + // Second pass, copy the strings + char *string = (char *)(result + countdata.count + 1); + CopyEnvStringsData cpydata = { result, string, 0 }; + SDL_IterateHashTable(env->strings, CopyEnvStrings, &cpydata); + SDL_assert(countdata.count == cpydata.count); + result[cpydata.count] = NULL; } - result[count] = NULL; } SDL_UnlockMutex(env->lock); @@ -235,26 +258,23 @@ bool SDL_SetEnvironmentVariable(SDL_Environment *env, const char *name, const ch SDL_LockMutex(env->lock); { - const void *existing_value; - bool insert = true; - - if (SDL_FindInHashTable(env->strings, name, &existing_value)) { - if (!overwrite) { - result = true; - insert = false; - } else { - SDL_RemoveFromHashTable(env->strings, name); - } - } - - if (insert) { - char *string = NULL; - if (SDL_asprintf(&string, "%s=%s", name, value) > 0) { - size_t len = SDL_strlen(name); - string[len] = '\0'; - name = string; - value = string + len + 1; - result = SDL_InsertIntoHashTable(env->strings, name, value); + char *string = NULL; + if (SDL_asprintf(&string, "%s=%s", name, value) > 0) { + const size_t len = SDL_strlen(name); + string[len] = '\0'; + const char *origname = name; + name = string; + value = string + len + 1; + result = SDL_InsertIntoHashTable(env->strings, name, value, overwrite); + if (!result) { + SDL_free(string); + if (!overwrite) { + const void *existing_value = NULL; + // !!! FIXME: InsertIntoHashTable does this lookup too, maybe we should have a means to report that, to avoid duplicate work? + if (SDL_FindInHashTable(env->strings, origname, &existing_value)) { + result = true; // it already existed, and we refused to overwrite it. Call it success. + } + } } } } diff --git a/src/video/SDL_pixels.c b/src/video/SDL_pixels.c index 7242f71718..5fede2255a 100644 --- a/src/video/SDL_pixels.c +++ b/src/video/SDL_pixels.c @@ -648,7 +648,7 @@ const SDL_PixelFormatDetails *SDL_GetPixelFormatDetails(SDL_PixelFormat format) SDL_PixelFormatDetails *details; if (SDL_ShouldInit(&SDL_format_details_init)) { - SDL_format_details = SDL_CreateHashTable(NULL, 8, SDL_HashID, SDL_KeyMatchID, SDL_NukeFreeValue, true, false); + SDL_format_details = SDL_CreateHashTable(0, true, SDL_HashID, SDL_KeyMatchID, SDL_DestroyHashValue, NULL); if (!SDL_format_details) { SDL_SetInitialized(&SDL_format_details_init, false); return NULL; @@ -671,9 +671,13 @@ const SDL_PixelFormatDetails *SDL_GetPixelFormatDetails(SDL_PixelFormat format) return NULL; } - if (!SDL_InsertIntoHashTable(SDL_format_details, (const void *)(uintptr_t)format, (void *)details)) { + if (!SDL_InsertIntoHashTable(SDL_format_details, (const void *)(uintptr_t)format, (void *)details, false)) { SDL_free(details); - return NULL; + // uh...did another thread beat us to inserting this? + if (SDL_FindInHashTable(SDL_format_details, (const void *)(uintptr_t)format, (const void **)&details)) { + return details; + } + return NULL; // oh well. } return details; @@ -1133,7 +1137,7 @@ Uint8 SDL_LookupRGBAColor(SDL_HashTable *palette_map, Uint32 pixel, const SDL_Pa Uint8 b = (Uint8)((pixel >> 8) & 0xFF); Uint8 a = (Uint8)((pixel >> 0) & 0xFF); color_index = SDL_FindColor(pal, r, g, b, a); - SDL_InsertIntoHashTable(palette_map, (const void *)(uintptr_t)pixel, (const void *)(uintptr_t)color_index); + SDL_InsertIntoHashTable(palette_map, (const void *)(uintptr_t)pixel, (const void *)(uintptr_t)color_index, true); } return color_index; } @@ -1499,7 +1503,7 @@ bool SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst) } else { if (SDL_ISPIXELFORMAT_INDEXED(dstfmt->format)) { // BitField --> Palette - map->info.palette_map = SDL_CreateHashTable(NULL, 32, SDL_HashID, SDL_KeyMatchID, NULL, false, false); + map->info.palette_map = SDL_CreateHashTable(0, false, SDL_HashID, SDL_KeyMatchID, NULL, NULL); } else { // BitField --> BitField if (srcfmt == dstfmt) {