libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.73k stars 1.8k forks source link

The "Blend" blending mode when used with SDL_BlitSurface yields incorrect alpha values for semi-transparent pixels #6990

Closed Marin-MK closed 3 months ago

Marin-MK commented 1 year ago

When using SDL_BlitSurface or SDL_BlitScaled with two surfaces, all pixels with less than 255 transparency will have their RGBA values changed in an arbitrary way.

For instance, a pixel with RGBA values (126, 126, 227, 254) may be turned into (125, 125, 225, 253) after the blit operation. I haven't found (or really looked for) a pattern in which the RGB values decrease, but every single pixel has its alpha value reduced by one unless it was 255. That's the best description I can give for this issue.

In the video below you can see a surface bilinearly interpolated over the colors (255, 0, 0, 255) in the top left, (0, 255, 0, 254) in the top right, (0, 0, 255, 254) in the bottom left, and (255, 255, 255, 255) in the bottom right.

https://streamable.com/5fn6dq

In this video, I blit the surface you can see in the window to a new surface, and then display that new surface and reuse that for subsequent blit operations.

You can see very clearly from this video that all colors that have an alpha value of 254 eventually become transparent. When I changed the color of the bilinearly interpolated corners to all have an alpha value of 255, blitting did not change the result of the image (as expected). No pixels were changed in this scenario.

Some specs: Windows 11 Pro 22621.963 SDL 2.26.2 SDL_image 2.6.2 (not used or relevant here) SDL_ttf 2.20.1 (not used or relevant here) C#11 with .NET 7.0

1bsyl commented 1 year ago

do you have some test case ? preferably in c

Marin-MK commented 1 year ago

I don't right now, but I'll strip my code down to pure SDL so you can test it in an hour or two.

Marin-MK commented 1 year ago

This is a stripped version of what my code does, in its simplest form. I've omitted the SDL_Init and SDL_Quit calls because the way my libraries are loaded is very specific and rather unimportant.

Here's the gradient.png image I used, which is the same gradient with two corners with an alpha of 254 as used in the video in which I demonstrated the bug: gradient

