| Summary: | SDL_ConvertSurface does not convert color keys consistently | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Edmund Horner <ejrh00> |
| Component: | video | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | etc0de, icculus, p.sangra, sdl-bugzilla, sylvain.becker |
| Version: | 2.0.2 | Keywords: | target-2.0.6 |
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: |
Test case showing colour key conversion bug
patch patch patch for the case of the input surface has a palette. |
||
FWIW, I can also reproduce on Linux x86_64 (Fedora 21) with SDL 2.0.3. Hi Edmund Horner,
when RGB565 format surface is converted to 32 bit ARGB8888 format using API SDL_ConvertSurfaceFormat.It adds up alpha channel in the converted surface.
SDL_SetColorKey API sets the RGB color pixel to transparent.
But after converting surface to 32 bit ARGB8888 by blitting. Alpha channel with 0xff value is coming.
Example :
Pixel in RGB565 format = 0x0000b54a (180,170,82)
After blitting 16 bit SDL_Surface to 32 bit SDL_Surface
Pixel in ARGB8888 format = 0xffb4a952 (180,169,82)
In your example:
After blitting RGB has changed.(In above case it is coming different in case of Green value)
But it is blitted with correct values when I use pixel values other than 0xb54a. Like 0xBA54 in place of 0xb54a.There RGB value comes same.
If color key set is 0xb54a. And it does not contain alpha channel. So when it goes for matching pixels from converted surface with color key set. It fails to match and shows opaque pixels because of alpha channel in converted surface with the RGB value.
So this is expected behaviour
The alpha channel is not the issue. The issue is that colour key conversion maps a 6-bit green value 42 to 170 (which seems correct, as 42/63 = 170/255). But 565 pixel conversion maps the same 6-bit value 42 to 169. So even though the newly converted surface has a colour key, it's now a different set of pixels that will appear transparent when it is blitted. The pixel conversion happens in Blit_RGB565_32 (SDL_blit_N.c) using a lookup table. The problem is the lookup table can't replicate the same rounding effects, since it maps the top 3 bits of green to one value, and the bottom bits to a different value, and then sums them. This isn't a problem for Red and Blue whose values come from only one lookup. Any suggestions on a fix? Created attachment 2608 [details]
patch
Some idea of patch ...
It uses a temporary surface to have the color-key converted.
Created attachment 2609 [details]
patch
some mistake, updated patch!
Two ideas come to mind (and I haven't looked at Sylvain's patch yet): 1. Perform the conversion of the colour key using the same mapping table. The inaccuracy caused by the table is extremely minor, being +/- 1 for all 256 possible green inputs, I believe. 2. Allow a mode (perhaps controlled by a SDL hint) that uses a slower conversion of the pixel data, using the same formula as is currently used for the colour key. PS. Thanks so much for your work on SDL! This fix is so monstrous, I can't even tell you. However, I think it is the one way to be 100% correct, so I'm adding it, thanks! https://hg.libsdl.org/SDL/rev/ecc1f6b82d95 Agree this is not the best thing ... It would be better if the pixel conversion routine could be called directly. That can be cleanup sometime in the future. :) I am reopening, because there might be an issue. when converting a surface with palette and colorkey, to a rgb surface. the "new" colorkey is wrong. I think when do a "SDL_FillRect(tmp, NULL, surface->map->info.colorkey);" with the real color, instead of using the key color. Any idea ? some more precisions. if "surface" has a palette, int ck = SDL_MapRGB(surface->format, 255,0,255); SDL_SetColorKey(surface, SDL_TRUE, ck); int ck2 = SDL_MapRGB(surface->format, 255,0,255); ck != ck2 because internally, the palette has an alpha value, and SDL_SetColorKey toggles with opaque/transparency. I think this is an issue for the previous patch :( Created attachment 2936 [details]
patch for the case of the input surface has a palette.
Ok, this is more clear now:
When the input surface has a palette, we need to share it internally for the color-key conversion. That's the patch.
There is another modification in the patch (SDL_pixels.c): to be applied, the palette does not need to be exactly "palette->ncolors == (1 << format->BitsPerPixel)". Palette are not always power of two.
*** Bug 3826 has been marked as a duplicate of this bug. *** (In reply to Sylvain from comment #13) > Created attachment 2936 [details] > patch for the case of the input surface has a palette. This patch is now https://hg.libsdl.org/SDL/rev/3d1698bc2747, thanks! --ryan. I just found out that this (or a very similar) problem reappeared in the current stable SDL release 2.0.12 (and it is also in 2.0.13 in Mercurial as of today): Summary: Using the current stable SDL2 2.0.12 release, there are apparently certain cases where SDL_ConvertSurface() corrupts a surface's color key that was explicitly set using SDL_SetColorKey(). How to reproduce the problem: Build and execute the minimal code example with PNG image file attached to the report for bug 3826 (by typing "make"). (Tested on macOS X 10.14.) Expected behaviour: The black background of the image should be displayed transparently (as it is the case with SDL2 2.0.10). Observed behaviour: The black pixels of the image are non-transparent. This bug potentially breaks every SDL2 program that uses SDL_SetColorKey() followed by SDL_ConvertSurface() to set a certain pixel color in a surface as being transparent and convert the surface to a different surface format (in my case to the “native” format for blitting to the target background surface). The code example mentioned above can be found in my bug report from 2017 (which was marked as a duplicate of this bug): https://bugzilla.libsdl.org/show_bug.cgi?id=3826 I haven't debugged this problem any further so far. P.S.: To run the code example without failing on SDL_GetColorKey(), and to display the image with the transparency problem, just add a "return;" as the very first line of the function "PrintColorkey()" at the beginning of the example code. I think this is fix in https://hg.libsdl.org/SDL/rev/5c4ad9fc7b21 Fix issue with colorkey, palette and format conversion Set the colorkey information on the converted surface. Test-case in bug 3826/2979, conflicting with bug 4798 ( With bug 4798, the colorkey is applied by setting the palette colour to alpha and blitting. see https://hg.libsdl.org/SDL/rev/0c66be754e29 ) Suite: https://hg.libsdl.org/SDL/rev/c0689749d4b4 Only need to set the colorkey on the converted surface if it has no alpha channel. Thanks a lot! I can confirm that using the current SDL2 from Mercurial containing your two patches, not only my minimal code example program from bug 3826 works correctly again, but also the images from my game "Rocks'n'Diamonds" now have correct transparency again! :-) Is it possible to add the mentioned code example as an automated regression test somehow to check future SDL versions against this potential pitfall? ok great, so I mark this as fixed! Yes it would be nice to have a complete test-suite ! Please submit a patch to improve what exists |
Created attachment 2153 [details] Test case showing colour key conversion bug When a 16-bit "565 format" surface has a colour key set, it will blit with correct transparency. If, however, it has its colour key set then is converted to a 32-bit ARGB format surface, the colour key in the converted image will not necessarily be the same pixel value as the transparent pixels. It may not blit correctly, because the colour key does not match the right pixels. In my case, with an image using 0xB54A for transparency, the colour key was converted to 180,170,82; but the corresponding pixels (with the same original value) were converted to 180,169,82. Blitting the converted image did not use transparency where expected. I have attached a test case. The bug has been replicated on both x86_64 Linux (SDL 2.0.2), and 32-bit MS C++ 2010 on Windows (SDL 2.0.0).