We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4964 - XPM having white as color key (transparency)
Summary: XPM having white as color key (transparency)
Status: RESOLVED FIXED
Alias: None
Product: SDL_image
Classification: Unclassified
Component: misc (show other bugs)
Version: 2.0.5
Hardware: x86_64 Linux
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-29 22:34 UTC by pudim
Modified: 2020-02-13 14:10 UTC (History)
2 users (show)

See Also:


Attachments
Fix XPM having white as color key (transparency) (67.36 KB, patch)
2020-01-29 22:34 UTC, pudim
Details | Diff
Test program (7.46 KB, text/x-csrc)
2020-01-29 22:38 UTC, pudim
Details
Fix XPM having white as color key (transparency) (v2) (67.44 KB, patch)
2020-02-08 03:44 UTC, pudim
Details | Diff
Test program (v2) (8.03 KB, text/x-csrc)
2020-02-08 03:45 UTC, pudim
Details

Note You need to log in before you can comment on or make changes to this bug.
Description pudim 2020-01-29 22:34:19 UTC
Created attachment 4180 [details]
Fix XPM having white as color key (transparency)

Hello there! Thank you for making SDL_image!

I'm sending a patch to a bug I found on the XPM loader while building my game. Here is the gist:

> Fix XPM having white as color key (transparency)
> 
> XPM files with transparency were having white set as their color key,
> causing unexpected blitting results.
> 
> Instead of white, use alpha=0 since the type is Uint32 anyway.

I'll attach a test program if the platform allow more than one file.

<3
Comment 1 pudim 2020-01-29 22:38:06 UTC
Created attachment 4181 [details]
Test program
Comment 2 Sylvain 2020-02-04 16:34:17 UTC
hello,

I've looked at your issue, the patch seems to large to be the fix I guess.

the test is also large too: using blending + blitting hide thing.
so not sure what you are reporting.

Can you precise what you think is wrong.

some code
adding:
    int c1 = -2, c2 = -2, c3 = -2, c4 = -2;
    SDL_GetColorKey(bg8,   &c1);
    SDL_GetColorKey(fg8,   &c2);
    SDL_GetColorKey(bg_lg, &c3);
    SDL_GetColorKey(fg_lg, &c4);
    SDL_Log("bg8 has_colorkey=%d %08x fmt=%s", SDL_HasColorKey(bg8), c1, SDL_GetPixelFormatName(bg8->format->format));
    SDL_Log("fg8 has_colorkey=%d %08x fmt=%s", SDL_HasColorKey(fg8), c2, SDL_GetPixelFormatName(fg8->format->format));
    SDL_Log("bg_lg has_colorkey=%d %08x fmt=%s", SDL_HasColorKey(bg_lg), c3, SDL_GetPixelFormatName(bg_lg->format->format));
    SDL_Log("fg_lg has_colorkey=%d %08x fmt=%s", SDL_HasColorKey(fg_lg), c4, SDL_GetPixelFormatName(fg_lg->format->format));

shows:
INFO: bg8 has_colorkey=1 00000000 fmt=SDL_PIXELFORMAT_INDEX8
INFO: fg8 has_colorkey=1 00000000 fmt=SDL_PIXELFORMAT_INDEX8
INFO: bg_lg has_colorkey=1 ffffffff fmt=SDL_PIXELFORMAT_RGB888
INFO: fg_lg has_colorkey=1 ffffffff fmt=SDL_PIXELFORMAT_RGB888
Comment 3 pudim 2020-02-05 19:17:47 UTC
> I've looked at your issue, the patch seems to large to be the fix I guess.
> 
> the test is also large too: using blending + blitting hide thing.
> so not sure what you are reporting.
> 
> Can you precise what you think is wrong.

Sorry if I wasn't very clear. I'll try to explain better:

The problem happens when you load an XPM image that has both None and
White (or literal #ffffff) on the palette and convert it to ARGB. White
then becomes transparent (alpha=0), yielding incorrect blit results.

It doesn't affect images that don't have None on the palette, and it
doesn't affect colors other than White.

If you run the test program, you should see a block of output like this:

=== palette converted to argb:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00ffffff, 00ffffff, ff666666 }
blit	= { ff000000, ff000000, ff666666 }

- expected:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00000000, ffffffff, ff666666 }
blit	= { ff000000, ffffffff, ff666666 }

