From b890dab2b410b0b114047cf7bbf49e7e89d21482 Mon Sep 17 00:00:00 2001
From: Peter Nelson <peter1138@openttd.org>
Date: Sat, 23 Nov 2024 12:14:46 +0000
Subject: [PATCH] Change: Treat recolour sprites as regular sprites in the
 SpriteCache. (#13107)

Recolour sprites are loaded when seen, instead of being loaded when needed. This could result in the sprite cache being filled up with recolour sprites, and also mean that replacing recolour sprites didn't release the previously allocated memory.

Instead, allow recolour sprites to be loaded as needed and freed when unneeded, like regular sprites.
---
 src/spritecache.cpp        | 22 ++++++++++++++++------
 src/spritecache_internal.h |  1 +
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/spritecache.cpp b/src/spritecache.cpp
index 35242178d5..2c97a830a7 100644
--- a/src/spritecache.cpp
+++ b/src/spritecache.cpp
@@ -418,10 +418,12 @@ static bool ResizeSprites(SpriteLoader::SpriteCollection &sprite, uint8_t sprite
 /**
  * Load a recolour sprite into memory.
  * @param file GRF we're reading from.
+ * @param file_pos Position within file.
  * @param num Size of the sprite in the GRF.
+ * @param allocator Sprite allocator to use.
  * @return Sprite data.
  */
-static void *ReadRecolourSprite(SpriteFile &file, uint num, SpriteAllocator &allocator)
+static void *ReadRecolourSprite(SpriteFile &file, size_t file_pos, uint num, SpriteAllocator &allocator)
 {
 	/* "Normal" recolour sprites are ALWAYS 257 bytes. Then there is a small
 	 * number of recolour sprites that are 17 bytes that only exist in DOS
@@ -430,6 +432,7 @@ static void *ReadRecolourSprite(SpriteFile &file, uint num, SpriteAllocator &all
 	static const uint RECOLOUR_SPRITE_SIZE = 257;
 	uint8_t *dest = allocator.Allocate<uint8_t>(std::max(RECOLOUR_SPRITE_SIZE, num));
 
+	file.SeekTo(file_pos, SEEK_SET);
 	if (file.NeedsPaletteRemap()) {
 		uint8_t *dest_tmp = new uint8_t[std::max(RECOLOUR_SPRITE_SIZE, num)];
 
@@ -634,9 +637,9 @@ bool LoadNextSprite(SpriteID load_index, SpriteFile &file, uint file_sprite_id)
 			file.ReadByte();
 			return false;
 		}
+		file_pos = file.GetPos();
 		type = SpriteType::Recolour;
-		CacheSpriteAllocator allocator;
-		data = ReadRecolourSprite(file, num, allocator);
+		file.SkipBytes(num);
 	} else if (file.GetContainerVersion() >= 2 && grf_type == 0xFD) {
 		if (num != 4) {
 			/* Invalid sprite section include, ignore. */
@@ -675,6 +678,7 @@ bool LoadNextSprite(SpriteID load_index, SpriteFile &file, uint file_sprite_id)
 	SpriteCache *sc = AllocateSpriteCache(load_index);
 	sc->file = &file;
 	sc->file_pos = file_pos;
+	sc->length = num;
 	sc->ptr = data;
 	sc->lru = 0;
 	sc->id = file_sprite_id;
@@ -835,7 +839,7 @@ static void DeleteEntryFromSpriteCache()
 	cur_lru = 0xffff;
 	for (SpriteID i = 0; i != _spritecache_items; i++) {
 		SpriteCache *sc = GetSpriteCache(i);
-		if (sc->type != SpriteType::Recolour && sc->ptr != nullptr && sc->lru < cur_lru) {
+		if (sc->ptr != nullptr && sc->lru < cur_lru) {
 			cur_lru = sc->lru;
 			best = i;
 		}
@@ -977,7 +981,13 @@ void *GetRawSprite(SpriteID sprite, SpriteType type, SpriteAllocator *allocator,
 		sc->lru = ++_sprite_lru_counter;
 
 		/* Load the sprite, if it is not loaded, yet */
-		if (sc->ptr == nullptr) sc->ptr = ReadSprite(sc, sprite, type, cache_allocator, nullptr);
+		if (sc->ptr == nullptr) {
+			if (sc->type == SpriteType::Recolour) {
+				sc->ptr = ReadRecolourSprite(*sc->file, sc->file_pos, sc->length, cache_allocator);
+			} else {
+				sc->ptr = ReadSprite(sc, sprite, type, cache_allocator, nullptr);
+			}
+		}
 
 		return sc->ptr;
 	} else {
@@ -1056,7 +1066,7 @@ void GfxClearSpriteCache()
 	/* Clear sprite ptr for all cached items */
 	for (uint i = 0; i != _spritecache_items; i++) {
 		SpriteCache *sc = GetSpriteCache(i);
-		if (sc->type != SpriteType::Recolour && sc->ptr != nullptr) DeleteEntryFromSpriteCache(i);
+		if (sc->ptr != nullptr) DeleteEntryFromSpriteCache(i);
 	}
 
 	VideoDriver::GetInstance()->ClearSystemSprites();
diff --git a/src/spritecache_internal.h b/src/spritecache_internal.h
index f4f328eb4d..76f8aac7b2 100644
--- a/src/spritecache_internal.h
+++ b/src/spritecache_internal.h
@@ -24,6 +24,7 @@ struct SpriteCache {
 	void *ptr;
 	size_t file_pos;
 	SpriteFile *file;    ///< The file the sprite in this entry can be found in.
+	uint32_t length; ///< Length of sprite data.
 	uint32_t id;
 	int16_t lru;
 	SpriteType type;     ///< In some cases a single sprite is misused by two NewGRFs. Once as real sprite and once as recolour sprite. If the recolour sprite gets into the cache it might be drawn as real sprite which causes enormous trouble.