My best guess it that it's related to the pixel format of the surfaces. Good to note as well is that a nint data type is just an arbitrary pointer in C# (an alias for IntPtr). Every time you press enter/return with this demo, the image becomes slightly more transparent.

        SDL_WindowFlags flags = SDL_WindowFlags.SDL_WINDOW_ALLOW_HIGHDPI;
        nint win = SDL_CreateWindow("Untitled Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 640, 480, flags);

        SDL_RendererFlags renderflags = SDL_RendererFlags.SDL_RENDERER_TARGETTEXTURE | SDL_RendererFlags.SDL_RENDERER_ACCELERATED;
        nint renderer = SDL_CreateRenderer(win, -1, renderflags);

        SDL_ShowWindow(win);

        nint surface = IMG_Load("gradient.png");
        nint texture = SDL_CreateTextureFromSurface(renderer, surface);

        bool Quit = false;
        while (!Quit)
        {
            SDL_Event e;
            while (SDL_PollEvent(out e) > 0)
            {
                if (Quit) break;
                if (e.type == SDL_EventType.SDL_WINDOWEVENT && e.window.windowevent == SDL_WindowEventID.SDL_WINDOWEVENT_CLOSE)
                {
                    Quit = true;
                }
                else if (e.type == SDL_EventType.SDL_KEYDOWN && e.key.keysym.sym == SDL_Keycode.SDLK_RETURN)
                {
                    nint newsurface = SDL_CreateRGBSurfaceWithFormat(0, 100, 100, 32, SDL_PixelFormatEnum.SDL_PIXELFORMAT_ABGR8888/* = 376840196*/);
                    SDL_Rect blit_src = new SDL_Rect();
                    blit_src.w = 100;
                    blit_src.h = 100;
                    SDL_Rect blit_dest = new SDL_Rect();
                    blit_dest.w = 100;
                    blit_dest.h = 100;
                    SDL_BlitSurface(surface, ref blit_src, newsurface, ref blit_dest);
                    SDL_FreeSurface(surface);
                    SDL_DestroyTexture(texture);
                    surface = newsurface;
                    texture = SDL_CreateTextureFromSurface(renderer, newsurface);
                }
            }
            SDL_RenderClear(renderer);
            SDL_Rect src = new SDL_Rect();
            src.w = 100;
            src.h = 100;
            SDL_Rect dest = new SDL_Rect();
            dest.w = 100;
            dest.h = 100;
            SDL_RenderCopy(renderer, texture, ref src, ref dest);
            SDL_RenderPresent(renderer);
        }

        SDL_DestroyRenderer(renderer);
        SDL_DestroyWindow(win);
1bsyl commented 1 year ago

I haven't really looked but some guess: you image is 100x100 so there is not scaling involved ? right ? since it has alpha maybe the blend mode is activated by default. your first newsurface is not initialized. (to be double checked...?, clear it to white or black) so the first blit would have invalid result etc.

madebr commented 1 year ago

This is your example in .c code:

#include "SDL.h"
#include "SDL_image.h"

static void analyze_surface(SDL_Surface *surface) {
    SDL_LockSurface(surface);
    if (surface->format->format == SDL_PIXELFORMAT_ABGR8888) {
//        Uint32 tl = ((Uint32*)surface->pixels)[0];
        Uint32 tr = ((Uint32*)surface->pixels)[surface->w-1];
        Uint8 r, g, b, a;
        SDL_GetRGBA(tr, surface->format, &r, &g, &b, &a);
        SDL_Log("R=%d G=%d B=%d A=%d", r, g, b, a);
    }

    SDL_UnlockSurface(surface);
}

int main(int argc, char *argv[]) {
    (void)argc;
    (void)argv;
    if (SDL_Init(SDL_INIT_EVERYTHING) < 0) {
        SDL_Log("SDL_Init failed: %s", SDL_GetError());
        return 1;
    }
    if (IMG_Init(IMG_INIT_PNG) != IMG_INIT_PNG) {
        SDL_Log("IMG_Init failed: %s", IMG_GetError());
        return 1;
    }
    SDL_Window *win = SDL_CreateWindow("Untitled Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 640, 480, SDL_WINDOW_ALLOW_HIGHDPI | SDL_WINDOW_RESIZABLE);

    SDL_Renderer *renderer = SDL_CreateRenderer(win, -1, SDL_RENDERER_TARGETTEXTURE | SDL_RENDERER_ACCELERATED);

    SDL_ShowWindow(win);

    SDL_Surface *surface = IMG_Load("gradient.png");
    if (surface == NULL) {
        SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "gradient.png missing", "Cannot find gradient.png", win);
        return 1;
    }
    analyze_surface(surface);
    SDL_Texture *texture = SDL_CreateTextureFromSurface(renderer, surface);

    SDL_bool Quit = SDL_FALSE;
    while (!Quit)
    {
        SDL_Event e;
        while (SDL_PollEvent(&e) > 0)
        {
            if (Quit) break;
            if (e.type == SDL_WINDOWEVENT && e.window.event == SDL_WINDOWEVENT_CLOSE)
            {
                Quit = SDL_TRUE;
            }
            else if (e.type == SDL_KEYDOWN && e.key.keysym.sym == SDLK_RETURN)
            {
                SDL_Surface *newsurface = SDL_CreateRGBSurfaceWithFormat(0, 100, 100, 32, SDL_PIXELFORMAT_ABGR8888);
                SDL_Rect blit_src = {0};
                blit_src.w = 100;
                blit_src.h = 100;
                SDL_Rect blit_dest = {0};
                blit_dest.w = 100;
                blit_dest.h = 100;
                SDL_BlitSurface(surface, &blit_src, newsurface, &blit_dest);
                SDL_FreeSurface(surface);
                SDL_DestroyTexture(texture);
                surface = newsurface;
                texture = SDL_CreateTextureFromSurface(renderer, newsurface);
                analyze_surface(surface);
            }
        }
        SDL_RenderClear(renderer);
        SDL_Rect src = {0};
        src.w = 100;
        src.h = 100;
        SDL_Rect dest = {0};
        SDL_GetWindowSize(win, &dest.w, &dest.h);
        SDL_RenderCopy(renderer, texture, &src, &dest);
        SDL_RenderPresent(renderer);
    }

    SDL_DestroyTexture(texture);
    SDL_FreeSurface(surface);
    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(win);
    IMG_Quit();
    SDL_Quit();
    return 0;
}

It outputs the following rgba pixels for the top right green pixel:

INFO: R=0 G=255 B=0 A=254
INFO: R=0 G=253 B=0 A=253
INFO: R=0 G=250 B=0 A=252
INFO: R=0 G=246 B=0 A=251
INFO: R=0 G=241 B=0 A=250
INFO: R=0 G=235 B=0 A=249
INFO: R=0 G=228 B=0 A=248
INFO: R=0 G=220 B=0 A=247
INFO: R=0 G=212 B=0 A=246
INFO: R=0 G=203 B=0 A=245
INFO: R=0 G=194 B=0 A=244
INFO: R=0 G=184 B=0 A=243
INFO: R=0 G=174 B=0 A=242
INFO: R=0 G=164 B=0 A=241
INFO: R=0 G=154 B=0 A=240
INFO: R=0 G=144 B=0 A=239
INFO: R=0 G=134 B=0 A=238
INFO: R=0 G=124 B=0 A=237
INFO: R=0 G=114 B=0 A=236
INFO: R=0 G=105 B=0 A=235
INFO: R=0 G=96 B=0 A=234
INFO: R=0 G=87 B=0 A=233
INFO: R=0 G=79 B=0 A=232
INFO: R=0 G=71 B=0 A=231
INFO: R=0 G=64 B=0 A=230
INFO: R=0 G=57 B=0 A=229
INFO: R=0 G=50 B=0 A=228
INFO: R=0 G=44 B=0 A=227
INFO: R=0 G=39 B=0 A=226
INFO: R=0 G=34 B=0 A=225
INFO: R=0 G=29 B=0 A=224
INFO: R=0 G=25 B=0 A=223
INFO: R=0 G=21 B=0 A=222
INFO: R=0 G=18 B=0 A=221
INFO: R=0 G=15 B=0 A=220
INFO: R=0 G=12 B=0 A=219
INFO: R=0 G=10 B=0 A=218
INFO: R=0 G=8 B=0 A=217
INFO: R=0 G=6 B=0 A=216
INFO: R=0 G=5 B=0 A=215
INFO: R=0 G=4 B=0 A=214
INFO: R=0 G=3 B=0 A=213
INFO: R=0 G=2 B=0 A=212

You can calculate the next green value by calculating: G_next = G_prev * A_prev / 256

Marin-MK commented 1 year ago

It outputs the following rgba pixels for the top right green pixel:

This is accurate with what I've been observing.

I haven't really looked but some guess: you image is 100x100 so there is not scaling involved ? right ? since it has alpha maybe the blend mode is activated by default. your first newsurface is not initialized. (to be double checked...?, clear it to white or black) so the first blit would have invalid result etc.

There's no scaling involved in SDL_BlitSurface, and even if it was capable of that, I set the source and destination rectangles to be the same size as the image (100x100). Surfaces are clear and ready to be used when they're created, so filling it with some other color would be a bit pointless when I override it entirely with the next method call. I test it nonetheless, and it didn't change anything.

What did make a difference, though, was the surface blend mode. When I set the blend mode to None for either the source surface or the destination surface, the new surface appears as I would expect. The bug reoccurs whenever I set the blend mode to Blend, however.

1bsyl commented 1 year ago

I was mislead when you talk about SDL_BlitScaled and a surface bilinearly interpolated. This is in fact your .png image which is a gradient.

So, you have a just blend the image over and over. we can close the issue ?

Marin-MK commented 1 year ago

I'd say blending a blank surface and a fully filled surface should result in exactly the fully filled surface back.

1bsyl commented 1 year ago

what would you expect ? Here's the blend mode: https://github.com/libsdl-org/SDL/blob/main/include/SDL3/SDL_blendmode.h#L40

slime73 commented 1 year ago

what would you expect ?

According to that formula, alpha-blending a source surface with a specific alpha onto a destination surface with 0 alpha should keep the source's alpha.

e.g. if the source pixel alpha is 254, then the result alpha will be dstA = 254 + (0 * (255 - 254)) == 254, and similar with RGB. But according to @madebr's results that's not actually happening. Unless I'm misunderstanding?

Marin-MK commented 1 year ago

According to that formula, alpha-blending a source surface with a specific alpha onto a destination surface with 0 alpha should keep the source's alpha.

e.g. if the source pixel alpha is 254, then the result alpha will be dstA = 254 + (0 * (255 - 254)) == 254, and similar with RGB. But according to @madebr's results that's not actually happening. Unless I'm misunderstanding?

That's correct. The formula of the Blend blend mode should keep the alpha value as 254 when blending 254 and 0, but the final result becomes 253 (and it also affects the RGB values).

I've updated the title of the issue to reflect the bug more adequately.

1bsyl commented 1 year ago

indeed, sounds a bug. it doesn't happens when selecting SDL_BlitSlow function internally.

INFO: R=0 G=255 B=0 A=254
INFO: R=0 G=254 B=0 A=254
INFO: R=0 G=253 B=0 A=254
INFO: R=0 G=252 B=0 A=254
INFO: R=0 G=251 B=0 A=254
INFO: R=0 G=250 B=0 A=254
INFO: R=0 G=249 B=0 A=254
INFO: R=0 G=24

It happens with BlitRGBtoRGBPixelAlphaMMX().

There is also another fall back to BlitRGBtoRGBPixelAlpha() which seems ok:

INFO: R=0 G=255 B=0 A=254
INFO: R=0 G=253 B=0 A=254
INFO: R=0 G=251 B=0 A=254
INFO: R=0 G=249 B=0 A=254
1bsyl commented 1 year ago

https://github.com/libsdl-org/SDL/blob/main/src/video/SDL_blit_A.c#L322

1bsyl commented 1 year ago

feel free to look at it :)