the first 2 lines are the pixels of the XPMs bg and fg converted to
ARGB, the 3rd is the blitting of fg over bg and then the 3 next lines
are just the hardcoded outputs I expect...

As you can see, the real and expected values for fg are different both
on the first and second pixels:

fg	= { 00ffffff, 00ffffff, ff666666 } # real
fg	= { 00000000, ffffffff, ff666666 } # expected

causing an unexpected blit result:

blit	= { ff000000, ff000000, ff666666 } # real
blit	= { ff000000, ffffffff, ff666666 } # expected

This happens because the code is setting #ffffffff as color key, which
is not representable as RGB, and probably gets truncated to White in
some places, but not in others.

Also note that the output you provided suggests something not so obvious
is happening:

> INFO: bg8 has_colorkey=1 00000000 fmt=SDL_PIXELFORMAT_INDEX8
> INFO: fg8 has_colorkey=1 00000000 fmt=SDL_PIXELFORMAT_INDEX8

as the code does sets it like this:

            if (rgb == 0xffffffff)
                SDL_SetColorKey(image, SDL_TRUE, pixel);

> INFO: bg_lg has_colorkey=1 ffffffff fmt=SDL_PIXELFORMAT_RGB888
> INFO: fg_lg has_colorkey=1 ffffffff fmt=SDL_PIXELFORMAT_RGB888

Also note² that #ffffffff has 4 bytes and shouldn't be representable in
RGB888, which only has, or at least should only care about, 3bytes.

Indeed the patch is big, but most of it is just prepending `0xff` to
most elements of that long name-to-color map, so that the regular ones
get alpha=255 and None gets alpha=0.

By the way, it also removes a duplicate entry for None, that set it to
#00ffffff (the same as White), from the end of that list ~ but you can
see that it isn't being used, otherwise the color key wouldn't be set by
the code quoted above.

Apart from that, the rest of the patch is just putting alpha in the
right places and replacing rgb with argb.

Love <3

-- 
pudim
Comment 4 Sylvain 2020-02-05 20:30:33 UTC
Thanks for clarifying!

btw: here's the way colorkey is used :
https://hg.libsdl.org/SDL/file/cec913fbe656/include/SDL_surface.h#l427

A note: it doesn't say about SDL_BLENDMODE_ADD vs SDL_BLENDMODE_BLEND ...
Then: I don't get exactly the same output:

=== palette converted to argb:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00ffffff, ffffffff, ff666666 }
blit	= { ff000000, ffffffff, ff666666 }

maybe something get fixed in latest HG SDL Source code ?

otherwise, not sure how it should be.
Comment 5 pudim 2020-02-07 12:43:55 UTC
> Then: I don't get exactly the same output:
> 
> === palette converted to argb:
> bg	= { ff000000, ff000000, ff000000 }
> fg	= { 00ffffff, ffffffff, ff666666 }
> blit	= { ff000000, ffffffff, ff666666 }
> 
> maybe something get fixed in latest HG SDL Source code ?
> 
> otherwise, not sure how it should be.

The patch was originally made on top of the mercurial tip:

changeset:   749:ec1f8032810e
tag:         tip
user:        Sam Lantinga <slouken@libsdl.org>
date:        Thu Jan 16 20:51:54 2020 -0800
summary:     Updated copyright date for 2020


And I do get the bug without the patch!

What did I do:

```
hg clone https://hg.libsdl.org/SDL_image/
cd SDL_image
./autogen.sh
./configure
make
LD_LIBRARY_PATH=./.libs/ ./xpm-white-alpha
```

which yielded the following:

```
=== palette:
bg	= {        1,        1,        1 }
fg	= {        0,        2,        3 }
blit	= {        1,        2,        3 }

- expected:
bg	= {        1,        1,        1 }
fg	= {        0,        2,        3 }
blit	= {        1,        2,        3 }

=== palette converted to argb:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00ffffff, 00ffffff, ff666666 } # <------ BAD!
blit	= { ff000000, ff000000, ff666666 } # <------ BAD!

- expected:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00000000, ffffffff, ff666666 }
blit	= { ff000000, ffffffff, ff666666 }

=== palette with 257 colors:
bg	= { 00000000, 00000000, 00000000 } # <------ blit works, but
fg	= { ffffffff, 00ffffff, 00666666 } #         alpha is funny...
blit	= { 00000000, 00ffffff, 00666666 }

- expected:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00000000, ffffffff, ff666666 }
blit	= { ff000000, ffffffff, ff666666 }
```

