Codechange: Use structure to hold recolour remap sprite.

src/blitter/32bpp_anim.cpp

@@ -36,7 +36,7 @@ inline void Blitter_32bppAnim::Draw(const Blitter::BlitterParams *bp, ZoomLevel
 	Colour *dst = (Colour *)bp->dst + bp->top * bp->pitch + bp->left;
 	uint16_t *anim = this->anim_buf + this->ScreenToAnimOffset((uint32_t *)bp->dst) + bp->top * this->anim_buf_pitch + bp->left;
 
-	const uint8_t *remap = bp->remap; // store so we don't have to access it via bp every time
+	const uint8_t *remap = bp->remap->GetPaletteRemap(); // store so we don't have to access it via bp every time
 
 	for (int y = 0; y < bp->height; y++) {
 		Colour *dst_ln = dst + bp->pitch;

src/blitter/32bpp_anim_sse4.cpp

@@ -33,7 +33,7 @@ template <BlitterMode mode, Blitter_32bppSSE2::ReadMode read_mode, Blitter_32bpp
 GNU_TARGET("sse4.1")
 inline void Blitter_32bppSSE4_Anim::Draw(const BlitterParams *bp, ZoomLevel zoom)
 {
-	const uint8_t * const remap = bp->remap;
+	const uint8_t * const remap = bp->remap->GetPaletteRemap();
 	Colour *dst_line = (Colour *) bp->dst + bp->top * bp->pitch + bp->left;
 	uint16_t *anim_line = this->anim_buf + this->ScreenToAnimOffset((uint32_t *)bp->dst) + bp->top * this->anim_buf_pitch + bp->left;
 	int effective_width = bp->width;

src/blitter/32bpp_optimized.cpp

@@ -48,7 +48,7 @@ inline void Blitter_32bppOptimized::Draw(const Blitter::BlitterParams *bp, ZoomL
 	Colour *dst = (Colour *)bp->dst + bp->top * bp->pitch + bp->left;
 
 	/* store so we don't have to access it via bp every time (compiler assumes pointer aliasing) */
-	const uint8_t *remap = bp->remap;
+	const uint8_t *remap = bp->remap->GetPaletteRemap();
 
 	for (int y = 0; y < bp->height; y++) {
 		/* next dst line begins here */

src/blitter/32bpp_simple.cpp

@@ -23,6 +23,7 @@ void Blitter_32bppSimple::Draw(Blitter::BlitterParams *bp, BlitterMode mode, Zoo
 {
 	const Blitter_32bppSimple::Pixel *src, *src_line;
 	Colour *dst, *dst_line;
+	const uint8_t *remap = bp->remap->GetPaletteRemap();
 
 	/* Find where to start reading in the source sprite */
 	src_line = (const Blitter_32bppSimple::Pixel *)bp->sprite + (bp->skip_top * bp->sprite_width + bp->skip_left) * ScaleByZoom(1, zoom);
@@ -42,7 +43,7 @@ void Blitter_32bppSimple::Draw(Blitter::BlitterParams *bp, BlitterMode mode, Zoo
 					if (src->m == 0) {
 						if (src->a != 0) *dst = ComposeColourRGBA(src->r, src->g, src->b, src->a, *dst);
 					} else {
-						if (bp->remap[src->m] != 0) *dst = ComposeColourPA(AdjustBrightness(this->LookupColourInPalette(bp->remap[src->m]), src->v), src->a, *dst);
+						if (remap[src->m] != 0) *dst = ComposeColourPA(AdjustBrightness(this->LookupColourInPalette(remap[src->m]), src->v), src->a, *dst);
 					}
 					break;
 
@@ -53,7 +54,7 @@ void Blitter_32bppSimple::Draw(Blitter::BlitterParams *bp, BlitterMode mode, Zoo
 							*dst = ComposeColourRGBA(g, g, g, src->a, *dst);
 						}
 					} else {
-						if (bp->remap[src->m] != 0) *dst = ComposeColourPA(AdjustBrightness(this->LookupColourInPalette(bp->remap[src->m]), src->v), src->a, *dst);
+						if (remap[src->m] != 0) *dst = ComposeColourPA(AdjustBrightness(this->LookupColourInPalette(remap[src->m]), src->v), src->a, *dst);
 					}
 					break;
 
@@ -73,7 +74,7 @@ void Blitter_32bppSimple::Draw(Blitter::BlitterParams *bp, BlitterMode mode, Zoo
 				case BlitterMode::TransparentRemap:
 					/* Apply custom transparency remap. */
 					if (src->a != 0) {
-						*dst = this->LookupColourInPalette(bp->remap[GetNearestColourIndex(*dst)]);
+						*dst = this->LookupColourInPalette(remap[GetNearestColourIndex(*dst)]);
 					}
 					break;
 

src/blitter/32bpp_sse_func.hpp

@@ -221,7 +221,7 @@ inline void Blitter_32bppSSSE3::Draw(const Blitter::BlitterParams *bp, ZoomLevel
 inline void Blitter_32bppSSE4::Draw(const Blitter::BlitterParams *bp, ZoomLevel zoom)
 #endif
 {
-	const uint8_t * const remap = bp->remap;
+	const uint8_t * const remap = bp->remap->GetPaletteRemap();
 	Colour *dst_line = (Colour *) bp->dst + bp->top * bp->pitch + bp->left;
 	int effective_width = bp->width;
 

src/blitter/40bpp_anim.cpp

@@ -114,7 +114,7 @@ inline void Blitter_40bppAnim::Draw(const Blitter::BlitterParams *bp, ZoomLevel
 	uint8_t *anim = VideoDriver::GetInstance()->GetAnimBuffer() + ((uint32_t *)bp->dst - (uint32_t *)_screen.dst_ptr) + bp->top * bp->pitch + bp->left;
 
 	/* store so we don't have to access it via bp every time (compiler assumes pointer aliasing) */
-	const uint8_t *remap = bp->remap;
+	const uint8_t *remap = bp->remap->GetPaletteRemap();
 
 	for (int y = 0; y < bp->height; y++) {
 		/* next dst line begins here */
@@ -374,7 +374,8 @@ void Blitter_40bppAnim::DrawColourMappingRect(void *dst, int width, int height,
 			anim = anim - width + _screen.pitch;
 		} while (--height);
 	} else if (pal == PALETTE_NEWSPAPER) {
-		const uint8_t *remap = GetNonSprite(pal, SpriteType::Recolour) + 1;
+		const RecolourSprite *rs = GetRecolourSprite(pal);
+		const uint8_t *remap = rs->GetPaletteRemap();
 		do {
 			for (int i = 0; i != width; i++) {
 				if (*anim == 0) {
@@ -389,7 +390,8 @@ void Blitter_40bppAnim::DrawColourMappingRect(void *dst, int width, int height,
 			anim = anim - width + _screen.pitch;
 		} while (--height);
 	} else {
-		const uint8_t *remap = GetNonSprite(pal, SpriteType::Recolour) + 1;
+		const RecolourSprite *rs = GetRecolourSprite(pal);
+		const uint8_t *remap = rs->GetPaletteRemap();
 		do {
 			for (int i = 0; i != width; i++) {
 				if (*anim != 0) *anim = remap[*anim];

src/blitter/8bpp_base.cpp

@@ -16,7 +16,7 @@
 
 void Blitter_8bppBase::DrawColourMappingRect(void *dst, int width, int height, PaletteID pal)
 {
-	const uint8_t *ctab = GetNonSprite(pal, SpriteType::Recolour) + 1;
+	const uint8_t *ctab = GetRecolourSprite(pal)->GetPaletteRemap();
 
 	do {
 		for (int i = 0; i != width; i++) *((uint8_t *)dst + i) = ctab[((uint8_t *)dst)[i]];

src/blitter/8bpp_optimized.cpp

@@ -85,7 +85,7 @@ void Blitter_8bppOptimized::Draw(Blitter::BlitterParams *bp, BlitterMode mode, Z
 			switch (mode) {
 				case BlitterMode::ColourRemap:
 				case BlitterMode::CrashRemap: {
-					const uint8_t *remap = bp->remap;
+					const uint8_t *remap = bp->remap->GetPaletteRemap();
 					do {
 						uint m = remap[*src];
 						if (m != 0) *dst = m;
@@ -101,7 +101,7 @@ void Blitter_8bppOptimized::Draw(Blitter::BlitterParams *bp, BlitterMode mode, Z
 
 				case BlitterMode::Transparent:
 				case BlitterMode::TransparentRemap: {
-					const uint8_t *remap = bp->remap;
+					const uint8_t *remap = bp->remap->GetPaletteRemap();
 					src += pixels;
 					do {
 						*dst = remap[*dst];

src/blitter/8bpp_simple.cpp

@@ -20,6 +20,7 @@ void Blitter_8bppSimple::Draw(Blitter::BlitterParams *bp, BlitterMode mode, Zoom
 {
 	const uint8_t *src, *src_line;
 	uint8_t *dst, *dst_line;
+	const uint8_t *remap = bp->remap->GetPaletteRemap();
 
 	/* Find where to start reading in the source sprite */
 	src_line = (const uint8_t *)bp->sprite + (bp->skip_top * bp->sprite_width + bp->skip_left) * ScaleByZoom(1, zoom);
@@ -38,12 +39,12 @@ void Blitter_8bppSimple::Draw(Blitter::BlitterParams *bp, BlitterMode mode, Zoom
 			switch (mode) {
 				case BlitterMode::ColourRemap:
 				case BlitterMode::CrashRemap:
-					colour = bp->remap[*src];
+					colour = remap[*src];
 					break;
 
 				case BlitterMode::Transparent:
 				case BlitterMode::TransparentRemap:
-					if (*src != 0) colour = bp->remap[*dst];
+					if (*src != 0) colour = remap[*dst];
 					break;
 
 				case BlitterMode::BlackRemap:

src/blitter/base.hpp

@@ -31,7 +31,7 @@ public:
 	/** Parameters related to blitting. */
 	struct BlitterParams {
 		const void *sprite; ///< Pointer to the sprite how ever the encoder stored it
-		const uint8_t *remap;  ///< XXX -- Temporary storage for remap array
+		const RecolourSprite *remap;  ///< Pointer to the RecolourSprite to use with remapping.
 
 		int skip_left;      ///< How much pixels of the source to skip on the left (based on zoom of dst)
 		int skip_top;       ///< How much pixels of the source to skip on the top (based on zoom of dst)

src/gfx.cpp

@@ -72,8 +72,8 @@ int _gui_scale_cfg;                         ///< GUI scale in config.
  * @ingroup dirty
  */
 static Rect _invalid_rect;
-static const uint8_t *_colour_remap_ptr;
+static const RecolourSprite *_colour_remap_ptr;
-static uint8_t _string_colourremap[3]; ///< Recoloursprite for stringdrawing. The grf loader ensures that #SpriteType::Font sprites only use colours 0 to 2.
+static RecolourSprite _string_colourremap; ///< RecolourSprite for string drawing.
 
 static const uint DIRTY_BLOCK_HEIGHT   = 8;
 static const uint DIRTY_BLOCK_WIDTH    = 64;
@@ -480,9 +480,9 @@ static void SetColourRemap(TextColour colour)
 	bool raw_colour = (colour & TC_IS_PALETTE_COLOUR) != 0;
 	colour &= ~(TC_NO_SHADE | TC_IS_PALETTE_COLOUR | TC_FORCED);
 
-	_string_colourremap[1] = raw_colour ? (uint8_t)colour : _string_colourmap[colour].p;
+	_string_colourremap.palette[1] = raw_colour ? (uint8_t)colour : _string_colourmap[colour].p;
-	_string_colourremap[2] = no_shade ? 0 : 1;
+	_string_colourremap.palette[2] = no_shade ? 0 : 1;
-	_colour_remap_ptr = _string_colourremap;
+	_colour_remap_ptr = &_string_colourremap;
 }
 
 /**
@@ -640,7 +640,7 @@ static int DrawLayoutLine(const ParagraphLayouter::Line &line, int y, int left,
 	}
 
 	if (underline) {
-		GfxFillRect(left, y + h, right, y + h + WidgetDimensions::scaled.bevel.top - 1, PixelColour{_string_colourremap[1]});
+		GfxFillRect(left, y + h, right, y + h + WidgetDimensions::scaled.bevel.top - 1, PixelColour{_string_colourremap.GetPaletteRemap()[1]});
 	}
 
 	return (align & SA_HOR_MASK) == SA_RIGHT ? left : right;
@@ -1006,13 +1006,13 @@ void DrawSpriteViewport(SpriteID img, PaletteID pal, int x, int y, const SubSpri
 	SpriteID real_sprite = GB(img, 0, SPRITE_WIDTH);
 	if (HasBit(img, PALETTE_MODIFIER_TRANSPARENT)) {
 		pal = GB(pal, 0, PALETTE_WIDTH);
-		_colour_remap_ptr = GetNonSprite(pal, SpriteType::Recolour) + 1;
+		_colour_remap_ptr = GetRecolourSprite(pal);
 		GfxMainBlitterViewport(GetSprite(real_sprite, SpriteType::Normal), x, y, pal == PALETTE_TO_TRANSPARENT ? BlitterMode::Transparent : BlitterMode::TransparentRemap, sub, real_sprite);
 	} else if (pal != PAL_NONE) {
 		if (HasBit(pal, PALETTE_TEXT_RECOLOUR)) {
 			SetColourRemap((TextColour)GB(pal, 0, PALETTE_WIDTH));
 		} else {
-			_colour_remap_ptr = GetNonSprite(GB(pal, 0, PALETTE_WIDTH), SpriteType::Recolour) + 1;
+			_colour_remap_ptr = GetRecolourSprite(GB(pal, 0, PALETTE_WIDTH));
 		}
 		GfxMainBlitterViewport(GetSprite(real_sprite, SpriteType::Normal), x, y, GetBlitterMode(pal), sub, real_sprite);
 	} else {
@@ -1034,13 +1034,13 @@ void DrawSprite(SpriteID img, PaletteID pal, int x, int y, const SubSprite *sub,
 	SpriteID real_sprite = GB(img, 0, SPRITE_WIDTH);
 	if (HasBit(img, PALETTE_MODIFIER_TRANSPARENT)) {
 		pal = GB(pal, 0, PALETTE_WIDTH);
-		_colour_remap_ptr = GetNonSprite(pal, SpriteType::Recolour) + 1;
+		_colour_remap_ptr = GetRecolourSprite(pal);
 		GfxMainBlitter(GetSprite(real_sprite, SpriteType::Normal), x, y, pal == PALETTE_TO_TRANSPARENT ? BlitterMode::Transparent : BlitterMode::TransparentRemap, sub, real_sprite, zoom);
 	} else if (pal != PAL_NONE) {
 		if (HasBit(pal, PALETTE_TEXT_RECOLOUR)) {
 			SetColourRemap((TextColour)GB(pal, 0, PALETTE_WIDTH));
 		} else {
-			_colour_remap_ptr = GetNonSprite(GB(pal, 0, PALETTE_WIDTH), SpriteType::Recolour) + 1;
+			_colour_remap_ptr = GetRecolourSprite(GB(pal, 0, PALETTE_WIDTH));
 		}
 		GfxMainBlitter(GetSprite(real_sprite, SpriteType::Normal), x, y, GetBlitterMode(pal), sub, real_sprite, zoom);
 	} else {

src/main_gui.cpp

@@ -553,10 +553,11 @@ void ShowSelectGameWindow();
 void SetupColoursAndInitialWindow()
 {
 	for (Colours i = COLOUR_BEGIN; i != COLOUR_END; i++) {
-		const uint8_t *b = GetNonSprite(GetColourPalette(i), SpriteType::Recolour) + 1;
+		const RecolourSprite *rs = GetRecolourSprite(GetColourPalette(i));
-		assert(b != nullptr);
+		assert(rs != nullptr);
+		const uint8_t *remap = rs->GetPaletteRemap();
 		for (ColourShade j = SHADE_BEGIN; j < SHADE_END; j++) {
-			SetColourGradient(i, j, PixelColour{b[0xC6 + j]});
+			SetColourGradient(i, j, PixelColour{remap[0xC6 + j]});
 		}
 	}
 

src/spritecache.cpp

@@ -406,6 +406,27 @@ static bool ResizeSprites(SpriteLoader::SpriteCollection &sprite, ZoomLevels spr
 	return  true;
 }
 
+/**
+ * Read recolour sprite data.
+ * @param file SpriteFile to read from.
+ * @param entries Number of entries to read.
+ */
+void RecolourSprite::Read(SpriteFile &file, size_t entries)
+{
+	assert(entries <= RecolourSprite::PALETTE_SIZE);
+	if (file.NeedsPaletteRemap()) {
+		/* Remapping from "Windows" to "DOS" requires a temporary buffer as entries are overwritten. */
+		std::array<uint8_t, RecolourSprite::PALETTE_SIZE> tmp{};
+		file.ReadBlock(tmp.data(), entries);
+
+		for (uint i = 0; i < entries; ++i) {
+			this->palette[i] = _palmap_w2d[tmp[_palmap_d2w[i]]];
+		}
+	} else {
+		file.ReadBlock(this->palette.data(), entries);
+	}
+}
+
 /**
  * Load a recolour sprite into memory.
  * @param file GRF we're reading from.
@@ -416,31 +437,22 @@ static bool ResizeSprites(SpriteLoader::SpriteCollection &sprite, ZoomLevels spr
  */
 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
-	 * GRFs which are the same as 257 byte recolour sprites, but with the last
-	 * 240 bytes zeroed.  */
-	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)];
 
-		/* Only a few recolour sprites are less than 257 bytes */
+	/* The first byte of the recolour sprite records the number of entries, with
-		if (num < RECOLOUR_SPRITE_SIZE) std::fill_n(dest_tmp, RECOLOUR_SPRITE_SIZE, 0);
+	 * the caveat that as this is a single byte 256 is recorded as 0. */
-		file.ReadBlock(dest_tmp, num);
+	uint entries = file.ReadByte();
+	if (entries == 0) entries = 256;
+	--num;
 
-		/* The data of index 0 is never used; "literal 00" according to the (New)GRF specs. */
+	if (entries > num) {
-		for (uint i = 1; i < RECOLOUR_SPRITE_SIZE; i++) {
+		Debug(grf, 1, "ReadRecolourSprite: Expected recolour sprite with {} entries but only {} present", entries, num);
-			dest[i] = _palmap_w2d[dest_tmp[_palmap_d2w[i - 1] + 1]];
+		entries = num;
-		}
-		delete[] dest_tmp;
-	} else {
-		file.ReadBlock(dest, num);
 	}
 
-	return dest;
+	RecolourSprite *rs = allocator.New<RecolourSprite>();
+	rs->Read(file, entries);
+	return rs;
 }
 
 /**
@@ -784,6 +796,10 @@ void IncreaseSpriteLRU()
 
 void SpriteCache::ClearSpriteData()
 {
+	if (this->ptr == nullptr) return;
+
+	if (this->type == SpriteType::Recolour) reinterpret_cast<RecolourSprite *>(this->ptr.get())->~RecolourSprite();
+
 	_spritecache_bytes_used -= this->length;
 	this->ptr.reset();
 }

src/spritecache.h

@@ -41,10 +41,9 @@ inline const Sprite *GetSprite(SpriteID sprite, SpriteType type)
 	return (Sprite*)GetRawSprite(sprite, type);
 }
 
-inline const uint8_t *GetNonSprite(SpriteID sprite, SpriteType type)
+inline const RecolourSprite *GetRecolourSprite(SpriteID sprite)
 {
-	assert(type == SpriteType::Recolour);
+	return static_cast<RecolourSprite *>(GetRawSprite(sprite, SpriteType::Recolour));
-	return (uint8_t*)GetRawSprite(sprite, type);
 }
 
 void GfxInitSpriteMem();

src/spritecache_type.h

@@ -11,6 +11,7 @@
 #define SPRITECACHE_TYPE_H
 
 #include "core/enum_type.hpp"
+#include "spriteloader/sprite_file_type.hpp"
 
 /** Data structure describing a sprite. */
 struct Sprite {
@@ -21,6 +22,17 @@ struct Sprite {
 	std::byte data[]; ///< Sprite data.
 };
 
+/** Data structure describing a palette remap. */
+class RecolourSprite {
+public:
+	static constexpr size_t PALETTE_SIZE = 256; ///< Number of entries in a recolour sprite.
+
+	void Read(SpriteFile &file, size_t entries);
+	inline const uint8_t *GetPaletteRemap() const { return this->palette.data(); }
+
+	std::array<uint8_t, PALETTE_SIZE> palette{}; ///< Palette index remap, mapping from one palette index to another.
+};
+
 enum class SpriteCacheCtrlFlag : uint8_t {
 	AllowZoomMin1xPal, ///< Allow use of sprite min zoom setting at 1x in palette mode.
 	AllowZoomMin1x32bpp, ///< Allow use of sprite min zoom setting at 1x in 32bpp mode.

src/spriteloader/spriteloader.hpp

@@ -117,6 +117,18 @@ public:
 		return static_cast<T *>(this->AllocatePtr(size));
 	}
 
+	/**
+	 * Allocate memory and construct an object in the sprite cache.
+	 * @tparam T Type to construct.
+	 * @tparam Targs The constructor parameter types.
+	 * @param args Parameters for the constructor.
+	 */
+	template <typename T, typename... Targs>
+	T *New(Targs &&... args)
+	{
+		return new (this->Allocate<T>(sizeof(T))) T(std::forward<Targs &&>(args)...);
+	}
+
 protected:
 	/**
 	 * Allocate memory for a sprite.

src/video/opengl.cpp

@@ -1290,7 +1290,7 @@ void OpenGLBackend::RenderOglSprite(const OpenGLSprite *gl_sprite, PaletteID pal
 			_glBindBuffer(GL_PIXEL_UNPACK_BUFFER, OpenGLSprite::pal_pbo);
 			_glPixelStorei(GL_UNPACK_ROW_LENGTH, 0);
 
-			_glBufferSubData(GL_PIXEL_UNPACK_BUFFER, 0, 256, GetNonSprite(GB(pal, 0, PALETTE_WIDTH), SpriteType::Recolour) + 1);
+			_glBufferSubData(GL_PIXEL_UNPACK_BUFFER, 0, 256, GetRecolourSprite(GB(pal, 0, PALETTE_WIDTH))->GetPaletteRemap());
 			_glTexSubImage1D(GL_TEXTURE_1D, 0, 0, 256, GL_RED, GL_UNSIGNED_BYTE, nullptr);
 
 			_glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);