Fix rgbgfx -Z palette overgeneration on merged color sets

include/gfx/color_set.hpp

@@ -22,9 +22,9 @@ class ColorSet {
 	void add(uint16_t color);
 
 	enum ComparisonResult {
-		NEITHER,
-		WE_BIGGER,
-		THEY_BIGGER = -1,
+		INCOMPARABLE,    // the two sets are incomparable
+		SUBSET_OR_EQUAL, // this set is a subset of or identical to the other
+		STRICT_SUPERSET, // this set is a strict superset of the other
 	};
 	ComparisonResult compare(ColorSet const &other) const;
 

src/gfx/color_set.cpp

@@ -42,29 +42,44 @@ void ColorSet::add(uint16_t color) {
 }
 
 ColorSet::ComparisonResult ColorSet::compare(ColorSet const &other) const {
-	// This works because the sets are sorted numerically
+	// This algorithm works because the sets are sorted numerically
 	assume(std::is_sorted(RANGE(_colorIndices)));
 	assume(std::is_sorted(RANGE(other._colorIndices)));
 
-	auto ours = _colorIndices.begin(), theirs = other._colorIndices.begin();
-	bool weBigger = true, theyBigger = true;
+	auto self_item = begin(), other_item = other.begin();
+	auto const self_end = end(), other_end = other.end();
+	bool self_has_unique = false, other_has_unique = false;
 
-	while (ours != end() && theirs != other.end()) {
-		if (*ours == *theirs) {
-			++ours;
-			++theirs;
-		} else if (*ours < *theirs) {
-			++ours;
-			theyBigger = false;
-		} else { // *ours > *theirs
-			++theirs;
-			weBigger = false;
+	while (self_item != self_end && other_item != other_end) {
+		if (*self_item < *other_item) {
+			// *self_item is not in other, so self cannot be a strict subset of other
+			self_has_unique = true;
+			++self_item;
+		} else if (*self_item > *other_item) {
+			// *other_item is not in self, so self cannot be a strict superset of other
+			other_has_unique = true;
+			++other_item;
+		} else {
+			// *self_item == *other_item, so continue comparing
+			++self_item;
+			++other_item;
 		}
+
+		// Early return optimization: we already know self and other are incomparable
+		if (self_has_unique && other_has_unique) {
+			return INCOMPARABLE;
+		}
+	}
+
+	// Check if either color set has unique items remaining after one set has been fully iterated
+	if (self_item != self_end) {
+		self_has_unique = true;
+	}
+	if (other_item != other_end) {
+		other_has_unique = true;
 	}
-	weBigger &= theirs == other.end();
-	theyBigger &= ours == end();
 
-	return theyBigger ? THEY_BIGGER : (weBigger ? WE_BIGGER : NEITHER);
+	return self_has_unique ? other_has_unique ? INCOMPARABLE : STRICT_SUPERSET : SUBSET_OR_EQUAL;
 }
 
 size_t ColorSet::size() const {

src/gfx/process.cpp

@@ -1027,29 +1027,75 @@ void process() {
 		// Insert the color set, making sure to avoid overlaps
 		for (size_t n = 0; n < colorSets.size(); ++n) {
 			switch (colorSet.compare(colorSets[n])) {
-			case ColorSet::WE_BIGGER:
-				colorSets[n] = colorSet; // Override them
-				// Remove any other color sets that we encompass
-				// (Example [(0, 1), (0, 2)], inserting (0, 1, 2))
+			case ColorSet::STRICT_SUPERSET:
+				// Override the previous color set that this one is a strict superset of
+
+				if (checkVerbosity(VERB_DEBUG)) {
+					fprintf(stderr, "- tile (%" PRIu32 ", %" PRIu32 ") overrides color set #%zu: [", tile.x, tile.y, n);
+					for (uint16_t color : colorSets[n]) {
+						fprintf(stderr, "$%04x, ", color);
+					}
+					fputs("] becomes [", stderr);
+					for (uint16_t color : colorSet) {
+						fprintf(stderr, "$%04x, ", color);
+					}
+					fputs("]\n", stderr);
+				}
+
+				colorSets[n] = colorSet;
+				// Remove any other color sets that we are also a strict superset of
+				// (example: we have [(0, 1), (0, 2)] and are inserting (0, 1, 2))
+				for (size_t m = n + 1; m < colorSets.size();) {
+					if (colorSet.compare(colorSets[m]) != ColorSet::STRICT_SUPERSET) {
+						++m;
+						continue;
+					}
+
+					for (size_t i = 0; i + 1 < attrmap.size(); ++i) {
+						AttrmapEntry &entry = attrmap[i];
+						if (entry.colorSetID == AttrmapEntry::transparent
+						    || entry.colorSetID == AttrmapEntry::background) {
+							continue;
+						}
+						if (entry.colorSetID == m) {
+							entry.colorSetID = n;
+						} else if (entry.colorSetID > m) {
+							--entry.colorSetID;
+						}
+					}
+
+					colorSets.erase(colorSets.begin() + m);
+				}
 				[[fallthrough]];
 
-			case ColorSet::THEY_BIGGER:
-				// Do nothing, they already contain us
+			case ColorSet::SUBSET_OR_EQUAL:
+				// Use the previous color set that this one is a subset or duplicate of
 				attrs.colorSetID = n;
 				goto continue_visiting_tiles; // Can't `continue` from within a nested loop
 
-			case ColorSet::NEITHER:
-				break; // Keep going
+			case ColorSet::INCOMPARABLE:
+				// This color set is incomparable so far, so keep going
+				break;
 			}
 		}
 
+		// This color set is incomparable with all previous ones, so add it as a new one
 		attrs.colorSetID = colorSets.size();
 		if (colorSets.size() == AttrmapEntry::background) { // Check for overflow
 			fatal(
 			    "Reached %zu color sets... sorry, this image is too much for me to handle :(",
 			    AttrmapEntry::transparent
 			);
 		}
+
+		if (checkVerbosity(VERB_DEBUG)) {
+			fprintf(stderr, "- tile (%" PRIu32 ", %" PRIu32 ") adds color set #%zu: [", tile.x, tile.y, colorSets.size());
+			for (uint16_t color : colorSet) {
+				fprintf(stderr, "$%04x, ", color);
+			}
+			fputs("]\n", stderr);
+		}
+
 		colorSets.push_back(colorSet);
 continue_visiting_tiles:;
 	}

test/gfx/issue1887_gloom_z.flags

@@ -0,0 +1 @@
+-Z

test/gfx/issue1887_gloom_z.out.pal

@@ -0,0 +1 @@
+"XH9g‘bpIH9gS"g