libsdl-org / sdl12-compat

An SDL-1.2 compatibility layer that uses SDL 2.0 behind the scenes.
Other
193 stars 40 forks source link

bumprace: sometimes hangs on Wayland #271

Open icculus opened 1 year ago

icculus commented 1 year ago

With https://github.com/libsdl-org/sdl12-compat/commit/67f8b3a85b782eefb4db90f34d5b0742ef2cb5fc (1.2.58 + 29 commits) the copyright message, explosion and CRASHED appear as expected, but on Wayland the game sometimes (50%) becomes unresponsive anyway. With X11 it seems to be reliable?

Originally posted by @smcv in https://github.com/libsdl-org/sdl12-compat/issues/253#issuecomment-1289059180

icculus commented 1 year ago

I can't reproduce this, either with Wayland or XWayland. Maybe we fixed it?

smcv commented 1 year ago

I'm still seeing this with Debian 12, GNOME 43 (Wayland mode), sdl12-compat 1.2.64 and SDL 2.26.5, with or withotu SDL_VIDEODRIVER=wayland. It doesn't happen every time.

icculus commented 1 year ago

If you get a chance, when it hangs, break in the debugger and see if it's definitely hanging in a function indefinitely or if the main loop is still running but we're failing to render anything further.

I'll try again to reproduce here if you don't have time, though!

Kontrabant commented 1 year ago

I can replicate this with current sdl12-compat running via the real SDL2 library, but when running sdl12-compat→sdl2-compat→SDL3, it's fine.

Looking at the bumprace code, it doesn't pump events when fading in the 'crashed' image. I'm guessing that the event queue is being implicitly run somewhere in SDL3, but not in the real SDL2, which leads to it fading in very slowly and eventually getting the "Bumprace is not responding" dialog.

icculus commented 1 year ago

Yeah, I think you're right:

//crash
  if ( user[pl].crashed )
  {
    DontReadKeys();
#ifdef SOUND    
    if (sound) {Mix_PlayChannel(-1,explode_sound,0);}    
#endif
    SDL_SetAlpha(explosion_pic,(SDL_SRCALPHA),255-80);
    DrawExplosion();
    DrawExplosion();
    DrawExplosion();
    DisplayMsg(pic_crashed,1);
    SDL_Delay(1000);
    endgame=1;
    fadeout();
  }

There's a full second blocking while the CRASHED message is on the screen, then the fadeout...

    rect.x = 0;
    rect.w = 800;
    rect.h = 1;
    for (y=0; y<600; y+=2) {
        time = SDL_GetTicks();
        rect.y = y;
        SDL_FillRect(Screen, &rect, SDL_MapRGB(Screen->format, 0,0,0));
        SDL_UpdateRects(Screen, 1, &rect);
        if (time == SDL_GetTicks())
            SDL_Delay(1);
    }
    for (y=599; y>0; y-=2) {
        rect.y = y;
        SDL_FillRect(Screen, &rect, SDL_MapRGB(Screen->format, 0,0,0));
        SDL_UpdateRects(Screen, 1, &rect);
    }

I think it's probably the SDL_Delay(1000) killing us here, but we might be able to squeak by with a quirk that always calls PumpEvents when presenting to the screen--something we currently only do when simulating SDL_INIT_EVENTTHREAD, but we could change an if to change that--and let the several-hundred calls to SDL_UpdateRects get the event queue moving sooner.

icculus commented 1 year ago

Does the hang go away with this patch?

diff --git a/src/SDL12_compat.c b/src/SDL12_compat.c
index 5b4536a..6ec6f07 100644
--- a/src/SDL12_compat.c
+++ b/src/SDL12_compat.c
@@ -6689,7 +6689,7 @@ PresentScreen(void)
      * Just pumping the event loop here simulates an event thread well enough
      * for most things.
      */
-    if (EventThreadEnabled) {
+    if (SDL_TRUE) {
         SDL_PumpEvents();
     }
Kontrabant commented 1 year ago

With that change, 'Crashed' still fades in extremely slowly and the game remains unresponsive, but the "Bumprace is not responding" dialog doesn't appear.

icculus commented 1 year ago

Okay, so that's why this isn't reproducing for me: the fade is very fast here, so I was beating the app-responsiveness timeout. That's the actual issue we need to solve here.