From bab982f2e0b7cc6d1c574e06d95c35a38f7d028e Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Sun, 21 Jul 2024 10:31:48 -0700
Subject: [PATCH] Don't recalculate the blit mapping when changing surfaces
We don't actually need to change the blit mapping if we're targeting a new surface with the same format and palette.
---
src/video/SDL_RLEaccel.c | 2 +-
src/video/SDL_blit.c | 5 +--
src/video/SDL_blit.h | 3 +-
src/video/SDL_blit_0.c | 4 +-
src/video/SDL_blit_1.c | 4 +-
src/video/SDL_blit_A.c | 2 +-
src/video/SDL_blit_N.c | 2 +-
src/video/SDL_pixels.c | 49 ++++++++++++------------
src/video/SDL_pixels_c.h | 2 +-
src/video/SDL_surface.c | 16 +-------
src/video/SDL_surface_c.h | 3 --
test/testautomation_surface.c | 70 +++++++++++++++++++++++++++++++++++
12 files changed, 105 insertions(+), 57 deletions(-)
diff --git a/src/video/SDL_RLEaccel.c b/src/video/SDL_RLEaccel.c
index 14db1a951f372..7cfcda66c31d4 100644
--- a/src/video/SDL_RLEaccel.c
+++ b/src/video/SDL_RLEaccel.c
@@ -1002,7 +1002,7 @@ static int RLEAlphaSurface(SDL_Surface *surface)
int (*copy_transl)(void *, const Uint32 *, int,
const SDL_PixelFormatDetails *, const SDL_PixelFormatDetails *);
- dest = surface->internal->map.dst;
+ dest = surface->internal->map.info.dst_surface;
if (!dest) {
return -1;
}
diff --git a/src/video/SDL_blit.c b/src/video/SDL_blit.c
index 2972bc823bf08..6efa3f6edc847 100644
--- a/src/video/SDL_blit.c
+++ b/src/video/SDL_blit.c
@@ -176,11 +176,10 @@ static SDL_BlitFunc SDL_ChooseBlitFunc(SDL_PixelFormat src_format, SDL_PixelForm
#endif /* SDL_HAVE_BLIT_AUTO */
/* Figure out which of many blit routines to set up on a surface */
-int SDL_CalculateBlit(SDL_Surface *surface)
+int SDL_CalculateBlit(SDL_Surface *surface, SDL_Surface *dst)
{
SDL_BlitFunc blit = NULL;
SDL_BlitMap *map = &surface->internal->map;
- SDL_Surface *dst = map->dst;
SDL_Colorspace src_colorspace = surface->internal->colorspace;
SDL_Colorspace dst_colorspace = dst->internal->colorspace;
@@ -201,11 +200,9 @@ int SDL_CalculateBlit(SDL_Surface *surface)
map->info.src_surface = surface;
map->info.src_fmt = surface->internal->format;
map->info.src_pal = surface->internal->palette;
- map->info.src_pitch = surface->pitch;
map->info.dst_surface = dst;
map->info.dst_fmt = dst->internal->format;
map->info.dst_pal = dst->internal->palette;
- map->info.dst_pitch = dst->pitch;
#if SDL_HAVE_RLE
/* See if we can do RLE acceleration */
diff --git a/src/video/SDL_blit.h b/src/video/SDL_blit.h
index 5f29a1cdf8972..5cf7bcc04aaa5 100644
--- a/src/video/SDL_blit.h
+++ b/src/video/SDL_blit.h
@@ -89,7 +89,6 @@ typedef int (SDLCALL *SDL_Blit) (struct SDL_Surface *src, const SDL_Rect *srcrec
/* Blit mapping definition */
typedef struct SDL_BlitMap
{
- SDL_Surface *dst;
int identity;
SDL_Blit blit;
void *data;
@@ -102,7 +101,7 @@ typedef struct SDL_BlitMap
} SDL_BlitMap;
/* Functions found in SDL_blit.c */
-extern int SDL_CalculateBlit(SDL_Surface *surface);
+extern int SDL_CalculateBlit(SDL_Surface *surface, SDL_Surface *dst);
/* Functions found in SDL_blit_*.c */
extern SDL_BlitFunc SDL_CalculateBlit0(SDL_Surface *surface);
diff --git a/src/video/SDL_blit_0.c b/src/video/SDL_blit_0.c
index a2b18f4acf346..ab06e4154426b 100644
--- a/src/video/SDL_blit_0.c
+++ b/src/video/SDL_blit_0.c
@@ -919,10 +919,10 @@ SDL_BlitFunc SDL_CalculateBlit0(SDL_Surface *surface)
{
int which;
- if (SDL_BITSPERPIXEL(surface->internal->map.dst->format) < 8) {
+ if (SDL_BITSPERPIXEL(surface->internal->map.info.dst_fmt->format) < 8) {
which = 0;
} else {
- which = SDL_BYTESPERPIXEL(surface->internal->map.dst->format);
+ which = SDL_BYTESPERPIXEL(surface->internal->map.info.dst_fmt->format);
}
if (SDL_PIXELTYPE(surface->format) == SDL_PIXELTYPE_INDEX1) {
diff --git a/src/video/SDL_blit_1.c b/src/video/SDL_blit_1.c
index 0ed8b37ddc263..0c7cdc099a00d 100644
--- a/src/video/SDL_blit_1.c
+++ b/src/video/SDL_blit_1.c
@@ -518,10 +518,10 @@ SDL_BlitFunc SDL_CalculateBlit1(SDL_Surface *surface)
{
int which;
- if (SDL_BITSPERPIXEL(surface->internal->map.dst->format) < 8) {
+ if (SDL_BITSPERPIXEL(surface->internal->map.info.dst_fmt->format) < 8) {
which = 0;
} else {
- which = SDL_BYTESPERPIXEL(surface->internal->map.dst->format);
+ which = SDL_BYTESPERPIXEL(surface->internal->map.info.dst_fmt->format);
}
switch (surface->internal->map.info.flags & ~SDL_COPY_RLE_MASK) {
diff --git a/src/video/SDL_blit_A.c b/src/video/SDL_blit_A.c
index 7283073df95aa..61942465d59d6 100644
--- a/src/video/SDL_blit_A.c
+++ b/src/video/SDL_blit_A.c
@@ -1217,7 +1217,7 @@ static void BlitNtoNPixelAlpha(SDL_BlitInfo *info)
SDL_BlitFunc SDL_CalculateBlitA(SDL_Surface *surface)
{
const SDL_PixelFormatDetails *sf = surface->internal->format;
- const SDL_PixelFormatDetails *df = surface->internal->map.dst->internal->format;
+ const SDL_PixelFormatDetails *df = surface->internal->map.info.dst_fmt;
switch (surface->internal->map.info.flags & ~SDL_COPY_RLE_MASK) {
case SDL_COPY_BLEND:
diff --git a/src/video/SDL_blit_N.c b/src/video/SDL_blit_N.c
index 26dbf6c6c7226..2efa9fcaff62b 100644
--- a/src/video/SDL_blit_N.c
+++ b/src/video/SDL_blit_N.c
@@ -3344,7 +3344,7 @@ SDL_BlitFunc SDL_CalculateBlitN(SDL_Surface *surface)
/* Set up data for choosing the blit */
srcfmt = surface->internal->format;
- dstfmt = surface->internal->map.dst->internal->format;
+ dstfmt = surface->internal->map.info.dst_fmt;
/* We don't support destinations less than 8-bits */
if (dstfmt->bits_per_pixel < 8) {
diff --git a/src/video/SDL_pixels.c b/src/video/SDL_pixels.c
index 34c0f35f7d67f..d8003f1039b71 100644
--- a/src/video/SDL_pixels.c
+++ b/src/video/SDL_pixels.c
@@ -1419,30 +1419,34 @@ static Uint8 *MapNto1(const SDL_PixelFormatDetails *src, const SDL_Palette *pal,
return Map1to1(&dithered, pal, identical);
}
-void SDL_InvalidateAllBlitMap(SDL_Surface *surface)
+int SDL_ValidateMap(SDL_Surface *src, SDL_Surface *dst)
{
- SDL_ListNode *l = surface->internal->list_blitmap;
-
- surface->internal->list_blitmap = NULL;
-
- while (l) {
- SDL_ListNode *tmp = l;
- SDL_InvalidateMap((SDL_BlitMap *)l->entry);
- l = l->next;
- SDL_free(tmp);
+ SDL_BlitMap *map = &src->internal->map;
+
+ if (map->info.dst_fmt != dst->internal->format ||
+ map->info.dst_pal != dst->internal->palette ||
+ (dst->internal->palette &&
+ map->dst_palette_version != dst->internal->palette->version) ||
+ (src->internal->palette &&
+ map->src_palette_version != src->internal->palette->version)) {
+ if (SDL_MapSurface(src, dst) < 0) {
+ return -1;
+ }
+ /* just here for debugging */
+ /* printf */
+ /* ("src = 0x%08X src->flags = %08X map->info.flags = %08x\ndst = 0x%08X dst->flags = %08X dst->internal->map.info.flags = %08X\nmap->blit = 0x%08x\n", */
+ /* src, dst->flags, map->info.flags, dst, dst->flags, */
+ /* dst->internal->map.info.flags, map->blit); */
+ } else {
+ map->info.dst_surface = dst;
}
+ return 0;
}
void SDL_InvalidateMap(SDL_BlitMap *map)
{
- if (!map) {
- return;
- }
- if (map->dst) {
- /* Un-register from the destination surface */
- SDL_ListRemove(&map->dst->internal->list_blitmap, map);
- }
- map->dst = NULL;
+ map->info.dst_fmt = NULL;
+ map->info.dst_pal = NULL;
map->src_palette_version = 0;
map->dst_palette_version = 0;
SDL_free(map->info.table);
@@ -1515,13 +1519,6 @@ int SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst)
}
}
- map->dst = dst;
-
- if (map->dst) {
- /* Register BlitMap to the destination surface, to be invalidated when needed */
- SDL_ListAdd(&map->dst->internal->list_blitmap, map);
- }
-
if (dstpal) {
map->dst_palette_version = dstpal->version;
} else {
@@ -1535,6 +1532,6 @@ int SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst)
}
/* Choose your blitters wisely */
- return SDL_CalculateBlit(src);
+ return SDL_CalculateBlit(src, dst);
}
diff --git a/src/video/SDL_pixels_c.h b/src/video/SDL_pixels_c.h
index a4b177efb49e0..e444dab04a3af 100644
--- a/src/video/SDL_pixels_c.h
+++ b/src/video/SDL_pixels_c.h
@@ -43,9 +43,9 @@ extern const float *SDL_GetColorPrimariesConversionMatrix(SDL_ColorPrimaries src
extern void SDL_ConvertColorPrimaries(float *fR, float *fG, float *fB, const float *matrix);
/* Blit mapping functions */
+extern int SDL_ValidateMap(SDL_Surface *src, SDL_Surface *dst);
extern void SDL_InvalidateMap(SDL_BlitMap *map);
extern int SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst);
-extern void SDL_InvalidateAllBlitMap(SDL_Surface *surface);
/* Miscellaneous functions */
extern void SDL_DitherPalette(SDL_Palette *palette);
diff --git a/src/video/SDL_surface.c b/src/video/SDL_surface.c
index f56242cec7a73..63351d5ce4260 100644
--- a/src/video/SDL_surface.c
+++ b/src/video/SDL_surface.c
@@ -849,19 +849,8 @@ int SDL_BlitSurfaceUnchecked(SDL_Surface *src, const SDL_Rect *srcrect,
SDL_Surface *dst, const SDL_Rect *dstrect)
{
/* Check to make sure the blit mapping is valid */
- if ((src->internal->map.dst != dst) ||
- (dst->internal->palette &&
- src->internal->map.dst_palette_version != dst->internal->palette->version) ||
- (src->internal->palette &&
- src->internal->map.src_palette_version != src->internal->palette->version)) {
- if (SDL_MapSurface(src, dst) < 0) {
- return -1;
- }
- /* just here for debugging */
- /* printf */
- /* ("src = 0x%08X src->flags = %08X src->internal->map.info.flags = %08x\ndst = 0x%08X dst->flags = %08X dst->internal->map.info.flags = %08X\nsrc->internal->map.blit = 0x%08x\n", */
- /* src, dst->flags, src->internal->map.info.flags, dst, dst->flags, */
- /* dst->internal->map.info.flags, src->internal->map.blit); */
+ if (SDL_ValidateMap(src, dst) < 0) {
+ return -1;
}
return src->internal->map.blit(src, srcrect, dst, dstrect);
}
@@ -2705,7 +2694,6 @@ void SDL_DestroySurface(SDL_Surface *surface)
SDL_DestroyProperties(surface->internal->props);
SDL_InvalidateMap(&surface->internal->map);
- SDL_InvalidateAllBlitMap(surface);
while (surface->internal->locked > 0) {
SDL_UnlockSurface(surface);
diff --git a/src/video/SDL_surface_c.h b/src/video/SDL_surface_c.h
index 70c24c3b24278..7aa23703abf7e 100644
--- a/src/video/SDL_surface_c.h
+++ b/src/video/SDL_surface_c.h
@@ -60,9 +60,6 @@ struct SDL_SurfaceData
/** info for fast blit mapping to other surfaces */
SDL_BlitMap map;
-
- /** list of BlitMap that hold a reference to this surface */
- SDL_ListNode *list_blitmap;
};
typedef struct SDL_InternalSurface
diff --git a/test/testautomation_surface.c b/test/testautomation_surface.c
index b8a1093dda3ac..022e097a87c8c 100644
--- a/test/testautomation_surface.c
+++ b/test/testautomation_surface.c
@@ -539,6 +539,71 @@ static int surface_testBlit9Grid(void *arg)
return TEST_COMPLETED;
}
+/**
+ * Tests blitting between multiple surfaces of the same format
+ */
+static int surface_testBlitMultiple(void *arg)
+{
+ SDL_Surface *source, *surface;
+ SDL_Palette *palette;
+ Uint8 *pixels;
+
+ palette = SDL_CreatePalette(2);
+ SDLTest_AssertCheck(palette != NULL, "SDL_CreatePalette()");
+ palette->colors[0].r = 0;
+ palette->colors[0].g = 0;
+ palette->colors[0].b = 0;
+ palette->colors[1].r = 0xFF;
+ palette->colors[1].g = 0;
+ palette->colors[1].b = 0;
+
+ source = SDL_CreateSurface(1, 1, SDL_PIXELFORMAT_INDEX8);
+ SDLTest_AssertCheck(source != NULL, "SDL_CreateSurface()");
+ SDL_SetSurfacePalette(source, palette);
+ *(Uint8 *)source->pixels = 1;
+
+ /* Set up a blit to a surface using the palette */
+ surface = SDL_CreateSurface(1, 1, SDL_PIXELFORMAT_INDEX8);
+ SDLTest_AssertCheck(surface != NULL, "SDL_CreateSurface()");
+ SDL_SetSurfacePalette(surface, palette);
+ pixels = (Uint8 *)surface->pixels;
+ *pixels = 0;
+ SDL_BlitSurface(source, NULL, surface, NULL);
+ SDLTest_AssertCheck(*pixels == 1, "Expected *pixels == 1 got %u", *pixels);
+
+ /* Set up a blit to another surface using the same palette */
+ SDL_DestroySurface(surface);
+ surface = SDL_CreateSurface(1, 1, SDL_PIXELFORMAT_INDEX8);
+ SDLTest_AssertCheck(surface != NULL, "SDL_CreateSurface()");
+ SDL_SetSurfacePalette(surface, palette);
+ pixels = (Uint8 *)surface->pixels;
+ *pixels = 0;
+ SDL_BlitSurface(source, NULL, surface, NULL);
+ SDLTest_AssertCheck(*pixels == 1, "Expected *pixels == 1 got %u", *pixels);
+
+ /* Set up a blit to new surface with a different format */
+ SDL_DestroySurface(surface);
+ surface = SDL_CreateSurface(1, 1, SDL_PIXELFORMAT_RGBA32);
+ SDLTest_AssertCheck(surface != NULL, "SDL_CreateSurface()");
+ pixels = (Uint8 *)surface->pixels;
+ SDL_BlitSurface(source, NULL, surface, NULL);
+ SDLTest_AssertCheck(*pixels == 0xFF, "Expected *pixels == 0xFF got 0x%.2X", *pixels);
+
+ /* Set up a blit to another surface with the same format */
+ SDL_DestroySurface(surface);
+ surface = SDL_CreateSurface(1, 1, SDL_PIXELFORMAT_RGBA32);
+ SDLTest_AssertCheck(surface != NULL, "SDL_CreateSurface()");
+ pixels = (Uint8 *)surface->pixels;
+ SDL_BlitSurface(source, NULL, surface, NULL);
+ SDLTest_AssertCheck(*pixels == 0xFF, "Expected *pixels == 0xFF got 0x%.2X", *pixels);
+
+ SDL_DestroyPalette(palette);
+ SDL_DestroySurface(source);
+ SDL_DestroySurface(surface);
+
+ return TEST_COMPLETED;
+}
+
/**
* Tests surface conversion.
*/
@@ -1227,6 +1292,10 @@ static const SDLTest_TestCaseReference surfaceTestBlit9Grid = {
(SDLTest_TestCaseFp)surface_testBlit9Grid, "surface_testBlit9Grid", "Tests 9-grid blitting.", TEST_ENABLED
};
+static const SDLTest_TestCaseReference surfaceTestBlitMultiple = {
+ (SDLTest_TestCaseFp)surface_testBlitMultiple, "surface_testBlitMultiple", "Tests blitting between multiple surfaces of the same format.", TEST_ENABLED
+};
+
static const SDLTest_TestCaseReference surfaceTestLoadFailure = {
(SDLTest_TestCaseFp)surface_testLoadFailure, "surface_testLoadFailure", "Tests sprite loading. A failure case.", TEST_ENABLED
};
@@ -1297,6 +1366,7 @@ static const SDLTest_TestCaseReference *surfaceTests[] = {
&surfaceTestBlit,
&surfaceTestBlitTiled,
&surfaceTestBlit9Grid,
+ &surfaceTestBlitMultiple,
&surfaceTestLoadFailure,
&surfaceTestSurfaceConversion,
&surfaceTestCompleteSurfaceConversion,