1bsyl commented 1 year ago

Here https://github.com/libsdl-org/SDL/blob/main/src/video/SDL_blit_A.c#L365 we do (srcA * 255) >> 8 so it doesn't keep srcA. And the alpha is decremented at each blit.

Now, I replaced it by: (srcA * 256) >> 8

please give a try to this PR ! #7006

Marin-MK commented 1 year ago

It appears to still be happening. Or I messed up cloning/compiling, but I don't think so.

madebr commented 1 year ago

It behaves different: the alpha remains constant, but the green component decreases in value:

/home/maarten/programming/SDL/cmake-build-debug/a
INFO: R=0 G=255 B=0 A=254
INFO: R=0 G=253 B=0 A=254
INFO: R=0 G=251 B=0 A=254
INFO: R=0 G=249 B=0 A=254
INFO: R=0 G=247 B=0 A=254
INFO: R=0 G=245 B=0 A=254
INFO: R=0 G=243 B=0 A=254
INFO: R=0 G=241 B=0 A=254
INFO: R=0 G=239 B=0 A=254
INFO: R=0 G=237 B=0 A=254
INFO: R=0 G=235 B=0 A=254
INFO: R=0 G=233 B=0 A=254
INFO: R=0 G=231 B=0 A=254
INFO: R=0 G=229 B=0 A=254
INFO: R=0 G=227 B=0 A=254
INFO: R=0 G=225 B=0 A=254
INFO: R=0 G=223 B=0 A=254
INFO: R=0 G=221 B=0 A=254
INFO: R=0 G=219 B=0 A=254
INFO: R=0 G=217 B=0 A=254
INFO: R=0 G=215 B=0 A=254
INFO: R=0 G=213 B=0 A=254
INFO: R=0 G=211 B=0 A=254
INFO: R=0 G=209 B=0 A=254
1bsyl commented 1 year ago

