From 678cf9d96054b53cd5cdff5df6c69a0c648c010f Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Sat, 10 Aug 2024 20:05:51 +0200 Subject: [PATCH 01/12] Implement `--input-tileset` As discussed in https://github.com/gbdev/rgbds/issues/575#issuecomment-1991456862 --- contrib/bash_compl/_rgbgfx.bash | 1 + contrib/zsh_compl/_rgbgfx | 1 + include/gfx/main.hpp | 3 +- man/rgbgfx.1 | 40 +++++++++++++++- src/gfx/main.cpp | 5 ++ src/gfx/process.cpp | 82 +++++++++++++++++++++++++++++---- src/gfx/reverse.cpp | 4 ++ 7 files changed, 126 insertions(+), 10 deletions(-) diff --git a/contrib/bash_compl/_rgbgfx.bash b/contrib/bash_compl/_rgbgfx.bash index 7729b39b8..d1b1e1651 100755 --- a/contrib/bash_compl/_rgbgfx.bash +++ b/contrib/bash_compl/_rgbgfx.bash @@ -21,6 +21,7 @@ _rgbgfx_completions() { [b]="base-tiles:unk" [c]="colors:unk" [d]="depth:unk" + [o]="input-tileset:glob-*.2bpp" [L]="slice:unk" [N]="nb-tiles:unk" [n]="nb-palettes:unk" diff --git a/contrib/zsh_compl/_rgbgfx b/contrib/zsh_compl/_rgbgfx index f009502c8..b64aa7af9 100644 --- a/contrib/zsh_compl/_rgbgfx +++ b/contrib/zsh_compl/_rgbgfx @@ -30,6 +30,7 @@ local args=( '(-b --base-tiles)'{-b,--base-tiles}'+[Base tile IDs for tile map output]:base tile IDs:' '(-c --colors)'{-c,--colors}'+[Specify color palettes]:palette spec:' '(-d --depth)'{-d,--depth}'+[Set bit depth]:bit depth:_depths' + '(-i --input-tileset)'{-i,--input-tileset}'+[Use specific tiles]:tileset file:_files -g "*.2bpp"' '(-L --slice)'{-L,--slice}'+[Only process a portion of the image]:input slice:' '(-N --nb-tiles)'{-N,--nb-tiles}'+[Limit number of tiles]:tile count:' '(-n --nb-palettes)'{-n,--nb-palettes}'+[Limit number of palettes]:palette count:' diff --git a/include/gfx/main.hpp b/include/gfx/main.hpp index 9716d4619..d7ae6dcbd 100644 --- a/include/gfx/main.hpp +++ b/include/gfx/main.hpp @@ -28,7 +28,8 @@ struct Options { EMBEDDED, } palSpecType = NO_SPEC; // -c std::vector, 4>> palSpec{}; - uint8_t bitDepth = 2; // -d + uint8_t bitDepth = 2; // -d + std::string inputTileset{}; // -i struct { uint16_t left; uint16_t top; diff --git a/man/rgbgfx.1 b/man/rgbgfx.1 index fa65b916d..81a4590ad 100644 --- a/man/rgbgfx.1 +++ b/man/rgbgfx.1 @@ -16,6 +16,7 @@ .Op Fl b Ar base_ids .Op Fl c Ar pal_spec .Op Fl d Ar depth +.Op Fl i Ar input_tiles .Op Fl L Ar slice .Op Fl N Ar nb_tiles .Op Fl n Ar nb_pals @@ -164,6 +165,37 @@ for a list of formats and their descriptions. .It Fl d Ar depth , Fl \-depth Ar depth Set the bit depth of the output tile data, in bits per pixel (bpp), either 1 or 2 (the default). This changes how tile data is output, and the maximum number of colors per palette (2 and 4 respectively). +.It Fl i Ar input_tiles , Fl \-input-tileset Ar input_tiles +Read tiles that will be used to convert this image, and that will always be given the first IDs. +.Ar input_tiles +must contain tile data in +.Dq raw +format, as generated through the +.Fl o +option. +.Pp +If used together with +.Fl o , +then the image can contain tiles not in +.Ar input_tiles . +Otherwise, the image must be able to be generated using +.Em only +the tiles from +.Ar input_tiles , +and thus generate different tile data. +The former is more useful if you want several images to share a given set of tiles, such as different levels sharing a single tileset; the latter, if you want to control more precisely the numeric IDs of specific tiles. +.Pp +If more than one color palette is in use, it is also +.Sy strongly +advised to dump it together with the tile data, and to pass it using +.Fl c Cm gbc: Ns Ar input_palette . +This is because +.Nm +may not pack the palettes the same way that it did when generating +.Ar input_tiles . +See +.Sx EXAMPLES +for an example of how to use this. .It Fl L Ar slice , Fl \-slice Ar slice Only process a given rectangle of the image. This is useful for example if the input image is a sheet of some sort, and you want to convert each cel individually. @@ -637,7 +669,13 @@ without needing an input image. .Pp .Dl $ rgbgfx -c '#fff,#ff0,#f80,#000' -p colors.pal .Pp -TODO: more examples. +The following will convert two levels using the same tileset, and error out of any of the level images contain tiles not in the tileset. +.Pp +.Bd -literal -offset Ds +$ rgbgfx tileset.png -o tileset.2bpp -O -P +$ rgbgfx -i tileset.2bpp -c gbc:tileset.pal level1.png -t level1.tilemap -a level1.attrmap +$ rgbgfx -i tileset.2bpp -c gbc:tileset.pal level2.png -t level2.tilemap -a level2.attrmap +.Ed .Sh BUGS Please report bugs and mistakes in this man page on .Lk https://github.com/gbdev/rgbds/issues GitHub . diff --git a/src/gfx/main.cpp b/src/gfx/main.cpp index a4464e88a..7322a8fc2 100644 --- a/src/gfx/main.cpp +++ b/src/gfx/main.cpp @@ -427,6 +427,11 @@ static char *parseArgv(int argc, char *argv[]) { options.bitDepth = 2; } break; + case 'i': + if (!options.inputTileset.empty()) + warning("Overriding input tileset file %s", options.inputTileset.c_str()); + options.inputTileset = musl_optarg; + break; case 'L': options.inputSlice.left = parseNumber(arg, "Input slice left coordinate"); if (options.inputSlice.left > INT16_MAX) { diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index aa6c28356..98b2931d9 100644 --- a/src/gfx/process.cpp +++ b/src/gfx/process.cpp @@ -692,7 +692,7 @@ static void outputPalettes(std::vector const &palettes) { if (!options.palettes.empty()) { File output; if (!output.open(options.palettes, std::ios_base::out | std::ios_base::binary)) { - fatal("Failed to open \"%s\": %s", output.c_str(options.palettes), strerror(errno)); + fatal("Failed to create \"%s\": %s", output.c_str(options.palettes), strerror(errno)); } for (Palette const &palette : palettes) { @@ -736,6 +736,20 @@ class TileData { return row; } + TileData(std::array &&raw) : _data(raw), _hash(0) { + for (uint8_t y = 0; y < 8; ++y) { + uint16_t bitplanes = _data[y * 2] | _data[y * 2 + 1] << 8; + + _hash ^= bitplanes; + if (options.allowMirroring) { + // Count the line itself as mirrorred; vertical mirroring is + // already taken care of because the symmetric line will be XOR'd + // the same way. (...which is a problem, but probably benign.) + _hash ^= flipTable[bitplanes >> 8] << 8 | flipTable[bitplanes & 0xFF]; + } + } + } + TileData(Png::TilesVisitor::Tile const &tile, Palette const &palette) : _hash(0) { size_t writeIndex = 0; for (uint32_t y = 0; y < 8; ++y) { @@ -836,7 +850,7 @@ static void outputTileData( ) { File output; if (!output.open(options.output, std::ios_base::out | std::ios_base::binary)) { - fatal("Failed to open \"%s\": %s", output.c_str(options.output), strerror(errno)); + fatal("Failed to create \"%s\": %s", output.c_str(options.output), strerror(errno)); } uint16_t widthTiles = options.inputSlice.width ? options.inputSlice.width : png.getWidth() / 8; @@ -875,7 +889,7 @@ static void outputMaps( if (!path.empty()) { file.emplace(); if (!file->open(path, std::ios_base::out | std::ios_base::binary)) { - fatal("Failed to open \"%s\": %s", file->c_str(options.tilemap), strerror(errno)); + fatal("Failed to create \"%s\": %s", file->c_str(options.tilemap), strerror(errno)); } } }; @@ -923,12 +937,10 @@ struct UniqueTiles { /* * Adds a tile to the collection, and returns its ID */ - std::tuple - addTile(Png::TilesVisitor::Tile const &tile, Palette const &palette) { - TileData newTile(tile, palette); + std::tuple addTile(TileData newTile) { auto [tileData, inserted] = tileset.insert(newTile); - TileData::MatchType matchType = TileData::EXACT; + TileData::MatchType matchType = TileData::NOPE; if (inserted) { // Give the new tile the next available unique ID tileData->tileID = static_cast(tiles.size()); @@ -963,8 +975,56 @@ static UniqueTiles dedupTiles( // by caching the full tile data anyway, so we might as well.) UniqueTiles tiles; + if (!options.inputTileset.empty()) { + File inputTileset; + if (!inputTileset.open(options.inputTileset, std::ios::in | std::ios::binary)) { + fatal("Failed to open \"%s\": %s", options.inputTileset.c_str(), strerror(errno)); + } + + std::array tile; + size_t const tileSize = options.bitDepth * 8; + for (;;) { + // It's okay to cast between character types. + size_t len = inputTileset->sgetn(reinterpret_cast(tile.data()), tileSize); + if (len == 0) { // EOF! + break; + } else if (len != tileSize) { + fatal( + "\"%s\" does not contain a multiple of %zu bytes; is it actually tile data?", + options.inputTileset.c_str(), + tileSize + ); + } else if (len == 8) { + // Expand the tile data to 2bpp. + for (size_t i = 8; i--;) { + tile[i * 2 + 1] = 0; + tile[i * 2] = tile[i]; + } + } + + auto [tileID, matchType] = tiles.addTile(std::move(tile)); + switch (matchType) { + case TileData::NOPE: + break; + case TileData::HFLIP: + case TileData::VFLIP: + case TileData::VHFLIP: + if (!options.allowMirroring) { + break; + } + [[fallthrough]]; + case TileData::EXACT: + error("The input tileset contains tiles that were deduplicated; please check that your deduplication flags (`-u`, `-m`) are consistent with what was used to generate the input tileset"); + } + } + } + for (auto [tile, attr] : zip(png.visitAsTiles(), attrmap)) { - auto [tileID, matchType] = tiles.addTile(tile, palettes[mappings[attr.protoPaletteID]]); + auto [tileID, matchType] = tiles.addTile({tile, palettes[mappings[attr.protoPaletteID]]}); + + if (matchType == TileData::NOPE && options.output.empty()) { + error("Tile at (%" PRIu32 ", %" PRIu32 ") is not within the input tileset, and `-o` was not given!", tile.x, tile.y); + } attr.xFlip = matchType == TileData::HFLIP || matchType == TileData::VHFLIP; attr.yFlip = matchType == TileData::VFLIP || matchType == TileData::VHFLIP; @@ -1186,6 +1246,12 @@ continue_visiting_tiles:; ); } + // I currently cannot figure out useful semantics for this combination of flags. + if (!options.inputTileset.empty()) { + fatal("Input tilesets are not supported without `-u`\nPlease consider explaining your " + "use case to RGBDS' developers!"); + } + if (!options.output.empty()) { options.verbosePrint(Options::VERB_LOG_ACT, "Generating unoptimized tile data...\n"); unoptimized::outputTileData(png, attrmap, palettes, mappings); diff --git a/src/gfx/reverse.cpp b/src/gfx/reverse.cpp index a622ba4c1..be9a934ce 100644 --- a/src/gfx/reverse.cpp +++ b/src/gfx/reverse.cpp @@ -136,6 +136,10 @@ void reverse() { ); } + if (!options.inputTileset.empty()) { + // TODO: check that the tile data is contained within the tileset + } + // By default, assume tiles are not deduplicated, and add the (allegedly) trimmed tiles size_t const nbTiles = tiles.size() / tileSize; options.verbosePrint(Options::VERB_INTERM, "Read %zu tiles.\n", nbTiles); From be14f193479c57a45e3768586db15b0dfeef2fe2 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Mon, 12 Aug 2024 15:37:05 +0200 Subject: [PATCH 02/12] Fix inverted proposition order --- man/rgbgfx.1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/rgbgfx.1 b/man/rgbgfx.1 index 81a4590ad..8890cfa22 100644 --- a/man/rgbgfx.1 +++ b/man/rgbgfx.1 @@ -183,7 +183,7 @@ Otherwise, the image must be able to be generated using the tiles from .Ar input_tiles , and thus generate different tile data. -The former is more useful if you want several images to share a given set of tiles, such as different levels sharing a single tileset; the latter, if you want to control more precisely the numeric IDs of specific tiles. +The former is more useful if you want to control more precisely the numeric IDs of specific tiles; the latter, if you want several images to share a given set of tiles, such as different levels sharing a single tileset. .Pp If more than one color palette is in use, it is also .Sy strongly From 8a94f1bf8a7928711ed85bab86dda419d751b918 Mon Sep 17 00:00:00 2001 From: Eldred Habert Date: Mon, 12 Aug 2024 20:04:57 +0200 Subject: [PATCH 03/12] Improve wording of documentation Co-authored-by: Sylvie <35663410+Rangi42@users.noreply.github.com> --- contrib/bash_compl/_rgbgfx.bash | 2 +- man/rgbgfx.1 | 31 +++++++++++++++++++------------ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/contrib/bash_compl/_rgbgfx.bash b/contrib/bash_compl/_rgbgfx.bash index d1b1e1651..9ac6a2724 100755 --- a/contrib/bash_compl/_rgbgfx.bash +++ b/contrib/bash_compl/_rgbgfx.bash @@ -21,7 +21,7 @@ _rgbgfx_completions() { [b]="base-tiles:unk" [c]="colors:unk" [d]="depth:unk" - [o]="input-tileset:glob-*.2bpp" + [i]="input-tileset:glob-*.2bpp" [L]="slice:unk" [N]="nb-tiles:unk" [n]="nb-palettes:unk" diff --git a/man/rgbgfx.1 b/man/rgbgfx.1 index 8890cfa22..9d361e44c 100644 --- a/man/rgbgfx.1 +++ b/man/rgbgfx.1 @@ -168,9 +168,7 @@ This changes how tile data is output, and the maximum number of colors per palet .It Fl i Ar input_tiles , Fl \-input-tileset Ar input_tiles Read tiles that will be used to convert this image, and that will always be given the first IDs. .Ar input_tiles -must contain tile data in -.Dq raw -format, as generated through the +must contain 2bpp tile data, as could be previously generated with the .Fl o option. .Pp @@ -183,19 +181,28 @@ Otherwise, the image must be able to be generated using the tiles from .Ar input_tiles , and thus generate different tile data. -The former is more useful if you want to control more precisely the numeric IDs of specific tiles; the latter, if you want several images to share a given set of tiles, such as different levels sharing a single tileset. -.Pp -If more than one color palette is in use, it is also -.Sy strongly -advised to dump it together with the tile data, and to pass it using -.Fl c Cm gbc: Ns Ar input_palette . +Using +.Fl o +with +.Fl i +is useful if you want to precisely control the tile IDs of its tile map. +Using +.Fl i +alone is more useful if you want several images to use a subset of shared tiles. +.Pp +If the image will use more than one color palette, it is +.Em strongly +advised to generate the palette set along with the input tile data, and pass +.Fl c Cm gbc: Ns Ar input_palette +along with +.Fl i input_tiles . This is because .Nm -may not pack the palettes the same way that it did when generating -.Ar input_tiles . +may not generate an identical palette set for this image as for its input tileset. +.Pp See .Sx EXAMPLES -for an example of how to use this. +for examples of how to use this option. .It Fl L Ar slice , Fl \-slice Ar slice Only process a given rectangle of the image. This is useful for example if the input image is a sheet of some sort, and you want to convert each cel individually. From b38fddd916f8de5cfd6e76bd3cfcd27de1c8975a Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Wed, 4 Sep 2024 00:15:44 +0200 Subject: [PATCH 04/12] Make compatible with individual flip flags --- src/gfx/process.cpp | 68 ++++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index 98b2931d9..2bdcefa4b 100644 --- a/src/gfx/process.cpp +++ b/src/gfx/process.cpp @@ -706,6 +706,17 @@ static void outputPalettes(std::vector const &palettes) { } } +static void hashBitplanes(uint16_t bitplanes, uint16_t &hash) { + hash ^= bitplanes; + if (options.allowMirroringX) { + // Count the line itself as mirrored, which ensures the same hash as the tile's horizontal + // flip; vertical mirroring is already taken care of because the symmetric line will be + // XOR'd the same way. (This can trivially create some collisions, but real-world tile data + // generally doesn't trigger them.) + hash ^= flipTable[bitplanes >> 8] << 8 | flipTable[bitplanes & 0xFF]; + } +} + class TileData { std::array _data; // The hash is a bit lax: it's the XOR of all lines, and every other nibble is identical @@ -739,14 +750,7 @@ class TileData { TileData(std::array &&raw) : _data(raw), _hash(0) { for (uint8_t y = 0; y < 8; ++y) { uint16_t bitplanes = _data[y * 2] | _data[y * 2 + 1] << 8; - - _hash ^= bitplanes; - if (options.allowMirroring) { - // Count the line itself as mirrorred; vertical mirroring is - // already taken care of because the symmetric line will be XOR'd - // the same way. (...which is a problem, but probably benign.) - _hash ^= flipTable[bitplanes >> 8] << 8 | flipTable[bitplanes & 0xFF]; - } + hashBitplanes(bitplanes, _hash); } } @@ -754,19 +758,12 @@ class TileData { size_t writeIndex = 0; for (uint32_t y = 0; y < 8; ++y) { uint16_t bitplanes = rowBitplanes(tile, palette, y); + hashBitplanes(bitplanes, _hash); + _data[writeIndex++] = bitplanes & 0xFF; if (options.bitDepth == 2) { _data[writeIndex++] = bitplanes >> 8; } - - // Update the hash - _hash ^= bitplanes; - if (options.allowMirroringX) { - // Count the line itself as mirrorred horizontally; vertical mirroring is already - // taken care of because the symmetric line will be XOR'd the same way. - // (This reduces the hash's efficiency, but seems benign with most real-world data.) - _hash ^= flipTable[bitplanes >> 8] << 8 | flipTable[bitplanes & 0xFF]; - } } } @@ -774,11 +771,11 @@ class TileData { uint16_t hash() const { return _hash; } enum MatchType { - NOPE, EXACT, HFLIP, VFLIP, - VHFLIP, + VHFLIP = VFLIP | HFLIP, + NOPE, }; MatchType tryMatching(TileData const &other) const { // Check for strict equality first, as that can typically be optimized, and it allows @@ -1003,18 +1000,22 @@ static UniqueTiles dedupTiles( } auto [tileID, matchType] = tiles.addTile(std::move(tile)); - switch (matchType) { - case TileData::NOPE: - break; - case TileData::HFLIP: - case TileData::VFLIP: - case TileData::VHFLIP: - if (!options.allowMirroring) { - break; + + if (matchType != TileData::NOPE) { + static_assert( + (TileData::HFLIP & TileData::VFLIP) == 0, + "Cannot use `MatchType` as a bitfield!" + ); + uint8_t disallowed = (options.allowMirroringX ? 0 : TileData::HFLIP) + | (options.allowMirroringY ? 0 : TileData::VFLIP); + if (matchType & disallowed) { + error( + "The input tileset's tile #%hu was deduplicated; please check that your " + "deduplication flags (`-u`, `-m`) are consistent with what was used to " + "generate the input tileset", + tileID + ); } - [[fallthrough]]; - case TileData::EXACT: - error("The input tileset contains tiles that were deduplicated; please check that your deduplication flags (`-u`, `-m`) are consistent with what was used to generate the input tileset"); } } } @@ -1023,7 +1024,12 @@ static UniqueTiles dedupTiles( auto [tileID, matchType] = tiles.addTile({tile, palettes[mappings[attr.protoPaletteID]]}); if (matchType == TileData::NOPE && options.output.empty()) { - error("Tile at (%" PRIu32 ", %" PRIu32 ") is not within the input tileset, and `-o` was not given!", tile.x, tile.y); + error( + "Tile at (%" PRIu32 ", %" PRIu32 + ") is not within the input tileset, and `-o` was not given!", + tile.x, + tile.y + ); } attr.xFlip = matchType == TileData::HFLIP || matchType == TileData::VHFLIP; From ea1ddbefe97ac8306aa7e887d2330b8ffe4320e0 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Wed, 4 Sep 2024 00:29:31 +0200 Subject: [PATCH 05/12] Note that `-r` ignores `-i` --- man/rgbgfx.1 | 3 +++ src/gfx/reverse.cpp | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/man/rgbgfx.1 b/man/rgbgfx.1 index 9d361e44c..b15a59de0 100644 --- a/man/rgbgfx.1 +++ b/man/rgbgfx.1 @@ -203,6 +203,9 @@ may not generate an identical palette set for this image as for its input tilese See .Sx EXAMPLES for examples of how to use this option. +.Pp +This option is ignored in +.Sx REVERSE MODE . .It Fl L Ar slice , Fl \-slice Ar slice Only process a given rectangle of the image. This is useful for example if the input image is a sheet of some sort, and you want to convert each cel individually. diff --git a/src/gfx/reverse.cpp b/src/gfx/reverse.cpp index be9a934ce..a622ba4c1 100644 --- a/src/gfx/reverse.cpp +++ b/src/gfx/reverse.cpp @@ -136,10 +136,6 @@ void reverse() { ); } - if (!options.inputTileset.empty()) { - // TODO: check that the tile data is contained within the tileset - } - // By default, assume tiles are not deduplicated, and add the (allegedly) trimmed tiles size_t const nbTiles = tiles.size() / tileSize; options.verbosePrint(Options::VERB_INTERM, "Read %zu tiles.\n", nbTiles); From faed611ac5d418b9a490a9ee80198ca451eccf67 Mon Sep 17 00:00:00 2001 From: Eldred Habert Date: Wed, 4 Sep 2024 00:31:14 +0200 Subject: [PATCH 06/12] Improve some phrasing of the docs Co-authored-by: Sylvie <35663410+Rangi42@users.noreply.github.com> --- man/rgbgfx.1 | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/man/rgbgfx.1 b/man/rgbgfx.1 index b15a59de0..278e25bf7 100644 --- a/man/rgbgfx.1 +++ b/man/rgbgfx.1 @@ -166,21 +166,31 @@ for a list of formats and their descriptions. Set the bit depth of the output tile data, in bits per pixel (bpp), either 1 or 2 (the default). This changes how tile data is output, and the maximum number of colors per palette (2 and 4 respectively). .It Fl i Ar input_tiles , Fl \-input-tileset Ar input_tiles -Read tiles that will be used to convert this image, and that will always be given the first IDs. +Use the specified input tiles in addition to having +.Nm +automatically determine some. +The input tiles will always be first in +.Fl o +image output, and will always get the first IDs in the +.Fl t +tilemap output. .Ar input_tiles -must contain 2bpp tile data, as could be previously generated with the +must contain 1bpp or 2bpp tile data +.Pq whichever matches the Fl d No option used here , +as could be previously generated with the .Fl o option. .Pp -If used together with -.Fl o , -then the image can contain tiles not in -.Ar input_tiles . -Otherwise, the image must be able to be generated using +If the +.Fl o +option is also specified, then the input tiles will be assigned the first tile IDs, and any tiles from the input image that are not in the input tileset will be assigned subsequent IDs. +But if the +.Fl o +option is +.Em not +specified, then the tile map can .Em only -the tiles from -.Ar input_tiles , -and thus generate different tile data. +use tiles from the input tileset. Using .Fl o with From 03ba675d2acaa0da568f6c972f90bdbd26cc797e Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Wed, 4 Sep 2024 01:25:56 +0200 Subject: [PATCH 07/12] Add tests for `rgbgfx -i` --- test/gfx/input_tileset.flags | 3 +++ test/gfx/input_tileset.in.2bpp | Bin 0 -> 272 bytes test/gfx/input_tileset.out.2bpp | Bin 0 -> 320 bytes test/gfx/input_tileset.png | Bin 0 -> 691 bytes test/gfx/input_tileset_deduped.err | 2 ++ test/gfx/input_tileset_deduped.flags | 3 +++ test/gfx/input_tileset_deduped.in.2bpp | Bin 0 -> 272 bytes test/gfx/input_tileset_deduped.png | Bin 0 -> 691 bytes test/gfx/input_tileset_extra.err | 4 ++++ test/gfx/input_tileset_extra.flags | 2 ++ test/gfx/input_tileset_extra.in.2bpp | Bin 0 -> 272 bytes test/gfx/input_tileset_extra.png | Bin 0 -> 691 bytes 12 files changed, 14 insertions(+) create mode 100644 test/gfx/input_tileset.flags create mode 100644 test/gfx/input_tileset.in.2bpp create mode 100644 test/gfx/input_tileset.out.2bpp create mode 100644 test/gfx/input_tileset.png create mode 100644 test/gfx/input_tileset_deduped.err create mode 100644 test/gfx/input_tileset_deduped.flags create mode 100644 test/gfx/input_tileset_deduped.in.2bpp create mode 100644 test/gfx/input_tileset_deduped.png create mode 100644 test/gfx/input_tileset_extra.err create mode 100644 test/gfx/input_tileset_extra.flags create mode 100644 test/gfx/input_tileset_extra.in.2bpp create mode 100644 test/gfx/input_tileset_extra.png diff --git a/test/gfx/input_tileset.flags b/test/gfx/input_tileset.flags new file mode 100644 index 000000000..273127642 --- /dev/null +++ b/test/gfx/input_tileset.flags @@ -0,0 +1,3 @@ +-i input_tileset.in.2bpp +-u +-o result.2bpp diff --git a/test/gfx/input_tileset.in.2bpp b/test/gfx/input_tileset.in.2bpp new file mode 100644 index 0000000000000000000000000000000000000000..c26ee3b351d2f38255703cd22bbcd7a7135b9d4b GIT binary patch literal 272 zcmZXOy9om^3_#_$Q0WmgidV@X0UNT<-3m@0$O;-D5SJ>19XWSrIHK1G{hqv{poDtR z;YIcm!WH_}6!nR9%&}-Mv`}JIxb?Gg&aZN#t3T*sEfDM9pwIjsi6O@ZWGkP(n oS@&=-MZ56ykI8&NM)B%wckq}4xBEz$N-Qy_S^iBy&uP?|zL0Wt!2kdN literal 0 HcmV?d00001 diff --git a/test/gfx/input_tileset.out.2bpp b/test/gfx/input_tileset.out.2bpp new file mode 100644 index 0000000000000000000000000000000000000000..bb4aef59f0522fab7d6dc5abea9949e1bb444408 GIT binary patch literal 320 zcmZXPK?=e!5JjhEk!6mMiwL=jH*h5=2rYP@_6l(?cmc6FKx?}xjS%vkSX(I0+l0x? zycrRsmtOTVpf$bnGKfUX0u{9S)}*Fh?Pky;bj|%%l~swD$22XFXL@qXUnrW;WOh76 zyR?b)n?R9)WhYyIkEX>4Tx04R}tkv&MmKp2MKriwpQI#@)+AwzYti;6hbDionYs1;guFuC*(nlvOS zE{=k0!NH%!s)LKOt`4q(Aov5~?BJy6A|-y86k5c1$8itueecWNcYx5SFwN?U1DbA| z>10C8=2pd?SM(u>A(~;CS;m|srQkch?h)YoU5sb>*ZnyL)V#%jfJhu?hG`RT5YKGd z2Iqa^2rJ1d@j3B?Nf#u3-7k*wF$VPP0?oSPd>=bb^8^S!16O+6Uu^)hpQP8@ zTJ#7AZvz+CZB5w&E_Z<8Cqp)6R|?V+3I*W(jJ_!c4Bi6$YhK@4=Qw=;GSsWo4RCM> zj29_;-RIrCoqhYarq#b6Pc(9v8*?w-00009a7bBm001r{001r{0eGc9b^rhX2XskI zMF;2!0~R(b6m3Z-0000PbVXQnLvL+uWo~o;Lvm$dbY)~9cWHEJAV*0}P*;Ht7XSbN zok>JNR4C75%-ZY200Re0UNc|=_C2C-HVb~G$g&V^OlYdFoI7#l+zDjWXj~>VIUOSl zJgONO7?|)l22DRiB@-Uwu>wYILvseAfqjoC6IO|%FCQO$`53Dd6E4pY2qhgO3uFmv zaan6|S*+2Aa1xpxTp__?-y;eSSxh~c;l>C_OZbxpa<AOkPAKK4ej3+b|7c!EYWW Z00839N^x9VEuH`X002ovPDHLkV1f(r88ZL? literal 0 HcmV?d00001 diff --git a/test/gfx/input_tileset_deduped.err b/test/gfx/input_tileset_deduped.err new file mode 100644 index 000000000..f1cb734ea --- /dev/null +++ b/test/gfx/input_tileset_deduped.err @@ -0,0 +1,2 @@ +error: The input tileset's tile #7 was deduplicated; please check that your deduplication flags (`-u`, `-m`) are consistent with what was used to generate the input tileset +Conversion aborted after 1 error diff --git a/test/gfx/input_tileset_deduped.flags b/test/gfx/input_tileset_deduped.flags new file mode 100644 index 000000000..1ebfde88e --- /dev/null +++ b/test/gfx/input_tileset_deduped.flags @@ -0,0 +1,3 @@ +-i input_tileset_deduped.in.2bpp +-u +-o result.2bpp diff --git a/test/gfx/input_tileset_deduped.in.2bpp b/test/gfx/input_tileset_deduped.in.2bpp new file mode 100644 index 0000000000000000000000000000000000000000..13004ab5df48c0a5f22dba2ae9f2d9aca95725e9 GIT binary patch literal 272 zcmZXO!41P83`I>j_{a#1!YUo4QX+MqWCe1s)D=8HrE>Ja!Zk`jLiIaf+4%qO1sOS% zj>1d{jN%*B39K-!BZTudQb&u*_w#=3+rDXCYEu(nqW~Fy72qU7x|>L(mIV1GPz}xI lX|7kf-i&t`Uje3kX6`c*vKMDsjbnUxJX=dKr<~TMkAK-kZOH%t literal 0 HcmV?d00001 diff --git a/test/gfx/input_tileset_deduped.png b/test/gfx/input_tileset_deduped.png new file mode 100644 index 0000000000000000000000000000000000000000..cb1cc9ee79d639722d9cd55574f54797040e0240 GIT binary patch literal 691 zcmV;k0!;mhP)EX>4Tx04R}tkv&MmKp2MKriwpQI#@)+AwzYti;6hbDionYs1;guFuC*(nlvOS zE{=k0!NH%!s)LKOt`4q(Aov5~?BJy6A|-y86k5c1$8itueecWNcYx5SFwN?U1DbA| z>10C8=2pd?SM(u>A(~;CS;m|srQkch?h)YoU5sb>*ZnyL)V#%jfJhu?hG`RT5YKGd z2Iqa^2rJ1d@j3B?Nf#u3-7k*wF$VPP0?oSPd>=bb^8^S!16O+6Uu^)hpQP8@ zTJ#7AZvz+CZB5w&E_Z<8Cqp)6R|?V+3I*W(jJ_!c4Bi6$YhK@4=Qw=;GSsWo4RCM> zj29_;-RIrCoqhYarq#b6Pc(9v8*?w-00009a7bBm001r{001r{0eGc9b^rhX2XskI zMF;2!0~R(b6m3Z-0000PbVXQnLvL+uWo~o;Lvm$dbY)~9cWHEJAV*0}P*;Ht7XSbN zok>JNR4C75%-ZY200Re0UNc|=_C2C-HVb~G$g&V^OlYdFoI7#l+zDjWXj~>VIUOSl zJgONO7?|)l22DRiB@-Uwu>wYILvseAfqjoC6IO|%FCQO$`53Dd6E4pY2qhgO3uFmv zaan6|S*+2Aa1xpxTp__?-y;eSSxh~c;l>C_OZbxpa<AOkPAKK4ej3+b|7c!EYWW Z00839N^x9VEuH`X002ovPDHLkV1f(r88ZL? literal 0 HcmV?d00001 diff --git a/test/gfx/input_tileset_extra.err b/test/gfx/input_tileset_extra.err new file mode 100644 index 000000000..796f72141 --- /dev/null +++ b/test/gfx/input_tileset_extra.err @@ -0,0 +1,4 @@ +error: Tile at (0, 0) is not within the input tileset, and `-o` was not given! +error: Tile at (0, 8) is not within the input tileset, and `-o` was not given! +error: Tile at (8, 8) is not within the input tileset, and `-o` was not given! +Conversion aborted after 3 errors diff --git a/test/gfx/input_tileset_extra.flags b/test/gfx/input_tileset_extra.flags new file mode 100644 index 000000000..f7b31597e --- /dev/null +++ b/test/gfx/input_tileset_extra.flags @@ -0,0 +1,2 @@ +-i input_tileset.in.2bpp +-u diff --git a/test/gfx/input_tileset_extra.in.2bpp b/test/gfx/input_tileset_extra.in.2bpp new file mode 100644 index 0000000000000000000000000000000000000000..c26ee3b351d2f38255703cd22bbcd7a7135b9d4b GIT binary patch literal 272 zcmZXOy9om^3_#_$Q0WmgidV@X0UNT<-3m@0$O;-D5SJ>19XWSrIHK1G{hqv{poDtR z;YIcm!WH_}6!nR9%&}-Mv`}JIxb?Gg&aZN#t3T*sEfDM9pwIjsi6O@ZWGkP(n oS@&=-MZ56ykI8&NM)B%wckq}4xBEz$N-Qy_S^iBy&uP?|zL0Wt!2kdN literal 0 HcmV?d00001 diff --git a/test/gfx/input_tileset_extra.png b/test/gfx/input_tileset_extra.png new file mode 100644 index 0000000000000000000000000000000000000000..cb1cc9ee79d639722d9cd55574f54797040e0240 GIT binary patch literal 691 zcmV;k0!;mhP)EX>4Tx04R}tkv&MmKp2MKriwpQI#@)+AwzYti;6hbDionYs1;guFuC*(nlvOS zE{=k0!NH%!s)LKOt`4q(Aov5~?BJy6A|-y86k5c1$8itueecWNcYx5SFwN?U1DbA| z>10C8=2pd?SM(u>A(~;CS;m|srQkch?h)YoU5sb>*ZnyL)V#%jfJhu?hG`RT5YKGd z2Iqa^2rJ1d@j3B?Nf#u3-7k*wF$VPP0?oSPd>=bb^8^S!16O+6Uu^)hpQP8@ zTJ#7AZvz+CZB5w&E_Z<8Cqp)6R|?V+3I*W(jJ_!c4Bi6$YhK@4=Qw=;GSsWo4RCM> zj29_;-RIrCoqhYarq#b6Pc(9v8*?w-00009a7bBm001r{001r{0eGc9b^rhX2XskI zMF;2!0~R(b6m3Z-0000PbVXQnLvL+uWo~o;Lvm$dbY)~9cWHEJAV*0}P*;Ht7XSbN zok>JNR4C75%-ZY200Re0UNc|=_C2C-HVb~G$g&V^OlYdFoI7#l+zDjWXj~>VIUOSl zJgONO7?|)l22DRiB@-Uwu>wYILvseAfqjoC6IO|%FCQO$`53Dd6E4pY2qhgO3uFmv zaan6|S*+2Aa1xpxTp__?-y;eSSxh~c;l>C_OZbxpa<AOkPAKK4ej3+b|7c!EYWW Z00839N^x9VEuH`X002ovPDHLkV1f(r88ZL? literal 0 HcmV?d00001 From dba40110dedd2a374ee05d4e98b6219f5f097c69 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Wed, 4 Sep 2024 01:26:15 +0200 Subject: [PATCH 08/12] Wire up `-i` to the CLI --- src/gfx/main.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/gfx/main.cpp b/src/gfx/main.cpp index 7322a8fc2..8271c6f8a 100644 --- a/src/gfx/main.cpp +++ b/src/gfx/main.cpp @@ -108,7 +108,7 @@ void Options::verbosePrint(uint8_t level, char const *fmt, ...) const { } // Short options -static char const *optstring = "-Aa:b:Cc:Dd:FfhL:mN:n:Oo:Pp:Qq:r:s:Tt:U:uVvx:Z"; +static char const *optstring = "-Aa:b:Cc:Dd:Ffhi:L:mN:n:Oo:Pp:Qq:r:s:Tt:U:uVvx:Z"; /* * Equivalent long options @@ -127,6 +127,7 @@ static option const longopts[] = { {"color-curve", no_argument, nullptr, 'C'}, {"colors", required_argument, nullptr, 'c'}, {"depth", required_argument, nullptr, 'd'}, + {"input-tileset", required_argument, nullptr, 'i'}, {"slice", required_argument, nullptr, 'L'}, {"mirror-tiles", no_argument, nullptr, 'm'}, {"nb-tiles", required_argument, nullptr, 'N'}, @@ -154,9 +155,10 @@ static option const longopts[] = { static void printUsage() { fputs( "Usage: rgbgfx [-r stride] [-CmOuVXYZ] [-v [-v ...]] [-a | -A]\n" - " [-b ] [-c ] [-d ] [-L ] [-N ]\n" - " [-n ] [-o ] [-p | -P] [-q | -Q]\n" - " [-s ] [-t | -T] [-x ] \n" + " [-b ] [-c ] [-d ] [-i ] [-L ]\n" + " [-N ] [-n ] [-o ] [-p | -P]\n" + " [-q | -Q] [-s ] [-t | -T] [-x ]\n" + " \n" "Useful options:\n" " -m, --mirror-tiles optimize out mirrored tiles\n" " -o, --output output the tile data to this path\n" From 049c1ed1777302f40b86ddb00507fc15a978a9ac Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Wed, 4 Sep 2024 01:31:05 +0200 Subject: [PATCH 09/12] Fix detection of deduplicated tiles in input tileset Turns out `addTile` only returns match kinds depending on `options`... --- src/gfx/process.cpp | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index 2bdcefa4b..44973d869 100644 --- a/src/gfx/process.cpp +++ b/src/gfx/process.cpp @@ -771,11 +771,11 @@ class TileData { uint16_t hash() const { return _hash; } enum MatchType { + NOPE, EXACT, HFLIP, VFLIP, - VHFLIP = VFLIP | HFLIP, - NOPE, + VHFLIP, }; MatchType tryMatching(TileData const &other) const { // Check for strict equality first, as that can typically be optimized, and it allows @@ -1002,20 +1002,12 @@ static UniqueTiles dedupTiles( auto [tileID, matchType] = tiles.addTile(std::move(tile)); if (matchType != TileData::NOPE) { - static_assert( - (TileData::HFLIP & TileData::VFLIP) == 0, - "Cannot use `MatchType` as a bitfield!" + error( + "The input tileset's tile #%hu was deduplicated; please check that your " + "deduplication flags (`-u`, `-m`) are consistent with what was used to " + "generate the input tileset", + tileID ); - uint8_t disallowed = (options.allowMirroringX ? 0 : TileData::HFLIP) - | (options.allowMirroringY ? 0 : TileData::VFLIP); - if (matchType & disallowed) { - error( - "The input tileset's tile #%hu was deduplicated; please check that your " - "deduplication flags (`-u`, `-m`) are consistent with what was used to " - "generate the input tileset", - tileID - ); - } } } } From 165451087dab98a7555f268888563cee10858537 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Wed, 4 Sep 2024 02:15:23 +0200 Subject: [PATCH 10/12] Remove redundant `-o` flag Oops --- test/gfx/input_tileset.flags | 1 - 1 file changed, 1 deletion(-) diff --git a/test/gfx/input_tileset.flags b/test/gfx/input_tileset.flags index 273127642..f7b31597e 100644 --- a/test/gfx/input_tileset.flags +++ b/test/gfx/input_tileset.flags @@ -1,3 +1,2 @@ -i input_tileset.in.2bpp -u --o result.2bpp From a863c497ba193a81ff32ee1c219b074d770fb908 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Wed, 4 Sep 2024 09:29:56 +0200 Subject: [PATCH 11/12] Touch up the documentation one last time --- man/rgbgfx.1 | 6 +++--- src/gfx/main.cpp | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/man/rgbgfx.1 b/man/rgbgfx.1 index 278e25bf7..cdf95134c 100644 --- a/man/rgbgfx.1 +++ b/man/rgbgfx.1 @@ -169,7 +169,7 @@ This changes how tile data is output, and the maximum number of colors per palet Use the specified input tiles in addition to having .Nm automatically determine some. -The input tiles will always be first in +The input tiles will always be first in the .Fl o image output, and will always get the first IDs in the .Fl t @@ -208,7 +208,7 @@ along with .Fl i input_tiles . This is because .Nm -may not generate an identical palette set for this image as for its input tileset. +might not generate the same palette set for this image as it did for its input tileset. .Pp See .Sx EXAMPLES @@ -689,7 +689,7 @@ without needing an input image. .Pp .Dl $ rgbgfx -c '#fff,#ff0,#f80,#000' -p colors.pal .Pp -The following will convert two levels using the same tileset, and error out of any of the level images contain tiles not in the tileset. +The following will convert two level images using the same tileset, and error out if any of them contain tiles not in the tileset. .Pp .Bd -literal -offset Ds $ rgbgfx tileset.png -o tileset.2bpp -O -P diff --git a/src/gfx/main.cpp b/src/gfx/main.cpp index 8271c6f8a..96c2405af 100644 --- a/src/gfx/main.cpp +++ b/src/gfx/main.cpp @@ -155,10 +155,10 @@ static option const longopts[] = { static void printUsage() { fputs( "Usage: rgbgfx [-r stride] [-CmOuVXYZ] [-v [-v ...]] [-a | -A]\n" - " [-b ] [-c ] [-d ] [-i ] [-L ]\n" - " [-N ] [-n ] [-o ] [-p | -P]\n" - " [-q | -Q] [-s ] [-t | -T] [-x ]\n" - " \n" + " [-b ] [-c ] [-d ] [-i ]\n" + " [-L ] [-N ] [-n ] [-o ]\n" + " [-p | -P] [-q | -Q] [-s ]\n" + " [-t | -T] [-x ] \n" "Useful options:\n" " -m, --mirror-tiles optimize out mirrored tiles\n" " -o, --output output the tile data to this path\n" From e55177db997d13c05a9d63e42c13fbac912a426d Mon Sep 17 00:00:00 2001 From: Eldred Habert Date: Wed, 4 Sep 2024 18:53:56 +0200 Subject: [PATCH 12/12] Tweak documentation a bit Co-authored-by: Sylvie <35663410+Rangi42@users.noreply.github.com> --- man/rgbgfx.1 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/man/rgbgfx.1 b/man/rgbgfx.1 index cdf95134c..4f81f23b2 100644 --- a/man/rgbgfx.1 +++ b/man/rgbgfx.1 @@ -205,7 +205,7 @@ If the image will use more than one color palette, it is advised to generate the palette set along with the input tile data, and pass .Fl c Cm gbc: Ns Ar input_palette along with -.Fl i input_tiles . +.Fl i Ar input_tiles . This is because .Nm might not generate the same palette set for this image as it did for its input tileset. @@ -693,8 +693,8 @@ The following will convert two level images using the same tileset, and error ou .Pp .Bd -literal -offset Ds $ rgbgfx tileset.png -o tileset.2bpp -O -P -$ rgbgfx -i tileset.2bpp -c gbc:tileset.pal level1.png -t level1.tilemap -a level1.attrmap -$ rgbgfx -i tileset.2bpp -c gbc:tileset.pal level2.png -t level2.tilemap -a level2.attrmap +$ rgbgfx level1.png -i tileset.2bpp -c gbc:tileset.pal -t level1.tilemap -a level1.attrmap +$ rgbgfx level2.png -i tileset.2bpp -c gbc:tileset.pal -t level2.tilemap -a level2.attrmap .Ed .Sh BUGS Please report bugs and mistakes in this man page on