From: =?utf-8?q?Lars_M=C3=BCller?=
 <34514239+appgurueu@users.noreply.github.com>
Date: Tue, 19 Nov 2024 13:37:05 +0100
Subject: Fix rendering regression with TGA type 1 files with BGRA8 color
 (#15402)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Origin: backport: https://github.com/minetest/minetest/commit/15e8f9e6a06c3ad87bae3ffaf4ec58f551612ff6
Forwarded: not-needed
Applied-Upstream: 5.11.0

TGA uses BGR(A)8, stored in memory in that order. Irrlicht typically expects 0xAARRGGBB, which depends on endianness.
(This means that on little endian, no [B][G][R][A] -> 0xAARRGGBB conversion needs to be done, but Irrlicht was swapping the bytes.)

This makes both conversion functions consistently convert from [B][G][R]([A]) to 0xAARRGGBB (SColor), documents them properly and moves them to CImageLoaderTGA.cpp
so no poor soul shall be fooled by them in the near future.

---------

Co-authored-by: Ælla Chiana Moskopp <erle@dieweltistgarnichtso.net>
---
 irr/src/CColorConverter.cpp | 38 +++++++++-----------------------------
 irr/src/CColorConverter.h   |  2 --
 irr/src/CImage.cpp          |  2 ++
 irr/src/CImageLoaderTGA.cpp | 23 +++++++++++++++++++++--
 4 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/irr/src/CColorConverter.cpp b/irr/src/CColorConverter.cpp
index 96b22c5..d3a5cf6 100644
--- a/irr/src/CColorConverter.cpp
+++ b/irr/src/CColorConverter.cpp
@@ -7,6 +7,15 @@
 #include "os.h"
 #include "irrString.h"
 
+// Warning: The naming of Irrlicht color formats
+// is not consistent regarding actual component order in memory.
+// E.g. in CImage, ECF_R8G8B8 is handled per-byte and stored as [R][G][B] in memory
+// while ECF_A8R8G8B8 is handled as an u32 0xAARRGGBB so [B][G][R][A] (little endian) in memory.
+// The conversions suffer from the same inconsistencies, e.g.
+// convert_R8G8B8toA8R8G8B8 converts [R][G][B] into 0xFFRRGGBB = [B][G][R][FF] (little endian);
+// convert_A1R5G5B5toR8G8B8 converts 0bARRRRRGGGGGBBBBB into [R][G][B].
+// This also means many conversions may be broken on big endian.
+
 namespace irr
 {
 namespace video
@@ -480,19 +489,6 @@ void CColorConverter::convert_R8G8B8toA1R5G5B5(const void *sP, s32 sN, void *dP)
 	}
 }
 
-void CColorConverter::convert_B8G8R8toA8R8G8B8(const void *sP, s32 sN, void *dP)
-{
-	u8 *sB = (u8 *)sP;
-	u32 *dB = (u32 *)dP;
-
-	for (s32 x = 0; x < sN; ++x) {
-		*dB = 0xff000000 | (sB[2] << 16) | (sB[1] << 8) | sB[0];
-
-		sB += 3;
-		++dB;
-	}
-}
-
 void CColorConverter::convert_A8R8G8B8toR8G8B8A8(const void *sP, s32 sN, void *dP)
 {
 	const u32 *sB = (const u32 *)sP;
@@ -515,22 +511,6 @@ void CColorConverter::convert_A8R8G8B8toA8B8G8R8(const void *sP, s32 sN, void *d
 	}
 }
 