and then:

```
patch -p1 < xpm-white-alpha.patch
make
LD_LIBRARY_PATH=./.libs/ ./xpm-white-alpha
```

which yielded:

```
=== palette:
bg	= {        1,        1,        1 }
fg	= {        0,        2,        3 }
blit	= {        1,        2,        3 }

- expected:
bg	= {        1,        1,        1 }
fg	= {        0,        2,        3 }
blit	= {        1,        2,        3 }

=== palette converted to argb:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00000000, ffffffff, ff666666 } # <------ GOOD!
blit	= { ff000000, ffffffff, ff666666 } # <------ GOOD!

- expected:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00000000, ffffffff, ff666666 }
blit	= { ff000000, ffffffff, ff666666 }

=== palette with 257 colors:
bg	= { ff000000, ff000000, ff000000 } # <------ makes more sense
fg	= { 00000000, ffffffff, ff666666 } #         to me
blit	= { ff000000, ffffffff, ff666666 }

- expected:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00000000, ffffffff, ff666666 }
blit	= { ff000000, ffffffff, ff666666 }
```

Are you sure you ran it against an unpatched build? Maybe it will break
on some systems but not on others?

<3
Comment 6 Sylvain 2020-02-07 21:18:15 UTC
Still get a different output.
Using also the latest SDL_image from source.
And latest SDL main lib (involved when converting palette to rgba, then blitting) 
I tried various old SDL version and I still got the output.

=== palette:
bg	= {        1,        1,        1 }
fg	= {        0,        2,        3 }
blit	= {        1,        2,        3 }

- expected:
bg	= {        1,        1,        1 }
fg	= {        0,        2,        3 }
blit	= {        1,        2,        3 }

=== palette converted to argb:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00ffffff, ffffffff, ff666666 } # There
blit	= { ff000000, ffffffff, ff666666 } # and here

- expected:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00000000, ffffffff, ff666666 }
blit	= { ff000000, ffffffff, ff666666 }

=== palette with 257 colors:
bg	= { 00000000, 00000000, 00000000 }
fg	= { ffffffff, 00ffffff, 00666666 }
blit	= { 00000000, 00ffffff, 00666666 }

- expected:
bg	= { ff000000, ff000000, ff000000 }
fg	= { 00000000, ffffffff, ff666666 }
blit	= { ff000000, ffffffff, ff666666 }
Comment 7 pudim 2020-02-07 22:57:54 UTC
Then I suppose that `SDL_ConvertSurfaceFormat()` is different on our
systems.

I'm on the latest stable SDL2:

```
sdl2-config --version
2.0.10
```

packaged by Void-Linux (revision 4): SDL2-2.0.10_4

I'm building SDL2 from HG now to be certain of what configure flags were
used.

Could you please tell me which version/configure flags you're on?

PS: just realized how nice it would be to have an option on
`sdl2-config` to print all configure options used when building.
Comment 8 pudim 2020-02-08 03:42:54 UTC
I can confirm that `SDL_ConvertSurfaceFormat()` is behaving differently
on the following versions:

* SDL2-2.0.10 (latest stable)
* SDL-2.0.11-13486 (latest mercurial snapshot updated Wed Feb 5)

and that the later does not cause White to become transparent after
conversion from Indexed to ARGB...

However, the issue is not resolved! I added another case to the test
program to convert the image with more than 256 colors to ARGB ~ and
it's broken, even with the patch!

I'd guess it has to do with creating an RGB image with 32bit pixels
without the alpha channel, but hacking the extra byte to flag
transparency.

So I made a change to the patch to only set a color key if the image is
Indexed, which fixed the issue for me. I'm attaching the new versions of
it and the test program.

And here goes my combination of results:

### SDL2-2.0.10
### SDL_image unpatched

```
# palette (4 colors)

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg |        1        1        1 |        1        1        1 | yes
      fg |        0        2        3 |        0        2        3 | yes
    blit |        1        2        3 |        1        2        3 | yes

# palette (4 colors) converted to argb

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00ffffff 00ffffff ff666666 | 00000000 ffffffff ff666666 | NO
    blit | ff000000 ff000000 ff666666 | ff000000 ffffffff ff666666 | NO

# palette (257 colors)

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | 00000000 00000000 00000000 | ff000000 ff000000 ff000000 | NO
      fg | ffffffff 00ffffff 00666666 | 00000000 ffffffff ff666666 | NO
    blit | 00000000 00ffffff 00666666 | ff000000 ffffffff ff666666 | NO

# palette (257 colors) converted to argb

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00ffffff 00ffffff ff666666 | 00000000 ffffffff ff666666 | NO
    blit | ff000000 ff000000 ff666666 | ff000000 ffffffff ff666666 | NO

```

