libsdl-org / sdl12-compat

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

bad video with icculus.org quake2 + ref_softsdl #216

Open sezero opened 2 years ago

sezero commented 2 years ago

Quake2 built from http://svn.icculus.org/quake2/trunk with only SDL options enabled in Makefile and run with ./quake2 +set vid_ref softsdl gives funky colors in video when run against sdl12-compat, like bad palette usage. Real SDL1.2 is OK. (Note that the sdlquake binary coredumps, so use the quake2 binary.) Curiously the pcx screenshot taken with F12 key looks OK, therefore the attached shot is taken using Gnome functionalities.

Screenshot

sezero commented 2 years ago

Changing SDL_SetPalette to SDL_SetColors fixes display, like:

Index: src/linux/rw_sdl.c
===================================================================
--- src/linux/rw_sdl.c  (revision 205)
+++ src/linux/rw_sdl.c  (working copy)
@@ -747,7 +747,7 @@ void SWimp_SetPalette( const unsigned ch
        colors[i].b = palette[i*4+2];
    }

-   SDL_SetPalette(surface, sdl_palettemode, colors, 0, 256);
+   SDL_SetColors(surface, colors, 0, 256);
 }
 #endif

SDL_SetColors itself calls SDL_SetPalette with SDL12_LOGPAL | SDL12_PHYSPAL: https://github.com/libsdl-org/sdl12-compat/blob/main/src/SDL12_compat.c#L7068

On the other hand, rw_sdl.c sets sdl_palettemode to 0x1, i.e. SDL12_LOGPAL, so, calling SDL_SetPalette there only calls first the SDL20_SetPaletteColors with palette20, but not the second one with VideoPhysicalPalette20: https://github.com/libsdl-org/sdl12-compat/blob/main/src/SDL12_compat.c#L7043

The result is an incompatibility with real SDL-1.2.

sezero commented 2 years ago

This is possibly a bug in rw_sdl.c -- can't tell now exactly, tired, will sleep. @icculus: What can you see in there?

slouken commented 2 years ago

This seems like it ought to work. What quake is doing is setting the palette on the surface, but only the logical palette, not the physical palette, because the display isn't 8-bit. Then presumably it does a blit from its internal surface to the screen, which should use the palette it just set.

icculus commented 2 years ago

I'm actually surprised we haven't seen more paletted games, that a bug like this has lived this long. I'll look at it.

icculus commented 2 years ago

So this only uses SDL_LOGPAL because earlier they called...

vinfo = SDL_GetVideoInfo();
sdl_palettemode = (vinfo->vfmt->BitsPerPixel == 8) ? (SDL_PHYSPAL|SDL_LOGPAL) : SDL_LOGPAL;

And since the display isn't 8-bit, they don't try to set the physical palette.

But they call SDL_SetVideoMode(bpp=8), so we give them an 8-bit surface as requested.

So I'm thinking the problem here is that we try to simulate a physical palette at all, whereas SDL-1.2 is (probably) not offering a hardware palette on x11 (or maybe anywhere on modern machines).

So the solution is probably just to remove any code that references VideoPhysicalPalette20.

(indeed, commenting out this line in SDL_UpdateRects...

surface12->surface20->format->palette = VideoPhysicalPalette20;

...fixes quake2.)

sezero commented 2 years ago

Hmm. Doing that does seem to fix it. However, regardless of that change, changing q2 to use SDL_SetColors instead, results in lots of flicker when effects are applied to screen, e.g. when using quad or when taking damage, which most probably is q2's fault.

icculus commented 2 years ago

changing q2 to use SDL_SetColors instead, results in lots of flicker when effects are applied to screen

That's probably this code in sdl12-compat's SDL_SetPalette, which gets called by SDL_SetColors with SDL_LOGPAL|SDL_PHYSPAL:

    if ((surface12 == VideoSurface12) && (flags & SDL12_PHYSPAL)) {
        SDL_UpdateRect(surface12, 0, 0, 0, 0);   /* force screen to reblit with new palette. */
    }

We basically end up uploading the surface twice...the theory is this will work if the game is not calling UpdateRects() but just changing the physical palette to cause some sort of animation effect.

This code should probably go away with the fake hardware palette. I'd have to see what happens on real SDL-1.2, it's possible this trick I described doesn't work there anyhow.

icculus commented 2 years ago

Yeah, most places in SDL-1.2 wouldn't ever offer a hardware palette anyhow, so it's best to just dump that code, which I now have.

sezero commented 2 years ago

changing q2 to use SDL_SetColors instead, results in lots of flicker when effects are applied to screen

That's probably this code in sdl12-compat's SDL_SetPalette, which gets called by SDL_SetColors with SDL_LOGPAL|SDL_PHYSPAL:

    if ((surface12 == VideoSurface12) && (flags & SDL12_PHYSPAL)) {
        SDL_UpdateRect(surface12, 0, 0, 0, 0);   /* force screen to reblit with new palette. */
    }

We basically end up uploading the surface twice...the theory is this will work if the game is not calling UpdateRects() but just changing the physical palette to cause some sort of animation effect.

The bad thing is, the same happens with real SDL-1.2. sigh...

I had changed our private port of Sin to use SDL_SetColors, now I will revert it.

icculus commented 2 years ago

Obviously this fix broke a bunch of other things, so I reverted it and need to investigate this more.