#347 OS X: color blue missing with colors above 256

[Dominik Reichardt]

as reported at http://vogons.zetafleet.com/viewtopic.php?t=23087 with SDL 1.2.14 and above colors are screwed up when you are using more than 256 colors it seems.
Test cases Windows 3.1 and Tomb Raider intro movie

[Dominik Reichardt]

interestingly, the color problem cannot be captured either by video or screencapture :)

[Dominik Reichardt]

Windows 95 with 16bit and 32bit colors does show the problem as well.
16bit:http://wedemandhtml.com/tmp/win95_dosbox_8.png
32bit:http://wedemandhtml.com/tmp/win95_dosbox_9.png

[clone2727]

Confirmed here. I've found a simple workaround until the bug is fixed: the screen shows normal in fullscreen mode.

[Jonas Maebe]

I have a hack to "fix" this problem. My test program is Pro Pinball: Timeshock (GOG version), running in either 65000+ or 65000 colors mode (both use the same video mode, it seems; not sure what the difference is).

It's not a proper fix, but it does show where the problem lies. I've used debug logging similar to what's suggested by wd at http://vogons.zetafleet.com/viewtopic.php?t=25446&postdays=0&postorder=asc&start=20 . There are two separate problems:

• In windowed mode: under certain Mac OS X versions, the format of the SDL surface in windowed mode is BGRA (well, the "A" is not used in this case, but the 8 lower bits are reserved). The code in src/gui/render_templates.h completely ignores the surface format however, and is hardcoded to expect RGB (with the upper 8 bits unused).

I can hack around this problem by changing the code for the 16->32 pixel packing from

#define PMAKE(_VAL) (((_VAL&(31<<11))<<8)|((_VAL&(63<<5))<<5)|((_VAL&31)<<3))

into

#define PMAKE(_VAL) (((_VAL&(31<<11)))|((_VAL&(63<<5))<<(5+8))|((_VAL&31)<<(3+24)))

(as mentioned earlier: this is not a proper fix, and will break things when the 32 bpp format is not BGRA)

Logging output:
Render src bpp: 16
windowed SDL_SetVideoMode bpp==20, flags==148
-> flags==4, bpp==[4;32], r==ff00:8:0, g==ff0000:16:0, b==ff000000:24:0, a==0:0:8
render.scale.outmode: 3

• in fullscreen mode: here the problem is that SDL allocates a 15 bpp surface, but it is treated by DOSBox as a 16 bpp surface, causing a color shift to green (no color conversion is performed, because the game itself also uses 16 bpp). I don't know whether the bug is in DOSBox or in SDL (logging output is below).

This problem was made a little more interesting by the fact my first hack (replacing the 16 bpp -> 16 bpp conversion with the 16 bpp -> 15 bpp conversion) didn't entirely work because there's a bug in DOSBox' 16 bpp -> 15 bpp conversion (I've posted a patch for that at https://sourceforge.net/tracker/index.php?func=detail&aid=3315935&group_id=52551&atid=467232 ).

Anyway, this problem can be hacked around by replacing the line for 16 bpp -> 16 bpp conversion from

#define PMAKE(_VAL) (_VAL)

into

#define PMAKE(_VAL) (((_VAL&~63)>>1)|(_VAL&31))

Logging output:
Render src bpp: 16
full screen SDL_SetVideoMode bpp==10, flags==144
-> flags==81000001, bpp==[2;16], r==7c00:10:3, g==3e0:5:3, b==1f:0:3, a==0:0:8

I'll attach a patch that applies both hacks.

I guess the proper fix will be to either use the actual pixel packing information from the SDL surface in the pixel packing macros (although this will obviously slow them down), or to extend the pre-generated pixel conversion routines with at least one extra variant (for BGRA) and to adjust the detection of which variant to use based on the surface pixel packing information rather than just on the reported bit depth.

[Jonas Maebe]

For some reason I don't appear to have permissions to attach files to this bug report (there's no "add a file" link when I click on "Attached File").

I'll put a text version of the patch below, but it'll probably get mangled so that it won't be possible to directly apply it. Also note that it supposes that the patch attached to https://sourceforge.net/tracker/index.php?func=detail&aid=3315935&group_id=52551&atid=467232 is already applied.

--- a/src/gui/render_templates.h
+++ b/src/gui/render_templates.h
@@ -109,9 +109,9 @@
 #if DBPP == 15
 #define PMAKE(_VAL) (((_VAL&~63)>>1)|(_VAL&31))
 #elif DBPP == 16
-#define PMAKE(_VAL) (_VAL)
+#define PMAKE(_VAL) (((_VAL&~63)>>1)|(_VAL&31))
 #elif DBPP == 32
-#define PMAKE(_VAL)  (((_VAL&(31<<11))<<8)|((_VAL&(63<<5))<<5)|((_VAL&31)<<3))
+#define PMAKE(_VAL)  (((_VAL&(31<<11)))|((_VAL&(63<<5))<<(5+8))|((_VAL&31)<<(3+24)))
 #endif
 #define SRCTYPE Bit16u
 #endif

[Dominik Reichardt]

Thanks for your patch. SF doesn't allow normal users to attach files to other users track item.

Interestingly your patch work for Windows 3.11 64k colors but not 32k colors

[Dominik Reichardt]

and Tomb Raider intro movie also not in correct color :(

[Jonas Maebe]

It's normal that the patch doesn't help with 32k colors. The bug is that SDL returns a 15 bit context, but claims it is a 16 bit context. My patch only hacked around this problem by changing the DOSBox 16 bit -> 16 bit blitting code to use the 16 bit -> 15 bit blitting code instead.

To make 15 bit video modes work (in fullscreen), you have to also change the 15 bit -> 16 bit blitting code so that it doesn't perform any conversion at all (since the "16 bit" destination context is actually a 15 bit context and hence the source and destination pixel formats are identical).

I've filed a bug with libsdl about this: http://bugzilla.libsdl.org/show_bug.cgi?id=1235

The problem with the color shifts in windowed mode is a DOSBox bug as described in my earlier comment.

[Jonas Maebe]

I've now attached a patch to the libsdl bug report (http://bugzilla.libsdl.org/show_bug.cgi?id=1235) that fixes the bug in SDL. If you recompile libSDL with that patch (and remove the hack I posted in the comments of this bug report), things should work fine both in 32k and 64k colour modes (only in full screen mode though; in windowed mode, it's a DOSBox bug that needs fixing).

[Dominik Reichardt]

I'm very late in replying to this again... sorry,... been distracted by other aspects and by opengl being much faster on OS X and not showing this problem.

• The SDL patch makes no difference when applied to current Mercurial of the SDL1.2 branch. Fullscreen still shows the wrong colors
• the hack fix for DOSBox no longer applies to the DBPP == 32 part cleanly. If I apply it anyway, the hack fix still works as before (for 64k colors and not for the tomb raider movie)

[Dominik Reichardt]

and... it all works fine with SDL2 :)