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

@@ -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;
     }

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 */

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);

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) {

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) {

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:

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) {

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;
+    SDL_BlitMap *map = &src->internal->map;
 
-    surface->internal->list_blitmap = NULL;
+    if (map->info.dst_fmt != dst->internal->format ||
-
+        map->info.dst_pal != dst->internal->palette ||
-    while (l) {
+        (dst->internal->palette &&
-        SDL_ListNode *tmp = l;
+         map->dst_palette_version != dst->internal->palette->version) ||
-        SDL_InvalidateMap((SDL_BlitMap *)l->entry);
+        (src->internal->palette &&
-        l = l->next;
+         map->src_palette_version != src->internal->palette->version)) {
-        SDL_free(tmp);
+        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) {
+    map->info.dst_fmt = NULL;
-        return;
+    map->info.dst_pal = NULL;
-    }
-    if (map->dst) {
-        /* Un-register from the destination surface */
-        SDL_ListRemove(&map->dst->internal->list_blitmap, map);
-    }
-    map->dst = 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);
 }
 

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);

src/video/SDL_surface.c

@@ -849,20 +849,9 @@ 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) ||
+    if (SDL_ValidateMap(src, dst) < 0) {
-        (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); */
-    }
     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);

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

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,