From 2b365983dbf2f8a86b5b21caa483a63b00c0a851 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Sat, 8 Nov 2025 12:35:53 -0800 Subject: [PATCH] Save the original pixels when RLE encoding is enabled Trying to reconstruct the original image when undoing RLE encoding was never completely accurate. Fixes https://github.com/libsdl-org/SDL/issues/14424 --- src/video/SDL_RLEaccel.c | 210 +++---------------------------------- src/video/SDL_RLEaccel_c.h | 2 +- src/video/SDL_blit.c | 2 +- src/video/SDL_pixels.c | 2 +- src/video/SDL_surface.c | 4 +- src/video/SDL_surface_c.h | 3 + 6 files changed, 20 insertions(+), 203 deletions(-) diff --git a/src/video/SDL_RLEaccel.c b/src/video/SDL_RLEaccel.c index 2ca6d7e0db..2f0c680522 100644 --- a/src/video/SDL_RLEaccel.c +++ b/src/video/SDL_RLEaccel.c @@ -878,23 +878,6 @@ static int copy_opaque_16(void *dst, const Uint32 *src, int n, return n * 2; } -// decode opaque pixels from 16bpp to 32bpp rgb + a -static int uncopy_opaque_16(Uint32 *dst, const void *src, int n, - const SDL_PixelFormatDetails *sfmt, const SDL_PixelFormatDetails *dfmt) -{ - int i; - const Uint16 *s = (const Uint16 *)src; - unsigned alpha = dfmt->Amask ? 255 : 0; - for (i = 0; i < n; i++) { - unsigned r, g, b; - RGB_FROM_PIXEL(*s, sfmt, r, g, b); - PIXEL_FROM_RGBA(*dst, dfmt, r, g, b, alpha); - s++; - dst++; - } - return n * 2; -} - // encode 32bpp rgb + a into 32bpp G0RAB format for blitting into 565 static int copy_transl_565(void *dst, const Uint32 *src, int n, const SDL_PixelFormatDetails *sfmt, const SDL_PixelFormatDetails *dfmt) @@ -931,24 +914,6 @@ static int copy_transl_555(void *dst, const Uint32 *src, int n, return n * 4; } -// decode translucent pixels from 32bpp GORAB to 32bpp rgb + a -static int uncopy_transl_16(Uint32 *dst, const void *src, int n, - const SDL_PixelFormatDetails *sfmt, const SDL_PixelFormatDetails *dfmt) -{ - int i; - const Uint32 *s = (const Uint32 *)src; - for (i = 0; i < n; i++) { - unsigned r, g, b, a; - Uint32 pix = *s++; - a = (pix & 0x3e0) >> 2; - pix = (pix & ~0x3e0) | pix >> 16; - RGB_FROM_PIXEL(pix, sfmt, r, g, b); - PIXEL_FROM_RGBA(*dst, dfmt, r, g, b, a); - dst++; - } - return n * 4; -} - // encode 32bpp rgba into 32bpp rgba, keeping alpha (dual purpose) static int copy_32(void *dst, const Uint32 *src, int n, const SDL_PixelFormatDetails *sfmt, const SDL_PixelFormatDetails *dfmt) @@ -965,23 +930,6 @@ static int copy_32(void *dst, const Uint32 *src, int n, return n * 4; } -// decode 32bpp rgba into 32bpp rgba, keeping alpha (dual purpose) -static int uncopy_32(Uint32 *dst, const void *src, int n, - const SDL_PixelFormatDetails *sfmt, const SDL_PixelFormatDetails *dfmt) -{ - int i; - const Uint32 *s = (const Uint32 *)src; - for (i = 0; i < n; i++) { - unsigned r, g, b, a; - Uint32 pixel = *s++; - RGB_FROM_PIXEL(pixel, sfmt, r, g, b); - a = pixel >> 24; - PIXEL_FROM_RGBA(*dst, dfmt, r, g, b, a); - dst++; - } - return n * 4; -} - #define ISOPAQUE(pixel, fmt) ((((pixel)&fmt->Amask) >> fmt->Ashift) == 255) #define ISTRANSL(pixel, fmt) \ @@ -1177,17 +1125,6 @@ static bool RLEAlphaSurface(SDL_Surface *surface) #undef ADD_OPAQUE_COUNTS #undef ADD_TRANSL_COUNTS - // Now that we have it encoded, release the original pixels - if (!(surface->flags & SDL_SURFACE_PREALLOCATED)) { - if (surface->flags & SDL_SURFACE_SIMD_ALIGNED) { - SDL_aligned_free(surface->pixels); - surface->flags &= ~SDL_SURFACE_SIMD_ALIGNED; - } else { - SDL_free(surface->pixels); - } - surface->pixels = NULL; - } - // reallocate the buffer to release unused memory { Uint8 *p = (Uint8 *)SDL_realloc(rlebuf, dst - rlebuf); @@ -1353,17 +1290,6 @@ static bool RLEColorkeySurface(SDL_Surface *surface) #undef ADD_COUNTS - // Now that we have it encoded, release the original pixels - if (!(surface->flags & SDL_SURFACE_PREALLOCATED)) { - if (surface->flags & SDL_SURFACE_SIMD_ALIGNED) { - SDL_aligned_free(surface->pixels); - surface->flags &= ~SDL_SURFACE_SIMD_ALIGNED; - } else { - SDL_free(surface->pixels); - } - surface->pixels = NULL; - } - // reallocate the buffer to release unused memory { // If SDL_realloc returns NULL, the original block is left intact @@ -1383,7 +1309,7 @@ bool SDL_RLESurface(SDL_Surface *surface) // Clear any previous RLE conversion if (surface->internal_flags & SDL_INTERNAL_SURFACE_RLEACCEL) { - SDL_UnRLESurface(surface, true); + SDL_UnRLESurface(surface); } // We don't support RLE encoding of bitmaps @@ -1432,6 +1358,11 @@ bool SDL_RLESurface(SDL_Surface *surface) surface->map.info.flags |= SDL_COPY_RLE_ALPHAKEY; } + if (!(surface->flags & SDL_SURFACE_PREALLOCATED)) { + surface->saved_pixels = surface->pixels; + surface->pixels = NULL; + } + // The surface is now accelerated surface->internal_flags |= SDL_INTERNAL_SURFACE_RLEACCEL; SDL_UpdateSurfaceLockFlag(surface); @@ -1439,135 +1370,18 @@ bool SDL_RLESurface(SDL_Surface *surface) return true; } -/* - * Un-RLE a surface with pixel alpha - * This may not give back exactly the image before RLE-encoding; all - * completely transparent pixels will be lost, and color and alpha depth - * may have been reduced (when encoding for 16bpp targets). - */ -static bool UnRLEAlpha(SDL_Surface *surface) -{ - Uint8 *srcbuf; - Uint32 *dst; - const SDL_PixelFormatDetails *sf = surface->fmt; - const SDL_PixelFormatDetails *df = SDL_GetPixelFormatDetails(*(SDL_PixelFormat *)surface->map.data); - int (*uncopy_opaque)(Uint32 *, const void *, int, - const SDL_PixelFormatDetails *, const SDL_PixelFormatDetails *); - int (*uncopy_transl)(Uint32 *, const void *, int, - const SDL_PixelFormatDetails *, const SDL_PixelFormatDetails *); - int w = surface->w; - int bpp = df->bytes_per_pixel; - size_t size; - - if (bpp == 2) { - uncopy_opaque = uncopy_opaque_16; - uncopy_transl = uncopy_transl_16; - } else { - uncopy_opaque = uncopy_transl = uncopy_32; - } - - if (!SDL_size_mul_check_overflow(surface->h, surface->pitch, &size)) { - return false; - } - - surface->pixels = SDL_aligned_alloc(SDL_GetSIMDAlignment(), size); - if (!surface->pixels) { - return false; - } - surface->flags |= SDL_SURFACE_SIMD_ALIGNED; - // fill background with transparent pixels - SDL_memset(surface->pixels, 0, (size_t)surface->h * surface->pitch); - - dst = (Uint32 *)surface->pixels; - srcbuf = (Uint8 *)surface->map.data + sizeof(SDL_PixelFormat); - for (;;) { - // copy opaque pixels - int ofs = 0; - do { - unsigned run; - if (bpp == 2) { - ofs += srcbuf[0]; - run = srcbuf[1]; - srcbuf += 2; - } else { - ofs += ((Uint16 *)srcbuf)[0]; - run = ((Uint16 *)srcbuf)[1]; - srcbuf += 4; - } - if (run) { - srcbuf += uncopy_opaque(dst + ofs, srcbuf, run, df, sf); - ofs += run; - } else if (!ofs) { - goto end_function; - } - } while (ofs < w); - - // skip padding if needed - if (bpp == 2) { - srcbuf += (uintptr_t)srcbuf & 2; - } - - // copy translucent pixels - ofs = 0; - do { - unsigned run; - ofs += ((Uint16 *)srcbuf)[0]; - run = ((Uint16 *)srcbuf)[1]; - srcbuf += 4; - if (run) { - srcbuf += uncopy_transl(dst + ofs, srcbuf, run, df, sf); - ofs += run; - } - } while (ofs < w); - dst += surface->pitch >> 2; - } - -end_function: - return true; -} - -void SDL_UnRLESurface(SDL_Surface *surface, bool recode) +void SDL_UnRLESurface(SDL_Surface *surface) { if (surface->internal_flags & SDL_INTERNAL_SURFACE_RLEACCEL) { surface->internal_flags &= ~SDL_INTERNAL_SURFACE_RLEACCEL; - if (recode && !(surface->flags & SDL_SURFACE_PREALLOCATED)) { - if (surface->map.info.flags & SDL_COPY_RLE_COLORKEY) { - SDL_Rect full; - size_t size; - - // re-create the original surface - if (!SDL_size_mul_check_overflow(surface->h, surface->pitch, &size)) { - // Memory corruption? - surface->internal_flags |= SDL_INTERNAL_SURFACE_RLEACCEL; - return; - } - surface->pixels = SDL_aligned_alloc(SDL_GetSIMDAlignment(), size); - if (!surface->pixels) { - // Oh crap... - surface->internal_flags |= SDL_INTERNAL_SURFACE_RLEACCEL; - return; - } - surface->flags |= SDL_SURFACE_SIMD_ALIGNED; - - // fill it with the background color - SDL_FillSurfaceRect(surface, NULL, surface->map.info.colorkey); - - // now render the encoded surface - full.x = full.y = 0; - full.w = surface->w; - full.h = surface->h; - SDL_RLEBlit(surface, &full, surface, &full); - } else { - if (!UnRLEAlpha(surface)) { - // Oh crap... - surface->internal_flags |= SDL_INTERNAL_SURFACE_RLEACCEL; - return; - } - } - } surface->map.info.flags &= ~(SDL_COPY_RLE_COLORKEY | SDL_COPY_RLE_ALPHAKEY); + if (!(surface->flags & SDL_SURFACE_PREALLOCATED)) { + surface->pixels = surface->saved_pixels; + surface->saved_pixels = NULL; + } + SDL_free(surface->map.data); surface->map.data = NULL; diff --git a/src/video/SDL_RLEaccel_c.h b/src/video/SDL_RLEaccel_c.h index 5a89a09d26..252ba1029f 100644 --- a/src/video/SDL_RLEaccel_c.h +++ b/src/video/SDL_RLEaccel_c.h @@ -27,6 +27,6 @@ // Useful functions and variables from SDL_RLEaccel.c extern bool SDL_RLESurface(SDL_Surface *surface); -extern void SDL_UnRLESurface(SDL_Surface *surface, bool recode); +extern void SDL_UnRLESurface(SDL_Surface *surface); #endif // SDL_RLEaccel_c_h_ diff --git a/src/video/SDL_blit.c b/src/video/SDL_blit.c index ea7e070074..ffe1ed3c4b 100644 --- a/src/video/SDL_blit.c +++ b/src/video/SDL_blit.c @@ -201,7 +201,7 @@ bool SDL_CalculateBlit(SDL_Surface *surface, SDL_Surface *dst) #ifdef SDL_HAVE_RLE // Clean everything out to start if (surface->internal_flags & SDL_INTERNAL_SURFACE_RLEACCEL) { - SDL_UnRLESurface(surface, true); + SDL_UnRLESurface(surface); } #endif diff --git a/src/video/SDL_pixels.c b/src/video/SDL_pixels.c index 70ea85b570..1a6d8d87e4 100644 --- a/src/video/SDL_pixels.c +++ b/src/video/SDL_pixels.c @@ -1560,7 +1560,7 @@ bool SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst) map = &src->map; #ifdef SDL_HAVE_RLE if (src->internal_flags & SDL_INTERNAL_SURFACE_RLEACCEL) { - SDL_UnRLESurface(src, true); + SDL_UnRLESurface(src); } #endif SDL_InvalidateMap(map); diff --git a/src/video/SDL_surface.c b/src/video/SDL_surface.c index df8f5d30a5..efab1081db 100644 --- a/src/video/SDL_surface.c +++ b/src/video/SDL_surface.c @@ -1725,7 +1725,7 @@ bool SDL_LockSurface(SDL_Surface *surface) #ifdef SDL_HAVE_RLE // Perform the lock if (surface->internal_flags & SDL_INTERNAL_SURFACE_RLEACCEL) { - SDL_UnRLESurface(surface, true); + SDL_UnRLESurface(surface); surface->internal_flags |= SDL_INTERNAL_SURFACE_RLEACCEL; // save accel'd state SDL_UpdateSurfaceLockFlag(surface); } @@ -3068,7 +3068,7 @@ void SDL_DestroySurface(SDL_Surface *surface) } #ifdef SDL_HAVE_RLE if (surface->internal_flags & SDL_INTERNAL_SURFACE_RLEACCEL) { - SDL_UnRLESurface(surface, false); + SDL_UnRLESurface(surface); } #endif SDL_SetSurfacePalette(surface, NULL); diff --git a/src/video/SDL_surface_c.h b/src/video/SDL_surface_c.h index 64d9eb3ad4..3b38fe0070 100644 --- a/src/video/SDL_surface_c.h +++ b/src/video/SDL_surface_c.h @@ -78,6 +78,9 @@ struct SDL_Surface /** info for fast blit mapping to other surfaces */ SDL_BlitMap map; + + /** Original pixels when RLE is enabled */ + void *saved_pixels; }; // Surface functions