libsdl-org / sdl12-compat

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

Do'SSi Zo'la: some UI elements disappear #233

Closed smcv closed 1 year ago

smcv commented 1 year ago

Prerequisites:

To reproduce:

Expected result: Both players are visible at all times, with the active player animated and the inactive player standing still. All UI text is visible, including "Exit Now : [ No ] [ Yes ]" in the exit confirmation prompt.

Actual result: The active player is visible and animated. The inactive player is invisible. The prompt to move or fire appears for player 1, but not for player 2. When I press Esc to exit, I see "Exit Now : [ Yes ]", with the "No" button invisible.

icculus commented 1 year ago

This is something wrong with SDL_SetAlpha; if I comment it out, the game renders (with some things blending incorrectly, of course) without any pieces suddenly vanishing.

icculus commented 1 year ago

This is failing because eventually we end up in SDL2's SDL_BlitSurface (SDL_UpperBlit), and when trying to figure out what to do with these surfaces, we land in this switch statement in SDL_CalculateBlit1 ...

https://github.com/libsdl-org/SDL/blob/237348c772b4ff0e758ace83f471dbf8570535e2/src/video/SDL_blit_1.c#L529-L544

...and surface->map->info.flags is SDL_COPY_COLORKEY | SDL_COPY_BLEND, which this switch statement doesn't handle, so we get a NULL and it fails all the way back out of SDL_BlitSurface without blitting anything.

I'm not really sure how to deal with this yet.

icculus commented 1 year ago

This fixes it in sdl12-compat...

diff --git a/src/SDL12_compat.c b/src/SDL12_compat.c
index f29e98f..0c89e77 100644
--- a/src/SDL12_compat.c
+++ b/src/SDL12_compat.c
@@ -6540,7 +6540,8 @@ SDL_SetAlpha(SDL12_Surface *surface12, Uint32 flags12, Uint8 value)
             }
         }
         surface12->flags |= SDL12_SRCALPHA;
-        SDL20_SetSurfaceBlendMode(surface12->surface20, SDL_BLENDMODE_BLEND);
+        /* SDL2 can get into ugly situations in corner case where blending a colorkey'd 8-bit surface does nothing. Turn off blending if we're at full alpha. */
+        SDL20_SetSurfaceBlendMode(surface12->surface20, (surface12->format->alpha != 255) ? SDL_BLENDMODE_BLEND : SDL_BLENDMODE_NONE);
     } else {
         if (!surface12->format->Amask) {  /* whole-surface alpha is ignored if surface has an alpha channel. */
             retval = SDL20_SetSurfaceAlphaMod(surface12->surface20, 255);

Basically we just turn off blending when we're at full alpha, so we hit the SDL_COPY_COLORKEY case of that switch instead and SDL2 can handle the blit. But I also sort of think it would be better to patch this in SDL2.

We toggle the blend mode all over the place in sdl12-compat, so if this isn't worth fixing in SDL2, I'll probably add a quirk so we only do this fix for this game.

icculus commented 1 year ago

Here's the SDL2 fix. I think this is probably more correct than working around this in sdl12-compat, and for something as obscure as this game, I'm willing to leave it broken for people that aren't on the upcoming SDL 2.28.0 release.

diff --git a/src/video/SDL_blit_1.c b/src/video/SDL_blit_1.c
index ba56979bf..f7c5b8099 100644
--- a/src/video/SDL_blit_1.c
+++ b/src/video/SDL_blit_1.c
@@ -533,6 +533,9 @@ SDL_CalculateBlit1(SDL_Surface *surface)
     case SDL_COPY_COLORKEY:
         return one_blitkey[which];

+    case SDL_COPY_COLORKEY | SDL_COPY_BLEND:  /* this is not super-robust but handles a specific case we found sdl12-compat. */
+        return (surface->map->info.a == 255) ? one_blitkey[which] : (SDL_BlitFunc)NULL;
+

@slouken, any objection to me applying this to SDL2/SDL3?

slouken commented 1 year ago

No, that seems fine to me.

icculus commented 1 year ago

Okay, we're good here. Closing!