@Marin-MK yes please re-try, check the commit is present, that the function is used.

slime73 commented 1 year ago

The green channel shouldn't be decreasing either though, so it's not fixed.

1bsyl commented 1 year ago

@slime73 is it sure ? because BlitSlow show that the green channel is decreasing ? x_n+1 = x_n * 254 / 255 ?

slime73 commented 1 year ago

Actually maybe you're right: the contents of the destination texture have pre-multiplied alpha after the source texture is drawn with alpha blending to it, because the alpha blending formula multiplies source RGB with source A. Therefore that same destination texture isn't really valid for use with regular alpha blending in the next iteration, because the alpha blend formula doesn't expect pre-multiplied alpha as the source.

SDL would need a premultiplied alpha blend mode that works with surfaces for it to work out. (Or blending could be turned off.)

Related: https://github.com/libsdl-org/SDL/issues/2485

1bsyl commented 1 year ago

Also modified for SDL2 / BlitRGBtoRGBPixelAlphaMMX3DNOW: 7bf4319eb271363ee711d209d1db76e55d54fedd And applied for SDL3: 13ab10031790f9aec74f8501be1f34e99abf6e5c

Marin-MK commented 1 year ago

The alpha value stays at 254 now, but the RGB values still go to 0. The higher the RGB value, the faster it decreases, until it eventually reaches 0.

1bsyl commented 1 year ago

@Marin-MK what do you expect?

    SDL_BLENDMODE_BLEND = 0x00000001,    /**< alpha blending
                                              dstRGB = (srcRGB * srcA) + (dstRGB * (1-srcA))
                                              dstA = srcA + (dstA * (1-srcA)) */

dstRGB is 0. and srcA = 254/255 so it should go to 0, shouldn't it?

slouken commented 3 months ago

We've added support for premultiplied alpha blending in https://github.com/libsdl-org/SDL/commit/df573391b156dd53831659b6964fc2d92b797f32 for SDL 3.0.