From 9e785495ffaa1807a66b0e719d7706a55cae20b0 Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:04:31 -0400 Subject: [PATCH] Fix crash when a background tile first occurs after both tile banks are full (#2070) Co-authored-by: ISSOtm --- src/gfx/process.cpp | 57 ++++++++++++---------- test/gfx/bg_after_tiles.flags | 1 + test/gfx/bg_after_tiles.out.2bpp | Bin 0 -> 256 bytes test/gfx/bg_after_tiles.out.attrmap | Bin 0 -> 17 bytes test/gfx/bg_after_tiles.out.pal | Bin 0 -> 32 bytes test/gfx/bg_after_tiles.out.tilemap | Bin 0 -> 17 bytes test/gfx/bg_after_tiles.png | Bin 0 -> 145 bytes test/gfx/complete_unoptimized.out.attrmap | Bin 544 -> 544 bytes 8 files changed, 33 insertions(+), 25 deletions(-) create mode 100644 test/gfx/bg_after_tiles.flags create mode 100644 test/gfx/bg_after_tiles.out.2bpp create mode 100644 test/gfx/bg_after_tiles.out.attrmap create mode 100644 test/gfx/bg_after_tiles.out.pal create mode 100644 test/gfx/bg_after_tiles.out.tilemap create mode 100644 test/gfx/bg_after_tiles.png diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index 81f82ab1..8fb4bd18 100644 --- a/src/gfx/process.cpp +++ b/src/gfx/process.cpp @@ -730,38 +730,45 @@ static void outputUnoptimizedMaps( autoOpenPath(options.attrmap, attrmapOutput); autoOpenPath(options.palmap, palmapOutput); + auto const emit = [](std::optional &output, uint8_t byte) { + if (output.has_value()) { + output.value()->sputc(byte); + } + }; uint16_t tileIdx = 0; uint8_t bank = 0; 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); - - // The unsigned overflow for `tileID` and `palID` is intentional, since - // nonzero base IDs may overflow beyond 255 and continue with IDs from 0. - if (tilemapOutput.has_value()) { - uint8_t tileID = (attr.isBackgroundTile() ? 0 : tileIdx) + options.baseTileIDs[bank]; - (*tilemapOutput)->sputc(tileID); - } + // 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. uint8_t palID = attr.getPalID(mappings) + options.basePalID; - if (attrmapOutput.has_value()) { - (*attrmapOutput)->sputc((palID & 0b111) | bank << 3); // The other flags are all 0 - } - if (palmapOutput.has_value()) { - (*palmapOutput)->sputc(palID); - } - - // Background tiles were not emitted in the tile data, so their ID and bank do not update. if (attr.isBackgroundTile()) { - continue; - } + // The tile bank may be 2 here, which is fine since background tiles are emitted as + // if they used the base tile ID and bank 0. + assume(bank <= 2); - if (tileIdx + 1 < options.maxNbTiles[bank]) { - ++tileIdx; + emit(tilemapOutput, options.baseTileIDs[0]); + emit(attrmapOutput, palID & 0b111); // The other flags are all zeros. + emit(palmapOutput, palID); + // Since background tiles are not in tile data, they do not increment the tile index. } else { - ++bank; - tileIdx = 0; + // The only valid tile banks are 0 and 1. + assume(bank < 2); + + // 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. + uint8_t tileID = tileIdx + options.baseTileIDs[bank]; + emit(tilemapOutput, tileID); + emit(attrmapOutput, (palID & 0b111) | bank << 3); // The other flags are all zeros. + emit(palmapOutput, palID); + + ++tileIdx; + // The `bank` may increment from 1 to 2, if banks 0 and 1 are both full. By then all + // the tiles should have been emitted, since there cannot be more tiles than could fit + // in both banks, but there may still be background tiles to skip. + if (tileIdx >= options.maxNbTiles[bank]) { + tileIdx = 0; + ++bank; + } } } } diff --git a/test/gfx/bg_after_tiles.flags b/test/gfx/bg_after_tiles.flags new file mode 100644 index 00000000..b003d4a5 --- /dev/null +++ b/test/gfx/bg_after_tiles.flags @@ -0,0 +1 @@ +-N 8,8 -B #0080ff diff --git a/test/gfx/bg_after_tiles.out.2bpp b/test/gfx/bg_after_tiles.out.2bpp new file mode 100644 index 0000000000000000000000000000000000000000..3a40dfe02c39164f711b92856584505b84c260d8 GIT binary patch literal 256 scmZSh&wvUUP{DsRfGP{*q3ZsR#>J3F*NbRQW2gmjv*Cu-cH;o$e_q`_=Nwo zi<@q4j*L*yT2YWV^~R5)$r(@3m#2!*(ebRVn)uAdHhzWKXcO2eLy=HJYD@<);T3K0RTBTII92v literal 0 HcmV?d00001 diff --git a/test/gfx/complete_unoptimized.out.attrmap b/test/gfx/complete_unoptimized.out.attrmap index c9721d31760c03fe7ed883cef7daeb897011dd7f..5aee6379f33de787a5ef16b74c364edbad0e3232 100644 GIT binary patch literal 544 XcmZQz7zM~7z<~-FP`Ml