From e5944a4d9e6ab659e8ee40415a235af0ce7834f2 Mon Sep 17 00:00:00 2001 From: aplefull Date: Tue, 6 May 2025 21:32:35 +0200 Subject: [PATCH] LibGfx: Correctly handle OS/2 BMPs with 3-byte color entries We now properly handle OS/2 format BMPs that use 3 bytes per color entry instead of 4. While OS/2 2.x officially specified 4 bytes per color, some tools still produce files with 3-byte entries. We can identify such files by checking the available color table space. --- Libraries/LibGfx/ImageFormats/BMPLoader.cpp | 25 +++++++++++++++++--- Tests/LibGfx/TestImageDecoder.cpp | 11 +++++++++ Tests/LibGfx/test-inputs/bmp/os2_3bpc.bmp | Bin 0 -> 8084 bytes 3 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 Tests/LibGfx/test-inputs/bmp/os2_3bpc.bmp diff --git a/Libraries/LibGfx/ImageFormats/BMPLoader.cpp b/Libraries/LibGfx/ImageFormats/BMPLoader.cpp index ca4545d1f97..a3c08be99de 100644 --- a/Libraries/LibGfx/ImageFormats/BMPLoader.cpp +++ b/Libraries/LibGfx/ImageFormats/BMPLoader.cpp @@ -958,7 +958,26 @@ static ErrorOr decode_bmp_color_table(BMPLoadingContext& context) return {}; } - auto bytes_per_color = context.dib_type == DIBType::Core ? 3 : 4; + // OS/2 1.x (Core) uses 3 bytes per color + // OS/2 2.x (OSV2) can use 3 or 4 bytes per color + u8 bytes_per_color; + if (context.dib_type == DIBType::Core) { + bytes_per_color = 3; + } else if (context.dib_type == DIBType::OSV2 || context.dib_type == DIBType::OSV2Short) { + // For OS/2 2.x we need to determine the number of bytes per color based on the size of the color table + u8 header_size = context.is_included_in_ico ? 0 : bmp_header_size; + u32 available_size = context.data_offset - header_size - context.dib_size(); + u32 max_colors = 1 << context.dib.core.bpp; + + if (available_size == 3 * max_colors) { + bytes_per_color = 3; + } else { + bytes_per_color = 4; + } + } else { + bytes_per_color = 4; + } + u32 max_colors = 1 << context.dib.core.bpp; u8 header_size = !context.is_included_in_ico ? bmp_header_size : 0; @@ -974,9 +993,9 @@ static ErrorOr decode_bmp_color_table(BMPLoadingContext& context) if (context.dib_type <= DIBType::OSV2) { // Partial color tables are not supported, so the space of the color // table must be at least enough for the maximum amount of colors - if (size_of_color_table < 3 * max_colors) { + if (size_of_color_table < bytes_per_color * max_colors) { // This is against the spec, but most viewers process it anyways - dbgln("BMP with CORE header does not have enough colors. Has: {}, expected: {}", size_of_color_table, (3 * max_colors)); + dbgln("BMP with CORE header does not have enough colors. Has: {}, expected: {}", size_of_color_table, (bytes_per_color * max_colors)); } } diff --git a/Tests/LibGfx/TestImageDecoder.cpp b/Tests/LibGfx/TestImageDecoder.cpp index 79be2640922..cacf10a841e 100644 --- a/Tests/LibGfx/TestImageDecoder.cpp +++ b/Tests/LibGfx/TestImageDecoder.cpp @@ -94,6 +94,17 @@ TEST_CASE(test_bmp_v4) EXPECT_EQ(frame.image->get_pixel(0, 0), Gfx::Color::NamedColor::Red); } +TEST_CASE(test_bmp_os2_3bit) +{ + auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("bmp/os2_3bpc.bmp"sv))); + EXPECT(Gfx::BMPImageDecoderPlugin::sniff(file->bytes())); + auto plugin_decoder = TRY_OR_FAIL(Gfx::BMPImageDecoderPlugin::create(file->bytes())); + + auto frame = TRY_OR_FAIL(expect_single_frame_of_size(*plugin_decoder, { 300, 200 })); + EXPECT_EQ(frame.image->get_pixel(150, 100), Gfx::Color::NamedColor::Black); + EXPECT_EQ(frame.image->get_pixel(152, 100), Gfx::Color::NamedColor::White); +} + TEST_CASE(test_ico_malformed_frame) { Array test_inputs = { diff --git a/Tests/LibGfx/test-inputs/bmp/os2_3bpc.bmp b/Tests/LibGfx/test-inputs/bmp/os2_3bpc.bmp new file mode 100644 index 0000000000000000000000000000000000000000..c40b0c1476d7f7c4f44a0f677472f6d6272bf9d6 GIT binary patch literal 8084 zcmZ?rog&Ww1|bX#3=Rwo3_6Sq3?~>E7#JBC!Qu{3c@T#QL@+Qg9NqbSKL{_lS<+ab za*KhcE7&%|G#_p%kuX>`~TbR?d~VH59Po8zJNi1 zf#HAs@|ybf|8M?(KL7vw`~Uy{{{H{>-#?%KpZxv*@BeSV{_FqWS;5J`z@XSE(8nQ> z(kSH8Y5C{=|G!^<-~Y4v_UYRA?H~T^*RB2Z=WY-S$Xy{e?Ut9kn3I!xZ$q~AQ(glVP?S{M-~0de?)v-v>#u)*|Nj5~|NsB~{{HRv|Gyx6 z|Ns2|??Y1r2gn~2Cac`s`DVtZjQi{BK+68VzhfT%&+Py2^Z$Q8fBU&iQUPQfL&F45 z289Vi6FgKE&1T&`Ub}4HrKs%Wj;2eWvhBaLUGhHwR>i>JcGKAJ=YOBy<|+60|NsC0 z{r~#P{oB7EHvj*>?(grqyPw%6K+;p1*j~R3eck)h{Qug6oLB$<{r~SrfBt{7{`+1O**ft=REz_y4c`e(f(PdH(-j|Np!E@BjC2 zf_(n||9=Cp8W2%-VwX@Bh}`egAv=<7+z$0wlmHAjG@7|Gj=}y?6WfzyJIH-~a#q|Gxjf zE8hOU_W!?wCqxB=^t(MVfA03U+yCE#)z$z1{`+=4$aOze;C=w96p?wrz{SB*TmAg+ z|Nr~<|2@9?>)l^>X6IKgU|?W?L>WjK!R|L@=J`r4KI!tUqi*l&v! zNC3;ji0}2~^Z)PvegFS=kY)e>{QqA68^rost1{<97EBcbgYFq2my=>Czrm6F{r~&l zuYA5fyZ>8(p$VY|#QMr1l%lD|ayviv{lBmG>))=uKkavB4MPLN1aN@^QUboy&i}u;{`UX+|NsB}{{M%Og{PT` zvjHja7#JA#&6ke9t8WhqivJ+1Z$DvRVB$!G7=lE$aSEn(PGY$q|Nj5~`}beJeQV&r zsNf-h>RAQ`1{F^swN9bb{r~^p|M&jC@thwF4h#w&3`i*oVyM~w{`&g;pb`UQ_4k(t z7(Xx^Ji)-Q0U`}|XH>izy-_y2#*0Re_c6R4sF28Ir>Nf3U--TnFhx98pe zpZ|X{g9j_iqz%4ciKIpr5eX&+3kC)zK9Ead6odpknONS||7GB8U|?`)U|;~JV^FOF z)&M2aqA%r~UG)8S^#TS3o(3h5I*<$t3xk6P15^)4<9ATa^8f$;Hwsfe_-m>@5MbbN zU}9hr@N{5fG59C-kY$5`pf{*SYn+sFb5lwqLrn(*s8k0T#=yY9!qC9Rz{J48zyK0v zU|=v{kk--N!om!dM^K>ZP=P@Jq!bjGRk8d3-QS+)rS8Dk1X2w03#iV!nV`nX6_==) zFau;b0|VRK>HkhHRhb~bzyXV+1_nk3fkQJGI5-#_CV_p!z`!7MgPBLM4HTfP8^M+` zF$jU3!>Pcy1uOs|oIMvlxxFdHQz7TBkpqJY1A_sFv;qTzNa{+p-pOu0ji4|Asbt$W z!Ts*)EkahW|NjB;IaGUu4lpqAEx5F*^J$971W?@$s*hMs3iB)!dN8}3cV&YE2LnR` zQ@~^f4hEJRjVcb47(hWSz%oIAfx*pFh^0}8!MFI{|7E@hK)SVFt9K%?5IzYzh3`?L4l#sfdN#a zaexXn;jpuV1_J{#10y5L1O*0$y0fw@ z(?NyR|9|%RH_hMP`{o;^_Vhu50B1>ql$V1B8|xHs0j$En#K3NQ?>{K?SQ?rf7?nUx zBLz@0^#HjD4Dp^q zE#Pwd|Nrm#`A_d%bY@MJe=@6q^MY!F@gdF>7BvPTP`#hV(9|Fg%Hj_?6?8fccsiLF zW-xd#GO!}bQ&1o>h%_)U=x+P||DNtbhK3Uypmy2~29F600s;yQ3@Qwa;B?Bsz>t@r zl>Gny`}_a(Z*R}NZOVGX$@BKMm75$myWI>n1$eGVxv`%KR2%9GusMLN{hxDtyUOfI z3?4@xPEhETVqjUpwQ;u-|Q_ z0Z#+dxt0Wn2@G75RXiuLc}`L}DYV2>fPsNw|GzH`0xX?OE)A1h8kvL`6dYO=K*7TR?F6di&HL6N;c+0n^vCnZ$oOiJ@qWSKaDfgvJ( zug1K2r*EgLo}0#h+x*$&YbSqyd;9y_-`{nA|JR+hufM;&Dt-e4g9@V%1ET<_^x|Y- zX=HL>RB)K0;J~Pm(V*fX07}H13@RZ1sxe4uI5Ma>2sv;lw9H^~&}8zNpqJZfc48K* z$HZmMml<9*s5A;NFogclxBt4U{(b%P`Tx)V{k|SlyZ--w`~TniZ-4*a`~5!m`~3R< zAbT4&xOgh5+%(F$X_PZV$Vbx5XR?}4r;6(&rIUI zySt=yciG+Dd3QHu`EFofU=dl6cT$Ci$=Er1d#;4}TW+(L&1PqBr!AQrmI(@Vj#yiT zITJv2G6M?(8{2KC)CPtI1}2t;Z5IwIFf^z+2`G37s7VF!FbIi&(kTOn0!ssv3WtEo zY=e9WBcBOk8Qn@gvlx5@)jTJwc{Zz@oRo5M0Rw}A04NbLFkE0{Xg$bK|NOt;4&}@L zfB!K(z)+w2Zrh~j%(rW{f#UfD1IrI428I9zh871#2M)<;w-}XOCNOa{FfdGF^PB)S z52TKRiN!+*ToW}oFoH{M=7laZ6+EDovxbYLy7=Mor2Us1_p-R zzr%kpuo>_%Fz_%;+rYq(bVHF(g3-fKkkP0iL!yD9K|q7kkpWcjsr2r4<5Wz$J1GNX zEx68OU{D0ryWs50puhl92XAnIvM{J!(lFUUQ-MK1fC186Vqs8_0I7qT0qUWA0kIWM z?yqZF#NcgzWWr@nh64=D0t%Im4dB{^fgu3oUXUtKT_^-9qd=7!C_91#>l?A;4^ZXE z$fUp##KNKA1~F&01FUHZieQjV1_p+p>isp-Cuw=^(79J|+S`GJ=~#4S>H`J_a8MQ@ z>{aU6%)rRNBCyHx0J!#HU}4|_)pKBDSQ}6qT1+eq4ve5g398{49Kh8dSRsO7Vl-r7 z5b(RZJ?HSGoOm-80R!I?EGMA;Wn$D|5P_*><6>Z7Y7lllz{)wTiGhJdg9#jcFlngY zSvbHU&*{XnP{F0yfq{XEp-}-8fKYYd%mHFEPEcuJa`0qQnIPmaL&>Fq$w!jK1*9A- z32F&|7z_-88yq-vG}KSWH|iv?ZS-N_JhCqVRELjpqsOM`*KNoCau6Brm3 z5+sB;Iuv}Ml1NBhN1N{jrU*%p1rAIMEHjt_z*!QKE1Y5|bq6PbG;s?1a4;|k2XQheFbIJY3nU>lFfc@L@-j@>{gwW~XeKAtbp1)LP77 zU~ynz;8XyYFQELvIDvzKL1mVb7Xzpr;lQZE0xl0g;Q%fxAclfSkc$}^G(b#j*vZ3C zrC}mZypu&YiJf%X&p;Li@ zfyqHqiHCuKqfy0W0)q!6CcsT6P*RsJg*tRgBlRa z!EQ{sX{m5f$mIqDgMiSTPBV}?X;l^x1yBGnFmQs?y{7^L1Dl5sC`~Ye8xfvW7SREK9b1*P43J7s}KqDBUTEKvTq3nzei$D{X1B(iSxU1w+ z9tH*m4-u795Csej3JRdIseyrkhXc|th1d(024w^=1DW{Hpv1tl2%`3h2gs=`3`z`& zPXrgSnncnYvIfCE$j zY&%E_1216^;@~s@g=!B2%LGtQg@J(~0n~VdMj9yLGB7aIMef(TKR^Ej1H(OL_sHj< zHa3IE+a^fi&%^;vXZ!*T3<(jF{USH#+-P9%WMFAB0GBLG4?&p^6!{=~7#J8qX@ZG? zg=Nd68=wpVY6oCA3~F!M&CMw{H|N}JU=Ucqa8eB%5YRwjU~*yvm(Lsv7#QmA@2~dW zpMP%y12+r9Wr(vbpc%^6z`(FfQb?s!Ozd_90|T1_#97SD;6MT?2BiRSN@GwtDWuXV zl+Y=}z!<~;&JzsKKmn=q00$_j-BiDCbA065J<}!~U{D0fKrqB$Z~_JOCMPg3q@3Ig z>R;Ympu+$vBq7R>NRXdE>68UjzcPXIGuRF!CCCn7P?<8x%TrZl!UP8f7I5IBD1>kX z1Q;0d?(WaKJ}v9KX?l~uOOW@#AqbIYVB!Rcfa`Y_4j~3k22e>0>2yO?U{ERzlUzI% zR2qaF7??7U&4Q=|C0tNB#ZdqM|MP#JZ%67bnI;zSl7WGN3!(r*N~VBl#smfim4-

Rx=W^A^3G)4{)&M? z0#bUwOoGrX3{4D<3JMJZ&|JvL04o(B@-PyVgTR>zDzFfef}uPF<>jfXGG!Vl3s0Kl z>8ZNpasxvnB84K2!33&a){z#-589hxd9Pelb#y#OlLI6&!C$dglLDVib*xQr?aSo22|ha$M>O3GN$TfJ!pL3#&(GNx literal 0 HcmV?d00001