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

open
nobody
graphics (66)
5
2012-09-07
2010-05-10
No

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

Discussion

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

     
  • 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
    clone2727
    2010-06-03

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

     
  • Jonas Maebe
    Jonas Maebe
    2011-06-13

    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
    Jonas Maebe
    2011-06-13

    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
    
     
  • 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

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

     
  • Jonas Maebe
    Jonas Maebe
    2011-06-22

    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
    Jonas Maebe
    2011-07-01

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