Skip to content
This repository has been archived by the owner. It is now read-only.
Browse files

Fixed blit mapping problem when surfaces are freed and then newly all…

…ocated at the same address.

Tim Angus to SDL

void function( SDL_Surface* surface )
 SDL_Surface* anotherSurface =
   SDL_ConvertSurfaceFormat( surface, ... );

 // surface->map->dst is now equal to anotherSurface

 // Do some stuff with anotherSurface

 SDL_FreeSurface( anotherSurface );

 // anotherSurface is now a dead pointer,
 // but surface->map->dst still points to it

int main( )
 SDL_Surface* surface = CreateAValidSurface( );

 function( surface );

At this point blit something from surface. SDL_LowerBlit is called, which checks surface->map->dst against the blit destination. If the pointers happen to match (not that unlikely), the map is decided to be valid and bad things happen.

It seems to me like the whole idea of caching the blit mapping is fundamentally flawed in that the source surface has no knowledge of the lifetime of the destination surface.
  • Loading branch information
slouken committed Jan 17, 2012
1 parent 8b39405 commit 329564a79e72fb0272573b04999be18e4f98f30c
Showing with 17 additions and 0 deletions.
  1. +17 −0 src/video/SDL_pixels.c
@@ -968,6 +968,12 @@ SDL_InvalidateMap(SDL_BlitMap * map)
if (!map) {
if (map->dst) {
/* Release our reference to the surface - see the note below */
if (--map->dst->refcount <= 0) {
map->dst = NULL;
map->src_palette_version = 0;
map->dst_palette_version = 0;
@@ -1036,6 +1042,17 @@ SDL_MapSurface(SDL_Surface * src, SDL_Surface * dst)

map->dst = dst;

if (map->dst) {
/* Keep a reference to this surface so it doesn't get deleted
while we're still pointing at it.
A better method would be for the destination surface to keep
track of surfaces that are mapped to it and automatically
invalidate them when it is freed, but this will do for now.

if (dstfmt->palette) {
map->dst_palette_version = dstfmt->palette->version;
} else {

0 comments on commit 329564a

Please sign in to comment.