sdl2-compat

sdl2-compat copied to clipboard

As of commit 3f84910, any attempt to build sdl2-compat for WinRT fails because of unsupported calls to MessageBoxA, lstrcpyA and wsprintfA, and possibly LoadLibraryA.

Originally posted by @sezero in https://github.com/libsdl-org/sdl2-compat/issues/40#issuecomment-1422518698

This might not be super-important, but just so this isn't lost, I'm tossing this into a separate bug.

The lstrcpyA and wsprintfA dependencies are easily overcome by a local stpcpy() and itoa() implementation, and since those two are universal and standalone they work for everything: Patch below, OK to apply?

diff --git a/src/sdl2_compat.c b/src/sdl2_compat.c
index f8b7d24..e3e843d 100644
--- a/src/sdl2_compat.c
+++ b/src/sdl2_compat.c
@@ -146,6 +146,41 @@ static SDL_bool WantDebugLogging = SDL_FALSE;
 static Uint32 LinkedSDL3VersionInt = 0;
 
 
+static char *
+SDL2COMPAT_stpcpy(char *dst, const char *src)
+{
+    while ((*dst++ = *src++) != '\0') {
+        /**/;
+    }
+    return --dst;
+}
+
+static void
+SDL2COMPAT_itoa(char *dst, int val)
+{
+    char *ptr, temp;
+
+    if (val < 0) {
+        *dst++ = '-';
+        val = -val;
+    }
+    ptr = dst;
+
+    do {
+        *ptr++ = '0' + (val % 10);
+        val /= 10;
+    } while (val > 0);
+    *ptr-- = '\0';
+
+    /* correct the order of digits */
+    do {
+        temp = *dst;
+        *dst++ = *ptr;
+        *ptr-- = temp;
+    } while (ptr > dst);
+}
+
+
 /* Obviously we can't use SDL_LoadObject() to load SDL3.  :)  */
 /* FIXME: Updated library names after https://github.com/libsdl-org/SDL/issues/5626 solidifies.  */
 static char loaderror[256];
@@ -156,16 +191,12 @@ static char loaderror[256];
     #define LoadSDL3Library() ((Loaded_SDL3 = LoadLibraryA(SDL3_LIBNAME)) != NULL)
     #define LookupSDL3Sym(sym) (void *)GetProcAddress(Loaded_SDL3, sym)
     #define CloseSDL3Library() { if (Loaded_SDL3) { FreeLibrary(Loaded_SDL3); Loaded_SDL3 = NULL; } }
-    #define strcpy_fn  lstrcpyA
-    #define sprintf_fn wsprintfA
 #elif defined(__APPLE__)
     #include <dlfcn.h>
     #include <pwd.h>
     #include <unistd.h>
     #define SDL3_LIBNAME "libSDL3.dylib"
     #define SDL3_FRAMEWORK "SDL3.framework/Versions/A/SDL3"
-    #define strcpy_fn  strcpy
-    #define sprintf_fn sprintf
     static void *Loaded_SDL3 = NULL;
     #define LookupSDL3Sym(sym) dlsym(Loaded_SDL3, sym)
     #define CloseSDL3Library() { if (Loaded_SDL3) { dlclose(Loaded_SDL3); Loaded_SDL3 = NULL; } }
@@ -218,8 +249,6 @@ static char loaderror[256];
     #define LoadSDL3Library() ((Loaded_SDL3 = dlopen(SDL3_LIBNAME, RTLD_LOCAL|RTLD_NOW)) != NULL)
     #define LookupSDL3Sym(sym) dlsym(Loaded_SDL3, sym)
     #define CloseSDL3Library() { if (Loaded_SDL3) { dlclose(Loaded_SDL3); Loaded_SDL3 = NULL; } }
-    #define strcpy_fn  strcpy
-    #define sprintf_fn sprintf
 #else
     #error Please define your platform.
 #endif
@@ -243,7 +272,8 @@ LoadSDL3Symbol(const char *fn, int *okay)
     if (*okay) { /* only bother trying if we haven't previously failed. */
         retval = LookupSDL3Sym(fn);
         if (retval == NULL) {
-            sprintf_fn(loaderror, "%s missing in SDL3 library.", fn);
+            char *p = SDL2COMPAT_stpcpy(loaderror, fn);
+            SDL2COMPAT_stpcpy(p, " missing in SDL3 library.");
             *okay = 0;
         }
     }
