(svn r4486) - NewGRF: Create and use a memory pool to manage sprite groups. This

reduces the amount of house keeping we do and the chance of memory 
leaks.

Makefile

@@ -636,6 +636,7 @@ SRCS += network_server.c
 SRCS += network_udp.c
 SRCS += newgrf.c
 SRCS += newgrf_engine.c
+SRCS += newgrf_spritegroup.c
 SRCS += newgrf_station.c
 SRCS += news_gui.c
 SRCS += npf.c

newgrf.c

@@ -21,6 +21,8 @@
 #include "newgrf_engine.h"
 #include "vehicle.h"
 
+#include "newgrf_spritegroup.h"
+
 /* TTDPatch extended GRF format codec
  * (c) Petr Baudis 2004 (GPL'd)
  * Changes by Florian octo Forster are (c) by the OpenTTD development team.
@@ -1215,7 +1217,7 @@ static void VehicleChangeInfo(byte *buf, int len)
  */
 static SpriteGroup* NewCallBackResultSpriteGroup(uint16 value)
 {
-	SpriteGroup *group = calloc(1, sizeof(*group));
+	SpriteGroup *group = AllocateSpriteGroup();
 
 	group->type = SGT_CALLBACK;
 
@@ -1240,7 +1242,7 @@ static SpriteGroup* NewCallBackResultSpriteGroup(uint16 value)
  */
 static SpriteGroup* NewResultSpriteGroup(uint16 value, byte sprites)
 {
-	SpriteGroup *group = calloc(1, sizeof(*group));
+	SpriteGroup *group = AllocateSpriteGroup();
 	group->type = SGT_RESULT;
 	group->g.result.result = value;
 	group->g.result.sprites = sprites;
@@ -1348,7 +1350,7 @@ static void NewSpriteGroup(byte *buf, int len)
 		buf += 4; len -= 4;
 		check_length(len, 6, "NewSpriteGroup 0x81/0x82");
 
-		group = calloc(1, sizeof(*group));
+		group = AllocateSpriteGroup();
 		group->type = SGT_DETERMINISTIC;
 		dg = &group->g.determ;
 
@@ -1379,13 +1381,11 @@ static void NewSpriteGroup(byte *buf, int len)
 			groupid = grf_load_word(&buf);
 			if (HASBIT(groupid, 15)) {
 				dg->ranges[i].group = NewCallBackResultSpriteGroup(groupid);
-				dg->ranges[i].group->ref_count++;
 			} else if (groupid >= _cur_grffile->spritegroups_count || _cur_grffile->spritegroups[groupid] == NULL) {
 				grfmsg(GMS_WARN, "NewSpriteGroup(%02x:0x%x): Groupid %04x does not exist, leaving empty.", setid, numloaded, groupid);
 				dg->ranges[i].group = NULL;
 			} else {
 				dg->ranges[i].group = _cur_grffile->spritegroups[groupid];
-				dg->ranges[i].group->ref_count++;
 			}
 
 			dg->ranges[i].low = grf_load_byte(&buf);
@@ -1395,19 +1395,14 @@ static void NewSpriteGroup(byte *buf, int len)
 		groupid = grf_load_word(&buf);
 		if (HASBIT(groupid, 15)) {
 			dg->default_group = NewCallBackResultSpriteGroup(groupid);
-			dg->default_group->ref_count++;
 		} else if (groupid >= _cur_grffile->spritegroups_count || _cur_grffile->spritegroups[groupid] == NULL) {
 			grfmsg(GMS_WARN, "NewSpriteGroup(%02x:0x%x): Groupid %04x does not exist, leaving empty.", setid, numloaded, groupid);
 			dg->default_group = NULL;
 		} else {
 			dg->default_group = _cur_grffile->spritegroups[groupid];
-		dg->default_group->ref_count++;
 		}
 
-		if (_cur_grffile->spritegroups[setid] != NULL)
-			UnloadSpriteGroup(&_cur_grffile->spritegroups[setid]);
 		_cur_grffile->spritegroups[setid] = group;
-		group->ref_count++;
 		return;
 
 	} else if (numloaded == 0x80 || numloaded == 0x83) {
@@ -1421,7 +1416,7 @@ static void NewSpriteGroup(byte *buf, int len)
 		len -= 4;
 		check_length(len, 6, "NewSpriteGroup 0x80/0x83");
 
-		group = calloc(1, sizeof(*group));
+		group = AllocateSpriteGroup();
 		group->type = SGT_RANDOMIZED;
 		rg = &group->g.random;
 
@@ -1444,20 +1439,15 @@ static void NewSpriteGroup(byte *buf, int len)
 
 			if (HASBIT(groupid, 15)) {
 				rg->groups[i] = NewCallBackResultSpriteGroup(groupid);
-				rg->groups[i]->ref_count++;
 			} else if (groupid >= _cur_grffile->spritegroups_count || _cur_grffile->spritegroups[groupid] == NULL) {
 				grfmsg(GMS_WARN, "NewSpriteGroup(%02x:0x%x): Groupid %04x does not exist, leaving empty.", setid, numloaded, groupid);
 				rg->groups[i] = NULL;
 			} else {
 				rg->groups[i] = _cur_grffile->spritegroups[groupid];
-				rg->groups[i]->ref_count++;
 			}
 		}
 
-		if (_cur_grffile->spritegroups[setid] != NULL)
-			UnloadSpriteGroup(&_cur_grffile->spritegroups[setid]);
 		_cur_grffile->spritegroups[setid] = group;
-		group->ref_count++;
 		return;
 	}
 
