Implement -c #none (#1301)

Also adds a test case for round-tripping `-r` with `-c #none`.

include/gfx/main.hpp

@@ -5,6 +5,7 @@
 
 #include <array>
 #include <limits.h>
+#include <optional>
 #include <stdint.h>
 #include <string>
 #include <utility>
@@ -31,7 +32,7 @@ struct Options {
 		EXPLICIT,
 		EMBEDDED,
 	} palSpecType = NO_SPEC; // -c
-	std::vector<std::array<Rgba, 4>> palSpec{};
+	std::vector<std::array<std::optional<Rgba>, 4>> palSpec{};
 	uint8_t bitDepth = 2; // -d
 	struct {
 		uint16_t left;

man/rgbgfx.1

@@ -125,11 +125,14 @@ begins with a hash character
 .Ql # ,
 it is treated as an inline palette specification.
 It should contain a comma-separated list of hexadecimal colors, each beginning with a hash.
-Colors in are accepted either as
+Colors are accepted either as
 .Ql #rgb
 or
 .Ql #rrggbb
 format.
+To leave one or more gaps in the palette,
+.Ql #none
+can be used instead of any color.
 Palettes must be separated by a colon or semicolon (the latter may require quoting to avoid special handling by the shell), and spaces are allowed around colons, semicolons and commas; trailing commas and semicolons are allowed.
 See
 .Sx EXAMPLES

src/gfx/main.cpp

@@ -768,9 +768,16 @@ int main(int argc, char *argv[]) {
 		}());
 		if (options.palSpecType == Options::EXPLICIT) {
 			fputs("\t[\n", stderr);
-			for (std::array<Rgba, 4> const &pal : options.palSpec) {
-				fprintf(stderr, "\t\t#%06x, #%06x, #%06x, #%06x,\n", pal[0].toCSS() >> 8,
-				        pal[1].toCSS() >> 8, pal[2].toCSS() >> 8, pal[3].toCSS() >> 8);
+			for (const auto &pal : options.palSpec) {
+				fputs("\t\t", stderr);
+				for (auto &color : pal) {
+					if (color) {
+						fprintf(stderr, "#%06x, ", color->toCSS() >> 8);
+					} else {
+						fputs("#none, ", stderr);
+					}
+				}
+				fputc('\n', stderr);
 			}
 			fputs("\t]\n", stderr);
 		}
@@ -847,18 +854,27 @@ auto Palette::begin() -> decltype(colors)::iterator {
 	// Skip the first slot if reserved for transparency
 	return colors.begin() + options.hasTransparentPixels;
 }
+
 auto Palette::end() -> decltype(colors)::iterator {
-	return std::find(begin(), colors.end(), UINT16_MAX);
+	// Return an iterator pointing past the last non-empty element.
+	// Since the palette may contain gaps, we must scan from the end.
+	return std::find_if(colors.rbegin(), colors.rend(),
+	                    [](uint16_t c) { return c != UINT16_MAX; })
+	       .base();
 }
 
 auto Palette::begin() const -> decltype(colors)::const_iterator {
 	// Skip the first slot if reserved for transparency
 	return colors.begin() + options.hasTransparentPixels;
 }
+
 auto Palette::end() const -> decltype(colors)::const_iterator {
-	return std::find(begin(), colors.end(), UINT16_MAX);
+	// Same as the non-const end().
+	return std::find_if(colors.rbegin(), colors.rend(),
+	                    [](uint16_t c) { return c != UINT16_MAX; })
+	       .base();
 }
 
 uint8_t Palette::size() const {
-	return indexOf(UINT16_MAX);
+	return end() - colors.begin();
 }

src/gfx/pal_spec.cpp

@@ -54,7 +54,8 @@ static void skipWhitespace(Str const &str, typename Str::size_type &pos) {
 }
 
 void parseInlinePalSpec(char const * const rawArg) {
-	// List of #rrggbb/#rgb colors, comma-separated, palettes are separated by colons
+	// List of #rrggbb/#rgb colors (or #none); comma-separated.
+	// Palettes are separated by colons.
 
 	std::string_view arg(rawArg);
 	using size_type = decltype(arg)::size_type;
@@ -87,7 +88,12 @@ void parseInlinePalSpec(char const * const rawArg) {
 	for (;;) {
 		++n; // Ignore the '#' (checked either by caller or previous loop iteration)
 
-		Rgba &color = options.palSpec.back()[nbColors];
+		std::optional<Rgba> &color = options.palSpec.back()[nbColors];
+		// Check for #none first.
+		if (arg.compare(n, 4, "none"sv) == 0 || arg.compare(n, 4, "NONE"sv) == 0) {
+			color = {};
+			n += 4;
+		} else {
 			auto pos = std::min(arg.find_first_not_of("0123456789ABCDEFabcdef"sv, n), arg.length());
 			switch (pos - n) {
 			case 3:
@@ -106,6 +112,7 @@ void parseInlinePalSpec(char const * const rawArg) {
 				return;
 			}
 			n = pos;
+		}
 
 		// Skip whitespace, if any
 		skipWhitespace(arg, n);
@@ -442,7 +449,7 @@ static void parseACTFile(std::filebuf &file) {
 	char const *ptr = buf.data();
 	size_t colorIdx = 0;
 	for (uint16_t i = 0; i < nbColors; ++i) {
-		Rgba &color = options.palSpec.back()[colorIdx];
+		std::optional<Rgba> &color = options.palSpec.back()[colorIdx];
 		color = Rgba(ptr[0], ptr[1], ptr[2], 0xFF);
 
 		ptr += 3;
@@ -498,7 +505,7 @@ static void parseACOFile(std::filebuf &file) {
 			options.palSpec.emplace_back();
 		}
 
-		Rgba &color = options.palSpec.back()[i % options.nbColorsPerPal];
+		std::optional<Rgba> &color = options.palSpec.back()[i % options.nbColorsPerPal];
 		uint16_t colorType = readBE<uint16_t>(buf);
 		switch (colorType) {
 		case 0: // RGB

src/gfx/process.cpp

@@ -576,8 +576,11 @@ static std::tuple<DefaultInitVec<size_t>, std::vector<Palette>>
 	// Convert the palette spec to actual palettes
 	std::vector<Palette> palettes(options.palSpec.size());
 	for (auto [spec, pal] : zip(options.palSpec, palettes)) {
-		for (size_t i = 0; i < options.nbColorsPerPal && spec[i].isOpaque(); ++i) {
-			pal[i] = spec[i].cgbColor();
+		for (size_t i = 0; i < options.nbColorsPerPal && (!spec[i] || spec[i]->isOpaque()); ++i) {
+			// If the spec has a gap, there's no need to copy anything.
+			if (spec[i]) {
+				pal[i] = spec[i]->cgbColor();
+			}
 		}
 	}
 

src/gfx/reverse.cpp

@@ -137,7 +137,7 @@ void reverse() {
 
 	// TODO: `-U` to configure tile size beyond 8x8px ("deduplication units")
 
-	std::vector<std::array<Rgba, 4>> palettes{
+	std::vector<std::array<std::optional<Rgba>, 4>> palettes{
 	    {Rgba(0xFFFFFFFF), Rgba(0xAAAAAAFF), Rgba(0x555555FF), Rgba(0x000000FF)}
 	};
 	// If a palette file is used as input, it overrides the default colors.
@@ -313,7 +313,7 @@ void reverse() {
 				uint8_t *ptr = &rowPtrs[y][tx * 8 * SIZEOF_PIXEL];
 				for (uint8_t x = 0; x < 8; ++x) {
 					uint8_t bit0 = bitplane0 & 0x80, bit1 = bitplane1 & 0x80;
-					Rgba const &pixel = palette[bit0 >> 7 | bit1 >> 6];
+					Rgba const &pixel = *palette[bit0 >> 7 | bit1 >> 6];
 					*ptr++ = pixel.red;
 					*ptr++ = pixel.green;
 					*ptr++ = pixel.blue;

test/gfx/none_comma_ommission.err

@@ -0,0 +1,4 @@
+error: Unexpected character, expected ',', ';', or end of argument
+In inline palette spec: #000,#none#fff
+                                  ^
+Conversion aborted after 1 error

test/gfx/none_comma_ommission.flags

@@ -0,0 +1 @@
+-c #000,#none#fff

test/gfx/rgbgfx_test.cpp

@@ -281,6 +281,21 @@ public:
 };
 
 static char *execProg(char const *name, char * const *argv) {
+	auto formatArgv = [&argv] {
+		// This is `static` so that the returned `buf.c_str()` will live long enough
+		// for `fatal()` to use it below.
+		static std::string buf;
+
+		buf.clear();
+		for (char * const *arg = argv; *arg != nullptr; ++arg) {
+			buf.push_back('"');
+			buf.append(*arg);
+			buf.append("\", ");
+		}
+		buf.resize(buf.length() - 2);
+		return buf.c_str();
+	};
+
 #if !defined(_MSC_VER) && !defined(__MINGW32__)
 	pid_t pid;
 	int err = posix_spawn(&pid, argv[0], nullptr, nullptr, argv, nullptr);
@@ -293,10 +308,10 @@ static char *execProg(char const *name, char * const *argv) {
 		fatal("Error waiting for %s: %s", name, strerror(errno));
 	} else if (info.si_code != CLD_EXITED) {
 		assert(info.si_code == CLD_KILLED || info.si_code == CLD_DUMPED);
-		fatal("%s was terminated by signal %s%s", name, strsignal(info.si_status),
-		      info.si_code == CLD_DUMPED ? " (core dumped)" : "");
+		fatal("%s was terminated by signal %s%s\n\tThe command was: [%s]", name, strsignal(info.si_status),
+		      info.si_code == CLD_DUMPED ? " (core dumped)" : "", formatArgv());
 	} else if (info.si_status != 0) {
-		fatal("%s returned with status %d", name, info.si_status);
+		fatal("%s returned with status %d\n\tThe command was: [%s]", name, info.si_status, formatArgv());
 	}
 
 #else // defined(_MSC_VER) || defined(__MINGW32__)
@@ -362,7 +377,7 @@ static char *execProg(char const *name, char * const *argv) {
 	CloseHandle(child.hThread);
 
 	if (status != 0) {
-		fatal("%s returned with status %ld", name, status);
+		fatal("%s returned with status %ld\n\tThe command was: [%s]", name, status, formatArgv());
 	}
 #endif
 

test/gfx/test.sh

@@ -39,8 +39,15 @@ for f in *.bin; do
 	done
 done
 
+# Test round-tripping '-r' with '-c #none'
+reverse_cmd="$RGBGFX -c#none,#fff,#000 -o none_round_trip.2bpp -r 1 out.png"
+reconvert_cmd="$RGBGFX -c#none,#fff,#000 -o result.2bpp out.png"
+compare_cmd="cmp none_round_trip.2bpp result.2bpp"
+new_test "$reverse_cmd && $reconvert_cmd && $compare_cmd"
+test || fail $?
+
 # Remove temporaries (also ignored by Git) created by the above tests
-rm -f out*.png result.png
+rm -f out*.png result.png result.2bpp
 
 for f in *.png; do
 	flags="$([[ -e "${f%.png}.flags" ]] && echo "@${f%.png}.flags")"