diff --git a/src/video/SDL_bmp.c b/src/video/SDL_bmp.c index 118bf11d35..de1db2e986 100644 --- a/src/video/SDL_bmp.c +++ b/src/video/SDL_bmp.c @@ -591,6 +591,78 @@ done: return surface; } +typedef struct { + SDL_Surface *surface; + SDL_Surface *intermediate_surface; + bool save32bit; + bool saveLegacyBMP; +} BMPSaveState; + +static void FreeBMPSaveState(BMPSaveState *state) +{ + if (state->intermediate_surface && state->intermediate_surface != state->surface) { + SDL_DestroySurface(state->intermediate_surface); + } +} + +static bool InitBMPSaveState(BMPSaveState *state, SDL_Surface *surface) +{ + state->surface = surface; + state->intermediate_surface = NULL; + state->save32bit = false; + state->saveLegacyBMP = false; + + CHECK_PARAM(!SDL_SurfaceValid(surface)) { + return SDL_InvalidParamError("surface"); + } + +#ifdef SAVE_32BIT_BMP + // We can save alpha information in a 32-bit BMP + if (SDL_BITSPERPIXEL(surface->format) >= 8 && + (SDL_ISPIXELFORMAT_ALPHA(surface->format) || + (surface->map.info.flags & SDL_COPY_COLORKEY))) { + state->save32bit = true; + } +#endif // SAVE_32BIT_BMP + + if (surface->palette && !state->save32bit) { + if (SDL_BITSPERPIXEL(surface->format) == 8) { + state->intermediate_surface = surface; + } else { + SDL_SetError("%u bpp BMP files not supported", + SDL_BITSPERPIXEL(surface->format)); + goto error; + } + } else if ((surface->format == SDL_PIXELFORMAT_BGR24 && !state->save32bit) || + (surface->format == SDL_PIXELFORMAT_BGRA32 && state->save32bit)) { + state->intermediate_surface = surface; + } else { + SDL_PixelFormat pixel_format; + + /* If the surface has a colorkey or alpha channel we'll save a + 32-bit BMP with alpha channel, otherwise save a 24-bit BMP. */ + if (state->save32bit) { + pixel_format = SDL_PIXELFORMAT_BGRA32; + } else { + pixel_format = SDL_PIXELFORMAT_BGR24; + } + state->intermediate_surface = SDL_ConvertSurface(surface, pixel_format); + if (!state->intermediate_surface) { + SDL_SetError("Couldn't convert image to %d bpp", + (int)SDL_BITSPERPIXEL(pixel_format)); + goto error; + } + } + + if (state->save32bit) { + state->saveLegacyBMP = SDL_GetHintBoolean(SDL_HINT_BMP_SAVE_LEGACY_FORMAT, false); + } + return true; +error: + FreeBMPSaveState(state); + return false; +} + SDL_Surface *SDL_LoadBMP(const char *file) { SDL_IOStream *stream = SDL_IOFromFile(file, "rb"); @@ -599,16 +671,12 @@ SDL_Surface *SDL_LoadBMP(const char *file) } return SDL_LoadBMP_IO(stream, true); } - -bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) +static bool SDL_SaveBMP_IO_Internal(BMPSaveState *state, SDL_IOStream *dst, bool closeio) { bool was_error = true; Sint64 fp_offset, new_offset; int i, pad; - SDL_Surface *intermediate_surface = NULL; Uint8 *bits; - bool save32bit = false; - bool saveLegacyBMP = false; // The Win32 BMP file header (14 bytes) char magic[2] = { 'B', 'M' }; @@ -647,60 +715,8 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) Uint32 bV5ProfileSize = 0; Uint32 bV5Reserved = 0; - // Make sure we have somewhere to save - CHECK_PARAM(!SDL_SurfaceValid(surface)) { - SDL_InvalidParamError("surface"); - goto done; - } - CHECK_PARAM(!dst) { - SDL_InvalidParamError("dst"); - goto done; - } - -#ifdef SAVE_32BIT_BMP - // We can save alpha information in a 32-bit BMP - if (SDL_BITSPERPIXEL(surface->format) >= 8 && - (SDL_ISPIXELFORMAT_ALPHA(surface->format) || - surface->map.info.flags & SDL_COPY_COLORKEY)) { - save32bit = true; - } -#endif // SAVE_32BIT_BMP - - if (surface->palette && !save32bit) { - if (SDL_BITSPERPIXEL(surface->format) == 8) { - intermediate_surface = surface; - } else { - SDL_SetError("%u bpp BMP files not supported", - SDL_BITSPERPIXEL(surface->format)); - goto done; - } - } else if ((surface->format == SDL_PIXELFORMAT_BGR24 && !save32bit) || - (surface->format == SDL_PIXELFORMAT_BGRA32 && save32bit)) { - intermediate_surface = surface; - } else { - SDL_PixelFormat pixel_format; - - /* If the surface has a colorkey or alpha channel we'll save a - 32-bit BMP with alpha channel, otherwise save a 24-bit BMP. */ - if (save32bit) { - pixel_format = SDL_PIXELFORMAT_BGRA32; - } else { - pixel_format = SDL_PIXELFORMAT_BGR24; - } - intermediate_surface = SDL_ConvertSurface(surface, pixel_format); - if (!intermediate_surface) { - SDL_SetError("Couldn't convert image to %d bpp", - (int)SDL_BITSPERPIXEL(pixel_format)); - goto done; - } - } - - if (save32bit) { - saveLegacyBMP = SDL_GetHintBoolean(SDL_HINT_BMP_SAVE_LEGACY_FORMAT, false); - } - - if (SDL_LockSurface(intermediate_surface)) { - const size_t bw = intermediate_surface->w * intermediate_surface->fmt->bytes_per_pixel; + if (SDL_LockSurface(state->intermediate_surface)) { + const size_t bw = state->intermediate_surface->w * state->intermediate_surface->fmt->bytes_per_pixel; // Set the BMP file header values bfSize = 0; // We'll write this when we're done @@ -723,23 +739,23 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) // Set the BMP info values biSize = 40; - biWidth = intermediate_surface->w; - biHeight = intermediate_surface->h; + biWidth = state->intermediate_surface->w; + biHeight = state->intermediate_surface->h; biPlanes = 1; - biBitCount = intermediate_surface->fmt->bits_per_pixel; + biBitCount = state->intermediate_surface->fmt->bits_per_pixel; biCompression = BI_RGB; - biSizeImage = intermediate_surface->h * intermediate_surface->pitch; + biSizeImage = state->intermediate_surface->h * state->intermediate_surface->pitch; biXPelsPerMeter = 0; biYPelsPerMeter = 0; - if (intermediate_surface->palette) { - biClrUsed = intermediate_surface->palette->ncolors; + if (state->intermediate_surface->palette) { + biClrUsed = state->intermediate_surface->palette->ncolors; } else { biClrUsed = 0; } biClrImportant = 0; // Set the BMP info values - if (save32bit && !saveLegacyBMP) { + if (state->save32bit && !state->saveLegacyBMP) { biSize = 124; // Version 4 values biCompression = BI_BITFIELDS; @@ -775,7 +791,7 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) } // Write the BMP info values - if (save32bit && !saveLegacyBMP) { + if (state->save32bit && !state->saveLegacyBMP) { // Version 4 values if (!SDL_WriteU32LE(dst, bV4RedMask) || !SDL_WriteU32LE(dst, bV4GreenMask) || @@ -804,12 +820,12 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) } // Write the palette (in BGR color order) - if (intermediate_surface->palette) { + if (state->intermediate_surface->palette) { SDL_Color *colors; int ncolors; - colors = intermediate_surface->palette->colors; - ncolors = intermediate_surface->palette->ncolors; + colors = state->intermediate_surface->palette->colors; + ncolors = state->intermediate_surface->palette->ncolors; for (i = 0; i < ncolors; ++i) { if (!SDL_WriteU8(dst, colors[i].b) || !SDL_WriteU8(dst, colors[i].g) || @@ -833,10 +849,10 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) } // Write the bitmap image upside down - bits = (Uint8 *)intermediate_surface->pixels + (intermediate_surface->h * intermediate_surface->pitch); + bits = (Uint8 *)state->intermediate_surface->pixels + (state->intermediate_surface->h * state->intermediate_surface->pitch); pad = ((bw % 4) ? (4 - (bw % 4)) : 0); - while (bits > (Uint8 *)intermediate_surface->pixels) { - bits -= intermediate_surface->pitch; + while (bits > (Uint8 *)state->intermediate_surface->pixels) { + bits -= state->intermediate_surface->pitch; if (SDL_WriteIO(dst, bits, bw) != bw) { goto done; } @@ -867,15 +883,12 @@ bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) } // Close it up.. - SDL_UnlockSurface(intermediate_surface); + SDL_UnlockSurface(state->intermediate_surface); was_error = false; } done: - if (intermediate_surface && intermediate_surface != surface) { - SDL_DestroySurface(intermediate_surface); - } if (closeio && dst) { if (!SDL_CloseIO(dst)) { was_error = true; @@ -887,11 +900,36 @@ done: return true; } +bool SDL_SaveBMP_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) +{ + bool result = true; + BMPSaveState state; + if (!InitBMPSaveState(&state, surface)) { + return false; + } + CHECK_PARAM(!dst) { + result = SDL_InvalidParamError("dst"); + goto done; + } + result = SDL_SaveBMP_IO_Internal(&state, dst, closeio); +done: + FreeBMPSaveState(&state); + return result; +} + bool SDL_SaveBMP(SDL_Surface *surface, const char *file) { + BMPSaveState state; + if (!InitBMPSaveState(&state, surface)) { + return false; + } + SDL_IOStream *stream = SDL_IOFromFile(file, "wb"); if (!stream) { return false; } - return SDL_SaveBMP_IO(surface, stream, true); + + bool result = SDL_SaveBMP_IO_Internal(&state, stream, true); + FreeBMPSaveState(&state); + return result; } diff --git a/src/video/SDL_stb.c b/src/video/SDL_stb.c index 6748c9c598..a4b0a718ab 100644 --- a/src/video/SDL_stb.c +++ b/src/video/SDL_stb.c @@ -395,7 +395,7 @@ bool SDL_SavePNG_IO(SDL_Surface *surface, SDL_IOStream *dst, bool closeio) Uint8 *trns = NULL; bool free_surface = false; - // Make sure we have somewhere to save + // Make sure we have something to save CHECK_PARAM(!SDL_SurfaceValid(surface)) { SDL_InvalidParamError("surface"); goto done; @@ -478,6 +478,14 @@ done: bool SDL_SavePNG(SDL_Surface *surface, const char *file) { #ifdef SDL_HAVE_STB + // Make sure we have something to save + CHECK_PARAM(!SDL_SurfaceValid(surface)) { + return SDL_InvalidParamError("surface"); + } + + if (SDL_ISPIXELFORMAT_INDEXED(surface->format) && !surface->palette) { + return SDL_SetError("Indexed surfaces must have a palette"); + } SDL_IOStream *stream = SDL_IOFromFile(file, "wb"); if (!stream) { return false; diff --git a/test/testautomation_surface.c b/test/testautomation_surface.c index bb4e3e0744..6061cf0c49 100644 --- a/test/testautomation_surface.c +++ b/test/testautomation_surface.c @@ -328,6 +328,7 @@ static int SDLCALL surface_testSaveLoad(void *arg) int ret; const char *sampleFilename = "testSaveLoad.tmp"; SDL_Surface *face; + SDL_Surface *indexed_surface; SDL_Surface *rface; SDL_Palette *palette; SDL_Color colors[] = { @@ -344,6 +345,18 @@ static int SDLCALL surface_testSaveLoad(void *arg) return TEST_ABORTED; } + indexed_surface = SDL_CreateSurface(32, 32, SDL_PIXELFORMAT_INDEX8); + SDLTest_AssertCheck(indexed_surface != NULL, "SDL_CreateSurface(SDL_PIXELFORMAT_INDEX8)"); + + /* Delete test file; ignore errors */ + SDL_RemovePath(sampleFilename); + + /* Saving an indexed surface without palette as BMP fails */ + ret = SDL_SaveBMP(indexed_surface, sampleFilename); + SDLTest_AssertPass("Call to SDL_SaveBMP() using an indexed surface without palette"); + SDLTest_AssertCheck(ret == false, "Verify result of SDL_SaveBMP(indexed_surface without palette), expected: false, got: %i", ret); + SDLTest_AssertCheck(!SDL_GetPathInfo(sampleFilename, NULL), "No file is created after trying to save a indexed surface without palette"); + /* Delete test file; ignore errors */ SDL_RemovePath(sampleFilename); @@ -367,6 +380,15 @@ static int SDLCALL surface_testSaveLoad(void *arg) /* Delete test file; ignore errors */ SDL_RemovePath(sampleFilename); + /* Saving an indexed surface as PNG fails */ + ret = SDL_SavePNG(indexed_surface, sampleFilename); + SDLTest_AssertPass("Call to SDL_SavePNG() using an indexed surface without palette"); + SDLTest_AssertCheck(ret == false, "Verify result of SDL_SavePNG(indexed surface without palette), expected: false, got: %i", ret); + SDLTest_AssertCheck(ret == false, "Verify result of SDL_SavePNG(indexed surface without palette), expected: false, got: %i", ret); + + /* Delete test file; ignore errors */ + SDL_RemovePath(sampleFilename); + /* Save a PNG surface */ ret = SDL_SavePNG(face, sampleFilename); SDLTest_AssertPass("Call to SDL_SavePNG()"); @@ -416,6 +438,9 @@ static int SDLCALL surface_testSaveLoad(void *arg) if (stream == NULL) { return TEST_ABORTED; } + ret = SDL_SaveBMP_IO(indexed_surface, stream, false); + SDLTest_AssertCheck(ret == false, "Verify result from SDL_SaveBMP (indexed surface without palette), expected: false, got: %i", ret); + SDL_SeekIO(stream, 0, SDL_IO_SEEK_SET); ret = SDL_SaveBMP_IO(face, stream, false); SDLTest_AssertPass("Call to SDL_SaveBMP()"); SDLTest_AssertCheck(ret == true, "Verify result from SDL_SaveBMP, expected: true, got: %i", ret); @@ -447,6 +472,9 @@ static int SDLCALL surface_testSaveLoad(void *arg) if (stream == NULL) { return TEST_ABORTED; } + ret = SDL_SavePNG_IO(indexed_surface, stream, false); + SDLTest_AssertCheck(ret == false, "Verify result from SDL_SavePNG_IO (indexed surface without palette), expected: false, got: %i", ret); + SDL_SeekIO(stream, 0, SDL_IO_SEEK_SET); ret = SDL_SavePNG_IO(face, stream, false); SDLTest_AssertPass("Call to SDL_SavePNG()"); SDLTest_AssertCheck(ret == true, "Verify result from SDL_SavePNG, expected: true, got: %i", ret); @@ -472,6 +500,7 @@ static int SDLCALL surface_testSaveLoad(void *arg) SDL_CloseIO(stream); stream = NULL; + SDL_DestroySurface(indexed_surface); SDL_DestroySurface(face); return TEST_COMPLETED;