@@ -1480,7 +1470,7 @@ static void NewSpriteGroup(byte *buf, int len)
 	if (_cur_grffile->first_spriteset == 0)
 		_cur_grffile->first_spriteset = _cur_grffile->spriteset_start;
 
-	group = calloc(1, sizeof(*group));
+	group = AllocateSpriteGroup();
 	group->type = SGT_REAL;
 	rg = &group->g.real;
 
@@ -1503,7 +1493,6 @@ static void NewSpriteGroup(byte *buf, int len)
 		} else {
 			rg->loaded[i] = NewResultSpriteGroup(_cur_grffile->spriteset_start + spriteset_id * _cur_grffile->spriteset_numents, rg->sprites_per_set);
 		}
-		rg->loaded[i]->ref_count++;
 		DEBUG(grf, 8) ("NewSpriteGroup: + rg->loaded[%i]  = %u (subset %u)", i, rg->loaded[i]->g.result.result, spriteset_id);
 	}
 
@@ -1514,14 +1503,10 @@ static void NewSpriteGroup(byte *buf, int len)
 		} else {
 			rg->loading[i] = NewResultSpriteGroup(_cur_grffile->spriteset_start + spriteset_id * _cur_grffile->spriteset_numents, rg->sprites_per_set);
 		}
-		rg->loading[i]->ref_count++;
 		DEBUG(grf, 8) ("NewSpriteGroup: + rg->loading[%i] = %u (subset %u)", i, rg->loading[i]->g.result.result, spriteset_id);
 	}
 
-	if (_cur_grffile->spritegroups[setid] != NULL)
-		UnloadSpriteGroup(&_cur_grffile->spritegroups[setid]);
 	_cur_grffile->spritegroups[setid] = group;
-	group->ref_count++;
 }
 
 /* Action 0x03 */
@@ -1593,7 +1578,6 @@ static void NewVehicle_SpriteGroupMapping(byte *buf, int len)
 				}
 
 				stat->spritegroup[1] = _cur_grffile->spritegroups[groupid];
-				stat->spritegroup[1]->ref_count++;
 			}
 		}
 
@@ -1612,7 +1596,6 @@ static void NewVehicle_SpriteGroupMapping(byte *buf, int len)
 				StationSpec *stat = &_cur_grffile->stations[stid];
 
 				stat->spritegroup[0] = _cur_grffile->spritegroups[groupid];