### SDL-2.0.11-13486
### SDL_image unpatched

```
# palette (4 colors)

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg |        1        1        1 |        1        1        1 | yes
      fg |        0        2        3 |        0        2        3 | yes
    blit |        1        2        3 |        1        2        3 | yes

# palette (4 colors) converted to argb

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00ffffff ffffffff ff666666 | 00000000 ffffffff ff666666 | NO
    blit | ff000000 ffffffff ff666666 | ff000000 ffffffff ff666666 | yes

# palette (257 colors)

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | 00000000 00000000 00000000 | ff000000 ff000000 ff000000 | NO
      fg | ffffffff 00ffffff 00666666 | 00000000 ffffffff ff666666 | NO
    blit | 00000000 00ffffff 00666666 | ff000000 ffffffff ff666666 | NO

# palette (257 colors) converted to argb

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00ffffff 00ffffff ff666666 | 00000000 ffffffff ff666666 | NO
    blit | ff000000 ff000000 ff666666 | ff000000 ffffffff ff666666 | NO

```

### SDL-2.0.11-13486
### SDL_image patch v1

```
# palette (4 colors)

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg |        1        1        1 |        1        1        1 | yes
      fg |        0        2        3 |        0        2        3 | yes
    blit |        1        2        3 |        1        2        3 | yes

# palette (4 colors) converted to argb

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00000000 ffffffff ff666666 | 00000000 ffffffff ff666666 | yes
    blit | ff000000 ffffffff ff666666 | ff000000 ffffffff ff666666 | yes

# palette (257 colors)

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00000000 ffffffff ff666666 | 00000000 ffffffff ff666666 | yes
    blit | ff000000 ffffffff ff666666 | ff000000 ffffffff ff666666 | yes

# palette (257 colors) converted to argb

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | 00000000 00000000 00000000 | ff000000 ff000000 ff000000 | NO
      fg | 00000000 ffffffff ff666666 | 00000000 ffffffff ff666666 | yes
    blit | 00000000 ffffffff ff666666 | ff000000 ffffffff ff666666 | NO

```

### SDL-2.0.11-13486
### SDL_image patch v2

```
# palette (4 colors)

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg |        1        1        1 |        1        1        1 | yes
      fg |        0        2        3 |        0        2        3 | yes
    blit |        1        2        3 |        1        2        3 | yes

# palette (4 colors) converted to argb

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00000000 ffffffff ff666666 | 00000000 ffffffff ff666666 | yes
    blit | ff000000 ffffffff ff666666 | ff000000 ffffffff ff666666 | yes

# palette (257 colors)

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00000000 ffffffff ff666666 | 00000000 ffffffff ff666666 | yes
    blit | ff000000 ffffffff ff666666 | ff000000 ffffffff ff666666 | yes

# palette (257 colors) converted to argb

 surface | real                       | expected                   | eq?
 ------- | -------------------------- | -------------------------- | ---
      bg | ff000000 ff000000 ff000000 | ff000000 ff000000 ff000000 | yes
      fg | 00000000 ffffffff ff666666 | 00000000 ffffffff ff666666 | yes
    blit | ff000000 ffffffff ff666666 | ff000000 ffffffff ff666666 | yes

```

<3
Comment 9 pudim 2020-02-08 03:44:28 UTC
Created attachment 4196 [details]
Fix XPM having white as color key (transparency) (v2)
Comment 10 pudim 2020-02-08 03:45:08 UTC
Created attachment 4197 [details]
Test program (v2)
Comment 11 pudim 2020-02-08 03:45:42 UTC
Comment on attachment 4197 [details]
Test program (v2)

