Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Critical regression in SDL2 2.0.6 pre-release regarding transparency using color key #2616

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Labels
duplicate This issue or pull request already exists

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 11, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.1
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2017-09-13 21:13:10 +0000, Holger Schemel wrote:

Created attachment 2934
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.

On 2017-09-13 21:25:20 +0000, Holger Schemel wrote:

The link to the mentioned bug report is broken; here's the correct one:

https://bugzilla.libsdl.org/show_bug.cgi?id=2979

On 2017-09-13 21:37:19 +0000, Holger Schemel wrote:

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

On 2017-09-13 21:40:40 +0000, Holger Schemel wrote:

Created attachment 2935
Full patch (including indentation changes) to fix the reported problem.

On 2017-09-14 07:47:58 +0000, Sylvain wrote:

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.

On 2017-09-14 08:42:17 +0000, Holger Schemel wrote:

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! :-)

On 2017-09-14 12:33:58 +0000, Ryan C. Gordon wrote:

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

@SDLBugzilla SDLBugzilla added bug duplicate This issue or pull request already exists labels Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

1 participant