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

fix blend mode for converted surface with alpha #4690

Closed
wants to merge 1 commit into from

Conversation

gapkalov
Copy link

When we convert from non-alpha surface to surface with alpha, blend mode wrongly doesn't set.

@1bsyl
Copy link
Contributor

1bsyl commented Aug 23, 2021

I've quickly looked and not sure the patch is correct:
"convert" SDL_Surface has the format "format".
So checking both "convert->format" and "format" redundant (in your patch).
The initial code had no typo: it checks the source format (surface->format) and destination (format).
(Blend mode is enabled only if the source surface had a valid alpha channel (not a non-alpha) ).

Maybe the behaviour could be changed to "enable blending if destination has alpha" (... if (format->Amask) ... )
But SDL_ConvertSurface() is used at various places so this could bring other issues.

@gapkalov
Copy link
Author

gapkalov commented Aug 23, 2021

current code:

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

If old surface doesn't have alpha this condition is always false. So this is a problem. This is also conflict with comment above of this code.

Possible variant is just change to:
if ((format->Amask) || ...

but I left additional check is Alpha channel presented in converted surface, if it doesn't create for some reason. So my variant maybe little bit safer:
if ((converted->format->Amask && format->Amask) || ...

For example if we look at the code of SDL_CreateRGBSurfaceWithFormat we see the following code:
/* By default surface with an alpha mask are set up for blending */
if (surface->format->Amask) {
SDL_SetSurfaceBlendMode(surface, SDL_BLENDMODE_BLEND);
}

@slouken
Copy link
Collaborator

slouken commented Jul 17, 2024

For SDL3 we're going to be consistent that surfaces and textures with an alpha format will have blending enabled by default.

Thanks!

@slouken slouken closed this in 98bea25 Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants