From 2b365983dbf2f8a86b5b21caa483a63b00c0a851 Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
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 | 208 ++-----------------------------------
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, 19 insertions(+), 202 deletions(-)
diff --git a/src/video/SDL_RLEaccel.c b/src/video/SDL_RLEaccel.c
index 2ca6d7e0db00c..2f0c680522cca 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,134 +1370,17 @@ 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);
+ surface->map.info.flags &= ~(SDL_COPY_RLE_COLORKEY | SDL_COPY_RLE_ALPHAKEY);
- // 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;
- }
- }
+ if (!(surface->flags & SDL_SURFACE_PREALLOCATED)) {
+ surface->pixels = surface->saved_pixels;
+ surface->saved_pixels = NULL;
}
- surface->map.info.flags &= ~(SDL_COPY_RLE_COLORKEY | SDL_COPY_RLE_ALPHAKEY);
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 5a89a09d268c7..252ba1029fb63 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 ea7e070074af8..ffe1ed3c4b405 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 70ea85b57070a..1a6d8d87e4d79 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 df8f5d30a57d0..efab1081db509 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 64d9eb3ad4a92..3b38fe00709e5 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