-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fish Fillets - Next Generation: BadWindow in X_ChangeProperty (on GNOME + Xwayland) #234
Comments
This is calling SDL_GetWMInfo before SDL_SetVideoMode and that's upsetting things; it works if I |
This is what's triggering it in SDL2's x11 code: #ifdef X_HAVE_UTF8_STRING
if (SDL_X11_HAVE_UTF8 && videodata->im) {
data->ic =
X11_XCreateIC(videodata->im, XNClientWindow, w, XNFocusWindow, w,
XNInputStyle, XIMPreeditNothing | XIMStatusNothing,
NULL);
}
#endif I'm not sure why, and this little SDL2 program doesn't trigger it: #include "SDL.h"
#include "SDL_syswm.h"
int main(void)
{
SDL_Window *w;
SDL_SysWMinfo info20;
SDL_Init(SDL_INIT_VIDEO);
w = SDL_CreateWindow("Hello SDL", 100, 100, 100, 100, 0);
SDL_zero(info20);
SDL_VERSION(&info20.version);
SDL_GetWindowWMInfo(w, &info20);
SDL_DestroyWindow(w);
w = SDL_CreateWindow("Hello SDL", 100, 100, 100, 100, 0);
SDL_DestroyWindow(w);
SDL_Quit();
return 0;
} |
Thanks to libsdl-org/SDL@17322e2 we can see more exactly where SDL2 is getting called into. I still don't know what part of this is upsetting X11, but there's lots of stuff going on in here that my non-triggering reproduction case isn't doing, so that's more to research still. Also, it looks like it's not crashing with the X error, but cleanly shutting down after this cases SDL_CreateWindow() to fail, but I haven't looked more closely yet.
|
The SDL_CreateRGBSurface call is SDL_image loading a surface before we've created a window, and then sdl12-compat creating a surface for SDL_WM_SetIcon (I believe from the png that SDL_image just loaded), in case that's relevant. |
Nope, Xlib is calling exit() in that error handler, and the app has SDL_Quit in an atexit function. |
(sorry for all the spam, I'm just making notes here as I dig.) Digging deeper, I just have SDL_WM_SetIcon return immediately, since the crash still happens and that's just noise, then I run this 1.2 app with sdl12-compat and log the SDL2 calls: #include "SDL.h"
#include "SDL_syswm.h"
int main(int argc, char **argv)
{
SDL_SysWMinfo info12;
SDL_Init(SDL_INIT_VIDEO);
SDL_memset(&info12, '\0', sizeof (info12));
SDL_VERSION(&info12.version);
SDL_GetWMInfo(&info12);
SDL_SetVideoMode(100, 100, 0, 0);
SDL_Quit();
return 0;
} Then I do the same with fillets, and here's the diff between the two, up until it hits that fatal SDL_CreateWindow call: INFO: SDL2CALL SDL_GetDesktopDisplayMode
INFO: SDL2CALL SDL_AllocFormat
INFO: SDL2CALL SDL_strcasecmp
+INFO: SDL2CALL SDL_RWFromFile
+INFO: SDL2CALL SDL_malloc
+INFO: SDL2CALL SDL_memset
+INFO: SDL2CALL SDL_CreateRGBSurface
+INFO: SDL2CALL SDL_malloc
+INFO: SDL2CALL SDL_malloc
+INFO: SDL2CALL SDL_malloc
+INFO: SDL2CALL SDL_memset
+INFO: SDL2CALL SDL_memset
+INFO: SDL2CALL SDL_GetColorKey
+INFO: SDL2CALL SDL_GetSurfaceAlphaMod
+INFO: SDL2CALL SDL_memset
+INFO: SDL2CALL SDL_GetSurfaceBlendMode
+INFO: SDL2CALL SDL_SetColorKey
+INFO: SDL2CALL SDL_GetColorKey
+INFO: SDL2CALL SDL_free
+INFO: SDL2CALL SDL_FreeSurface
+INFO: SDL2CALL SDL_free
+INFO: SDL2CALL SDL_free
+INFO: SDL2CALL SDL_free
INFO: SDL2CALL SDL_CreateWindow
INFO: SDL2CALL SDL_memset
INFO: SDL2CALL SDL_GetWindowWMInfo
@@ -104,77 +124,14 @@
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_getenv
INFO: SDL2CALL SDL_CreateWindow |
valgrind shows no memory errors in fillets, so it's not an obvious memory corruption. I guess I'll add SDL_image into the mix here. |
This program gives (what appears to be) the exact same string of SDL2 calls but does not crash: #include "SDL.h"
#include "SDL_syswm.h"
#include "SDL_image.h"
int main(int argc, char **argv)
{
SDL_SysWMinfo info12;
SDL_Surface *surface;
SDL_RWops *io;
SDL_Init(SDL_INIT_VIDEO);
//printf("imginit=%d\n", IMG_Init(IMG_INIT_PNG));
io = SDL_RWFromFile("/usr/share/games/fillets-ng/images/icon.png", "rb");
//printf("io=%p\n", io);
surface = IMG_LoadTyped_RW(io, 1, "PNG");
//printf("surface=%p%s\n", surface, surface ? "" : IMG_GetError());
SDL_WM_SetIcon(surface, NULL);
SDL_FreeSurface(surface);
SDL_memset(&info12, '\0', sizeof (info12));
SDL_VERSION(&info12.version);
SDL_GetWMInfo(&info12);
SDL_SetVideoMode(100, 100, 0, 0);
SDL_Quit();
return 0;
} The same functions are called down to the crashing SDL_CreateWindow, so there's something else happening here, or some specific of a given call, I don't know, but it's probably time to either give up or debug this down into Xlib and see what's upsetting it. |
Ugh, the binary links directly to libX11, and calls Xutf8TextListToTextProperty, XSetWMName, and XFree between the calls to SDL_GetWMInfo and SDL_SetVideoMode. Returning immediately with an error from SDL_GetWMInfo was a red herring, since it obviously wanted the Display pointer to call these Xlib functions directly, and went on without them, avoiding the crash. I guess the solution now is to add a quirk that says "refuse to provide syswm info to this app," which could be useful for other apps that also do unnecessary X11 stuff like this but would otherwise work on Wayland. (Although in this case, forcing SDL_VIDEODRIVER=wayland also works, since it presumably discovers it can't do X11 stuff and carries on.) |
And hooked up "fillets" to it. Reference Issue #234.
Okay, the quirk gets us past the X11 error, and the game seems largely playable now, but there are bursts of static in the first level from some sound it's not playing/converting correctly, so I'll check that before closing this issue. |
Prerequisites:
pipewire-pulse
emulating PulseAudioapt install fillets-ng
(Debian package version1.0.1-4+b1
)libsdl1.2debian
(real SDL 1.2, with an odd package name for historical reasons) version1.2.15+dfsg2-8
libsdl1.2-compat
commit eba13ef, locally-builtlibsdl2-2.0-0
version2.24.1+dfsg-1
To reproduce:
fillets
LD_LIBRARY_PATH=.../sdl12-compat/_build fillets
Expected result: both work
Actual result: with sdl12-compat, it fails with:
Workaround:
SDL_VIDEODRIVER=wayland
works.The text was updated successfully, but these errors were encountered: