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 3826 - Critical regression in SDL2 2.0.6 pre-release regarding transparency using color key
Summary: Critical regression in SDL2 2.0.6 pre-release regarding transparency using co...
Status: RESOLVED DUPLICATE of bug 2979
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: HG 2.1
Hardware: x86_64 Linux
: P2 critical
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-13 21:13 UTC by Holger Schemel
Modified: 2017-09-14 12:33 UTC (History)
2 users (show)

See Also:

Attachments
Minimal test program and test image file to show the problem. (3.29 KB, application/zip)
2017-09-13 21:13 UTC, Holger Schemel 		Details
Full patch (including indentation changes) to fix the reported problem. (2.94 KB, patch)
2017-09-13 21:40 UTC, Holger Schemel 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Holger Schemel 2017-09-13 21:13:10 UTC 
Created attachment 2934 [details]
Minimal test program and test image file to show the problem.

Summary: Using the latest SDL2 2.0.6 pre-release, there are apparently certain cases where SDL_ConvertSurface() corrupts a surface's color key that was explicitly set using SetColorKey().

How to reproduce the problem: Build and execute the minimal code example with PNG image file attached to this bug report (by typing "make"). (Tested on Ubuntu 12.04 / x86_64 and Mac OS X 10.11.)

Expected behaviour: The black background of the image should be displayed transparently (as it is with SDL2 2.0.5).

Observed behaviour: The black pixels of the image are non-transparent, while all white pixels are transparent now (which they shouldn't).

Some more details:

The reason for this problem seems to be a regression in the SDL2 2.0.6 pre-release checkout from Mercurial that practically breaks the graphics of my game “Rocks’n’Diamonds” (https://www.artsoft.org/rocksndiamonds/) and potentially 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).

At least for my game, this is a real show-stopper with SDL2 2.0.6.

In the game, the problem is immediately visible at the “door” looking broken on the right side of the screen directly after starting the program. (In addition, all 100% white pixels of the images are transparent now.)

Some debugging revealed that the color key (explicitly set after loading the image files) was changed from totally black (0x00000000) on the old surface to totally white (0x00ffffff) on the new surface created by SDL_ConvertSurface().

I have used “hg bisect” to track down this problem, and found it to be caused by the following commit:

changeset: 11253:ecc1f6b82d95
user: Sam Lantinga slouken@libsdl.org
date: Sat Aug 12 16:59:00 2017 -0700
summary: Fixed bug 2979 - SDL_ConvertSurface does not convert color keys consistently

Related Bugzilla bug: https://bugzilla.libsdl.org/show_bug.cgi?id=29793

Undoing this specific commit in the SDL2 code fixes the problem, which can also be reproduced with the attached test program (or the Rocks’n’Diamonds source code; just download and extract from link above and do a “make run”). I was able to reproduce this problem with Linux (Ubuntu 12.04 x64) and Mac OS X (10.11.6) with the latest SDL2 checkout from Mercurial.
Comment 1 Holger Schemel 2017-09-13 21:25:20 UTC 
The link to the mentioned bug report is broken; here's the correct one:

https://bugzilla.libsdl.org/show_bug.cgi?id=2979
Comment 2 Holger Schemel 2017-09-13 21:37:19 UTC 
The following patch (against SDL2 revision 11494:d167d43d74f2 from 2017-09-11) should fix the problem for both 16-bit "565 format" surfaces (as mentioned in bug 2979 linked above) and surfaces of all other depths (by effectively rolling back the changeset 11253:ecc1f6b82d95 for those surface formats, while using the "special color key handling" from that patch only for those surfaces (16-bit depth) that need it).

The effect can be seen with the attached example programs from both bug 2979 (16-bit surface with color key) and the attached example programs from this bug report (8-bit image loaded to 32-bit surface).

Here's the "short and readable version" of the patch without white-space changes, while the full patch (including changed indentation) follows as an attachment:

--- snip ---

--- src/video/SDL_surface.c.orig        2017-09-12 22:40:34.696199701 +0200
+++ src/video/SDL_surface.c     2017-09-13 23:18:16.939879983 +0200
@@ -988,6 +988,9 @@
         }
 
         if (set_colorkey_by_color) {
+            if (surface->format->BitsPerPixel == 16) {
+                /* 16-bit "565 format" surfaces need special handling */
+
             SDL_Surface *tmp;
             SDL_Surface *tmp2;
             int converted_colorkey = 0;
@@ -1013,6 +1016,15 @@
 
             /* Set the converted colorkey on the new surface */
             SDL_SetColorKey(convert, 1, converted_colorkey);
+            } else {
+              /* Set the colorkey by color, which needs to be unique */
+              Uint8 keyR, keyG, keyB, keyA;
+
+              SDL_GetRGBA(surface->map->info.colorkey, surface->format, &keyR,
+                          &keyG, &keyB, &keyA);
+              SDL_SetColorKey(convert, 1,
+                              SDL_MapRGBA(convert->format, keyR, keyG, keyB, keyA));
+            }
 
             /* This is needed when converting for 3D texture upload */
             SDL_ConvertColorkeyToAlpha(convert);

--- snip ---
Comment 3 Holger Schemel 2017-09-13 21:40:40 UTC 
Created attachment 2935 [details]
Full patch (including indentation changes) to fix the reported problem.
Comment 4 Sylvain 2017-09-14 07:47:58 UTC 
I reopened and submitted a patch in the original bug 2979.

The patch was incomplete. It was missing the case where the input surface has colormap / palette. It has to be shared so that the conversion is performed correctly.
Comment 5 Holger Schemel 2017-09-14 08:42:17 UTC 
Great! I can confirm that with your new patch to bug 2979, not only do the example programs from both bug reports work correctly, but also the images from my game "Rocks'n'Diamonds" now have correct transparency again! :-)
Comment 6 Ryan C. Gordon 2017-09-14 12:33:58 UTC 
Marking this as a duplicate of Bug #2979, so we're all looking at the same place.

--ryan.

*** This bug has been marked as a duplicate of bug 2979 ***