-void CColorConverter::convert_B8G8R8A8toA8R8G8B8(const void *sP, s32 sN, void *dP)
-{
-	u8 *sB = (u8 *)sP;
-	u8 *dB = (u8 *)dP;
-
-	for (s32 x = 0; x < sN; ++x) {
-		dB[0] = sB[3];
-		dB[1] = sB[2];
-		dB[2] = sB[1];
-		dB[3] = sB[0];
-
-		sB += 4;
-		dB += 4;
-	}
-}
-
 void CColorConverter::convert_R8G8B8toB8G8R8(const void *sP, s32 sN, void *dP)
 {
 	u8 *sB = (u8 *)sP;
diff --git a/irr/src/CColorConverter.h b/irr/src/CColorConverter.h
index 7f16282..0bffa26 100644
--- a/irr/src/CColorConverter.h
+++ b/irr/src/CColorConverter.h
@@ -74,8 +74,6 @@ class CColorConverter
 	static void convert_R8G8B8toA1R5G5B5(const void *sP, s32 sN, void *dP);
 	static void convert_R8G8B8toB8G8R8(const void *sP, s32 sN, void *dP);
 	static void convert_R8G8B8toR5G6B5(const void *sP, s32 sN, void *dP);
-	static void convert_B8G8R8toA8R8G8B8(const void *sP, s32 sN, void *dP);
-	static void convert_B8G8R8A8toA8R8G8B8(const void *sP, s32 sN, void *dP);
 	static void convert_A8R8G8B8toR8G8B8A8(const void *sP, s32 sN, void *dP);
 	static void convert_A8R8G8B8toA8B8G8R8(const void *sP, s32 sN, void *dP);
 
diff --git a/irr/src/CImage.cpp b/irr/src/CImage.cpp
index 2095901..0c6d705 100644
--- a/irr/src/CImage.cpp
+++ b/irr/src/CImage.cpp
@@ -95,6 +95,8 @@ SColor CImage::getPixel(u32 x, u32 y) const
 	case ECF_A8R8G8B8:
 		return ((u32 *)Data)[y * Size.Width + x];
 	case ECF_R8G8B8: {
+		// FIXME this interprets the memory as [R][G][B], whereas SColor is stored as
+		// 0xAARRGGBB, meaning it is lies in memory as [B][G][R][A] on a little endian machine.
 		u8 *p = Data + (y * 3) * Size.Width + (x * 3);
 		return SColor(255, p[0], p[1], p[2]);
 	}
diff --git a/irr/src/CImageLoaderTGA.cpp b/irr/src/CImageLoaderTGA.cpp
index 274b155..75b0b56 100644
--- a/irr/src/CImageLoaderTGA.cpp
+++ b/irr/src/CImageLoaderTGA.cpp
@@ -93,6 +93,25 @@ bool CImageLoaderTGA::isALoadableFileFormat(io::IReadFile *file) const
 	return (!strcmp(footer.Signature, "TRUEVISION-XFILE.")); // very old tgas are refused.
 }
 
+/// Converts *byte order* BGR to *endianness order* ARGB (SColor "=" u32)
+static void convert_BGR8_to_SColor(const u8 *src, u32 n, u32 *dst)
+{
+	for (u32 i = 0; i < n; ++i) {
+		const u8 *bgr = &src[3 * i];
+		dst[i] = 0xff000000 | (bgr[2] << 16) | (bgr[1] << 8) | bgr[0];
+	}
+}
+
+/// Converts *byte order* BGRA to *endianness order* ARGB (SColor "=" u32)
+/// Note: This just copies from src to dst on little endian.
+static void convert_BGRA8_to_SColor(const u8 *src, u32 n, u32 *dst)
+{
+	for (u32 i = 0; i < n; ++i) {
+		const u8 *bgra = &src[4 * i];
+		dst[i] = (bgra[3] << 24) | (bgra[2] << 16) | (bgra[1] << 8) | bgra[0];
+	}
+}
+
 //! creates a surface from the file
 IImage *CImageLoaderTGA::loadImage(io::IReadFile *file) const
 {
@@ -139,10 +158,10 @@ IImage *CImageLoaderTGA::loadImage(io::IReadFile *file) const
 			CColorConverter::convert_A1R5G5B5toA8R8G8B8(colorMap, header.ColorMapLength, palette);
 			break;
 		case 24:
-			CColorConverter::convert_B8G8R8toA8R8G8B8(colorMap, header.ColorMapLength, palette);
+			convert_BGR8_to_SColor(colorMap, header.ColorMapLength, palette);
 			break;
 		case 32:
-			CColorConverter::convert_B8G8R8A8toA8R8G8B8(colorMap, header.ColorMapLength, palette);
+			convert_BGRA8_to_SColor(colorMap, header.ColorMapLength, palette);
 			break;
 		}
 		delete[] colorMap;