Use an iterator zip

Simplifies iterating over tiles and attributes at the same time

include/defaultinitalloc.hpp

@@ -2,7 +2,8 @@
  * Allocator adaptor that interposes construct() calls to convert value-initialization
  * (which is what you get with e.g. `vector::resize`) into default-initialization (which does not
  * zero out non-class types).
- * From https://stackoverflow.com/questions/21028299/is-this-behavior-of-vectorresizesize-type-n-under-c11-and-boost-container/21028912#21028912
+ * From
+ * https://stackoverflow.com/questions/21028299/is-this-behavior-of-vectorresizesize-type-n-under-c11-and-boost-container/21028912#21028912
  */
 
 #ifndef DEFAULT_INIT_ALLOC_H
@@ -23,11 +24,11 @@ public:
 	using A::A; // Inherit the allocator's constructors
 
 	template<typename U>
-	void construct(U * ptr) noexcept(std::is_nothrow_default_constructible_v<U>) {
+	void construct(U *ptr) noexcept(std::is_nothrow_default_constructible_v<U>) {
-		::new(static_cast<void *>(ptr)) U;
+		::new (static_cast<void *>(ptr)) U;
 	}
 	template<typename U, typename... Args>
-	void construct(U * ptr, Args && ... args) {
+	void construct(U *ptr, Args &&...args) {
 		a_t::construct(static_cast<A &>(*this), ptr, std::forward<Args>(args)...);
 	}
 };

include/itertools.hpp

@@ -0,0 +1,90 @@
+/*
+ * This file is part of RGBDS.
+ *
+ * Copyright (c) 2022, Eldred Habert and RGBDS contributors.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef RGBDS_ITERTOOLS_HPP
+#define RGBDS_ITERTOOLS_HPP
+
+#include <tuple>
+#include <utility>
+
+template<typename... Ts>
+static inline void report() {
+	puts(__PRETTY_FUNCTION__);
+}
+
+// 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;
+
+public:
+	explicit Zip(std::tuple<Iters...> &&iters) : _iters(iters) {}
+
+	Zip &operator++() {
+		std::apply([](auto &&...it) { (++it, ...); }, _iters);
+		return *this;
+	}
+
+	auto operator*() const {
+		return std::apply([](auto &&...it) { return std::tuple<decltype(*it)...>(*it...); },
+		                  _iters);
+	}
+
+	friend auto operator==(Zip const &lhs, Zip const &rhs) {
+		return std::get<0>(lhs._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);
+	}
+};
+
+namespace detail {
+template<typename... Containers>
+class ZipContainer {
+	std::tuple<Containers...> _containers;
+
+public:
+	ZipContainer(Containers &&...containers)
+	    : _containers(std::forward<Containers>(containers)...) {}
+
+	auto begin() {
+		return Zip(std::apply(
+		    [](auto &&...containers) {
+			    using std::begin;
+			    return std::make_tuple(begin(containers)...);
+		    },
+		    _containers));
+	}
+
+	auto end() {
+		return Zip(std::apply(
+		    [](auto &&...containers) {
+			    using std::end;
+			    return std::make_tuple(end(containers)...);
+		    },
+		    _containers));
+	}
+};
+
+// Take ownership of objects and rvalue refs passed to us, but not lvalue refs
+template<typename T>
+using Holder = std::conditional_t<std::is_lvalue_reference_v<T>, T,
+                                  std::remove_cv_t<std::remove_reference_t<T>>>;
+}
+
+/**
+ * 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)...);
+}
+
+#endif /* RGBDS_ITERTOOLS_HPP */

src/gfx/process.cpp

@@ -27,6 +27,7 @@
 
 #include "defaultinitalloc.hpp"
 #include "helpers.h"
+#include "itertools.hpp"
 
 #include "gfx/main.hpp"
 #include "gfx/pal_packing.hpp"
@@ -435,8 +436,12 @@ public:
 				return *this;
 			}
 
-			bool operator!=(iterator const &rhs) const {
+			friend bool operator==(iterator const &lhs, iterator const &rhs) {
-				return coords() != rhs.coords(); // Compare the returned coord pairs
+				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
 			}
 		};
 
@@ -565,12 +570,10 @@ static std::tuple<DefaultInitVec<size_t>, std::vector<Palette>>
 
 	// Convert the palette spec to actual palettes
 	std::vector<Palette> palettes(options.palSpec.size());