>#include <SDL.h>
>#include <SDL_image.h>
>
>#define PALETTE4 \
>	"000	c None", \
>	"001	c Black", \
>	"002	c White", \
>	"003	c #666666"
>
>#define PALETTE256 \
>	PALETTE4, \
>	"004\tc #000004", "005\tc #000005", "006\tc #000006", "007\tc #000007",\
>	"008\tc #000008", "009\tc #000009", "010\tc #000010", "011\tc #000011",\
>	"012\tc #000012", "013\tc #000013", "014\tc #000014", "015\tc #000015",\
>	"016\tc #000016", "017\tc #000017", "018\tc #000018", "019\tc #000019",\
>	"020\tc #000020", "021\tc #000021", "022\tc #000022", "023\tc #000023",\
>	"024\tc #000024", "025\tc #000025", "026\tc #000026", "027\tc #000027",\
>	"028\tc #000028", "029\tc #000029", "030\tc #000030", "031\tc #000031",\
>	"032\tc #000032", "033\tc #000033", "034\tc #000034", "035\tc #000035",\
>	"036\tc #000036", "037\tc #000037", "038\tc #000038", "039\tc #000039",\
>	"040\tc #000040", "041\tc #000041", "042\tc #000042", "043\tc #000043",\
>	"044\tc #000044", "045\tc #000045", "046\tc #000046", "047\tc #000047",\
>	"048\tc #000048", "049\tc #000049", "050\tc #000050", "051\tc #000051",\
>	"052\tc #000052", "053\tc #000053", "054\tc #000054", "055\tc #000055",\
>	"056\tc #000056", "057\tc #000057", "058\tc #000058", "059\tc #000059",\
>	"060\tc #000060", "061\tc #000061", "062\tc #000062", "063\tc #000063",\
>	"064\tc #000064", "065\tc #000065", "066\tc #000066", "067\tc #000067",\
>	"068\tc #000068", "069\tc #000069", "070\tc #000070", "071\tc #000071",\
>	"072\tc #000072", "073\tc #000073", "074\tc #000074", "075\tc #000075",\
>	"076\tc #000076", "077\tc #000077", "078\tc #000078", "079\tc #000079",\
>	"080\tc #000080", "081\tc #000081", "082\tc #000082", "083\tc #000083",\
>	"084\tc #000084", "085\tc #000085", "086\tc #000086", "087\tc #000087",\
>	"088\tc #000088", "089\tc #000089", "090\tc #000090", "091\tc #000091",\
>	"092\tc #000092", "093\tc #000093", "094\tc #000094", "095\tc #000095",\
>	"096\tc #000096", "097\tc #000097", "098\tc #000098", "099\tc #000099",\
>	"100\tc #000100", "101\tc #000101", "102\tc #000102", "103\tc #000103",\
>	"104\tc #000104", "105\tc #000105", "106\tc #000106", "107\tc #000107",\
>	"108\tc #000108", "109\tc #000109", "110\tc #000110", "111\tc #000111",\
>	"112\tc #000112", "113\tc #000113", "114\tc #000114", "115\tc #000115",\
>	"116\tc #000116", "117\tc #000117", "118\tc #000118", "119\tc #000119",\
>	"120\tc #000120", "121\tc #000121", "122\tc #000122", "123\tc #000123",\
>	"124\tc #000124", "125\tc #000125", "126\tc #000126", "127\tc #000127",\
>	"128\tc #000128", "129\tc #000129", "130\tc #000130", "131\tc #000131",\
>	"132\tc #000132", "133\tc #000133", "134\tc #000134", "135\tc #000135",\
>	"136\tc #000136", "137\tc #000137", "138\tc #000138", "139\tc #000139",\
>	"140\tc #000140", "141\tc #000141", "142\tc #000142", "143\tc #000143",\
>	"144\tc #000144", "145\tc #000145", "146\tc #000146", "147\tc #000147",\
>	"148\tc #000148", "149\tc #000149", "150\tc #000150", "151\tc #000151",\
>	"152\tc #000152", "153\tc #000153", "154\tc #000154", "155\tc #000155",\
>	"156\tc #000156", "157\tc #000157", "158\tc #000158", "159\tc #000159",\
>	"160\tc #000160", "161\tc #000161", "162\tc #000162", "163\tc #000163",\
>	"164\tc #000164", "165\tc #000165", "166\tc #000166", "167\tc #000167",\
>	"168\tc #000168", "169\tc #000169", "170\tc #000170", "171\tc #000171",\
>	"172\tc #000172", "173\tc #000173", "174\tc #000174", "175\tc #000175",\
>	"176\tc #000176", "177\tc #000177", "178\tc #000178", "179\tc #000179",\
>	"180\tc #000180", "181\tc #000181", "182\tc #000182", "183\tc #000183",\
>	"184\tc #000184", "185\tc #000185", "186\tc #000186", "187\tc #000187",\
>	"188\tc #000188", "189\tc #000189", "190\tc #000190", "191\tc #000191",\
>	"192\tc #000192", "193\tc #000193", "194\tc #000194", "195\tc #000195",\
>	"196\tc #000196", "197\tc #000197", "198\tc #000198", "199\tc #000199",\
>	"200\tc #000200", "201\tc #000201", "202\tc #000202", "203\tc #000203",\
>	"204\tc #000204", "205\tc #000205", "206\tc #000206", "207\tc #000207",\
>	"208\tc #000208", "209\tc #000209", "210\tc #000210", "211\tc #000211",\
>	"212\tc #000212", "213\tc #000213", "214\tc #000214", "215\tc #000215",\
>	"216\tc #000216", "217\tc #000217", "218\tc #000218", "219\tc #000219",\
>	"220\tc #000220", "221\tc #000221", "222\tc #000222", "223\tc #000223",\
>	"224\tc #000224", "225\tc #000225", "226\tc #000226", "227\tc #000227",\
>	"228\tc #000228", "229\tc #000229", "230\tc #000230", "231\tc #000231",\
>	"232\tc #000232", "233\tc #000233", "234\tc #000234", "235\tc #000235",\
>	"236\tc #000236", "237\tc #000237", "238\tc #000238", "239\tc #000239",\
>	"240\tc #000240", "241\tc #000241", "242\tc #000242", "243\tc #000243",\
>	"244\tc #000244", "245\tc #000245", "246\tc #000246", "247\tc #000247",\
>	"248\tc #000248", "249\tc #000249", "250\tc #000250", "251\tc #000251",\
>	"252\tc #000252", "253\tc #000253", "254\tc #000254", "255\tc #000255",\
>	"256\tc #000256"
>
>char* bg_xpm[] = {
>	"3 1 4 3",
>	PALETTE4,
>	"001001001"
>};
>
>char* fg_xpm[] = {
>	"3 1 4 3",
>	PALETTE4,
>	"000002003",
>};
>
>char* bg_lg_xpm[] = {
>	"3 1 257 3",
>	PALETTE256,
>	"001001001"
>};
>
>char* fg_lg_xpm[] = {
>	"3 1 257 3",
>	PALETTE256,
>	"000002003"
>};
>
>int main()
>{
>	SDL_Window *window;
>	SDL_Renderer *renderer;
>	SDL_Surface* bg8,    * fg8,    * bl8;
>	SDL_Surface* bg32,   * fg32,   * bl32;
>	SDL_Surface* bg_lg,  * fg_lg,  * bl_lg;
>	SDL_Surface* bg_lga, * fg_lga, * bl_lga;
>
>	SDL_Init(SDL_INIT_VIDEO);
>	IMG_Init(0);
>	SDL_CreateWindowAndRenderer(32, 32, 0, &window, &renderer);
>
>	bg8	= IMG_ReadXPMFromArray(bg_xpm);
>	bl8	= IMG_ReadXPMFromArray(bg_xpm);
>	fg8	= IMG_ReadXPMFromArray(fg_xpm);
>	bg_lg	= IMG_ReadXPMFromArray(bg_lg_xpm);
>	bl_lg	= IMG_ReadXPMFromArray(bg_lg_xpm);
>	fg_lg	= IMG_ReadXPMFromArray(fg_lg_xpm);
>
>	bg32	= SDL_ConvertSurfaceFormat(bg8, SDL_PIXELFORMAT_BGRA32, 0);
>	bl32	= SDL_ConvertSurfaceFormat(bg8, SDL_PIXELFORMAT_BGRA32, 0);
>	fg32	= SDL_ConvertSurfaceFormat(fg8, SDL_PIXELFORMAT_BGRA32, 0);
>
>	bg_lga	= SDL_ConvertSurfaceFormat(bg_lg, SDL_PIXELFORMAT_BGRA32, 0);
>	bl_lga	= SDL_ConvertSurfaceFormat(bg_lg, SDL_PIXELFORMAT_BGRA32, 0);
>	fg_lga	= SDL_ConvertSurfaceFormat(fg_lg, SDL_PIXELFORMAT_BGRA32, 0);
>
>	SDL_SetSurfaceBlendMode(fg32,  SDL_BLENDMODE_ADD);
>	SDL_SetSurfaceBlendMode(bl32,  SDL_BLENDMODE_ADD);
>	SDL_SetSurfaceBlendMode(bl_lg, SDL_BLENDMODE_ADD);
>	SDL_SetSurfaceBlendMode(bl_lga, SDL_BLENDMODE_ADD);
>
>	SDL_BlitSurface(fg8,	NULL, bl8,	NULL);
>	SDL_BlitSurface(fg32,	NULL, bl32,	NULL);
>	SDL_BlitSurface(fg_lg,	NULL, bl_lg,	NULL);
>	SDL_BlitSurface(fg_lga,	NULL, bl_lga,	NULL);
>
>#define print_example(s, f, x, a, b, c) \
>	printf("%8s | "f" "f" "f" | "f" "f" "f" | %s\n", s, \
>			(x)[0], (x)[1], (x)[2], (a), (b), (c), \
>			((x)[0] == (a) && (x)[1] == (b) && (x)[2] == (c)) ? \
>			"yes" : "NO");
>
>#define print_header_08x(s) \
>	puts("# "s"\n\n" \
>	" surface | real                       | expected                   | eql?\n" \
>	" ------- | -------------------------- | -------------------------- | ----" \
>	)
>
>	print_header_08x("palette (4 colors)");
>	print_example("bg",   "% 8d", (Uint8*) bg8->pixels, 1, 1, 1);
>	print_example("fg",   "% 8d", (Uint8*) fg8->pixels, 0, 2, 3);
>	print_example("blit", "% 8d", (Uint8*) bl8->pixels, 1, 2, 3);
>	putchar('\n');
>	
>	print_header_08x("palette (4 colors) converted to argb");
>	print_example("bg",   "%08x", (Uint32*) bg32->pixels, 0xff000000, 0xff000000, 0xff000000);
>	print_example("fg",   "%08x", (Uint32*) fg32->pixels, 0x00000000, 0xffffffff, 0xff666666);
>	print_example("blit", "%08x", (Uint32*) bl32->pixels, 0xff000000, 0xffffffff, 0xff666666);
>	putchar('\n');
>
>	print_header_08x("palette (257 colors)");
>	print_example("bg",   "%08x", (Uint32*) bg_lg->pixels, 0xff000000, 0xff000000, 0xff000000);
>	print_example("fg",   "%08x", (Uint32*) fg_lg->pixels, 0x00000000, 0xffffffff, 0xff666666);
>	print_example("blit", "%08x", (Uint32*) bl_lg->pixels, 0xff000000, 0xffffffff, 0xff666666);
>	putchar('\n');
>
>	print_header_08x("palette (257 colors) converted to argb");
>	print_example("bg",   "%08x", (Uint32*) bg_lga->pixels, 0xff000000, 0xff000000, 0xff000000);
>	print_example("fg",   "%08x", (Uint32*) fg_lga->pixels, 0x00000000, 0xffffffff, 0xff666666);
>	print_example("blit", "%08x", (Uint32*) bl_lga->pixels, 0xff000000, 0xffffffff, 0xff666666);
>	putchar('\n');
>
>	IMG_Quit();
>	SDL_Quit();
>
>	return 0;
>}
Comment 12 Sylvain 2020-02-09 21:59:45 UTC
For # palette (257 colors):

the surface created from SDL_image is SDL_PIXELFORMAT_RGB888 + a colorkey, so the alpha field shouldn't be considered. But the colorkey is actually with an alpha value. This is somehow conflicting.

Which may be the reason why, when converting to ARGB32, colorkey match both white and colorkey. and clear the alpha.

I'd say this happen here:
https://hg.libsdl.org/SDL/file/045f218436fe/src/video/SDL_surface.c#l308

So, yes, maybe the XPM code should create an alpha surface at first!
Comment 13 pudim 2020-02-09 23:00:56 UTC
I think it's also worth noting that the current code doesn't set the
alpha for any color on the indexed palette, not even for None, which
might break other things.

And IMHO it is more intuitive to have `0x00000000` as transparency,
rather than `0x00ffffff`, `0xff000000` or, even worse, `0xffffffff`, as
an alpha of 0 conventionally means transparent and adding the value 0 to
a pixel does nothing.
Comment 14 Sam Lantinga 2020-02-13 14:10:08 UTC
This is in, thanks!
https://hg.libsdl.org/SDL_image/rev/082214c629d1