From e9fc7b3135ebf8f0eb4b3f1ab528bb1dd4bf2ea5 Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Sun, 13 Sep 2026 13:18:24 -0400 Subject: [PATCH] Warn when tile or palette IDs cannot fit in the tilemap or attrmap respectively (#2077) Co-authored-by: Eldred Habert --- src/gfx/main.cpp | 4 ++ src/gfx/process.cpp | 56 ++++++++++++-------- test/gfx/attrmap_overflow.err | 1 + test/gfx/attrmap_overflow.flags | 1 + test/gfx/attrmap_overflow.out.2bpp | 1 + test/gfx/attrmap_overflow.out.attrmap | Bin 0 -> 9 bytes test/gfx/attrmap_overflow.out.tilemap | Bin 0 -> 9 bytes test/gfx/attrmap_overflow.png | Bin 0 -> 470 bytes test/gfx/tilemap_dedup_overflow.err | 1 + test/gfx/tilemap_dedup_overflow.out.2bpp | Bin 0 -> 5120 bytes test/gfx/tilemap_dedup_overflow.out.attrmap | Bin 0 -> 320 bytes test/gfx/tilemap_dedup_overflow.out.tilemap | Bin 0 -> 320 bytes test/gfx/tilemap_dedup_overflow.png | Bin 0 -> 997 bytes test/gfx/tilemap_overflow.err | 1 + test/gfx/tilemap_overflow.out.2bpp | Bin 0 -> 5120 bytes test/gfx/tilemap_overflow.out.attrmap | Bin 0 -> 320 bytes test/gfx/tilemap_overflow.out.tilemap | Bin 0 -> 320 bytes test/gfx/tilemap_overflow.png | Bin 0 -> 512 bytes 18 files changed, 42 insertions(+), 23 deletions(-) create mode 100644 test/gfx/attrmap_overflow.err create mode 100644 test/gfx/attrmap_overflow.flags create mode 100644 test/gfx/attrmap_overflow.out.2bpp create mode 100644 test/gfx/attrmap_overflow.out.attrmap create mode 100644 test/gfx/attrmap_overflow.out.tilemap create mode 100644 test/gfx/attrmap_overflow.png create mode 100644 test/gfx/tilemap_dedup_overflow.err create mode 100644 test/gfx/tilemap_dedup_overflow.out.2bpp create mode 100644 test/gfx/tilemap_dedup_overflow.out.attrmap create mode 100644 test/gfx/tilemap_dedup_overflow.out.tilemap create mode 100644 test/gfx/tilemap_dedup_overflow.png create mode 100644 test/gfx/tilemap_overflow.err create mode 100644 test/gfx/tilemap_overflow.out.2bpp create mode 100644 test/gfx/tilemap_overflow.out.attrmap create mode 100644 test/gfx/tilemap_overflow.out.tilemap create mode 100644 test/gfx/tilemap_overflow.png 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 0000000000000000000000000000000000000000..2257d9da0b8b2d9954007d52338a097f3d7c0e75 GIT binary patch literal 9 QcmZQzWMXDvWn*Um004Oa9RL6T literal 0 HcmV?d00001 diff --git a/test/gfx/attrmap_overflow.out.tilemap b/test/gfx/attrmap_overflow.out.tilemap new file mode 100644 index 0000000000000000000000000000000000000000..fd5323fa84b728194bfe7992a17d3ae5a00d7f5f GIT binary patch literal 9 QcmZQzWMXDvWn(^b literal 0 HcmV?d00001 diff --git a/test/gfx/attrmap_overflow.png b/test/gfx/attrmap_overflow.png new file mode 100644 index 0000000000000000000000000000000000000000..150c0fa109730554ae696347017f678b369659f8 GIT binary patch literal 470 zcmeAS@N?(olHy`uVBq!ia0vp^9ze{&!2~3)Ue~JuQjEnx?oJHr&dIz4a#+$GeH|GX zHuiJ>Nn{1`ISV`@iy0XB4ude`@%$AjKtah8*NBqf{Irtt#G+J&fW*wa5~kXKn^Q|6eBAGBar0<#L`eU$T=E}%wTblgA5s&7zBZI z6cA^&vw+2OfNT&*0Ai4S7>#Z#0|PSy!vuB)7N8mfBVz-`1rT#VdRP}g%$f#dg8&oI zBqp%RAWI7%3#!Y|zyKtBbNQ^PC%1FQ1DQ>pE{-7<{%@yPiycwmU}(3U>27TPIc>Jn z&x~m%D<9pi3~62@!XW*5?#h|^o!7a$)xRol{57dUcgv4SKfGA&9v2GspHP$&7E+4R zH~AgE+>x*Ur2W4>vu(2fkKJoa*ev eYNK7&rOEr199vZ1*T%OG_QhI zL_|b}LC7MM5=0qX46}35g>;dJbTPOS|A6+K@4kD#`^@9PoSbvdchC3Zy{AYaXVS)X zCqD`QUCOd7;xb&wIDR)OxF%zc4^e*yJXY2h(n>hj6K8+=(SOnPtdG3r^qPu;0>-}q-ayzT>o3(QTJ;lY&`eFZSbcb{TE!%`aC|@mwr~y^(TKC^xdH6 z`tRZ1@cuPV&-E?oR{B?5cb@tajFI>Mtjs&s{09}hE+dXL|A@SBKjzK6=09=0=AXwL z*m^QwFwM&3_>4-oNJQx}JFt!uX2!oafo%nBSD=pJ#K~agok?tAcA- zAJZSEvpA0YT3@8ImU+czuBRXUXI;;;A9>2K$^6?m&so2gPJavet$)qab$tW+ zRv2ILp7Y#W^vmnNkn-Dnb1$+!1|M`ENY;%5cyVXMa8aeEyrB^^sRU8;|+P9|wIm=$U^nJ^$5@`H#R~xt`}e z_OE!~dA=|6eCqpu+2@!2-eHV9f8%)9^8fPw=Xo9T`J^`UoNu0Q)6w3*=ddt7({BgPdKa~4_F6F-y#Rc{< z^GD45h7YmV*zcI@_Y*e0z~1BfWjtK2zxlI1^6F>$ndHw*S-u+Guls-L(f*G-v*mhS z{|kLUk_;{x9z1H563C*Hu< zp5a4$iwuBA%lZP}8pOscJ`H#z`7gMh^^sRUtDi~!>`?nFfO|o|hVkmSHoSk$({ud+ z^z|^l;=1#kTYhu=Smx*2EIQ`*y8iv6JhA@$qnO|S`uC6Voa?brPn`W(PyfYoydu>{ zUj3{ee*d$+9rXR+$M1jr`^Wm%Jbl+UvHp!pKg12^`Tp-Jn;rH z80P*T0FU~9Y9(wu_x~XHv!4D7u4jE7zbO5zp8KEtY0&qAp8LO!{x-aS&C_!|-}kY9 z#dYT`|M&j>*Zhie{){!h;+)@j%O5fG8Mgd}Eq}zEU&T3p<~6_KoL})J=Ia#m)AR8x zztvlQ;{W7chU68l6?qOsJRzTPeR4b`e?zlb!SCeSF{_7wzn1kwvX!v;DbD`%qyL5L zSs!`zv-;1;e??YrpqWSOT8N7aRzk8WNZ-07z^4Idj>lOdu`R?Uc)IRb2oiAS-ETYLtS657Zj%3n>scRp^|ShOQGPY(nbG(UvMs+?@@t-H O*KcD@V}8ZE;Qt4BO>dn5 literal 0 HcmV?d00001 diff --git a/test/gfx/tilemap_dedup_overflow.out.attrmap b/test/gfx/tilemap_dedup_overflow.out.attrmap new file mode 100644 index 0000000000000000000000000000000000000000..4c2a1b23f7ec2bb37ffe869ac4a099ad44bb9554 GIT binary patch literal 320 NcmZQz7zLDs0003%00961 literal 0 HcmV?d00001 diff --git a/test/gfx/tilemap_dedup_overflow.out.tilemap b/test/gfx/tilemap_dedup_overflow.out.tilemap new file mode 100644 index 0000000000000000000000000000000000000000..e12a5cab509e4a51082a0780656c40ff4a9cec13 GIT binary patch literal 320 zcmZQzWMXDvWn<^yMC+6cQE@6%&_`l#-T_m6KOcR8m$^Ra4i{)Y8_`)zddH zG%_|ZH8Z!cw6eCbwX=6{baHlab#wRd^z!!c_45x13RUz zF>}`JIdkXDU$Ah|;w4L$Enl&6)#^2C*R9{Mant54TeofBv2)k%J$v`vjzBsxc>kDAIJlaTa()7BevL9R^{>rK(-+x6N4a-jsoJ$b{4RB4v-B32|x_e52Mj-Wnch$asoR83s8-Lk+A{e z0*JXFJ**2LW=#XKL4XNp5))WukfjBX1=VF}U;vW6xqQ~tliRuDf$Hi#T^vI)?!Ap< zob*J2qeb52|Nl3f8jEJ{TooNO_0E*EYG#iKj7~;;fu$T$$96K#C}As?z4zrdD^r*d z-&4oF2gVuKGn-E?uMC#|)R1*s)KB{mgVp4cn0Y^1C9l8nyTs1l@FaEm?8es<{&LI7 zmtS$5oug!-VuDNWkShnq!ITCep5}zC%FMP2@iRZwK&I#)G9A2)ueqx(e?FR;r zxU!v+7NrH=_8*RDo=jZ!dCR9qe7ABfy!34tbXRd$n)#RUtG!&{lD&+*SJ7ENX{w-~ z?4G53H}ZmfC(JHrvH8g6mA=<)Tj`DC5_@*z zd z@MEF7{fdA4KJe~07XA36;%M>0J@o-~dLh3LR_e{Mx?r%Vqe!ZIv82Mf1)*%)6U;X> zS6-5QVRhrnTdBD@^=#5}Iqmo(OAk3j?2j;=5N4=fvv29PTCr>Aj|eOJ?{U0zGkHUs zs}S!@6Mo6)C$Y~WHI{FTV-I4CT;X!~X~Mh`wq1N?wU6vH-)wl&sr-WnW{|?=hRTO* zezJEi@JDr|wDnI9@LTXY;c~yulNZIOPW4~glMC+6cQE@6%&_`l#-T_m6KOcR8m$^Ra4i{)Y8_`)zddH zG%_|ZH8Z!cw6eCbwX=6{baHlab#wRd^z!!c_45x13RUz zF>}`JIdkXDU$Ah|;w4L$Enl&6)#^2C*R9{Mant54TeofBv2)k%J$v`vjzBsxB}__|Nk$&IsYz@#aZAHS>jqptK^weVD0CHFvq!?Kl7=bJ=AeM%* zK@QYlWCn{f0ojI(ObmiRItqw0+gZTkIY2fDBmgl;Ka57Vm4N~1$qDQXEI>5|M#ct= z3n1o#^sp{~m^BT^1_36ZNlajsL6#Ok7F3s^fdNSN=JHunPj2Uq2Qp`Qx;Tbp+N7Yw^U{iQ{^{B3%IFQO cP_#g_Chkp$z%uctcR()oboFyt=akR{08z4jSO5S3 literal 0 HcmV?d00001