@@ -484,7 +514,7 @@ LoadSDL3(void)
 
         okay = LoadSDL3Library();
         if (!okay) {
-            strcpy_fn(loaderror, "Failed loading SDL3 library.");
+            SDL2COMPAT_stpcpy(loaderror, "Failed loading SDL3 library.");
         } else {
             #define SDL3_SYM(rc,fn,params,args,ret) SDL3_##fn = (SDL3_##fn##_t) LoadSDL3Symbol("SDL_" #fn, &okay);
             #include "sdl3_syms.h"
@@ -494,7 +524,19 @@ LoadSDL3(void)
                 LinkedSDL3VersionInt = SDL_VERSIONNUM(v.major, v.minor, v.patch);
                 okay = (LinkedSDL3VersionInt >= SDL3_REQUIRED_VER);
                 if (!okay) {
-                    sprintf_fn(loaderror, "SDL3 %d.%d.%d library is too old.", v.major, v.minor, v.patch);
+                    char value[12];
+                    char *p = SDL2COMPAT_stpcpy(loaderror, "SDL3 ");
+
+                    SDL2COMPAT_itoa(value, v.major);
+                    p = SDL2COMPAT_stpcpy(p, value);
+                    *p++ = '.';
+                    SDL2COMPAT_itoa(value, v.minor);
+                    p = SDL2COMPAT_stpcpy(p, value);
+                    *p++ = '.';
+                    SDL2COMPAT_itoa(value, v.patch);
+                    p = SDL2COMPAT_stpcpy(p, value);
+
+                    SDL2COMPAT_stpcpy(p, " library is too old.");
                 } else {
                     WantDebugLogging = SDL2Compat_GetHintBoolean("SDL2COMPAT_DEBUG_LOGGING", SDL_FALSE);
                     if (WantDebugLogging) {
@@ -1070,7 +1112,7 @@ SDL2Compat_InitOnStartup(void)
     return 1;
 
 fail:
-    strcpy_fn(loaderror, "Failed to initialize sdl2-compat library.");
+    SDL2COMPAT_stpcpy(loaderror, "Failed to initialize sdl2-compat library.");
 
     if (EventWatchListMutex) {
         SDL3_DestroyMutex(EventWatchListMutex);

If the above patch is applied, MessageBoxA() seems to be the only problematic call: SDL has a winrt implementation in C++, but I have no experience with that.

Yep, seems fine to me.

Yep, seems fine to me.

Applied. (And it applies almost without any changes to sdl1.2-compat too: Should I?)

Now MessageBoxA, and possibly LoadLibraryA, seem to be the only problematic calls in sdl2-compat.

Applied. (And it applies almost without any changes to sdl1.2-compat too: Should I?)

Sure, seems fine.

For now, let's just change LoadLibraryA to LoadLibraryW and SDL3_LIBNAME to L"SDL3.dll" ... this should work out of the box on everything back to Windows XP anyhow.

For MessageBoxA, we can't use SDL_iconv because we only call this when SDL3 failed to load, but we completely control the error string afaict and it's all low-ASCII, so we could just do something like this:

static void error_dialog(const char *errorMsg)
{
    WCHAR werrmsg[256];
    int i;
    for (i = 0; (i < (SDL_arraysize(werrmsg) - 1)) && errorMsg[i]; i++) {
        werrmsg[i] = (WCHAR) errorMsg[i]; /* low-ASCII maps to WCHAR directly. */
     }
    werrmsg[i] = '\0';

    MessageBoxW(NULL, werrmsg, L"Error", MB_OK | MB_SETFOREGROUND | MB_ICONSTOP);
}

Again, this code will presumably work on both Win32 back to Windows XP and WinRT.

Then we need to actually test this on WinRT in any case. :)

Sound like a plan?

Yep!

Both MessageBoxW and LoadLibraryW are documented to be "destop apps only", so I can't see how that would help.

Maybe something like this? (Disclaimer: Not even compile tested, I don't do winrt or c++ and the cmake'ry is possibly bullsh?t: @icculus and @madebr to rescue.)

test.zip

Both MessageBoxW and LoadLibraryW are documented to be "destop apps only", so I can't see how that would help.

Huh, I didn't realize that. :/

LoadPackagedLibrary() is apparently the correct way to do this from UWP, but it has limitations (but I assume UWP developers will be shipping sdl2-compat and SDL3.dll packaged with their app anyhow, so I think it'll work out?).

https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-loadpackagedlibrary?redirectedfrom=MSDN

Maybe something like this?

I think yes, but we probably need someone to take the time to compile testsprite2 for UWP and see what the state of all of this is.

Until then, I don't think it hurts to get it in revision control as-is, since we know the current code is 100% broken until we do something to fix it.

Maybe something like this?

I think yes, but we probably need someone to take the time to compile testsprite2 for UWP and see what the state of all of this is.

Until then, I don't think it hurts to get it in revision control as-is, since we know the current code is 100% broken until we do something to fix it.

You mean push it to git as is for now?

P.S.: Is the winrt code not doable in C-only?

Yeah, push it to git. It's a starting point if someone comes along to finish it (or we find time to do so).

A lot of the Win32 API is available from C on WinRT, but when Microsoft decided to cut parts out, the replacements are all C++ (or whatever variant of C++ it uses..."C++/CX" or something...?). It's annoying.

My guess is that message boxes got replaced because they wanted to offer a real async API instead of MessageBoxA(), which blocks and runs the Windows event queue internally...so we're stuck with C++ for that.

OK, pushed to git main.

With these 2 patches (cmake + ci), I can get SDL2-compat compiling, but not linking. The linking errors disappear when I remove the _DllMainCRTStartup function, but that's no good.

These are the linking errors.

With these 2 patches (cmake + ci), I can get SDL2-compat compiling, but not linking.

Those changes should be good I guess and can go in

The linking errors disappear when I remove the _DllMainCRTStartup function, but that's no good.

Without _DllMainCRTStartup I don't know how we can do the dlopen magic..

I got it linking, and running with this additional patch. ci I know you aren't supposed to do LoadLibraryA in DllMain, but this works... Also, when the app can't find SDL3.dll, it hangs rather then showing a error dialog.

I can now start testviewport, but SDL_Init fails here.

I know you aren't supposed to do LoadLibraryA in DllMain, but this works...

Are you doing LoadLibraryA in your patch? Or am I looking at the wrong place?

Also, when the app can't find SDL3.dll, it hangs rather then showing a error dialog.

The error dialog procedure is not tested - it's just copied from SDL2 with some butchering. @icculus or @slouken may help

I can now start testviewport, but SDL_Init fails here.

We aren't building SDL_main, that may be the reason?

I know you aren't supposed to do LoadLibraryA in DllMain, but this works...

Are you doing LoadLibraryA in your patch? Or am I looking at the wrong place?

sdl2-compat uses LoadLibraryA/LoadPackagedLibrary to load SDL3.dll.

The error dialog procedure is not tested - it's just copied from SDL2 with some butchering. @icculus or @slouken may help

We aren't building SDL_main, that may be the reason?

I copied SDL2's SDL2main, but it needs extra care to be compatible with SDL3. It fails here.

I know you aren't supposed to do LoadLibraryA in DllMain, but this works...

Are you doing LoadLibraryA in your patch? Or am I looking at the wrong place?

sdl2-compat uses LoadLibraryA/LoadPackagedLibrary to load SDL3.dll.

Ah, I misread the meaning.

Other than calling LoadPackagedLibrary from DllMain, one can use a function marked as constructor like gcc does, but I don't know how / if that is done with MSVC.

Other than calling LoadPackagedLibrary from DllMain, one can use a function marked as constructor like gcc does, but I don't know how / if that is done with MSVC.

This alternative MSVC-only way has the same behavior (currently). It uses CRT initialization. I don't know of a destructor equivalent.

--- a/src/sdl2_compat.c
+++ b/src/sdl2_compat.c
@@ -668,7 +668,7 @@ __declspec(selectany) int _fltused = 1;
 #define _DllMainCRTStartup LibMain
 #endif
 #if defined(__WINRT__)
-#define _DllMainCRTStartup DllMain
+#define _DllMainCRTStartup SDL2compat_DllMain
 #endif
 BOOL WINAPI _DllMainCRTStartup(HANDLE dllhandle, DWORD reason, LPVOID reserved)
 {
@@ -706,6 +706,14 @@ BOOL WINAPI _DllMainCRTStartup(HANDLE dllhandle, DWORD reason, LPVOID reserved)
     return TRUE;
 }

+#if defined(__WINRT__)
+#pragma section(".CRT$XCZ", read)
+static void __cdecl sdl2compat_entry(void) {
+    SDL2compat_DllMain(NULL, DLL_PROCESS_ATTACH, NULL);
+}
+__declspec(dllexport) __declspec(allocate(".CRT$XCZ")) void(__cdecl* sdl2compat_entry_crt)(void) = sdl2compat_entry;
+#endif
+
 #else
     #error Please define an init procedure for your platform.
 #endif

Other than calling LoadPackagedLibrary from DllMain, one can use a function marked as constructor like gcc does, but I don't know how / if that is done with MSVC.

This alternative MSVC-only way has the same behavior (currently). It uses CRT initialization. I don't know of a destructor equivalent.

Not good without a destructor equivalent as far as I can see.

Other than calling LoadPackagedLibrary from DllMain, one can use a function marked as constructor like gcc does, but I don't know how / if that is done with MSVC.

This alternative MSVC-only way has the same behavior (currently). It uses CRT initialization. I don't know of a destructor equivalent.

Not good without a destructor equivalent as far as I can see.

If LoadLibrary in DllMain turns out problematic, we might combine the CRT initialization for doing LoadLibrary and initialization with DllMain for doing the clean up. But plugging in the CRT initialization is probably a bad idea anyways. WinRT uses c++, and the STL might not have been completely initialized.

If LoadLibrary in DllMain

That wisdom is not restricted to winrt, is it not? We've been using that for 'normal' windows for some time. Let's leave that discussion for later unless it truly bites us.

SDL3 killed winrt/uwp, therefore this ticket is now stale. Closing.