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 3309

Summary: SDL_ConvertSurface adds AlphaMod when input surface has ColorKey
Product: SDL Reporter: Sylvain <sylvain.becker>
Component: videoAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: icculus
Version: don't knowKeywords: target-2.0.6
Hardware: All   
OS: All   
Attachments: test case

Description Sylvain 2016-04-15 08:35:28 UTC
Hi,

Let's you have a SDL_Surface that has ColorKey, but no Alpha Modulation.
When this surface is duplicated with SDL_ConvertSurface function, the result has ColorKey and Alpha Modulation (BLEND, and Opaque 255).

I think SDL_ConvertSurface should strictly keeps the input format.


example
=======

SDL_Surface *input; // ... Set up a surface with ColorKey and no AlphaMod

SDL_Surface *output = SDL_ConvertSurface(input, input->format, input->flags);

// "output" surface has a ColorKey but *also* AlphaMod (BLEND, and Opaque 255).

Patch
=====

In the file "src/video/SDL_Surface.c":

 951     /* Enable alpha blending by default if the new surface has an
 952      * alpha channel or alpha modulation */
 953     if ((surface->format->Amask && format->Amask) ||
 954         (copy_flags & (SDL_COPY_COLORKEY|SDL_COPY_MODULATE_ALPHA))) {
 955         SDL_SetSurfaceBlendMode(convert, SDL_BLENDMODE_BLEND);
 956     }

Maybe "SDL_COPY_COLORKEY" flags should not be checked:

 951     /* Enable alpha blending by default if the new surface has an
 952      * alpha channel or alpha modulation */
 953     if ((surface->format->Amask && format->Amask) ||
 954         (copy_flags & SDL_COPY_MODULATE_ALPHA)) {
 955         SDL_SetSurfaceBlendMode(convert, SDL_BLENDMODE_BLEND);
 956     }
Comment 1 Sylvain 2016-04-16 12:40:44 UTC
Created attachment 2433 [details]
test case

Here's a test case.

The output means "surf1" has no AlphaMod, whereas "surf2" has it.

 INFO: 0xe2d290   Surf1 (480 x 854) bpp=4 SDL_PIXELFORMAT_RGB888    ColorKey  
 INFO: 0xe2dfc0   Surf2 (480 x 854) bpp=4 SDL_PIXELFORMAT_RGB888  AlphaMod=255 BLEND  ColorKey
Comment 2 Ryan C. Gordon 2017-08-10 20:53:36 UTC
Marking this as target-2.0.6 at Sulvaim's request, but I haven't looked at this at all and may remove the tag after I do.

--ryan.
Comment 3 Sam Lantinga 2017-08-12 01:10:27 UTC
Is this causing an issue that needs to be fixed for 2.0.6, or is this just a matter of correctness that can wait until after this release?
Comment 4 Sylvain 2017-08-12 08:31:24 UTC
It does not address a specific issue. Anyway, I ship always a custom version a SDL2 on my side (and I currently have this patch applied).

But when using often SDL_ConvertSurface surface it adds unused Alpha Modulation everywhere. It's kind of "contaminating".

Also, afterwards, when converting the surface to a texture, then renderer may choose a RGBA format when it would only need a RGB.
Comment 5 Sam Lantinga 2017-08-12 22:22:41 UTC
I explicitly added blending with colorkey in commit https://hg.libsdl.org/SDL/rev/2cd7bb613a83, but I didn't note why I did it, and thinking about it that seems wrong.

I applied your patch, thanks!
https://hg.libsdl.org/SDL/rev/1667e6dafc85