-				stat->spritegroup[0]->ref_count++;
 				stat->grfid = _cur_grffile->grfid;
 				stat->localidx = stid;
 				SetCustomStation(stat);
@@ -2372,46 +2355,16 @@ static void InitializeGRFSpecial(void)
 	                   | (1 << 0x10); /* autoreplace */
 }
 
-/**
- * Unload unused sprite groups from the specified GRF file.
- * Called after loading each GRF file.
- * @param file GRF file
- */
-static void ReleaseSpriteGroups(GRFFile *file)
-{
-	int i;
-
-	// Bail out if no spritegroups were defined.
-	if (file->spritegroups == NULL)
-		return;
-
-	DEBUG(grf, 6)("ReleaseSpriteGroups: Releasing for `%s'.", file->filename);
-	for (i = 0; i < file->spritegroups_count; i++) {
-		if (file->spritegroups[i] != NULL)
-			UnloadSpriteGroup(&file->spritegroups[i]);
-	}
-	free(file->spritegroups);
-	file->spritegroups = NULL;
-	file->spritegroups_count = 0;
-}
-
 static void ResetCustomStations(void)
 {
 	GRFFile *file;
 	uint i;
-	CargoID c;
 
 	for (file = _first_grffile; file != NULL; file = file->next) {
 		for (i = 0; i < file->num_stations; i++) {
 			if (file->stations[i].grfid != file->grfid) continue;
 
 			// TODO: Release renderdata, platforms and layouts
-
-			// Release this stations sprite groups.
-			for (c = 0; c < NUM_GLOBAL_CID; c++) {
-				if (file->stations[i].spritegroup[c] != NULL)
-					UnloadSpriteGroup(&file->stations[i].spritegroup[c]);
-			}
 		}
 
 		/* Free and reset the station data */
@@ -2467,6 +2420,8 @@ static void ResetNewGRFData(void)
 
 	// Add engine type to engine data. This is needed for the refit precalculation.
 	AddTypeToEngines();
+
+	InitializeSpriteGroupPool();
 }
 
 /** Reset all NewGRFData that was used only while processing data */
@@ -2480,6 +2435,11 @@ static void ClearTemporaryNewGRFData(void)
 		l = l2;
 	}
 	_cur_grffile->label = NULL;
+
+	/* Clear the list of spritegroups */
+	free(_cur_grffile->spritegroups);
+	_cur_grffile->spritegroups = NULL;
+	_cur_grffile->spritegroups_count = 0;
 }
 
 static void InitNewGRFFile(const char* filename, int sprite_offset)
@@ -2680,11 +2640,6 @@ static void LoadNewGRFFile(const char* filename, uint file_index, uint stage)
 
 		if (_skip_sprites > 0) _skip_sprites--;
 	}
-
-	// Release our sprite group references.
-	// Any groups that are referenced elsewhere will be cleaned up later.
-	// This removes groups that aren't used. (Perhaps skipped?)
-	ReleaseSpriteGroups(_cur_grffile);
 }
 
 

newgrf_engine.c

