Simplify some C++ abstractions (#1518)

* Remove namespaces
* Prefer `bool operator==`, not `friend auto operator==`
* Prefer not to use `using`
* Use a `constexpr` function instead of a template for `flipTable`

include/gfx/main.hpp

@@ -107,20 +107,18 @@ struct Palette {
 	uint8_t size() const;
 };
 
-namespace detail {
-template<typename T, T... i>
-static constexpr auto flipTable(std::integer_sequence<T, i...>) {
-	return std::array{[](uint8_t byte) {
+// Flipping tends to happen fairly often, so take a bite out of dcache to speed it up
+static constexpr auto flipTable = ([]() constexpr {
+	std::array<uint16_t, 256> table{};
+	for (uint16_t i = 0; i < table.size(); i++) {
 		// To flip all the bits, we'll flip both nibbles, then each nibble half, etc.
+		uint16_t byte = i;
 		byte = (byte & 0b0000'1111) << 4 | (byte & 0b1111'0000) >> 4;
 		byte = (byte & 0b0011'0011) << 2 | (byte & 0b1100'1100) >> 2;
 		byte = (byte & 0b0101'0101) << 1 | (byte & 0b1010'1010) >> 1;
-		return byte;
-	}(i)...};
-}
-} // namespace detail
-
-// Flipping tends to happen fairly often, so take a bite out of dcache to speed it up
-static constexpr auto flipTable = detail::flipTable(std::make_integer_sequence<uint16_t, 256>());
+		table[i] = byte;
+	}
+	return table;
+})();
 
 #endif // RGBDS_GFX_MAIN_HPP

include/gfx/pal_packing.hpp

@@ -6,19 +6,15 @@
 #include <tuple>
 #include <vector>
 
-#include "defaultinitalloc.hpp"
+#include "defaultinitvec.hpp"
 
 struct Palette;
 class ProtoPalette;
 
-namespace packing {
-
 /*
  * Returns which palette each proto-palette maps to, and how many palettes are necessary
  */
 std::tuple<DefaultInitVec<size_t>, size_t>
     overloadAndRemove(std::vector<ProtoPalette> const &protoPalettes);
 
-} // namespace packing
-
 #endif // RGBDS_GFX_PAL_PACKING_HPP

include/gfx/pal_sorting.hpp

@@ -12,20 +12,16 @@
 
 struct Palette;
 
-namespace sorting {
-
-void indexed(
+void sortIndexed(
     std::vector<Palette> &palettes,
     int palSize,
     png_color const *palRGB,
     int palAlphaSize,
     png_byte *palAlpha
 );
-void grayscale(
+void sortGrayscale(
     std::vector<Palette> &palettes, std::array<std::optional<Rgba>, 0x8001> const &colors
 );
-void rgb(std::vector<Palette> &palettes);
-
-} // namespace sorting
+void sortRgb(std::vector<Palette> &palettes);
 
 #endif // RGBDS_GFX_PAL_SORTING_HPP

include/gfx/rgba.hpp

@@ -40,8 +40,8 @@ struct Rgba {
 		auto shl = [](uint8_t val, unsigned shift) { return static_cast<uint32_t>(val) << shift; };
 		return shl(red, 24) | shl(green, 16) | shl(blue, 8) | shl(alpha, 0);
 	}
-	friend bool operator==(Rgba const &lhs, Rgba const &rhs) { return lhs.toCSS() == rhs.toCSS(); }
-	friend bool operator!=(Rgba const &lhs, Rgba const &rhs) { return lhs.toCSS() != rhs.toCSS(); }
+	bool operator==(Rgba const &rhs) const { return toCSS() == rhs.toCSS(); }
+	bool operator!=(Rgba const &rhs) const { return toCSS() != rhs.toCSS(); }
 
 	/*
 	 * CGB colors are RGB555, so we use bit 15 to signify that the color is transparent instead

include/itertools.hpp

@@ -6,52 +6,46 @@
 #include <tuple>
 #include <utility>
 
-template<typename T>
-class EnumSeqIterator {
-	T _value;
-
-public:
-	explicit EnumSeqIterator(T value) : _value(value) {}
-
-	EnumSeqIterator &operator++() {
-		_value = (T)(_value + 1);
-		return *this;
-	}
-
-	auto operator*() const { return _value; }
-
-	friend auto operator==(EnumSeqIterator const &lhs, EnumSeqIterator const &rhs) {
-		return lhs._value == rhs._value;
-	}
-
-	friend auto operator!=(EnumSeqIterator const &lhs, EnumSeqIterator const &rhs) {
-		return lhs._value != rhs._value;
-	}
-};
-
 template<typename T>
 class EnumSeq {
 	T _start;
 	T _stop;
 
+	class Iterator {
+		T _value;
+
+	public:
+		explicit Iterator(T value) : _value(value) {}
+
+		Iterator &operator++() {
+			_value = (T)(_value + 1);
+			return *this;
+		}
+
+		auto operator*() const { return _value; }
+
+		bool operator==(Iterator const &rhs) const { return _value == rhs._value; }
+		bool operator!=(Iterator const &rhs) const  { return _value != rhs._value; }
+	};
+
 public:
 	explicit EnumSeq(T stop) : _start((T)0), _stop(stop) {}
 	explicit EnumSeq(T start, T stop) : _start(start), _stop(stop) {}
 
-	EnumSeqIterator<T> begin() { return EnumSeqIterator(_start); }
-	EnumSeqIterator<T> end() { return EnumSeqIterator(_stop); }
+	Iterator begin() { return Iterator(_start); }
+	Iterator end() { return Iterator(_stop); }
 };
 
 // This is not a fully generic implementation; its current use cases only require for-loop behavior.
 // We also assume that all iterators have the same length.
-template<typename... Iters>
-class Zip {
-	std::tuple<Iters...> _iters;
+template<typename... Ts>
+class ZipIterator {
+	std::tuple<Ts...> _iters;
 
 public:
-	explicit Zip(std::tuple<Iters...> &&iters) : _iters(iters) {}
+	explicit ZipIterator(std::tuple<Ts...> &&iters) : _iters(iters) {}
 
-	Zip &operator++() {
+	ZipIterator &operator++() {
 		std::apply([](auto &&...it) { (++it, ...); }, _iters);
 		return *this;
 	}
@@ -62,26 +56,24 @@ public:
 		);
 	}
 
-	friend auto operator==(Zip const &lhs, Zip const &rhs) {
-		return std::get<0>(lhs._iters) == std::get<0>(rhs._iters);
+	bool operator==(ZipIterator const &rhs) const {
+		return std::get<0>(_iters) == std::get<0>(rhs._iters);
 	}
 
-	friend auto operator!=(Zip const &lhs, Zip const &rhs) {
-		return std::get<0>(lhs._iters) != std::get<0>(rhs._iters);
+	bool operator!=(ZipIterator const &rhs) const {
+		return std::get<0>(_iters) != std::get<0>(rhs._iters);
 	}
 };
 
-namespace detail {
-template<typename... Containers>
+template<typename... Ts>
 class ZipContainer {
-	std::tuple<Containers...> _containers;
+	std::tuple<Ts...> _containers;
 
 public:
-	explicit ZipContainer(Containers &&...containers)
-	    : _containers(std::forward<Containers>(containers)...) {}
+	explicit ZipContainer(Ts &&...containers) : _containers(std::forward<Ts>(containers)...) {}
 
 	auto begin() {
-		return Zip(std::apply(
+		return ZipIterator(std::apply(
 		    [](auto &&...containers) {
 			    using std::begin;
 			    return std::make_tuple(begin(containers)...);
@@ -91,7 +83,7 @@ public:
 	}
 
 	auto end() {
-		return Zip(std::apply(
+		return ZipIterator(std::apply(
 		    [](auto &&...containers) {
 			    using std::end;
 			    return std::make_tuple(end(containers)...);
@@ -105,12 +97,11 @@ public:
 template<typename T>
 using Holder = std::
     conditional_t<std::is_lvalue_reference_v<T>, T, std::remove_cv_t<std::remove_reference_t<T>>>;
-} // namespace detail
 
 // Does the same number of iterations as the first container's iterator!
-template<typename... Containers>
-static constexpr auto zip(Containers &&...cs) {
-	return detail::ZipContainer<detail::Holder<Containers>...>(std::forward<Containers>(cs)...);
+template<typename... Ts>
+static constexpr auto zip(Ts &&...cs) {
+	return ZipContainer<Holder<Ts>...>(std::forward<Ts>(cs)...);
 }
 
 #endif // RGBDS_ITERTOOLS_HPP

src/gfx/pal_packing.cpp

@@ -15,10 +15,6 @@
 #include "gfx/main.hpp"
 #include "gfx/proto_palette.hpp"
 
-using std::swap;
-
-namespace packing {
-
 // The solvers here are picked from the paper at https://arxiv.org/abs/1605.00558:
 // "Algorithms for the Pagination Problem, a Bin Packing with Overlapping Items"
 // Their formulation of the problem consists in packing "tiles" into "pages"; here is a
@@ -114,8 +110,8 @@ private:
 		}
 
 		friend void swap(Iter &lhs, Iter &rhs) {
-			swap(lhs._array, rhs._array);
-			swap(lhs._iter, rhs._iter);
+			std::swap(lhs._array, rhs._array);
+			std::swap(lhs._iter, rhs._iter);
 		}
 	};
 public:
@@ -537,5 +533,3 @@ std::tuple<DefaultInitVec<size_t>, size_t>
 	}
 	return {mappings, assignments.size()};
 }
-
-} // namespace packing

src/gfx/pal_sorting.cpp

@@ -8,9 +8,7 @@
 
 #include "gfx/main.hpp"
 
-namespace sorting {
-
-void indexed(
+void sortIndexed(
     std::vector<Palette> &palettes,
     int palSize,
     png_color const *palRGB,
@@ -47,7 +45,7 @@ void indexed(
 	}
 }
 
-void grayscale(
+void sortGrayscale(
     std::vector<Palette> &palettes, std::array<std::optional<Rgba>, 0x8001> const &colors
 ) {
 	options.verbosePrint(Options::VERB_LOG_ACT, "Sorting grayscale-only palette...\n");
@@ -73,7 +71,7 @@ static unsigned int legacyLuminance(uint16_t color) {
 	return 2126 * red + 7152 * green + 722 * blue;
 }
 
-void rgb(std::vector<Palette> &palettes) {
+void sortRgb(std::vector<Palette> &palettes) {
 	options.verbosePrint(Options::VERB_LOG_ACT, "Sorting palettes by \"\"\"luminance\"\"\"...\n");
 
 	for (Palette &pal : palettes) {
@@ -82,5 +80,3 @@ void rgb(std::vector<Palette> &palettes) {
 		});
 	}
 }
-
-} // namespace sorting

src/gfx/process.cpp

@@ -15,7 +15,7 @@
 #include <utility>
 #include <vector>
 
-#include "defaultinitalloc.hpp"
+#include "defaultinitvec.hpp"
 #include "file.hpp"
 #include "helpers.hpp"
 #include "itertools.hpp"
@@ -446,7 +446,7 @@ public:
 		};
 
 	private:
-		struct iterator {
+		struct Iterator {
 			TilesVisitor const &parent;
 			uint32_t const limit;
 			uint32_t x, y;
@@ -458,7 +458,7 @@ public:
 				return {parent._png, x + options.inputSlice.left, y + options.inputSlice.top};
 			}
 
-			iterator &operator++() {
+			Iterator &operator++() {
 				auto [major, minor] = parent._columnMajor ? std::tie(y, x) : std::tie(x, y);
 				major += 8;
 				if (major == limit) {
@@ -468,19 +468,14 @@ public:
 				return *this;
 			}
 
-			friend bool operator==(iterator const &lhs, iterator const &rhs) {
-				return lhs.coords() == rhs.coords(); // Compare the returned coord pairs
-			}
-
-			friend bool operator!=(iterator const &lhs, iterator const &rhs) {
-				return lhs.coords() != rhs.coords(); // Compare the returned coord pairs
-			}
+			bool operator==(Iterator const &rhs) const { return coords() == rhs.coords(); }
+			bool operator!=(Iterator const &rhs) const { return coords() != rhs.coords(); }
 		};
 
 	public:
-		iterator begin() const { return {*this, _limit, 0, 0}; }
-		iterator end() const {
-			iterator it{*this, _limit, _width - 8, _height - 8}; // Last valid one...
+		Iterator begin() const { return {*this, _limit, 0, 0}; }
+		Iterator end() const {
+			Iterator it{*this, _limit, _width - 8, _height - 8}; // Last valid one...
 			return ++it;                                         // ...now one-past-last!
 		}
 	};
@@ -567,7 +562,7 @@ static std::tuple<DefaultInitVec<size_t>, std::vector<Palette>>
     generatePalettes(std::vector<ProtoPalette> const &protoPalettes, Png const &png) {
 	// Run a "pagination" problem solver
 	// TODO: allow picking one of several solvers?
-	auto [mappings, nbPalettes] = packing::overloadAndRemove(protoPalettes);
+	auto [mappings, nbPalettes] = overloadAndRemove(protoPalettes);
 	assume(mappings.size() == protoPalettes.size());
 
 	if (options.verbosity >= Options::VERB_INTERM) {
@@ -601,11 +596,11 @@ static std::tuple<DefaultInitVec<size_t>, std::vector<Palette>>
 	// "Sort" colors in the generated palettes, see the man page for the flowchart
 	auto [embPalSize, embPalRGB, embPalAlphaSize, embPalAlpha] = png.getEmbeddedPal();
 	if (embPalRGB != nullptr) {
-		sorting::indexed(palettes, embPalSize, embPalRGB, embPalAlphaSize, embPalAlpha);
+		sortIndexed(palettes, embPalSize, embPalRGB, embPalAlphaSize, embPalAlpha);
 	} else if (png.isSuitableForGrayscale()) {
-		sorting::grayscale(palettes, png.getColors().raw());
+		sortGrayscale(palettes, png.getColors().raw());
 	} else {
-		sorting::rgb(palettes);
+		sortRgb(palettes);
 	}
 	return {mappings, palettes};
 }
@@ -826,9 +821,7 @@ public:
 
 		return MatchType::NOPE;
 	}
-	friend bool operator==(TileData const &lhs, TileData const &rhs) {
-		return lhs.tryMatching(rhs) != MatchType::NOPE;
-	}
+	bool operator==(TileData const &rhs) const { return tryMatching(rhs) != MatchType::NOPE; }
 };
 
 template<>
@@ -836,9 +829,7 @@ struct std::hash<TileData> {
 	std::size_t operator()(TileData const &tile) const { return tile.hash(); }
 };
 
-namespace unoptimized {
-
-static void outputTileData(
+static void outputUnoptimizedTileData(
     Png const &png,
     DefaultInitVec<AttrmapEntry> const &attrmap,
     std::vector<Palette> const &palettes,
@@ -877,7 +868,7 @@ static void outputTileData(
 	assume(remainingTiles == 0);
 }
 
-static void outputMaps(
+static void outputUnoptimizedMaps(
     DefaultInitVec<AttrmapEntry> const &attrmap, DefaultInitVec<size_t> const &mappings
 ) {
 	std::optional<File> tilemapOutput, attrmapOutput, palmapOutput;
@@ -916,10 +907,6 @@ static void outputMaps(
 	}
 }
 
-} // namespace unoptimized
-
-namespace optimized {
-
 struct UniqueTiles {
 	std::unordered_set<TileData> tileset;
 	std::vector<TileData const *> tiles;
@@ -1089,8 +1076,6 @@ static void outputPalmap(
 	}
 }
 
-} // namespace optimized
-
 void processPalettes() {
 	options.verbosePrint(Options::VERB_CFG, "Using libpng %s\n", png_get_libpng_ver(nullptr));
 
@@ -1251,7 +1236,7 @@ continue_visiting_tiles:;
 
 		if (!options.output.empty()) {
 			options.verbosePrint(Options::VERB_LOG_ACT, "Generating unoptimized tile data...\n");
-			unoptimized::outputTileData(png, attrmap, palettes, mappings);
+			outputUnoptimizedTileData(png, attrmap, palettes, mappings);
 		}
 
 		if (!options.tilemap.empty() || !options.attrmap.empty() || !options.palmap.empty()) {
@@ -1259,12 +1244,12 @@ continue_visiting_tiles:;
 			    Options::VERB_LOG_ACT,
 			    "Generating unoptimized tilemap and/or attrmap and/or palmap...\n"
 			);
-			unoptimized::outputMaps(attrmap, mappings);
+			outputUnoptimizedMaps(attrmap, mappings);
 		}
 	} else {
 		// All of these require the deduplication process to be performed to be output
 		options.verbosePrint(Options::VERB_LOG_ACT, "Deduplicating tiles...\n");
-		optimized::UniqueTiles tiles = optimized::dedupTiles(png, attrmap, palettes, mappings);
+		UniqueTiles tiles = dedupTiles(png, attrmap, palettes, mappings);
 
 		if (tiles.size() > options.maxNbTiles[0] + options.maxNbTiles[1]) {
 			fatal(
@@ -1277,22 +1262,22 @@ continue_visiting_tiles:;
 
 		if (!options.output.empty()) {
 			options.verbosePrint(Options::VERB_LOG_ACT, "Generating optimized tile data...\n");
-			optimized::outputTileData(tiles);
+			outputTileData(tiles);
 		}
 
 		if (!options.tilemap.empty()) {
 			options.verbosePrint(Options::VERB_LOG_ACT, "Generating optimized tilemap...\n");
-			optimized::outputTilemap(attrmap);
+			outputTilemap(attrmap);
 		}
 
 		if (!options.attrmap.empty()) {
 			options.verbosePrint(Options::VERB_LOG_ACT, "Generating optimized attrmap...\n");
-			optimized::outputAttrmap(attrmap, mappings);
+			outputAttrmap(attrmap, mappings);
 		}
 
 		if (!options.palmap.empty()) {
 			options.verbosePrint(Options::VERB_LOG_ACT, "Generating optimized palmap...\n");
-			optimized::outputPalmap(attrmap, mappings);
+			outputPalmap(attrmap, mappings);
 		}
 	}
 }

src/gfx/reverse.cpp

@@ -12,7 +12,7 @@
 #include <string.h>
 #include <vector>
 
-#include "defaultinitalloc.hpp"
+#include "defaultinitvec.hpp"
 #include "file.hpp"
 #include "helpers.hpp" // assume
 

test/gfx/rgbgfx_test.cpp

@@ -29,7 +29,7 @@
 #include <string>
 #include <vector>
 
-#include "defaultinitalloc.hpp"
+#include "defaultinitvec.hpp" // Reused from RGBDS
 
 #include "gfx/rgba.hpp" // Reused from RGBGFX