Fix a crash and a limitation in RGBGFX unoptimized output

- If both banks 0 and 1 were completely filled, then
  `assume(bank == 0)` would false and potentially crash.
- If the input image had fully-background tiles, they would be
  incorrectly counted towards `nbTiles` and could cause the
  "Image contains N tiles, exceeding the limit" check to fail.
This commit is contained in:
Rangi
2026-07-11 12:32:31 -04:00
parent 5525dd902b
commit 3b33c942ae
7 changed files with 21 additions and 14 deletions
+18 -14
View File
@@ -700,12 +700,17 @@ static void outputUnoptimizedMaps(
autoOpenPath(options.attrmap, attrmapOutput); autoOpenPath(options.attrmap, attrmapOutput);
autoOpenPath(options.palmap, palmapOutput); autoOpenPath(options.palmap, palmapOutput);
uint8_t tileID = 0; uint8_t tileIdx = 0;
uint8_t bank = 0; uint8_t bank = 0;
for (AttrmapEntry const &attr : attrmap) { for (AttrmapEntry const &attr : attrmap) {
// The update-increment logic at the end of this loop may increment `bank` from 1 to 2,
// if both banks 0 and 1 are full, but by then all the `attrmap` entries should have been
// processed, since there cannot be more tiles than could fit in both banks.
assume(bank < 2);
if (tilemapOutput.has_value()) { if (tilemapOutput.has_value()) {
(*tilemapOutput) uint8_t tileID = (attr.isBackgroundTile() ? 0 : tileIdx) + options.baseTileIDs[bank];
->sputc((attr.isBackgroundTile() ? 0 : tileID) + options.baseTileIDs[bank]); (*tilemapOutput)->sputc(tileID);
} }
uint8_t palID = attr.getPalID(mappings) + options.basePalID; uint8_t palID = attr.getPalID(mappings) + options.basePalID;
if (attrmapOutput.has_value()) { if (attrmapOutput.has_value()) {
@@ -715,18 +720,16 @@ static void outputUnoptimizedMaps(
(*palmapOutput)->sputc(palID); (*palmapOutput)->sputc(palID);
} }
// Background tiles are skipped in the tile data, so they should be skipped in the maps too. // Background tiles were not emitted in the tile data, so their ID and bank do not update.
if (attr.isBackgroundTile()) { if (attr.isBackgroundTile()) {
continue; continue;
} }
// Compare with `maxNbTiles` *before* incrementing, due to unsigned overflow! if (tileIdx + 1 < options.maxNbTiles[bank]) {
if (tileID + 1 < options.maxNbTiles[bank]) { ++tileIdx;
++tileID;
} else { } else {
assume(bank == 0); ++bank;
bank = 1; tileIdx = 0;
tileID = 0;
} }
} }
} }
@@ -1158,13 +1161,13 @@ continue_visiting_tiles:;
// If deduplication is not happening, we just need to output the tile data and/or maps as-is // If deduplication is not happening, we just need to output the tile data and/or maps as-is
if (!options.allowDedup) { if (!options.allowDedup) {
uint32_t const nbTilesH = image.png.height / 8, nbTilesW = image.png.width / 8;
// Check the tile count // Check the tile count
if (uint64_t nbTiles = nbTilesW * nbTilesH; if (size_t nbTiles = std::count_if(
RANGE(attrmap), [](AttrmapEntry const &attr) { return !attr.isBackgroundTile(); }
);
nbTiles > options.maxNbTiles[0] + options.maxNbTiles[1]) { nbTiles > options.maxNbTiles[0] + options.maxNbTiles[1]) {
fatal( fatal(
"Image contains %" PRIu64 " tiles, exceeding the limit of %" PRIu16 " + %" PRIu16, "Image contains %zu tiles, exceeding the limit of %" PRIu16 " + %" PRIu16,
nbTiles, nbTiles,
options.maxNbTiles[0], options.maxNbTiles[0],
options.maxNbTiles[1] options.maxNbTiles[1]
@@ -1192,6 +1195,7 @@ continue_visiting_tiles:;
verbosePrint(VERB_NOTICE, "Deduplicating tiles...\n"); verbosePrint(VERB_NOTICE, "Deduplicating tiles...\n");
UniqueTiles tiles = dedupTiles(image, attrmap, palettes, mappings); UniqueTiles tiles = dedupTiles(image, attrmap, palettes, mappings);
// Check the tile count
if (size_t nbTiles = tiles.size(); if (size_t nbTiles = tiles.size();
nbTiles > options.maxNbTiles[0] + options.maxNbTiles[1]) { nbTiles > options.maxNbTiles[0] + options.maxNbTiles[1]) {
fatal( fatal(
+3
View File
@@ -0,0 +1,3 @@
-B transparent
-c dmg
-N 256,256
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

After

Width:  |  Height:  |  Size: 3.6 KiB