sdl12-compat icon indicating copy to clipboard operation
sdl12-compat copied to clipboard

bad video with icculus.org quake2 + ref_softsdl

Open sezero opened this issue 3 years ago • 10 comments

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 avatar Sep 21 '22 23:09 sezero

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 avatar Sep 22 '22 00:09 sezero

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

sezero avatar Sep 22 '22 01:09 sezero

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.

slouken avatar Sep 22 '22 02:09 slouken

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 avatar Sep 22 '22 02:09 icculus

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.)

icculus avatar Sep 23 '22 14:09 icculus

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.

sezero avatar Sep 23 '22 14:09 sezero

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 avatar Sep 23 '22 15:09 icculus

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.

icculus avatar Sep 23 '22 16:09 icculus

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.

sezero avatar Sep 23 '22 18:09 sezero

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

icculus avatar Oct 14 '22 18:10 icculus