From 767a91cc47d964e9512feae7feae0a6f3dc762f6 Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Wed, 1 Jan 2025 12:54:34 -0800
Subject: [PATCH] Updated the SDL_ttf hashtable implementation to match SDL
This fixes a crash on emscripten where both implementations are linked as .a files and it's not clear which one will be used.
---
external/SDL | 2 +-
src/SDL_gpu_textengine.c | 6 +--
src/SDL_hashtable.c | 76 ++++++++++++++++++++++++++---------
src/SDL_hashtable.h | 14 +++++++
src/SDL_renderer_textengine.c | 6 +--
src/SDL_surface_textengine.c | 4 +-
src/SDL_ttf.c | 2 +-
7 files changed, 82 insertions(+), 28 deletions(-)
diff --git a/external/SDL b/external/SDL
index f79f2121..2b1d809b 160000
--- a/external/SDL
+++ b/external/SDL
@@ -1 +1 @@
-Subproject commit f79f21217bbb7aba82d68076af5119956b44d046
+Subproject commit 2b1d809b21575b0f87a7cf5f1d33704dfae8ada5
diff --git a/src/SDL_gpu_textengine.c b/src/SDL_gpu_textengine.c
index 40d29beb..ca5442d0 100644
--- a/src/SDL_gpu_textengine.c
+++ b/src/SDL_gpu_textengine.c
@@ -494,7 +494,7 @@ static bool CreateMissingGlyphs(TTF_GPUTextEngineData *enginedata, TTF_GPUTextEn
goto done;
}
- checked = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NULL, false);
+ checked = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NULL, false, false);
if (!checked) {
goto done;
}
@@ -810,7 +810,7 @@ static TTF_GPUTextEngineFontData *CreateFontData(TTF_GPUTextEngineData *engineda
}
data->font = font;
data->generation = font_generation;
- data->glyphs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NukeGlyph, false);
+ data->glyphs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NukeGlyph, false, false);
if (!data->glyphs) {
DestroyFontData(data);
return NULL;
@@ -859,7 +859,7 @@ static TTF_GPUTextEngineData *CreateEngineData(SDL_GPUDevice *device)
data->device = device;
data->winding = TTF_GPU_TEXTENGINE_WINDING_CLOCKWISE;
- data->fonts = SDL_CreateHashTable(NULL, 4, SDL_HashPointer, SDL_KeyMatchPointer, NukeFontData, false);
+ data->fonts = SDL_CreateHashTable(NULL, 4, SDL_HashPointer, SDL_KeyMatchPointer, NukeFontData, false, false);
if (!data->fonts) {
DestroyEngineData(data);
return NULL;
diff --git a/src/SDL_hashtable.c b/src/SDL_hashtable.c
index 47bf18c2..8b1b8b93 100644
--- a/src/SDL_hashtable.c
+++ b/src/SDL_hashtable.c
@@ -44,6 +44,7 @@ SDL_COMPILE_TIME_ASSERT(sizeof_SDL_HashItem, sizeof(SDL_HashItem) <= MAX_HASHITE
struct SDL_HashTable
{
+ SDL_RWLock *lock;
SDL_HashItem *table;
SDL_HashTable_HashFn hash;
SDL_HashTable_KeyMatchFn keymatch;
@@ -60,6 +61,7 @@ SDL_HashTable *SDL_CreateHashTable(void *data,
SDL_HashTable_HashFn hashfn,
SDL_HashTable_KeyMatchFn keymatchfn,
SDL_HashTable_NukeFn nukefn,
+ bool threadsafe,
bool stackable)
{
SDL_HashTable *table;
@@ -80,9 +82,14 @@ SDL_HashTable *SDL_CreateHashTable(void *data,
return NULL;
}
+ if (threadsafe) {
+ // Don't fail if we can't create a lock (single threaded environment?)
+ table->lock = SDL_CreateRWLock();
+ }
+
table->table = (SDL_HashItem *)SDL_calloc(num_buckets, sizeof(SDL_HashItem));
if (!table->table) {
- SDL_free(table);
+ SDL_DestroyHashTable(table);
return NULL;
}
@@ -289,17 +296,22 @@ bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *
{
SDL_HashItem *item;
Uint32 hash;
+ bool result = false;
if (!table) {
return false;
}
+ if (table->lock) {
+ SDL_LockRWLockForWriting(table->lock);
+ }
+
hash = calc_hash(table, key);
item = find_first_item(table, key, hash);
if (item && !table->stackable) {
- // TODO: Maybe allow overwrites? We could do it more efficiently here than unset followed by insert.
- return false;
+ // Allow overwrites, this might have been inserted on another thread
+ delete_item(table, item);
}
SDL_HashItem new_item;
@@ -313,18 +325,25 @@ bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *
if (!maybe_resize(table)) {
table->num_occupied_slots--;
- return false;
+ goto done;
}
// This never returns NULL
insert_item(&new_item, table->table, table->hash_mask, &table->max_probe_len);
- return true;
+ result = true;
+
+done:
+ if (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) {
@@ -333,26 +352,39 @@ bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void
return false;
}
+ if (table->lock) {
+ SDL_LockRWLockForReading(table->lock);
+ }
+
hash = calc_hash(table, key);
i = find_first_item(table, key, hash);
if (i) {
if (value) {
*value = i->value;
}
- return true;
+ result = true;
}
- return false;
+
+ if (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;
}
+ if (table->lock) {
+ SDL_LockRWLockForWriting(table->lock);
+ }
+
// 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,
@@ -361,13 +393,18 @@ bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key)
hash = calc_hash(table, key);
item = find_first_item(table, key, hash);
-
if (!item) {
- return false;
+ goto done;
}
delete_item(table, item);
- return true;
+ result = true;
+
+done:
+ if (table->lock) {
+ SDL_UnlockRWLock(table->lock);
+ }
+ return result;
}
bool SDL_IterateHashTableKey(const SDL_HashTable *table, const void *key, const void **_value, void **iter)
@@ -472,22 +509,25 @@ static void nuke_all(SDL_HashTable *table)
void SDL_EmptyHashTable(SDL_HashTable *table)
{
if (table) {
- if (table->nuke) {
- nuke_all(table);
- }
+ SDL_LockRWLockForWriting(table->lock);
+ {
+ if (table->nuke) {
+ nuke_all(table);
+ }
- SDL_memset(table->table, 0, sizeof(*table->table) * (table->hash_mask + 1));
- table->num_occupied_slots = 0;
+ SDL_memset(table->table, 0, sizeof(*table->table) * (table->hash_mask + 1));
+ table->num_occupied_slots = 0;
+ }
+ SDL_UnlockRWLock(table->lock);
}
}
void SDL_DestroyHashTable(SDL_HashTable *table)
{
if (table) {
- if (table->nuke) {
- nuke_all(table);
- }
+ SDL_EmptyHashTable(table);
+ SDL_DestroyRWLock(table->lock);
SDL_free(table->table);
SDL_free(table);
}
diff --git a/src/SDL_hashtable.h b/src/SDL_hashtable.h
index b54a7c81..5983b8d3 100644
--- a/src/SDL_hashtable.h
+++ b/src/SDL_hashtable.h
@@ -34,19 +34,33 @@ extern SDL_HashTable *SDL_CreateHashTable(void *data,
SDL_HashTable_HashFn hashfn,
SDL_HashTable_KeyMatchFn keymatchfn,
SDL_HashTable_NukeFn nukefn,
+ bool threadsafe,
bool stackable);
+// 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.
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);
+
+// This function is thread-safe if the hashtable was created with threadsafe = true
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);
+
+// This function is thread-safe if the hashtable was created with threadsafe = true
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 (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);
diff --git a/src/SDL_renderer_textengine.c b/src/SDL_renderer_textengine.c
index e472cb7d..da3cc7b9 100644
--- a/src/SDL_renderer_textengine.c
+++ b/src/SDL_renderer_textengine.c
@@ -430,7 +430,7 @@ static bool CreateMissingGlyphs(TTF_RendererTextEngineData *enginedata, TTF_Rend
goto done;
}
- checked = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NULL, false);
+ checked = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NULL, false, false);
if (!checked) {
goto done;
}
@@ -724,7 +724,7 @@ static TTF_RendererTextEngineFontData *CreateFontData(TTF_RendererTextEngineData
}
data->font = font;
data->generation = font_generation;
- data->glyphs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NukeGlyph, false);
+ data->glyphs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NukeGlyph, false, false);
if (!data->glyphs) {
DestroyFontData(data);
return NULL;
@@ -772,7 +772,7 @@ static TTF_RendererTextEngineData *CreateEngineData(SDL_Renderer *renderer)
}
data->renderer = renderer;
- data->fonts = SDL_CreateHashTable(NULL, 4, SDL_HashPointer, SDL_KeyMatchPointer, NukeFontData, false);
+ data->fonts = SDL_CreateHashTable(NULL, 4, SDL_HashPointer, SDL_KeyMatchPointer, NukeFontData, false, false);
if (!data->fonts) {
DestroyEngineData(data);
return NULL;
diff --git a/src/SDL_surface_textengine.c b/src/SDL_surface_textengine.c
index 6ea7ce2c..e5d313f3 100644
--- a/src/SDL_surface_textengine.c
+++ b/src/SDL_surface_textengine.c
@@ -181,7 +181,7 @@ static TTF_SurfaceTextEngineFontData *CreateFontData(TTF_SurfaceTextEngineData *
data->font = font;
data->generation = font_generation;
- data->glyphs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NukeGlyphData, false);
+ data->glyphs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NukeGlyphData, false, false);
if (!data->glyphs) {
DestroyFontData(data);
return NULL;
@@ -222,7 +222,7 @@ static TTF_SurfaceTextEngineData *CreateEngineData(void)
return NULL;
}
- data->fonts = SDL_CreateHashTable(NULL, 4, SDL_HashPointer, SDL_KeyMatchPointer, NukeFontData, false);
+ data->fonts = SDL_CreateHashTable(NULL, 4, SDL_HashPointer, SDL_KeyMatchPointer, NukeFontData, false, false);
if (!data->fonts) {
DestroyEngineData(data);
return NULL;
diff --git a/src/SDL_ttf.c b/src/SDL_ttf.c
index 9f13f837..769392c0 100644
--- a/src/SDL_ttf.c
+++ b/src/SDL_ttf.c
@@ -1963,7 +1963,7 @@ TTF_Font *TTF_OpenFontWithProperties(SDL_PropertiesID props)
font->hdpi = TTF_DEFAULT_DPI;
font->vdpi = TTF_DEFAULT_DPI;
- font->text = SDL_CreateHashTable(NULL, 16, SDL_HashPointer, SDL_KeyMatchPointer, NULL, false);
+ font->text = SDL_CreateHashTable(NULL, 16, SDL_HashPointer, SDL_KeyMatchPointer, NULL, false, false);
if (!font->text) {
TTF_CloseFont(font);
return NULL;