@@ -44,7 +44,6 @@ void SetWagonOverrideSprites(EngineID engine, SpriteGroup *group, byte *train_id
 	 * to prevent leaks. But first we need to refcount the SpriteGroup.
 	 * --pasky */
 	wo->group = group;
-	group->ref_count++;
 	wo->trains = trains;
 	wo->train_id = malloc(trains);
 	memcpy(wo->train_id, train_id, trains);
@@ -86,7 +85,7 @@ void UnloadWagonOverrides(void)
 		wos = &_engine_wagon_overrides[engine];
 		for (i = 0; i < wos->overrides_count; i++) {
 			wo = &wos->overrides[i];
-			UnloadSpriteGroup(&wo->group);
+			wo->group = NULL;
 			free(wo->train_id);
 		}
 		free(wos->overrides);
@@ -104,11 +103,9 @@ static SpriteGroup *engine_custom_sprites[TOTAL_NUM_ENGINES][NUM_GLOBAL_CID];
 void SetCustomEngineSprites(EngineID engine, byte cargo, SpriteGroup *group)
 {
 	if (engine_custom_sprites[engine][cargo] != NULL) {
-		DEBUG(grf, 6)("SetCustomEngineSprites: engine `%d' cargo `%d' already has group -- removing.", engine, cargo);
-		UnloadSpriteGroup(&engine_custom_sprites[engine][cargo]);
+		DEBUG(grf, 6)("SetCustomEngineSprites: engine `%d' cargo `%d' already has group -- replacing.", engine, cargo);
 	}
 	engine_custom_sprites[engine][cargo] = group;
-	group->ref_count++;
 }
 
 /**
@@ -121,10 +118,7 @@ void UnloadCustomEngineSprites(void)
 
 	for (engine = 0; engine < TOTAL_NUM_ENGINES; engine++) {
 		for (cargo = 0; cargo < NUM_GLOBAL_CID; cargo++) {
-			if (engine_custom_sprites[engine][cargo] != NULL) {
-				DEBUG(grf, 6)("UnloadCustomEngineSprites: Unloading group for engine `%d' cargo `%d'.", engine, cargo);
-				UnloadSpriteGroup(&engine_custom_sprites[engine][cargo]);
-			}
+			engine_custom_sprites[engine][cargo] = NULL;
 		}
 	}
 }

newgrf_spritegroup.c

@@ -0,0 +1,68 @@
+/* $Id$ */
+
+#include "stdafx.h"
+#include "openttd.h"
+#include "pool.h"
+#include "sprite.h"
+
+enum {
+	SPRITEGROUP_POOL_BLOCK_SIZE_BITS = 4, /* (1 << 4) == 16 items */
+	SPRITEGROUP_POOL_MAX_BLOCKS      = 8000,
+};
+
+static uint _spritegroup_count = 0;
+static MemoryPool _spritegroup_pool;
+
+
+static void SpriteGroupPoolCleanBlock(uint start_item, uint end_item)
+{
+	uint i;
+
+	for (i = start_item; i <= end_item; i++) {
+		SpriteGroup *group = (SpriteGroup*)GetItemFromPool(&_spritegroup_pool, i);
+
+		/* Free dynamically allocated memory */
+		switch (group->type) {
+			case SGT_REAL:
+				free(group->g.real.loaded);
+				free(group->g.real.loading);
+				break;
+
+			case SGT_DETERMINISTIC:
+				free(group->g.determ.ranges);
+				break;
+
+			case SGT_RANDOMIZED:
+				free(group->g.random.groups);
+				break;
+
+			default:
+				break;
+		}
+	}
+}
+
+
+/* Initialize the SpriteGroup pool */
+static MemoryPool _spritegroup_pool = { "SpriteGr", SPRITEGROUP_POOL_MAX_BLOCKS, SPRITEGROUP_POOL_BLOCK_SIZE_BITS, sizeof(SpriteGroup), NULL, &SpriteGroupPoolCleanBlock, 0, 0, NULL };
+
+
+/* Allocate a new SpriteGroup */
+SpriteGroup *AllocateSpriteGroup(void)
+{
+	/* This is totally different to the other pool allocators, as we never remove an item from the pool. */
+	if (_spritegroup_count == _spritegroup_pool.total_items) {
+		if (!AddBlockToPool(&_spritegroup_pool)) return NULL;
+	}
+
+	return (SpriteGroup*)GetItemFromPool(&_spritegroup_pool, _spritegroup_count++);
+}
+
+
+void InitializeSpriteGroupPool(void)
+{
+	CleanPool(&_spritegroup_pool);
+	AddBlockToPool(&_spritegroup_pool);
+
+	_spritegroup_count = 0;
+}

newgrf_spritegroup.h

@@ -0,0 +1,9 @@
+/* $Id$ */
+
+#ifndef NEWGRF_SPRITEGROUP_H
+#define NEWGRF_SPRITEGROUP_H
+
+SpriteGroup *AllocateSpriteGroup(void);
+void InitializeSpriteGroupPool(void);
+
+#endif /* NEWGRF_SPRITEGROUP_H */

sprite.c

@@ -97,74 +97,3 @@ byte RandomizedSpriteGroupTriggeredBits(const RandomizedSpriteGroup *rsg,
 
 	return (rsg->num_groups - 1) << rsg->lowest_randbit;
 }
-
-/**
- * Traverse a sprite group and release its and its child's memory.
- * A group is only released if its reference count is zero.
- * We pass a pointer to a pointer so that the original reference can be set to NULL.
- * @param group_ptr Pointer to sprite group reference.
- */
-void UnloadSpriteGroup(SpriteGroup **group_ptr)
-{
-	SpriteGroup *group;
-	int i;
-
-	assert(group_ptr != NULL);
-	assert(*group_ptr != NULL);
-
-	group = *group_ptr;
-	*group_ptr = NULL; // Remove this reference.
-
-	group->ref_count--;
-	if (group->ref_count > 0) {
-		DEBUG(grf, 6)("UnloadSpriteGroup: Group at `%p' (type %d) has %d reference(s) left.", group, group->type, group->ref_count);
-		return; // Still some references left, so don't clear up.
-	}
-
-	DEBUG(grf, 6)("UnloadSpriteGroup: Releasing group at `%p'.", group);
-	switch (group->type) {
-		case SGT_REAL:
-		{
-			RealSpriteGroup *rsg = &group->g.real;
-			for (i = 0; i < rsg->loading_count; i++) {
-				if (rsg->loading[i] != NULL) UnloadSpriteGroup(&rsg->loading[i]);
-			}
-			for (i = 0; i < rsg->loaded_count; i++) {
-				if (rsg->loaded[i] != NULL) UnloadSpriteGroup(&rsg->loaded[i]);
-			}
-			free(group->g.real.loaded);
-			free(group->g.real.loading);
-			free(group);
-			return;
-		}
-
-		case SGT_DETERMINISTIC:
-		{
-			DeterministicSpriteGroup *dsg = &group->g.determ;
-			for (i = 0; i < group->g.determ.num_ranges; i++) {
-				if (dsg->ranges[i].group != NULL) UnloadSpriteGroup(&dsg->ranges[i].group);
-			}
-			if (dsg->default_group != NULL) UnloadSpriteGroup(&dsg->default_group);
-			free(group->g.determ.ranges);
-			free(group);
-			return;
-		}
-
-		case SGT_RANDOMIZED:
-		{
-			for (i = 0; i < group->g.random.num_groups; i++) {
-				if (group->g.random.groups[i] != NULL) UnloadSpriteGroup(&group->g.random.groups[i]);
-			}
-			free(group->g.random.groups);
-			free(group);
-			return;
-		}
-
-		case SGT_CALLBACK:
-		case SGT_RESULT:
-			free(group);
-			return;
-	}
-
-	DEBUG(grf, 1)("Unable to remove unknown sprite group type `0x%x'.", group->type);
-}

sprite.h

@@ -118,6 +118,7 @@ typedef struct ResultSpriteGroup {
 } ResultSpriteGroup;
 
 typedef enum SpriteGroupType {
+	SGT_INVALID,
 	SGT_REAL,
 	SGT_DETERMINISTIC,
 	SGT_RANDOMIZED,
@@ -127,7 +128,6 @@ typedef enum SpriteGroupType {
 
 struct SpriteGroup {
 	SpriteGroupType type;
-	byte ref_count;
 
 	union {
 		RealSpriteGroup real;
@@ -160,6 +160,4 @@ SpriteGroup *EvalRandomizedSpriteGroup(const RandomizedSpriteGroup *rsg, byte ra
  * (then they are |ed to @waiting_triggers instead). */
 byte RandomizedSpriteGroupTriggeredBits(const RandomizedSpriteGroup *rsg, byte triggers, byte *waiting_triggers);
 
-void UnloadSpriteGroup(SpriteGroup **group_ptr);
-
 #endif /* SPRITE_H */