libsdl-org / SDL_image

Image decoding for many popular formats for Simple Directmedia Layer.
zlib License
513 stars 174 forks source link

Regression [SDL_image 2.8.0]: can not blit from a png file with palette and transparency #409

Closed hexagonrecursion closed 6 months ago

hexagonrecursion commented 6 months ago

SDL_image >= 2.8.0 loads png files with a palette as a surface with a palette (SDL_PIXELFORMAT_INDEX8). If one or more of the colours in the palette has transparency, the blend mode is set to SDL_BLENDMODE_BLEND, the ColorKey is not set and AlphaMod is not set. This causes issues because SDL_BlitSurface from a surface with this exact combination of options is not supported in SDL.

I consider this a regression because previously we could IMG_Load() those files and use them as a source for blitting, but now we can't.

To show that this exact combination of options is not supported:

g++ -lSDL2 main.cpp && ./a.out

Prints:

INFO: Oh no! error: Blit combination not supported

main.cpp:

#include <cstdlib>
#include <SDL2/SDL.h>

void expect(bool cond) {
    if(!cond) {
        SDL_Log("Something unexpectedly went wroung while setting up the test: %s\n", SDL_GetError());
        std::exit(1);
    }
}

int main(int, char**) {
    // Setup
    expect(SDL_Init(SDL_INIT_VIDEO) == 0);
    SDL_Surface *src = SDL_CreateRGBSurfaceWithFormat(0,1,1,8,SDL_PIXELFORMAT_INDEX8); expect(src);
    expect(SDL_SetSurfaceBlendMode(src, SDL_BLENDMODE_BLEND) == 0);
    SDL_Surface *dst = SDL_CreateRGBSurfaceWithFormat(0,1,1,32,SDL_PIXELFORMAT_RGBA32); expect(dst);
    // Check
    if(SDL_BlitSurface(src, nullptr, dst, nullptr) == 0) {
        SDL_Log("OK\n");
    } else {
        SDL_Log("Oh no! error: %s\n", SDL_GetError());
    }
    return 0;
}

To show that png files with a palette that contains colours with transparency trigger this:

g++ -lSDL2 -lSDL2_image main.cpp && ./a.out

Prints:

INFO: format: SDL_PIXELFORMAT_INDEX8; has color key: no; blend mode: SDL_BLENDMODE_BLEND; modulate alpha: 255
INFO: Oh no! error: Blit combination not supported

main.cpp:

#include <cstdlib>
#include <SDL2/SDL.h>
#include <SDL2/SDL_image.h>

void expect(bool cond) {
    if(!cond) {
        SDL_Log("Something unexpectedly went wroung while setting up the test: %s\n", SDL_GetError());
        std::exit(1);
    }
}

const char *blendModeStr(SDL_BlendMode blendMode) {
    switch(blendMode) {
        case SDL_BLENDMODE_NONE: return "SDL_BLENDMODE_NONE";
        case SDL_BLENDMODE_BLEND: return "SDL_BLENDMODE_BLEND";
        case SDL_BLENDMODE_ADD: return "SDL_BLENDMODE_ADD";
        case SDL_BLENDMODE_MOD: return "SDL_BLENDMODE_MOD";
        case SDL_BLENDMODE_MUL: return "SDL_BLENDMODE_MUL";
    }
    return "unknown";
}

int main(int, char**) {
    expect(SDL_Init(SDL_INIT_VIDEO) == 0);
    expect(IMG_Init(IMG_INIT_PNG) == IMG_INIT_PNG);
    SDL_Surface *src = IMG_Load("bug.png"); expect(src);
    SDL_Surface *dst = SDL_CreateRGBSurfaceWithFormat(0,1,1,32,SDL_PIXELFORMAT_RGBA32); expect(dst);
    SDL_BlendMode blendMode;
    expect(SDL_GetSurfaceBlendMode(src, &blendMode) == 0);
    Uint8 alpha;
    expect(SDL_GetSurfaceAlphaMod(src, &alpha) == 0);
    SDL_Log(
            "format: %s; has color key: %s; blend mode: %s; modulate alpha: %d\n",
            SDL_GetPixelFormatName(src->format->format),
            (SDL_HasColorKey(src) ? "yes" : "no"),
            blendModeStr(blendMode),
            alpha
    );
    if(SDL_BlitSurface(src, nullptr, dst, nullptr) == 0) {
        SDL_Log("OK\n");
    } else {
        SDL_Log("Oh no! error: %s\n", SDL_GetError());
    }
    return 0;
}

Here is the png file I used to test this: bug.png

This code in SDL_blit_1.c is missing case SDL_COPY_BLEND:

    switch (surface->map->info.flags & ~SDL_COPY_RLE_MASK) {
    case 0:
        return one_blit[which];

    case SDL_COPY_COLORKEY:
        return one_blitkey[which];

    case SDL_COPY_COLORKEY | SDL_COPY_BLEND:  /* this is not super-robust but handles a specific case we found sdl12-compat. */
        return (surface->map->info.a == 255) ? one_blitkey[which] : (SDL_BlitFunc)NULL;

    case SDL_COPY_MODULATE_ALPHA | SDL_COPY_BLEND:
        /* Supporting 8bpp->8bpp alpha is doable but requires lots of
           tables which consume space and takes time to precompute,
           so is better left to the user */
        return which >= 2 ? Blit1toNAlpha : (SDL_BlitFunc)NULL;

    case SDL_COPY_COLORKEY | SDL_COPY_MODULATE_ALPHA | SDL_COPY_BLEND:
        return which >= 2 ? Blit1toNAlphaKey : (SDL_BlitFunc)NULL;
    }

Version information:

slouken commented 6 months ago

Thanks for the report!

The showimage test program will also demonstrate this issue with the following patch:

diff --git a/examples/showimage.c b/examples/showimage.c
index d544869..faa597c 100644
--- a/examples/showimage.c
+++ b/examples/showimage.c
@@ -104,12 +104,24 @@ int main(int argc, char *argv[])
             continue;
         }

+#if 1
+        SDL_Surface *surface = IMG_Load(argv[i]);
+        if (surface) {
+            SDL_Surface *tmp = SDL_CreateSurface(surface->w, surface->h, SDL_PIXELFORMAT_RGBA32);
+            if (SDL_BlitSurface(surface, NULL, tmp, NULL) < 0) {
+                SDL_Log("SDL_BlitSurface() failed: %s\n", SDL_GetError());
+                return(2);
+            }
+            texture = SDL_CreateTextureFromSurface(renderer, tmp);
+        }
+#else
         /* Open the image file */
         texture = IMG_LoadTexture(renderer, argv[i]);
         if (!texture) {
             SDL_Log("Couldn't load %s: %s\n", argv[i], SDL_GetError());
             continue;
         }
+#endif
         SDL_QueryTexture(texture, NULL, NULL, &w, &h);

         /* Save the image file, if desired */
slouken commented 6 months ago

This will be fixed for the SDL 2.8.2 release. Thanks for the excellent bug report!

hexagonrecursion commented 6 months ago

Did you mean SDL_image 2.8.2? Because SDL is already at 2.28.5

slouken commented 6 months ago

Yes, sorry, I meant SDL_image 2.8.2.