-	auto palIter = palettes.begin(); // TODO: `zip`
+	for (auto [spec, pal] : zip(options.palSpec, palettes)) {
-	for (auto const &spec : options.palSpec) {
 		for (size_t i = 0; i < options.nbColorsPerPal; ++i) {
-			(*palIter)[i] = spec[i].cgbColor();
+			pal[i] = spec[i].cgbColor();
 		}
-		++palIter;
 	}
 
 	// Iterate through proto-palettes, and try mapping them to the specified palettes
@@ -728,10 +731,9 @@ static void outputTileData(Png const &png, DefaultInitVec<AttrmapEntry> const &a
 	}
 	remainingTiles -= options.trim;
 
-	auto iter = attrmap.begin();
+	for (auto [tile, attr] : zip(png.visitAsTiles(options.columnMajor), attrmap)) {
-	for (auto tile : png.visitAsTiles(options.columnMajor)) {
 		// If the tile is fully transparent, default to palette 0
-		Palette const &palette = palettes[iter->getPalID(mappings)];
+		Palette const &palette = palettes[attr.getPalID(mappings)];
 		for (uint32_t y = 0; y < 8; ++y) {
 			uint16_t bitplanes = TileData::rowBitplanes(tile, palette, y);
 			output.sputc(bitplanes & 0xFF);
@@ -739,7 +741,6 @@ static void outputTileData(Png const &png, DefaultInitVec<AttrmapEntry> const &a
 				output.sputc(bitplanes >> 8);
 			}
 		}
-		++iter;
 
 		--remainingTiles;
 		if (remainingTiles == 0) {
@@ -747,10 +748,9 @@ static void outputTileData(Png const &png, DefaultInitVec<AttrmapEntry> const &a
 		}
 	}
 	assert(remainingTiles == 0);
-	assert(iter + options.trim == attrmap.end());
 }
 
-static void outputMaps(Png const &png, DefaultInitVec<AttrmapEntry> const &attrmap,
+static void outputMaps(DefaultInitVec<AttrmapEntry> const &attrmap,
                        DefaultInitVec<size_t> const &mappings) {
 	std::optional<std::filebuf> tilemapOutput, attrmapOutput, palmapOutput;
 	if (!options.tilemap.empty()) {
@@ -768,8 +768,7 @@ static void outputMaps(Png const &png, DefaultInitVec<AttrmapEntry> const &attrm
 
 	uint8_t tileID = 0;
 	uint8_t bank = 0;
-	auto iter = attrmap.begin();
+	for (auto attr : attrmap) {
-	for ([[maybe_unused]] auto tile : png.visitAsTiles(options.columnMajor)) {
 		if (tileID == options.maxNbTiles[bank]) {
 			assert(bank == 0);
 			bank = 1;
@@ -780,16 +779,14 @@ static void outputMaps(Png const &png, DefaultInitVec<AttrmapEntry> const &attrm
 			tilemapOutput->sputc(tileID + options.baseTileIDs[bank]);
 		}
 		if (attrmapOutput.has_value()) {
-			uint8_t palID = iter->getPalID(mappings) & 7;
+			uint8_t palID = attr.getPalID(mappings) & 7;
 			attrmapOutput->sputc(palID | bank << 3); // The other flags are all 0
 		}
 		if (palmapOutput.has_value()) {
-			palmapOutput->sputc(iter->getPalID(mappings));
+			palmapOutput->sputc(attr.getPalID(mappings));
 		}
 		++tileID;
-		++iter;
 	}
-	assert(iter == attrmap.end());
 }
 
 } // namespace unoptimized
@@ -846,19 +843,15 @@ static UniqueTiles dedupTiles(Png const &png, DefaultInitVec<AttrmapEntry> &attr
 	// by caching the full tile data anyway, so we might as well.)
 	UniqueTiles tiles;
 
-	auto iter = attrmap.begin();
+	for (auto [tile, attr] : zip(png.visitAsTiles(options.columnMajor), attrmap)) {
-	for (auto tile : png.visitAsTiles(options.columnMajor)) {
+		auto [tileID, matchType] = tiles.addTile(tile, palettes[mappings[attr.protoPaletteID]]);
-		auto [tileID, matchType] = tiles.addTile(tile, palettes[mappings[iter->protoPaletteID]]);
 
-		iter->xFlip = matchType == TileData::HFLIP || matchType == TileData::VHFLIP;
+		attr.xFlip = matchType == TileData::HFLIP || matchType == TileData::VHFLIP;
-		iter->yFlip = matchType == TileData::VFLIP || matchType == TileData::VHFLIP;
+		attr.yFlip = matchType == TileData::VFLIP || matchType == TileData::VHFLIP;
-		iter->bank = tileID >= options.maxNbTiles[0];
+		attr.bank = tileID >= options.maxNbTiles[0];
-		iter->tileID = (iter->bank ? tileID - options.maxNbTiles[0] : tileID)
+		attr.tileID =
-		               + options.baseTileIDs[iter->bank];
+		    (attr.bank ? tileID - options.maxNbTiles[0] : tileID) + options.baseTileIDs[attr.bank];
-
-		++iter;
 	}
-	assert(iter == attrmap.end());
 
 	// Copy elision should prevent the contained `unordered_set` from being re-constructed
 	return tiles;
@@ -1057,7 +1050,7 @@ contained:;
 			options.verbosePrint(
 			    Options::VERB_LOG_ACT,
 			    "Generating unoptimized tilemap and/or attrmap and/or palmap...\n");
-			unoptimized::outputMaps(png, attrmap, mappings);
+			unoptimized::outputMaps(attrmap, mappings);
 		}
 	} else {
 		// All of these require the deduplication process to be performed to be output