diff --git a/src/gfx/main.cpp b/src/gfx/main.cpp index 3fc56f03..7e5751b5 100644 --- a/src/gfx/main.cpp +++ b/src/gfx/main.cpp @@ -299,6 +299,10 @@ static void parseArg(int ch, char *arg) { break; case 'N': + // Explicit numbers for either tile bank cannot be greater than 256. + // If they were greater than 256, it would permit tile IDs to be truncated in the tilemap. + // We do warn that tile IDs may be truncated for the implicit/default unlimited number of + // tiles in bank 0. options.maxNbTiles[0] = readNumber(argPtr, "Number of tiles in bank 0", 256); if (options.maxNbTiles[0] > 256) { error("Bank 0 cannot contain more than 256 tiles"); diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index 270e2a2e..2537790c 100644 --- a/src/gfx/process.cpp +++ b/src/gfx/process.cpp @@ -502,14 +502,14 @@ static void outputPalettes(std::vector const &palettes) { }); // LCOV_EXCL_STOP - if (palettes.size() > options.nbPalettes) { + if (size_t nbPals = palettes.size(); nbPals > options.nbPalettes) { // If the palette generation is wrong, other (dependee) operations are likely to be // nonsensical, so fatal-error outright - fatal( - "Generated %zu palettes, over the maximum of %" PRIu16, - palettes.size(), - options.nbPalettes - ); + fatal("Generated %zu palettes, over the maximum of %" PRIu16, nbPals, options.nbPalettes); + } else if (nbPals > 8 && !options.attrmap.empty() && options.palmap.empty()) { + // With `-n/--nb-palettes` greater than 8, palette IDs may be truncated in the attrmap + // (though not in the palmap), so warn about that. + warnx("Generated %zu palettes, of which only 8 are representable in the attrmap", nbPals); } if (!options.palettes.empty()) { @@ -745,6 +745,8 @@ static void outputUnoptimizedMaps( for (AttrmapEntry const &attr : attrmap) { // A non-zero base ID may make this addition overflow, wrapping around the available // palette IDs. Since the operands are unsigned, this won't cause undefined behavior. + // With `-n/--nb-palettes` greater than 8, palette IDs may be truncated in the attrmap + // (though not in the palmap), which was already warned about. uint8_t palID = attr.getPalID(mappings) + options.basePalID; if (attr.isBackgroundTile()) { // The tile bank may be 2 here, which is fine since background tiles are emitted as @@ -761,6 +763,8 @@ static void outputUnoptimizedMaps( // A non-zero base ID may make this addition overflow, wrapping around the available // tile IDs. Since the operands are unsigned, this won't cause undefined behavior. + // With `-N/--nb-tiles` unlimited (by default) for bank 0, tile IDs may be truncated in + // the tilemap, which was already warned about. uint8_t tileID = tileIdx + options.baseTileIDs[bank]; emit(tilemapOutput, tileID); emit(attrmapOutput, (palID & 0b111) | bank << 3); // The other flags are all zeros. @@ -940,6 +944,8 @@ static void outputTilemap(std::vector const &attrmap) { // LCOV_EXCL_STOP } + // With `-N/--nb-tiles` unlimited (by default) for bank 0, tile IDs may be truncated in the + // tilemap, which was already warned about. for (AttrmapEntry const &entry : attrmap) { output->sputc(entry.tileID); // The tile ID has already been converted } @@ -959,6 +965,8 @@ static void attr |= entry.bank << 3; // The unsigned underflow for the palette ID is intentional, since a // nonzero base palette ID may overflow and continue with IDs from 0. + // With `-n/--nb-palettes` greater than 8, palette IDs may be truncated in the attrmap + // (though not in the palmap), which was already warned about. attr |= (entry.getPalID(mappings) + options.basePalID) & 0b111; output->sputc(attr); } @@ -1191,20 +1199,31 @@ continue_visiting_tiles:; : makePalsAsSpecified(colorSets); outputPalettes(palettes); - // If deduplication is not happening, we just need to output the tile data and/or maps as-is - if (!options.allowDedup) { - // Check the tile count - if (size_t nbTiles = std::count_if( - RANGE(attrmap), [](AttrmapEntry const &attr) { return !attr.isBackgroundTile(); } - ); - nbTiles > options.maxNbTiles[0] + options.maxNbTiles[1]) { + auto checkTileCountLimit = [](size_t nbTiles) { + if (nbTiles > options.maxNbTiles[0] + options.maxNbTiles[1]) { fatal( "Image contains %zu tiles, exceeding the limit of %" PRIu16 " + %" PRIu16, nbTiles, options.maxNbTiles[0], options.maxNbTiles[1] ); + } else if (((nbTiles > 256 && options.maxNbTiles[0] > 256) + || (nbTiles > options.maxNbTiles[0] + 256u && options.maxNbTiles[1] > 256)) + && !options.tilemap.empty()) { + // With `-N/--nb-tiles` unlimited (by default) for bank 0, tile IDs may be truncated in + // the tilemap, so warn about that. + warnx( + "Image contains %zu tiles, of which only 256 are representable in the tilemap", + nbTiles + ); } + }; + + // If deduplication is not happening, we just need to output the tile data and/or maps as-is + if (!options.allowDedup) { + checkTileCountLimit(std::count_if(RANGE(attrmap), [](AttrmapEntry const &attr) { + return !attr.isBackgroundTile(); + })); // I currently cannot figure out useful semantics for this combination of flags. if (!options.inputTileset.empty()) { @@ -1227,16 +1246,7 @@ continue_visiting_tiles:; verbosePrint(VERB_NOTICE, "Deduplicating tiles...\n"); UniqueTiles tiles = dedupTiles(image, attrmap, palettes, mappings); - // Check the tile count - if (size_t nbTiles = tiles.size(); - nbTiles > options.maxNbTiles[0] + options.maxNbTiles[1]) { - fatal( - "Image contains %zu tiles, exceeding the limit of %" PRIu16 " + %" PRIu16, - nbTiles, - options.maxNbTiles[0], - options.maxNbTiles[1] - ); - } + checkTileCountLimit(tiles.size()); if (!options.output.empty()) { verbosePrint(VERB_NOTICE, "Generating optimized tile data...\n"); diff --git a/test/gfx/attrmap_overflow.err b/test/gfx/attrmap_overflow.err new file mode 100644 index 00000000..a77701a7 --- /dev/null +++ b/test/gfx/attrmap_overflow.err @@ -0,0 +1 @@ +warning: Generated 9 palettes, of which only 8 are representable in the attrmap diff --git a/test/gfx/attrmap_overflow.flags b/test/gfx/attrmap_overflow.flags new file mode 100644 index 00000000..333885a4 --- /dev/null +++ b/test/gfx/attrmap_overflow.flags @@ -0,0 +1 @@ +-n 9 diff --git a/test/gfx/attrmap_overflow.out.2bpp b/test/gfx/attrmap_overflow.out.2bpp new file mode 100644 index 00000000..66724819 --- /dev/null +++ b/test/gfx/attrmap_overflow.out.2bpp @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/test/gfx/attrmap_overflow.out.attrmap b/test/gfx/attrmap_overflow.out.attrmap new file mode 100644 index 00000000..2257d9da Binary files /dev/null and b/test/gfx/attrmap_overflow.out.attrmap differ diff --git a/test/gfx/attrmap_overflow.out.tilemap b/test/gfx/attrmap_overflow.out.tilemap new file mode 100644 index 00000000..fd5323fa Binary files /dev/null and b/test/gfx/attrmap_overflow.out.tilemap differ diff --git a/test/gfx/attrmap_overflow.png b/test/gfx/attrmap_overflow.png new file mode 100644 index 00000000..150c0fa1 Binary files /dev/null and b/test/gfx/attrmap_overflow.png differ diff --git a/test/gfx/tilemap_dedup_overflow.err b/test/gfx/tilemap_dedup_overflow.err new file mode 100644 index 00000000..8ad12e40 --- /dev/null +++ b/test/gfx/tilemap_dedup_overflow.err @@ -0,0 +1 @@ +warning: Image contains 320 tiles, of which only 256 are representable in the tilemap diff --git a/test/gfx/tilemap_dedup_overflow.out.2bpp b/test/gfx/tilemap_dedup_overflow.out.2bpp new file mode 100644 index 00000000..306e81ef Binary files /dev/null and b/test/gfx/tilemap_dedup_overflow.out.2bpp differ diff --git a/test/gfx/tilemap_dedup_overflow.out.attrmap b/test/gfx/tilemap_dedup_overflow.out.attrmap new file mode 100644 index 00000000..4c2a1b23 Binary files /dev/null and b/test/gfx/tilemap_dedup_overflow.out.attrmap differ diff --git a/test/gfx/tilemap_dedup_overflow.out.tilemap b/test/gfx/tilemap_dedup_overflow.out.tilemap new file mode 100644 index 00000000..e12a5cab Binary files /dev/null and b/test/gfx/tilemap_dedup_overflow.out.tilemap differ diff --git a/test/gfx/tilemap_dedup_overflow.png b/test/gfx/tilemap_dedup_overflow.png new file mode 100644 index 00000000..8424f8fa Binary files /dev/null and b/test/gfx/tilemap_dedup_overflow.png differ diff --git a/test/gfx/tilemap_overflow.err b/test/gfx/tilemap_overflow.err new file mode 100644 index 00000000..8ad12e40 --- /dev/null +++ b/test/gfx/tilemap_overflow.err @@ -0,0 +1 @@ +warning: Image contains 320 tiles, of which only 256 are representable in the tilemap diff --git a/test/gfx/tilemap_overflow.out.2bpp b/test/gfx/tilemap_overflow.out.2bpp new file mode 100644 index 00000000..6a63e0d6 Binary files /dev/null and b/test/gfx/tilemap_overflow.out.2bpp differ diff --git a/test/gfx/tilemap_overflow.out.attrmap b/test/gfx/tilemap_overflow.out.attrmap new file mode 100644 index 00000000..4c2a1b23 Binary files /dev/null and b/test/gfx/tilemap_overflow.out.attrmap differ diff --git a/test/gfx/tilemap_overflow.out.tilemap b/test/gfx/tilemap_overflow.out.tilemap new file mode 100644 index 00000000..e12a5cab Binary files /dev/null and b/test/gfx/tilemap_overflow.out.tilemap differ diff --git a/test/gfx/tilemap_overflow.png b/test/gfx/tilemap_overflow.png new file mode 100644 index 00000000..ef474566 Binary files /dev/null and b/test/gfx/tilemap_overflow.png differ