libsdl-org/sdl2-compat

textinput, textedit and drop events broken after recent SDL3 changes

Closed this issue · 10 comments

sezero commented

Just run checkkeys test program and hit a few keys.

CC: @slouken, @icculus

sezero commented

For Event3to2 I can do the following and it does fix checkkeys. It also fixes
e.g. quakespasm text input in game console.

diff --git a/src/sdl2_compat.c b/src/sdl2_compat.c
index 3749523..ffec361 100644
--- a/src/sdl2_compat.c
+++ b/src/sdl2_compat.c
@@ -1388,6 +1388,14 @@ Event3to2(const SDL_Event *event3, SDL2_Event *event2)
     SDL3_memcpy((&event2->common) + 1, (&event3->common) + 1, sizeof (SDL2_Event) - sizeof (SDL2_CommonEvent));
     /* mouse coords became floats in SDL3: */
     switch (event3->type) {
+    case SDL_EVENT_TEXT_INPUT:
+        SDL3_strlcpy(event2->text.text, event3->text.text, sizeof(event2->text.text));
+        break;
+    case SDL_EVENT_TEXT_EDITING:
+        SDL3_strlcpy(event2->edit.text, event3->edit.text, sizeof(event2->edit.text));
+        event2->edit.start = event3->edit.start;
+        event2->edit.length = event3->edit.length;
+        break;
     case SDL_EVENT_MOUSE_MOTION:
         renderer = SDL3_GetRenderer(SDL3_GetWindowFromID(event3->motion.windowID));
         if (renderer) {

However: What to do for Event2to3 ?
Should we SDL3_malloc the text and feed to SDL3? If so, where will it be freed
and whose job will it be to free it?

P.S.: There may be more issues with event changes other than those two I mentioned.

sezero commented

P.S.: There may be more issues with event changes other than those two I mentioned.

DropEvent should be broken by the looks of it. Won't be testing or working on it myself

However: What to do for Event2to3 ? Should we SDL3_malloc the text and feed to SDL3? If so, where will it be freed and whose job will it be to free it?

We should use SDL_AllocateEventMemory(), and it'll be automatically freed later.

sezero commented

OK, will be trying

sezero commented

Current progress below. Will need to do something for DropEvent too, but I'll have to read more to handle that.

diff --git a/src/sdl2_compat.c b/src/sdl2_compat.c
index 3749523..cb34756 100644
--- a/src/sdl2_compat.c
+++ b/src/sdl2_compat.c
@@ -793,6 +793,7 @@ typedef struct SDL2_TextEditingEvent
     Sint32 length;
 } SDL2_TextEditingEvent;
 
+#define SDL2_TEXTEDITING_EXT 0x305
 typedef struct SDL2_TextEditingExtEvent
 {
     Uint32 type;
@@ -1388,6 +1389,25 @@ Event3to2(const SDL_Event *event3, SDL2_Event *event2)
     SDL3_memcpy((&event2->common) + 1, (&event3->common) + 1, sizeof (SDL2_Event) - sizeof (SDL2_CommonEvent));
     /* mouse coords became floats in SDL3: */
     switch (event3->type) {
+    case SDL_EVENT_TEXT_INPUT:
+        SDL3_strlcpy(event2->text.text, event3->text.text, sizeof(event2->text.text));
+        break;
+    case SDL_EVENT_TEXT_EDITING:
+        if (SDL3_GetHintBoolean("SDL_IME_SUPPORT_EXTENDED_TEXT", SDL_FALSE) &&
+            SDL3_strlen(event3->edit.text) >= sizeof(event2->edit.text)) {
+            event2->editExt.type = SDL2_TEXTEDITING_EXT;
+            event2->editExt.windowID = event3->edit.windowID;
+            event2->editExt.text = SDL3_strdup(event3->edit.text);
+            event2->editExt.start = event3->edit.start;
+            event2->editExt.length = event3->edit.length;
+        }
+        else
+        {
+            SDL3_strlcpy(event2->edit.text, event3->edit.text, sizeof(event2->edit.text));
+            event2->edit.start = event3->edit.start;
+            event2->edit.length = event3->edit.length;
+        }
+        break;
     case SDL_EVENT_MOUSE_MOTION:
         renderer = SDL3_GetRenderer(SDL3_GetWindowFromID(event3->motion.windowID));
         if (renderer) {
@@ -1461,6 +1481,32 @@ Event2to3(const SDL2_Event *event2, SDL_Event *event3)
     SDL3_memcpy((&event3->common) + 1, (&event2->common) + 1, sizeof (SDL2_Event) - sizeof (SDL2_CommonEvent));
     /* mouse coords became floats in SDL3: */
     switch (event2->type) {
+    case SDL_EVENT_TEXT_INPUT: {
+        const size_t len = SDL3_strlen(event2->text.text) + 1;
+        event3->text.text = (char *)SDL3_AllocateEventMemory(len);
+        SDL3_memcpy(event3->text.text, event3->text.text, len);
+        break;
+    }
+    case SDL_EVENT_TEXT_EDITING: {
+        const size_t len = SDL3_strlen(event2->edit.text) + 1;
+        event3->edit.type = SDL_EVENT_TEXT_EDITING;
+        event3->edit.windowID = event2->edit.windowID;
+        event3->edit.start = event2->edit.start;
+        event3->edit.length = event2->edit.length;
+        event3->edit.text = (char *)SDL3_AllocateEventMemory(len);
+        SDL3_memcpy(event3->edit.text, event2->edit.text, len);
+        break;
+    }
+    case SDL2_TEXTEDITING_EXT: {
+        const size_t len = SDL3_strlen(event2->editExt.text) + 1;
+        event3->edit.type = SDL_EVENT_TEXT_EDITING;
+        event3->edit.windowID = event2->editExt.windowID;
+        event3->edit.start = event2->editExt.start;
+        event3->edit.length = event2->editExt.length;
+        event3->edit.text = (char *)SDL3_AllocateEventMemory(len);
+        SDL3_memcpy(event3->edit.text, event2->editExt.text, len);
+        break;
+    }
     case SDL_EVENT_MOUSE_MOTION:
         event3->motion.x = (float)event2->motion.x;
         event3->motion.y = (float)event2->motion.y;
diff --git a/src/sdl3_syms.h b/src/sdl3_syms.h
index 18ea97a..bb5f452 100644
--- a/src/sdl3_syms.h
+++ b/src/sdl3_syms.h
@@ -123,6 +123,7 @@ SDL3_SYM(SDL_bool,WaitEventTimeout,(SDL_Event *a, Sint32 b),(a,b),return)
 SDL3_SYM(int,PushEvent,(SDL_Event *a),(a),return)
 SDL3_SYM(void,SetEventFilter,(SDL_EventFilter a, void *b),(a,b),)
 SDL3_SYM(void,FilterEvents,(SDL_EventFilter a, void *b),(a,b),)
+SDL3_SYM(void *,AllocateEventMemory,(size_t a),(a),return)
 SDL3_SYM_PASSTHROUGH(Uint32,RegisterEvents,(int a),(a),return)
 SDL3_SYM_PASSTHROUGH(char*,GetBasePath,(void),(),return)
 SDL3_SYM_PASSTHROUGH(char*,GetPrefPath,(const char *a, const char *b),(a,b),return)
sezero commented

Question: How should an SDL_TEXTEDITING_EXT event be handled when it's
fed to SDL_PushEvent or SDL_PeepEvents with an SDL_ADDEVENT action?

In SDL2, as far as I understand, the char *text pointer is kept as is
and it will return to user program as is, and the user program will free
it later.

In SDL2-compat, however, there will be a duplication of it in Events3to2:
The user program will free the duplicated text pointer, is that what we
expect? If that's the case, should we free the original ptr in Events2to3
after duplicating it in order to avoid a leak?

(I'm probably over-thinking it..)

I don't think we need to handle SDL_TEXTEDITING_EXT, that's not an event that a user application would send, it just comes from an IME to the application.

Drop events are handled similarly to SDL2_TEXTEDITING_EXT, you just duplicate the data/file in either direction.

sezero commented

I don't think we need to handle SDL_TEXTEDITING_EXT, that's not an event that a user application would send, it just comes from an IME to the application.

OK, commenting out that part

Drop events are handled similarly to SDL2_TEXTEDITING_EXT, you just duplicate the data/file in either direction.

OK, and question: Are drop events also expected like SDL2_TEXTEDITING_EXT i.e.: only pushed to user program from SDL and not pushed to SDL by user?

OK, and question: Are drop events also expected like SDL2_TEXTEDITING_EXT i.e.: only pushed to user program from SDL and not pushed to SDL by user?

Yes, that’s correct.