From e0ceb6658073482428d50b49af5d6c0e24239a41 Mon Sep 17 00:00:00 2001
From: aplefull <aplefull@gmail.com>
Date: Wed, 23 Apr 2025 21:40:41 +0200
Subject: [PATCH] LibGfx: Fix incorrect colors in ICO-embedded BMPs

---
 Libraries/LibGfx/ImageFormats/BMPLoader.cpp |  16 +++++++++++-----
 Tests/LibGfx/TestImageDecoder.cpp           |  11 +++++++++++
 Tests/LibGfx/test-inputs/ico/yt-favicon.ico | Bin 0 -> 894 bytes
 3 files changed, 22 insertions(+), 5 deletions(-)
 create mode 100644 Tests/LibGfx/test-inputs/ico/yt-favicon.ico

diff --git a/Libraries/LibGfx/ImageFormats/BMPLoader.cpp b/Libraries/LibGfx/ImageFormats/BMPLoader.cpp
index b494ba7c148..534a992188a 100644
--- a/Libraries/LibGfx/ImageFormats/BMPLoader.cpp
+++ b/Libraries/LibGfx/ImageFormats/BMPLoader.cpp
@@ -1379,12 +1379,18 @@ static ErrorOr<void> decode_bmp_pixel_data(BMPLoadingContext& context)
                     return Error::from_string_literal("Cannot read 24 bits");
 
                 u32 pixel = streamer.read_u24();
-                u8 b = (pixel & 0xFF0000) >> 16;
-                u8 g = (pixel & 0x00FF00) >> 8;
-                u8 r = (pixel & 0x0000FF);
 
-                u32 rgbx_pixel = (r << 16) | (g << 8) | b;
-                context.bitmap->scanline(row)[column++] = rgbx_pixel;
+                if (context.is_included_in_ico) {
+                    // 24-bit ICO files already use BGRA8888 format
+                    context.bitmap->scanline(row)[column++] = pixel;
+                } else {
+                    u8 b = (pixel & 0x0000FF);
+                    u8 g = (pixel & 0x00FF00) >> 8;
+                    u8 r = (pixel & 0xFF0000) >> 16;
+
+                    context.bitmap->scanline(row)[column++] = (b << 16) | (g << 8) | r;
+                }
+
                 break;
             }
             case 32:
diff --git a/Tests/LibGfx/TestImageDecoder.cpp b/Tests/LibGfx/TestImageDecoder.cpp
index 441e2fc9685..c837821dc41 100644
--- a/Tests/LibGfx/TestImageDecoder.cpp
+++ b/Tests/LibGfx/TestImageDecoder.cpp
@@ -192,6 +192,17 @@ TEST_CASE(test_bmp_embedded_in_ico)
     EXPECT_EQ(frame.image->get_pixel(7, 4), Gfx::Color(161, 0, 0));
 }
 
+TEST_CASE(test_24bit_bmp_embedded_in_ico)
+{
+    auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("ico/yt-favicon.ico"sv)));
+    EXPECT(Gfx::ICOImageDecoderPlugin::sniff(file->bytes()));
+    auto plugin_decoder = TRY_OR_FAIL(Gfx::ICOImageDecoderPlugin::create(file->bytes()));
+
+    auto frame = TRY_OR_FAIL(expect_single_frame_of_size(*plugin_decoder, { 16, 16 }));
+    EXPECT_EQ(frame.image->get_pixel(14, 14), Gfx::Color(234, 0, 0));
+    EXPECT_EQ(frame.image->get_pixel(13, 15), Gfx::Color(255, 10, 15));
+}
+
 TEST_CASE(test_malformed_maskless_ico)
 {
     auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("ico/malformed_maskless.ico"sv)));
diff --git a/Tests/LibGfx/test-inputs/ico/yt-favicon.ico b/Tests/LibGfx/test-inputs/ico/yt-favicon.ico
new file mode 100644
index 0000000000000000000000000000000000000000..db55282ec976f4d1e8a6fcb659b64ebaca18d78d
GIT binary patch
literal 894
zcmZQzU}Ruq5D);-5)2v43=Con3=A3!3=9Gc3=9ek5OD@55awoJ0AVPX;lhRg{9ME$
zh++nYS41MX$^ZZVx3l}dX4QWPu(SJr^e~9IcoE3z&NdMH#4!*Xq6MP<|9_AO8oQ$v
zq!^+MPNJ!?vxBPn|NlQk{r~^}37Y@^KgfmrTp;rGYjpK+yC4jR+aPQ-5@a_RBa7ow
z4+&Cam3Y_}E`ZG?otXRZGXn#|-{0r||Nn2@u$qB^VOMF$|NsBv=AUL@V3-wThGryO
zJ%sW9|9^;fvDogvzt1x;Fc7I8q7p(PYk}B%d&fjH_drxaNM!X43=Ds!zQGhnRs(0p
z%|Fd=ne+eu|5-t15G&wPm<$F6J1zzWn`{OKyJQ9iy+j6voKyyeqD%$`y%Gk7>{JGZ
e<RS)!l6(dReUKWE+F}NV%sd8$<a`E(!~y{QzW3Vz

literal 0
HcmV?d00001