From 90286ccbbcb2322fb2be88e98d72b9c869431425 Mon Sep 17 00:00:00 2001
From: Sylvie <35663410+Rangi42@users.noreply.github.com>
Date: Sun, 20 Oct 2024 13:51:39 -0400
Subject: [PATCH] Fix detection of tiles with too many colors (#1546)

---
 include/gfx/proto_palette.hpp |  8 ++----
 src/gfx/process.cpp           | 46 +++++++++++++++++------------------
 src/gfx/proto_palette.cpp     |  9 +++----
 3 files changed, 29 insertions(+), 34 deletions(-)

diff --git a/include/gfx/proto_palette.hpp b/include/gfx/proto_palette.hpp
index fc75e9bd8..88f5476e9 100644
--- a/include/gfx/proto_palette.hpp
+++ b/include/gfx/proto_palette.hpp
@@ -18,12 +18,8 @@ class ProtoPalette {
 	std::array<uint16_t, capacity> _colorIndices{UINT16_MAX, UINT16_MAX, UINT16_MAX, UINT16_MAX};
 
 public:
-	/*
-	 * Adds the specified color to the set, or **silently drops it** if the set is full.
-	 *
-	 * Returns whether the color was unique.
-	 */
-	bool add(uint16_t color);
+	// Adds the specified color to the set, or **silently drops it** if the set is full.
+	void add(uint16_t color);
 
 	enum ComparisonResult {
 		NEITHER,
diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp
index 604ac1c84..21db52e8f 100644
--- a/src/gfx/process.cpp
+++ b/src/gfx/process.cpp
@@ -1115,33 +1115,44 @@ void process() {
 	DefaultInitVec<AttrmapEntry> attrmap{};
 
 	for (auto tile : png.visitAsTiles()) {
-		ProtoPalette tileColors;
 		AttrmapEntry &attrs = attrmap.emplace_back();
-		uint8_t nbColorsInTile = 0;
 
+		// Count the unique non-transparent colors for packing
+		std::unordered_set<uint16_t> tileColors;
 		for (uint32_t y = 0; y < 8; ++y) {
 			for (uint32_t x = 0; x < 8; ++x) {
-				Rgba color = tile.pixel(x, y);
-				if (!color.isTransparent()) { // Do not count transparency in for packing
-					// Add the color to the proto-pal (if not full), and count it if it was unique.
-					if (tileColors.add(color.cgbColor())) {
-						++nbColorsInTile;
-					}
+				if (Rgba color = tile.pixel(x, y); !color.isTransparent()) {
+					tileColors.insert(color.cgbColor());
 				}
 			}
 		}
 
+		if (tileColors.size() > options.maxOpaqueColors()) {
+			fatal(
+			    "Tile at (%" PRIu32 ", %" PRIu32 ") has %zu colors, more than %" PRIu8 "!",
+			    tile.x,
+			    tile.y,
+			    tileColors.size(),
+			    options.maxOpaqueColors()
+			);
+		}
+
 		if (tileColors.empty()) {
 			// "Empty" proto-palettes screw with the packing process, so discard those
 			attrs.protoPaletteID = AttrmapEntry::transparent;
 			continue;
 		}
 
+		ProtoPalette protoPalette;
+		for (uint16_t cgbColor : tileColors) {
+			protoPalette.add(cgbColor);
+		}
+
 		// Insert the proto-palette, making sure to avoid overlaps
 		for (size_t n = 0; n < protoPalettes.size(); ++n) {
-			switch (tileColors.compare(protoPalettes[n])) {
+			switch (protoPalette.compare(protoPalettes[n])) {
 			case ProtoPalette::WE_BIGGER:
-				protoPalettes[n] = tileColors; // Override them
+				protoPalettes[n] = protoPalette; // Override them
 				// Remove any other proto-palettes that we encompass
 				// (Example [(0, 1), (0, 2)], inserting (0, 1, 2))
 				/*
@@ -1151,7 +1162,7 @@ void process() {
 				 * Investigation is necessary, especially if pathological cases are found.
 				 *
 				 * for (size_t i = protoPalettes.size(); --i != n;) {
-				 *     if (tileColors.compare(protoPalettes[i]) == ProtoPalette::WE_BIGGER) {
+				 *     if (protoPalette.compare(protoPalettes[i]) == ProtoPalette::WE_BIGGER) {
 				 *         protoPalettes.erase(protoPalettes.begin() + i);
 				 *     }
 				 * }
@@ -1168,17 +1179,6 @@ void process() {
 			}
 		}
 
-		if (nbColorsInTile > options.maxOpaqueColors()) {
-			fatal(
-			    "Tile at (%" PRIu32 ", %" PRIu32 ") has %" PRIu8 " opaque colors, more than %" PRIu8
-			    "!",
-			    tile.x,
-			    tile.y,
-			    nbColorsInTile,
-			    options.maxOpaqueColors()
-			);
-		}
-
 		attrs.protoPaletteID = protoPalettes.size();
 		if (protoPalettes.size() == AttrmapEntry::transparent) { // Check for overflow
 			fatal(
@@ -1186,7 +1186,7 @@ void process() {
 			    AttrmapEntry::transparent
 			);
 		}
-		protoPalettes.push_back(tileColors);
+		protoPalettes.push_back(protoPalette);
 continue_visiting_tiles:;
 	}
 
diff --git a/src/gfx/proto_palette.cpp b/src/gfx/proto_palette.cpp
index faa0bb5ca..be845bd01 100644
--- a/src/gfx/proto_palette.cpp
+++ b/src/gfx/proto_palette.cpp
@@ -6,7 +6,7 @@
 
 #include "helpers.hpp"
 
-bool ProtoPalette::add(uint16_t color) {
+void ProtoPalette::add(uint16_t color) {
 	size_t i = 0;
 
 	// Seek the first slot greater than the new color
@@ -16,12 +16,12 @@ bool ProtoPalette::add(uint16_t color) {
 		++i;
 		if (i == _colorIndices.size()) {
 			// We reached the end of the array without finding the color, so it's a new one.
-			return true;
+			return;
 		}
 	}
 	// If we found it, great! Nothing else to do.
 	if (_colorIndices[i] == color) {
-		return false;
+		return;
 	}
 
 	// Swap entries until the end
@@ -30,12 +30,11 @@ bool ProtoPalette::add(uint16_t color) {
 		++i;
 		if (i == _colorIndices.size()) {
 			// The set is full, but doesn't include the new color.
-			return true;
+			return;
 		}
 	}
 	// Write that last one into the new slot
 	_colorIndices[i] = color;
-	return true;
 }
 
 ProtoPalette::ComparisonResult ProtoPalette::compare(ProtoPalette const &other) const {