Fix inconsistent handling of transparent colors in the middle of palettes

include/gfx/pal_spec.hpp

@@ -4,8 +4,12 @@
 #define RGBDS_GFX_PAL_SPEC_HPP
 
 #include <stdint.h>
+#include <string>
+
+#include "gfx/png.hpp"
 
 void parseInlinePalSpec(char const * const rawArg);
+void parseEmbeddedPalSpec(Png const &png);
 void parseExternalPalSpec(char const *arg);
 void parseDmgPalSpec(char const * const rawArg);
 void parseBackgroundPalSpec(char const *arg);

include/gfx/process.hpp

@@ -3,7 +3,9 @@
 #ifndef RGBDS_GFX_PROCESS_HPP
 #define RGBDS_GFX_PROCESS_HPP
 
+#include "gfx/png.hpp"
+
 void processPalettes();
-void process();
+void process(Png &&png);
 
 #endif // RGBDS_GFX_PROCESS_HPP

src/gfx/main.cpp

@@ -5,6 +5,7 @@
 #include <inttypes.h>
 #include <ios>
 #include <optional>
+#include <png.h>
 #include <stdarg.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -479,6 +480,7 @@ static void parseArg(int ch, char *arg) {
 // LCOV_EXCL_START
 static void verboseOutputConfig() {
 	usage.printVersion(true);
+	fprintf(stderr, "Using libpng %s\n", png_get_libpng_ver(nullptr));
 
 	printVVVVVVerbosity();
 
@@ -524,12 +526,18 @@ static void verboseOutputConfig() {
 		break;
 	case Options::INLINE:
 	case Options::EXTERNAL:
-		fputs("\tExplicit palette spec\n\t[\n", stderr);
+	case Options::EMBEDDED:
+		if (options.palSpecType == Options::EMBEDDED) {
+			fputs("\tEmbedded palette spec from PNG indexed PLTE chunk\n", stderr);
+		} else {
+			fputs("\tExplicit palette spec\n", stderr);
+		}
+		fputs("\t[\n", stderr);
 		for (auto const &pal : options.palSpec) {
 			fputs("\t\t", stderr);
 			for (auto const &color : pal) {
 				if (color.has_value()) {
-					fprintf(stderr, "#%06x, ", color->toCSS() >> 8);
+					fprintf(stderr, "#%08x, ", color->toCSS());
 				} else {
 					fputs("#none, ", stderr);
 				}
@@ -538,9 +546,6 @@ static void verboseOutputConfig() {
 		}
 		fputs("\t]\n", stderr);
 		break;
-	case Options::EMBEDDED:
-		fputs("\tEmbedded palette spec from PNG indexed PLTE chunk\n", stderr);
-		break;
 	case Options::DMG:
 		fprintf(stderr, "\tDMG palette spec $%02" PRIx8 "\n", options.palSpecDmg);
 		break;
@@ -671,13 +676,32 @@ int main(int argc, char *argv[]) {
 	autoOutPath(localOptions.autoPalettes, options.palettes, ".pal");
 	autoOutPath(localOptions.autoPalmap, options.palmap, ".palmap");
 
+	// Read the input image now since it may be needed for parsing an embedded palette spec
+	bool useInputImage = !options.input.empty() && !localOptions.reverse;
+	Png png;
+	if (useInputImage) {
+		if (File image;
+		    image.open(options.input, std::ios_base::in | std::ios_base::binary) == nullptr) {
+			fatal(
+			    "Failed to open PNG image (\"%s\"): %s", image.c_str(options.input), strerror(errno)
+			);
+		} else {
+			png = Png(image.c_str(options.input), *image);
+		}
+	}
+
 	// Execute deferred pal spec parsing, now that all other params are known.
 	// Do not parse pal specs if `options.nbColorsPerPal` is invalid.
 	if (options.nbColorsPerPal > 0 && options.nbColorsPerPal <= 4) {
 		switch (options.palSpecType) {
 		case Options::NO_SPEC:
+			assume(!localOptions.palSpec);
+			break;
 		case Options::EMBEDDED:
 			assume(!localOptions.palSpec);
+			if (useInputImage) {
+				parseEmbeddedPalSpec(png);
+			}
 			break;
 		case Options::INLINE:
 			assume(localOptions.palSpec);
@@ -703,7 +727,7 @@ int main(int argc, char *argv[]) {
 		if (localOptions.reverse) {
 			reverse();
 		} else {
-			process();
+			process(std::move(png));
 		}
 	} else if (!options.palettes.empty() && options.hasExplicitPalSpec() && !localOptions.reverse) {
 		processPalettes();

src/gfx/pal_spec.cpp

@@ -20,6 +20,7 @@
 #include <vector>
 
 #include "diagnostics.hpp"
+#include "file.hpp"
 #include "helpers.hpp"
 #include "platform.hpp"
 #include "util.hpp" // UpperMap, parseDigit
@@ -174,6 +175,28 @@ void parseInlinePalSpec(char const * const rawArg) {
 	}
 }
 
+void parseEmbeddedPalSpec(Png const &png) {
+	// Generate a palette spec from one palette's length of colors in the embedded palette
+	if (png.palette.empty()) {
+		error("\"-c embedded\" was given, but the PNG does not have an embedded palette");
+		return;
+	}
+
+	// Ignore extraneous colors if they are unused
+	size_t nbColors = png.palette.size();
+	if (nbColors > options.maxOpaqueColors()) {
+		nbColors = options.maxOpaqueColors();
+	}
+
+	// Fill in the palette spec
+	options.palSpec.clear();
+	auto &palette = options.palSpec.emplace_back();
+	assume(nbColors <= palette.size());
+	for (size_t i = 0; i < nbColors; ++i) {
+		palette[i] = png.palette[i];
+	}
+}
+
 // Appends the first line read from `file` to the end of the provided `buffer`.
 // Returns true if a line was read.
 [[nodiscard]]

src/gfx/process.cpp

@@ -8,7 +8,6 @@
 #include <inttypes.h>
 #include <ios>
 #include <optional>
-#include <png.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
@@ -77,8 +76,8 @@ public:
 };
 
 struct Image {
-	Png png{};
-	ImagePalette colors{};
+	Png png;
+	ImagePalette colors;
 
 	Rgba &pixel(uint32_t x, uint32_t y) { return png.pixels[y * png.width + x]; }
 	Rgba const &pixel(uint32_t x, uint32_t y) const { return png.pixels[y * png.width + x]; }
@@ -127,14 +126,7 @@ struct Image {
 		return {GrayscaleResult::GRAY_OK, std::nullopt};
 	}
 
-	explicit Image(std::string const &path) {
-		File input;
-		if (input.open(path, std::ios_base::in | std::ios_base::binary) == nullptr) {
-			fatal("Failed to open input image (\"%s\"): %s", input.c_str(path), strerror(errno));
-		}
-
-		png = Png(input.c_str(path), *input);
-
+	explicit Image(Png &&png_) : png(std::move(png_)), colors() {
 		// Validate input slice
 		if (options.inputSlice.width == 0 && png.width % 8 != 0) {
 			fatal("Image width (%" PRIu32 " pixels) is not a multiple of 8", png.width);
@@ -332,28 +324,6 @@ struct AttrmapEntry {
 	}
 };
 
-static void generatePalSpec(Image const &image) {
-	// Generate a palette spec from the first few colors in the embedded palette
-	std::vector<Rgba> const &embPal = image.png.palette;
-	if (embPal.empty()) {
-		fatal("\"-c embedded\" was given, but the PNG does not have an embedded palette");
-	}
-
-	// Ignore extraneous colors if they are unused
-	size_t nbColors = embPal.size();
-	if (nbColors > options.maxOpaqueColors()) {
-		nbColors = options.maxOpaqueColors();
-	}
-
-	// Fill in the palette spec
-	options.palSpec.clear();
-	auto &palette = options.palSpec.emplace_back();
-	assume(nbColors <= palette.size());
-	for (size_t i = 0; i < nbColors; ++i) {
-		palette[i] = embPal[i];
-	}
-}
-
 static std::pair<std::vector<size_t>, std::vector<Palette>>
     generatePalettes(std::vector<ColorSet> const &colorSets, Image const &image) {
 	// Run a "pagination" problem solver
@@ -450,7 +420,7 @@ static std::pair<std::vector<size_t>, std::vector<Palette>>
 			if (i + skipFirst >= options.nbColorsPerPal) {
 				error(
 				    "Each palette spec can only contain up to %" PRIu8
-				    " color%s plus the implict transparent color",
+				    " color%s plus the implicit transparent color",
 				    options.nbColorsPerPal - 1,
 				    options.nbColorsPerPal - 1 == 1 ? "" : "s"
 				);
@@ -992,8 +962,6 @@ static void
 }
 
 void processPalettes() {
-	verbosePrint(VERB_CONFIG, "Using libpng %s\n", png_get_libpng_ver(nullptr));
-
 	std::vector<ColorSet> colorSets;
 	std::vector<Palette> palettes;
 	std::tie(std::ignore, palettes) = makePalsAsSpecified(colorSets);
@@ -1001,11 +969,9 @@ void processPalettes() {
 	outputPalettes(palettes);
 }
 
-void process() {
-	verbosePrint(VERB_CONFIG, "Using libpng %s\n", png_get_libpng_ver(nullptr));
-
+void process(Png &&png) {
 	verbosePrint(VERB_NOTICE, "Reading tiles...\n");
-	Image image(options.input); // This also sets `hasTransparentPixels` as a side effect
+	Image image(std::move(png)); // This also sets `hasTransparentPixels` as a side effect
 
 	// LCOV_EXCL_START
 	verboseDo(VERB_INFO, [&]() {
@@ -1195,9 +1161,6 @@ continue_visiting_tiles:;
 		fatal("Image does not contain any colors");
 	}
 
-	if (options.palSpecType == Options::EMBEDDED) {
-		generatePalSpec(image);
-	}
 	auto [mappings, palettes] =
 	    options.palSpecType == Options::NO_SPEC || options.palSpecType == Options::DMG
 	        ? generatePalettes(colorSets, image)

src/gfx/reverse.cpp

@@ -107,8 +107,6 @@ static void printPalette(std::array<std::optional<Rgba>, 4> const &palette) {
 }
 
 void reverse() {
-	verbosePrint(VERB_CONFIG, "Using libpng %s\n", png_get_libpng_ver(nullptr));
-
 	// Check for weird flag combinations
 
 	if (options.output.empty()) {

test/gfx/alpha_embedded.err

@@ -1,4 +0,0 @@
-error: Failed to fit tile colors [GB:07,09,05; GB:12,14,05; GB:20,21,06] in specified palettes
-note: The following palette was specified:
-      - [GB:20,21,06; GB:12,14,05]
-Conversion aborted after 1 error

test/gfx/middle_transparent_embedded.err

@@ -0,0 +1,2 @@
+error: Each palette spec can only contain up to 3 colors plus the implict transparent color
+Conversion aborted after 1 error

test/gfx/middle_transparent_embedded.flags

@@ -0,0 +1 @@
+-c embedded

test/gfx/middle_transparent_gbc_spec.err

@@ -0,0 +1,2 @@
+error: Each palette spec can only contain up to 3 colors plus the implict transparent color
+Conversion aborted after 1 error

test/gfx/middle_transparent_gbc_spec.flags

@@ -0,0 +1 @@
+-c gbc:middle_transparent_gbc_spec.pal

test/gfx/middle_transparent_png_spec.err

@@ -0,0 +1,2 @@
+error: Each palette spec can only contain up to 3 colors plus the implict transparent color
+Conversion aborted after 1 error

test/gfx/middle_transparent_png_spec.flags

@@ -0,0 +1 @@
+-c png:middle_transparent_png_spec.pal.png