From 3b33c942aeb912bd16a0946dcf4ee447a56e86e1 Mon Sep 17 00:00:00 2001 From: Rangi Date: Sat, 11 Jul 2026 12:32:31 -0400 Subject: [PATCH] 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. --- src/gfx/process.cpp | 32 ++++++++++++---------- test/gfx/complete_unoptimized.flags | 3 ++ test/gfx/complete_unoptimized.out.2bpp | Bin 0 -> 8192 bytes test/gfx/complete_unoptimized.out.attrmap | Bin 0 -> 544 bytes test/gfx/complete_unoptimized.out.pal | Bin 0 -> 8 bytes test/gfx/complete_unoptimized.out.tilemap | Bin 0 -> 544 bytes test/gfx/complete_unoptimized.png | Bin 0 -> 3711 bytes 7 files changed, 21 insertions(+), 14 deletions(-) create mode 100644 test/gfx/complete_unoptimized.flags create mode 100644 test/gfx/complete_unoptimized.out.2bpp create mode 100644 test/gfx/complete_unoptimized.out.attrmap create mode 100644 test/gfx/complete_unoptimized.out.pal create mode 100644 test/gfx/complete_unoptimized.out.tilemap create mode 100644 test/gfx/complete_unoptimized.png diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index 56314b0f..02da54c4 100644 --- a/src/gfx/process.cpp +++ b/src/gfx/process.cpp @@ -700,12 +700,17 @@ static void outputUnoptimizedMaps( autoOpenPath(options.attrmap, attrmapOutput); autoOpenPath(options.palmap, palmapOutput); - uint8_t tileID = 0; + uint8_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); + if (tilemapOutput.has_value()) { - (*tilemapOutput) - ->sputc((attr.isBackgroundTile() ? 0 : tileID) + options.baseTileIDs[bank]); + uint8_t tileID = (attr.isBackgroundTile() ? 0 : tileIdx) + options.baseTileIDs[bank]; + (*tilemapOutput)->sputc(tileID); } uint8_t palID = attr.getPalID(mappings) + options.basePalID; if (attrmapOutput.has_value()) { @@ -715,18 +720,16 @@ static void outputUnoptimizedMaps( (*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()) { continue; } - // Compare with `maxNbTiles` *before* incrementing, due to unsigned overflow! - if (tileID + 1 < options.maxNbTiles[bank]) { - ++tileID; + if (tileIdx + 1 < options.maxNbTiles[bank]) { + ++tileIdx; } else { - assume(bank == 0); - bank = 1; - tileID = 0; + ++bank; + tileIdx = 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 (!options.allowDedup) { - uint32_t const nbTilesH = image.png.height / 8, nbTilesW = image.png.width / 8; - // 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]) { fatal( - "Image contains %" PRIu64 " tiles, exceeding the limit of %" PRIu16 " + %" PRIu16, + "Image contains %zu tiles, exceeding the limit of %" PRIu16 " + %" PRIu16, nbTiles, options.maxNbTiles[0], options.maxNbTiles[1] @@ -1192,6 +1195,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( diff --git a/test/gfx/complete_unoptimized.flags b/test/gfx/complete_unoptimized.flags new file mode 100644 index 00000000..7f79dc66 --- /dev/null +++ b/test/gfx/complete_unoptimized.flags @@ -0,0 +1,3 @@ +-B transparent +-c dmg +-N 256,256 diff --git a/test/gfx/complete_unoptimized.out.2bpp b/test/gfx/complete_unoptimized.out.2bpp new file mode 100644 index 0000000000000000000000000000000000000000..51748dd125f4fd551419403a1fb7fc4ff410dbe7 GIT binary patch literal 8192 zcmY*cL1d%(8y;(|wbtI^J5(&AYE<+VIaF-bsH!RzBVyD>&7pFOsHhrCMO8#@l|$85 zRgH>L6){F_MGui%RaMp4s2(D>s5y)|%zn@R@%-QS`zweh9FZ%t{ z)01!whrj*yB3zf3OG^vk67Kh>riR06)#E~8a#Hx<;PiAjoD3K9`~7+ySUj=mLl623 z3yMd+=c%XsnHlmYCl=r9S$sZUuUErWD5(C^|M}_l6ko6RdcJ<*YE^u*c@(avC*?0L zz7Cggzn@4vJyj|mzk4@6E_`s%Y(71Whf8|38nF0&AK3Jv2mRNt6_0$+Q&0IbGvtqs zEdJ`s;`8}htrD(x?^HkiTwN)?R=c|L^%GYr;!jTx4^K}A1LZF*&CV_@38P<+kM+98 ztE*E}!s!3wnGmX5s&!eflVLz(4U=EJn}tHJ?YQPkUu=M_+HQA z^Lb2vy}r6i{_>K3IvvGVtG%ADpLlCaJoXm!^!~6n;a~3udl~tL*Vf>V82uj%;9orU zn(^rWU|{;_KlFY7qdfJbkN$)A{qJ^75B=ZSfqy>#@{)dh|GQmZf4vTD{b#(+Tb%!> zPZ;&WzsK+|EdT!ehkx<%Zg+}--$W@IRC}tT^rB&H+{}O&VR+Dm!1c??2{WJp5z+egEOd_aF1Gc+>-2{r~r${xxs%FN}Wi{8PR3 zFJAur{KLO^`8Q1ee*Ss>NuT}|FaLi2dHyk8{{8&Jzn_1ef6}LaoByLD_Wze(xc^_j zMt_Dw;(mX8oc%Y<{u3V@938R$;wu&Q-(z?Z9v^4_g^}-h>M4F^hWyEi#k2p!`Fy3q z{u5tb(oe6a_)3NSkK$@o82xWHwLjSZ#YJKEzgF|OP)H<%+5bc$y8n%5|4koy3k!>j z(f#jvybH@`|C`O|{VrxCVoteq#UU=H|mC-0x3J z3*#=Km7Rnf4A%F$M?Th^Y?%H|M_Qq|LLFD{2Qi!V)HK^ ze&OF^)GI9i!t`gn{F68T7Ek}g=AW4NKloQX{hGe~ga7`0G>XOEgll>E_us#VYcxtG zTj4r86OP4hZ)34y(c|6S>uX_T$70vl|NhJTSZsYASUj=mLl64@{G)i}d!BmAZ?(w( z`KQGvlNP_byB@A$@$bLMCzGgXdHIh&6u-WnO#1qX|N0C3^Yhr)+i*$m`FVQ!Q@G?G z{KLc8nDUAD_T=yR8QEiF@(+G<6PWc2n?CfQPygVN?|JGe|I;V(^w0R|X^Y?6+YFcd zqyDg<&=39d`P0+B{>@Eb`X7zr@#W=gR(hjRI(>H5Zp%OT>+2v;K5?TVf1}aL%JOm$ z$UpdE5m@~rHht(pU;W4PgTV9DQ~t#TdG#Ohsg%X*MFD|_Q`+I1JkH_!t_xH`;%1SN=PulH}p_ufDKnE#E9blTTXO#ffLVD?tRMZGUylF75Pi*N~JmgDjF?>*k% z|NL1Pvlfqk{!IVSD;9yp6PrHtpilpNemw4Z>M6g~0$y36f5xZN7Qee&EWQsH=O6Xc z5B;;AblTTnECQ?l*n7VJsTBNM|FM^S|ABq~F+0Bhm=)iDV(S<3egE-%-+yA;KkYVo z^&hhyT`7o|M=VY zAM@|~56<@=&-eWo_Wu3+^Zc{fg^l{P_7Nj`|1h{rman z`DgRb^UvlV&-e2$O#k1$;d!_*v0vZ5vHz`>F#O^U=kf0D?X58Te|yXRLl6Bop4jx+ z|5l6ppU=k~RC=h#=d=HB-q1heamQ0U`@g=<{g3+Thx?!PB$K{=?1%Mr>whK#57fi` z_xwD%|8H*KL3}KBa}(YFnG8JH{x6k)EgsnPkssav$oD+;l;3KRxBh1`(fwa4MfX4b zX#QidQYpIsiRu6A*C5D-E0^Q*zJ5)m+Tr^6Q8)KJ=i^{`2`k;CbpPzugAT<>;UBsg%Vx8s%_pY_R{R9~I?t^v`-yDPMoN3{3ye z&;0$9$?$p4&;0#!elCo?h-?Pl`TSVS^VE|* zfB$4M^w0QY(&G91r&OYU)Q^fX8Tw~E$)v9zzkf<4?f)RiWc>XM6h7Ca?XEcz^$IG)g6Z|4*gp zNBbZ3`1`+b)PFXs=h1(=ElmIAvd4|a-JLM~-`z$18_)M)`t;v!>-$gto`+w{r~hm= b>fhq&zg%YjMrW~2Un{dgbcGW`DsC{OU> literal 0 HcmV?d00001 diff --git a/test/gfx/complete_unoptimized.out.attrmap b/test/gfx/complete_unoptimized.out.attrmap new file mode 100644 index 0000000000000000000000000000000000000000..c9721d31760c03fe7ed883cef7daeb897011dd7f GIT binary patch literal 544 ScmZQz7zM~7z%dF?LI41YoCtvc literal 0 HcmV?d00001 diff --git a/test/gfx/complete_unoptimized.out.pal b/test/gfx/complete_unoptimized.out.pal new file mode 100644 index 0000000000000000000000000000000000000000..dedaa6475e66b6e5508a3c760ea090711354a66e GIT binary patch literal 8 Ncmexg&(Hvb3;+*#0)GGi literal 0 HcmV?d00001 diff --git a/test/gfx/complete_unoptimized.out.tilemap b/test/gfx/complete_unoptimized.out.tilemap new file mode 100644 index 0000000000000000000000000000000000000000..0387f1bc21b8fe2e6e7bef931585e068e530325d GIT binary patch literal 544 zcmZQzWMXDvWn<^yE|QGBzFw*E zFmcl4DN`AyO`kDy*6cZR=gnWRaM9u=3`>_SU$Ju4>NRWEt>3V5(`JS(TeofBv2)k% zJ$v`@wXV0C#aFOBCo;%Ty=VCF@zduoU%!3-@$=X3KY#!IXBZ6+l<)um_b>ke literal 0 HcmV?d00001 diff --git a/test/gfx/complete_unoptimized.png b/test/gfx/complete_unoptimized.png new file mode 100644 index 0000000000000000000000000000000000000000..055e952972cdbbcae23d374b49529894f9566d60 GIT binary patch literal 3711 zcmZ8k30TtE76;2Iby85v%7qY3Z5-Dm#f2mloKe*3WtvNt=AQdPnKmhfx#04~HMP{T zUb!!+sbt!Sx#g03VuG40A*rBZ1AX z?k*q#BiinTGqCInJ&(TtoQ-@q1l$FQ1mYY*Kp^pVf(bsX`rBRL@K~t%<5x%Kj}2 zo~#Pqw|6cC$l72b5ENJ)f$9BV|K7PDAPdG?I2iO5@a&)U|E&Tq2Z2%!fc67-wRCi~ zz>k0E71$Jh{6kj;APa^Npjl||=6M5sK>GVz+FDwIPh(>=<$^yB(g0WwSelt$jC6Vb zC?S}R5;tQm@5X%3Al)=Nes)>E&YT>aC^HtP>aptgiN8s+rpFvzQ1?-#hm}L?Y`C7ZOiq5hWJO{g zq@LDs{!l=_Z59{U+>-uVfyK@htP&b^g`Jxcb+^S*w3C0u;z)4UWnr^7Lq7%W=EOXi zx_O6eOSSbpATs~KC_d<;I(~y=F`T`E2{kbs8|{Q%uzy-Q#KX5+SC>o4yEXSIOBLDP z2-@HoC^*GYhuZL`vOn{r3&YZ2d9}Q+2iXll54nO`*_#Dmt1tHepVf5>X@ggKOy^v3A`cmUe zjSU;WP+_dxk#0js=qpZn?mE|IzpP}|v}Bw{9QDz|JA&lD6j?x}Zu8|-5>g~H*SGMe z2zr#BkG#t32%_z>X(twI1?iUfNK|KalatjM87yCLze26)l=-?QW93=Ox7{Z3!b*M{PS_+{MT!vW+(t8WLxx z%SxKwCrKCbpWDl_9lmy7xyL(feAt>>EEa@b<4MNc0=;;Cs@Qdu?aNzF{yD*8%VY?! z;R)9(s2lonH}Sx{W-(+P-S&FV7teou5mRfQP-ov+F`mIV<_*70;;OqFQd?;)*kG)W z37~fO4Gke(NNB~%A4|A5d5&G3Tj7fx=wN$~pvtrq9@!t2&!+c%`&AHvChM~`SbxZ7 zIoDLwjxx1DreK|LUG`Nj{@oACM@>m2T83`6tKk6KudVK(9q+NQRzwcvczjNQq0X%E z(k+zUL{-{YPdTQ>WsxIlUUXjJOg^RQtf<}Y@4PNm%<9nqnX4tTn`NUXww^_c+Mzqc zAMCJE8)sS5a-Mhg3o7F)Y0lrhdHk{rTq^A${q?MsjAB@x6Jq#O^v8rzs6CF7!ZpXO z`MP|w!~RwHn0byTVW=3^@7CENN^d3H*Km)gyvr061}AGTV^?ARqTYn@8`;biCRAo7 z-D}FCi;0{wrN$M{tQe;m>U;_dPP+V(W2$)#8(Q+=D{1(X(YOGpBV7!0!-)2a(?9t( z4~>1P_v`EcACu^{0r029QjQC1f-Y2O>=RobBRw!9wqA!`SFrRPh*r{B#VDDx!*%}U z?<%VzXPJqqhB{2aQtg*e4HmEd#e<8J2OMODOPtqJiy`C9Wpjh#!%Y>HQ%=$hljzUT zXd`kR<2q4aMM4l1=JW+YP@=5g8mK<-kp;r6%1T-eA4QjktoCUK29i~@#VYsRASkgF zTT;V(t}Db@;ZDt|MZoEd=CX9(cE|S;jepcWfi$1mttA^}D=p4NR?HET*0;n@C8WSx z!p+voKXqqBh58ujXT#l-De36F#0mrV=lfhBbp*j`^JLfSLcq={vrF`A;h{?6IsI!? zfV@z(+7cN3oyPgi!_uXx8I?2^AKC% z51_;XDiVQ;4|;QV$;CMqQ|P?$K7M7lvGH|a-Q21@D9%iCS zn+7X(0BZH9AYhAV9Fu&&H~%4l!~d4Ijc@;-otG`>-g{@X#TvqRZ@h~UL4QFp#x(aM z!DA(A;bpxG+MXxTi>_WVRXvgGC5YC@JkUZfX9!0Qfc2c7AZo3C2~*M4qlAn}^7>QN z+tsb!-4>x#%=fXb_7pjzg1P=_+_Phsd2z#*d~egYI}W5zpHHjCZHy3~gOU+ujU2lh z@UZ~Tb_GfFUE)U3w#=GnZV>wdX?V;4FO_xXD@JLEE}QLhU#$P>MI5aY@M_378!8_J zE^d+0I=1sJAKekFrx{V)?k}k^6>l7deiD|}CuOONdKXpvhpQ|QuX2&G{eg1u(q`Dz zBRcPcEXp)Hh*t~cOIBS;rOH6dSwd||d!0w`=#`_DKa4}y4NH}fZ6~I*Li$r^@%m+o zN*%Gah(}h2Me-z|r`0rd$XDjx??hQMr&Vh3Yh0lqxTEJYHa!+I)W-^flYiI>a0hI4 z3RsR>6xNH$4D!Ptai3g_H8x4d6h`NhC2S`|WR^b6^Y95WaI8g=N+IV}QVr4@04_*J z>;VVFyL?NnE)XI3H6tXRjll}VDxwc?UV#SqU z<5^>u1&m|!_=%Q$v_rcq4bh((E|t=8tHdht-pV33#3(|kZ^EAjY0+qZ_}RuwUZ@{x z(~aX+w7!-Ah!i=c!FVmCpPVv0h1L?wo{?oKH2f@g=1xsy(-7q&ks8In?6>rY2++@8e27sYAY4WGPt<$CpN+ z;e7StxxoySZeD;OELgff#^vzapg6DFu5JKWFqS_PD*EKk4wY5j3^a$AgMjk5idfYR zDyFAgzLWdnC6~UA{{`UFjt}_(NTQd3tMA9A6D~dC?O&dRiuszXINIA2+DOSwNWMCw z9B>BWza5Z8v|@R3=VM?`AnVr?I-@Ik9S%0A_qscsJH6L}%=|}0x`=}otr(Z5h;#Ws za&m6vLdGi3vuhDKBZiNW#8K=4%i>Zg#wR*q9|~K#DidfN5!HQlScv3eeJ*gvyXL zJFJ!D$%Xce&t>wgz?ap^14as&QM!3Hf(&r&7{r(;-@U%t_g2xJwEL$G1*=;8US_<& zZ_l6qBsW?8ntNufqF70DrDdq6)+ENVyD7WS*4w`Ny-;ii1xZb7<(}sfRUY z=bO}xemTUHAWJ!t&ocTm07DYkP=+r3Ns5iwv_8i1iWHE_d<~N*vMmW#wQ;fg`?+N# zosGKxd)92eX0|!0$CcXDNHrUNaR@^eF|e#j(kNcJTDD|1snk)DP^)-FR@n39TnS{{ zy*wA7QJHWQ;#Qyzpi?}8Jt1iTw7lE}C+tD)@?~Pw@1O@8T@Gb zschuV^Nl9FBY<$0Io6Ei1WU4(=&UwBnSp=X`#6_qoZ?Rtpzcdr5$&rU9zP5r zBh_9r|CGi|Tv{dsoW8mJ+@doF9y zW@+Tz*Mq#PQ9i{TugNz)h_1SzBn`7)7&4ve7-0@15BWw2?=%FgV* zTf){JbDf)4+en^P)BjyXryP%be+G!#xBVYz=(@aa!Eg2*$(V1Gumzk|_-_ZQG}h*U zHm=uysfmKd4H|PqO4Fkzv5K>;KQUvXm`C2)#no&?r*d}(O!1X+s*~$;t^)L8xz1FB zo-B~AhmMO%?u@KM3G#9ruRc9HhZbM&I-_C_uYXljz1I(XGsP-R3i7OKzPH+V