From 7746ddbf2b61eaffd52760a6cdc541ba531c6ab9 Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:26:57 -0400 Subject: [PATCH] Fix reverse-graphics check for whether the tilemap references too-high tile IDs in bank 1 (#2086) --- src/gfx/reverse.cpp | 44 ++++++++++++++++++--------- test/gfx/reverse_tilemap_oob.2bpp | Bin 0 -> 128 bytes test/gfx/reverse_tilemap_oob.attrmap | Bin 0 -> 4 bytes test/gfx/reverse_tilemap_oob.err | 2 ++ test/gfx/reverse_tilemap_oob.flags | 4 +++ test/gfx/reverse_tilemap_oob.tilemap | Bin 0 -> 4 bytes test/gfx/test.sh | 12 ++++++-- 7 files changed, 44 insertions(+), 18 deletions(-) create mode 100644 test/gfx/reverse_tilemap_oob.2bpp create mode 100644 test/gfx/reverse_tilemap_oob.attrmap create mode 100644 test/gfx/reverse_tilemap_oob.err create mode 100644 test/gfx/reverse_tilemap_oob.flags create mode 100644 test/gfx/reverse_tilemap_oob.tilemap diff --git a/src/gfx/reverse.cpp b/src/gfx/reverse.cpp index 496a6dab..4bc1de96 100644 --- a/src/gfx/reverse.cpp +++ b/src/gfx/reverse.cpp @@ -279,7 +279,7 @@ void reverse() { } std::optional> attrmap; - uint16_t nbTilesInBank[2] = {0, 0}; // Only used if there is an attrmap. + uint16_t nbTilesMappedInBank[2] = {0, 0}; // Only used if there is an attrmap. if (!options.attrmap.empty()) { attrmap = readInto(options.attrmap); if (attrmap->size() != mapSize) { @@ -329,8 +329,8 @@ void reverse() { // The unsigned underflow for `tileOfs` is intentional, since a nonzero // base tile ID may overflow and continue with IDs from 0. if (uint8_t tileOfs = (*tilemap)[index] - options.baseTileIDs[bank]; - tileOfs >= nbTilesInBank[bank]) { - nbTilesInBank[bank] = tileOfs + 1; + tileOfs >= nbTilesMappedInBank[bank]) { + nbTilesMappedInBank[bank] = tileOfs + 1; } } } @@ -338,29 +338,43 @@ void reverse() { verbosePrint( VERB_INFO, "Number of tiles in bank {0: %" PRIu16 ", 1: %" PRIu16 "}\n", - nbTilesInBank[0], - nbTilesInBank[1] + nbTilesMappedInBank[0], + nbTilesMappedInBank[1] ); for (int bank = 0; bank < 2; ++bank) { - if (nbTilesInBank[bank] > options.maxNbTiles[bank]) { + if (nbTilesMappedInBank[bank] > options.maxNbTiles[bank]) { error( "Bank %d contains %" PRIu16 " tiles, but the specified limit is %" PRIu16, bank, - nbTilesInBank[bank], + nbTilesMappedInBank[bank], options.maxNbTiles[bank] ); } } - if (nbTilesInBank[0] + nbTilesInBank[1] > nbTiles + options.trim) { - fatal( - "The tilemap references %" PRIu16 " tiles in bank 0 and %" PRIu16 - " in bank 1, but only %zu have been read in total", - nbTilesInBank[0], - nbTilesInBank[1], - nbTiles - ); + if (uint16_t const maxTotalNbTiles = + nbTilesMappedInBank[1] > 0 + ? std::max( + nbTilesMappedInBank[0], options.maxNbTiles[0] + nbTilesMappedInBank[1] + ) + : nbTilesMappedInBank[0]; + maxTotalNbTiles > nbTiles + options.trim) { + std::string message = + "The tilemap references " + std::to_string(nbTilesMappedInBank[0]) + " tiles"; + if (nbTilesMappedInBank[1] > 0) { + if (nbTilesMappedInBank[0] != options.maxNbTiles[0]) { + message += " out of a maximum " + std::to_string(options.maxNbTiles[0]); + } + message += " in bank 0, and " + std::to_string(nbTilesMappedInBank[1]) + + " in bank 1 (total: " + std::to_string(maxTotalNbTiles) + ")"; + } + message += ", but only " + std::to_string(nbTiles) + " have been read"; + if (options.trim > 0) { + message += " plus " + std::to_string(options.trim) + + " trimmed (total: " + std::to_string(nbTiles + options.trim) + ")"; + } + fatal("%s", message.c_str()); } requireZeroErrors(); diff --git a/test/gfx/reverse_tilemap_oob.2bpp b/test/gfx/reverse_tilemap_oob.2bpp new file mode 100644 index 0000000000000000000000000000000000000000..079d05719ef4e0328c9bca89f77aed4d49c421f1 GIT binary patch literal 128 ccmZQzKn4F9P$3%kKRQ6w&48{PLjYYr02CuX!2kdN literal 0 HcmV?d00001 diff --git a/test/gfx/reverse_tilemap_oob.attrmap b/test/gfx/reverse_tilemap_oob.attrmap new file mode 100644 index 0000000000000000000000000000000000000000..8c2a6c0ef8c845cd8c29067a821a08ca1abbac19 GIT binary patch literal 4 LcmZQz;NSoN02}}j literal 0 HcmV?d00001 diff --git a/test/gfx/reverse_tilemap_oob.err b/test/gfx/reverse_tilemap_oob.err new file mode 100644 index 00000000..02c0fcf4 --- /dev/null +++ b/test/gfx/reverse_tilemap_oob.err @@ -0,0 +1,2 @@ +FATAL: The tilemap references 2 tiles out of a maximum 8 in bank 0, and 2 in bank 1 (total: 10), but only 8 have been read plus 1 trimmed (total: 9) +Conversion aborted after 1 error diff --git a/test/gfx/reverse_tilemap_oob.flags b/test/gfx/reverse_tilemap_oob.flags new file mode 100644 index 00000000..8a40d5b0 --- /dev/null +++ b/test/gfx/reverse_tilemap_oob.flags @@ -0,0 +1,4 @@ +-N 8,8 +-x 1 +-t reverse_tilemap_oob.tilemap +-a reverse_tilemap_oob.attrmap diff --git a/test/gfx/reverse_tilemap_oob.tilemap b/test/gfx/reverse_tilemap_oob.tilemap new file mode 100644 index 0000000000000000000000000000000000000000..693ddc9ccea2f9aa65dd6de5f6d8836eea782945 GIT binary patch literal 4 LcmZQzWMBjU00;mB literal 0 HcmV?d00001 diff --git a/test/gfx/test.sh b/test/gfx/test.sh index ae608ad4..bcd0c170 100755 --- a/test/gfx/test.sh +++ b/test/gfx/test.sh @@ -104,14 +104,20 @@ done for f in *.[12]bpp; do # Do not process outputs or sample outputs of other tests as test inputs themselves - if [[ "$f" = result.[12]bpp ]] || [[ "$f" = *.out.[12]bpp ]]; then + if [[ "$f" = result.[12]bpp ]] || [[ "$f" = *.in.[12]bpp ]] || [[ "$f" = *.out.[12]bpp ]]; then continue fi flags="$([[ -e "${f%.[12]bpp}.flags" ]] && echo "@${f%.[12]bpp}.flags") $([[ -e "${f%.1bpp}.flags" ]] && echo "-d 1")" - newTest "$RGBGFX $flags -o $f -r 1 result.png && $RGBGFX $flags -o result.2bpp result.png" - runTest && tryCmp "$f" result.2bpp || failTest $? + if [[ -e "${f%.[12]bpp}.err" ]]; then + newTest "$RGBGFX $flags -o $f -r 1 result.png" + runTest 2>"$errtmp" + diff -au --strip-trailing-cr <(sed "s/$f//g" "${f%.[12]bpp}.err") "$errtmp" || failTest + else + newTest "$RGBGFX $flags -o $f -r 1 result.png && $RGBGFX $flags -o result.2bpp result.png" + runTest && tryCmp "$f" result.2bpp || failTest $? + fi done # Test writing to stdout