From b4dadd35b6ea22192b3b68b042f439caa0dd0566 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Sat, 21 May 2022 10:41:56 +0200 Subject: [PATCH] Use an iterator `zip` Simplifies iterating over tiles and attributes at the same time --- include/defaultinitalloc.hpp | 9 ++-- include/itertools.hpp | 90 ++++++++++++++++++++++++++++++++++++ src/gfx/process.cpp | 53 +++++++++------------ 3 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 include/itertools.hpp diff --git a/include/defaultinitalloc.hpp b/include/defaultinitalloc.hpp index b2387bc9..43cb2146 100644 --- a/include/defaultinitalloc.hpp +++ b/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 - void construct(U * ptr) noexcept(std::is_nothrow_default_constructible_v) { - ::new(static_cast(ptr)) U; + void construct(U *ptr) noexcept(std::is_nothrow_default_constructible_v) { + ::new (static_cast(ptr)) U; } template - void construct(U * ptr, Args && ... args) { + void construct(U *ptr, Args &&...args) { a_t::construct(static_cast(*this), ptr, std::forward(args)...); } }; diff --git a/include/itertools.hpp b/include/itertools.hpp new file mode 100644 index 00000000..892bb89d --- /dev/null +++ b/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 +#include + +template +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 +class Zip { + std::tuple _iters; + +public: + explicit Zip(std::tuple &&iters) : _iters(iters) {} + + Zip &operator++() { + std::apply([](auto &&...it) { (++it, ...); }, _iters); + return *this; + } + + auto operator*() const { + return std::apply([](auto &&...it) { return std::tuple(*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 +class ZipContainer { + std::tuple _containers; + +public: + ZipContainer(Containers &&...containers) + : _containers(std::forward(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 +using Holder = std::conditional_t, T, + std::remove_cv_t>>; +} + +/** + * Does the same number of iterations as the first container's iterator! + */ +template +static constexpr auto zip(Containers &&...cs) { + return detail::ZipContainer...>(std::forward(cs)...); +} + +#endif /* RGBDS_ITERTOOLS_HPP */ diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index dd1be2b6..8b15f676 100644 --- a/src/gfx/process.cpp +++ b/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 { - return 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 + } + + 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, std::vector> // Convert the palette spec to actual palettes std::vector palettes(options.palSpec.size()); - auto palIter = palettes.begin(); // TODO: `zip` - for (auto const &spec : options.palSpec) { + for (auto [spec, pal] : zip(options.palSpec, palettes)) { 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 const &a } remainingTiles -= options.trim; - auto iter = attrmap.begin(); - for (auto tile : png.visitAsTiles(options.columnMajor)) { + for (auto [tile, attr] : zip(png.visitAsTiles(options.columnMajor), attrmap)) { // 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 const &a output.sputc(bitplanes >> 8); } } - ++iter; --remainingTiles; if (remainingTiles == 0) { @@ -747,10 +748,9 @@ static void outputTileData(Png const &png, DefaultInitVec const &a } } assert(remainingTiles == 0); - assert(iter + options.trim == attrmap.end()); } -static void outputMaps(Png const &png, DefaultInitVec const &attrmap, +static void outputMaps(DefaultInitVec const &attrmap, DefaultInitVec const &mappings) { std::optional tilemapOutput, attrmapOutput, palmapOutput; if (!options.tilemap.empty()) { @@ -768,8 +768,7 @@ static void outputMaps(Png const &png, DefaultInitVec const &attrm uint8_t tileID = 0; uint8_t bank = 0; - auto iter = attrmap.begin(); - for ([[maybe_unused]] auto tile : png.visitAsTiles(options.columnMajor)) { + for (auto attr : attrmap) { if (tileID == options.maxNbTiles[bank]) { assert(bank == 0); bank = 1; @@ -780,16 +779,14 @@ static void outputMaps(Png const &png, DefaultInitVec 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 &attr // by caching the full tile data anyway, so we might as well.) UniqueTiles tiles; - auto iter = attrmap.begin(); - for (auto tile : png.visitAsTiles(options.columnMajor)) { - auto [tileID, matchType] = tiles.addTile(tile, palettes[mappings[iter->protoPaletteID]]); + for (auto [tile, attr] : zip(png.visitAsTiles(options.columnMajor), attrmap)) { + auto [tileID, matchType] = tiles.addTile(tile, palettes[mappings[attr.protoPaletteID]]); - iter->xFlip = matchType == TileData::HFLIP || matchType == TileData::VHFLIP; - iter->yFlip = matchType == TileData::VFLIP || matchType == TileData::VHFLIP; - iter->bank = tileID >= options.maxNbTiles[0]; - iter->tileID = (iter->bank ? tileID - options.maxNbTiles[0] : tileID) - + options.baseTileIDs[iter->bank]; - - ++iter; + attr.xFlip = matchType == TileData::HFLIP || matchType == TileData::VHFLIP; + attr.yFlip = matchType == TileData::VFLIP || matchType == TileData::VHFLIP; + attr.bank = tileID >= options.maxNbTiles[0]; + attr.tileID = + (attr.bank ? tileID - options.maxNbTiles[0] : tileID) + options.baseTileIDs[attr.bank]; } - 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