LibGfx: Support BMP favicons with less than 32 bpp

Adapt BMPImageDecoderPlugin to support BMP images included in ICOns.
ICOImageDecoderPlugin now uses BMPImageDecoderPlugin to decode all
BMP images instead of it's own ad-hoc decoder which only supported
32 bpp BMPs.

Tests/LibGfx/TestImageDecoder.cpp

@@ -49,9 +49,8 @@ TEST_CASE(test_gif)
     EXPECT(frame.duration == 400);
 }
 
-TEST_CASE(test_ico)
+TEST_CASE(test_not_ico)
 {
-    // FIXME: Use an ico file
     auto file = Core::MappedFile::map("/res/graphics/buggie.png"sv).release_value();
     auto ico = Gfx::ICOImageDecoderPlugin((u8 const*)file->data(), file->size());
     EXPECT(ico.frame_count());
@@ -63,6 +62,19 @@ TEST_CASE(test_ico)
     EXPECT(ico.frame(0).is_error());
 }
 
+TEST_CASE(test_bmp_embedded_in_ico)
+{
+    auto file = Core::MappedFile::map("/res/icons/16x16/serenity.ico"sv).release_value();
+    auto ico = Gfx::ICOImageDecoderPlugin((u8 const*)file->data(), file->size());
+    EXPECT(ico.frame_count());
+
+    EXPECT(ico.sniff());
+    EXPECT(!ico.is_animated());
+    EXPECT(!ico.loop_count());
+
+    EXPECT(!ico.frame(0).is_error());
+}
+
 TEST_CASE(test_jpg)
 {
     auto file = Core::MappedFile::map("/res/html/misc/bmpsuite_files/rgb24.jpg"sv).release_value();

Userland/Libraries/LibGfx/BMPLoader.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2022, Bruno Conde <brunompconde@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -134,6 +135,8 @@ struct BMPLoadingContext {
     size_t file_size { 0 };
     u32 data_offset { 0 };
 
+    bool is_included_in_ico { false };
+
     DIB dib;
     DIBType dib_type;
 
@@ -749,23 +752,33 @@ static bool decode_bmp_dib(BMPLoadingContext& context)
     if (context.state >= BMPLoadingContext::State::DIBDecoded)
         return true;
 
-    if (context.state < BMPLoadingContext::State::HeaderDecoded && !decode_bmp_header(context))
+    if (!context.is_included_in_ico && context.state < BMPLoadingContext::State::HeaderDecoded && !decode_bmp_header(context))
         return false;
 
-    if (context.file_size < bmp_header_size + 4)
+    u8 header_size = context.is_included_in_ico ? 0 : bmp_header_size;
+
+    if (!context.is_included_in_ico && context.file_size < (u8)(header_size + 4))
         return false;
 
-    InputStreamer streamer(context.file_bytes + bmp_header_size, 4);
+    if (context.is_included_in_ico && context.file_size < 4)
+        return false;
+
+    InputStreamer streamer(context.file_bytes + (context.is_included_in_ico ? 0 : header_size), 4);
+
     u32 dib_size = streamer.read_u32();
 
-    if (context.file_size < bmp_header_size + dib_size)
+    if (context.file_size < header_size + dib_size)
         return false;
-    if (context.data_offset < bmp_header_size + dib_size) {
+
+    if (!context.is_included_in_ico && (context.data_offset < header_size + dib_size)) {
         dbgln("Shenanigans! BMP pixel data and header usually don't overlap.");
         return false;
     }
 
-    streamer = InputStreamer(context.file_bytes + bmp_header_size + 4, context.data_offset - bmp_header_size - 4);
+    // NOTE: If this is a headless BMP (embedded on ICO files), then we can only infer the data_offset after we know the data table size.
+    // We are also assuming that no Extra bit masks are present
+    u32 dib_offset = context.is_included_in_ico ? dib_size : context.data_offset - header_size - 4;
+    streamer = InputStreamer(context.file_bytes + header_size + 4, dib_offset);
 
     dbgln_if(BMP_DEBUG, "BMP dib size: {}", dib_size);
 
@@ -833,6 +846,18 @@ static bool decode_bmp_dib(BMPLoadingContext& context)
         return false;
     }
 
+    // NOTE: If this is a headless BMP (included on ICOns), the data_offset is set based on the number_of_palette_colors found on the DIB header
+    if (context.is_included_in_ico) {
+        if (context.dib.core.bpp > 8)
+            context.data_offset = dib_size;
+        else {
+            auto bytes_per_color = context.dib_type == DIBType::Core ? 3 : 4;
+            u32 max_colors = 1 << context.dib.core.bpp;
+            auto size_of_color_table = (context.dib.info.number_of_palette_colors > 0 ? context.dib.info.number_of_palette_colors : max_colors) * bytes_per_color;
+            context.data_offset = dib_size + size_of_color_table;
+        }
+    }
+
     context.state = BMPLoadingContext::State::DIBDecoded;
 
     return true;
@@ -856,8 +881,16 @@ static bool decode_bmp_color_table(BMPLoadingContext& context)
 
     auto bytes_per_color = context.dib_type == DIBType::Core ? 3 : 4;
     u32 max_colors = 1 << context.dib.core.bpp;
-    VERIFY(context.data_offset >= bmp_header_size + context.dib_size());
-    auto size_of_color_table = context.data_offset - bmp_header_size - context.dib_size();
+
+    u8 header_size = !context.is_included_in_ico ? bmp_header_size : 0;
+    VERIFY(context.data_offset >= header_size + context.dib_size());
+
+    u32 size_of_color_table;
+    if (!context.is_included_in_ico) {
+        size_of_color_table = context.data_offset - header_size - context.dib_size();
+    } else {
+        size_of_color_table = (context.dib.info.number_of_palette_colors > 0 ? context.dib.info.number_of_palette_colors : max_colors) * bytes_per_color;
+    }
 
     if (context.dib_type <= DIBType::OSV2) {
         // Partial color tables are not supported, so the space of the color
@@ -868,7 +901,7 @@ static bool decode_bmp_color_table(BMPLoadingContext& context)
         }
     }
 
-    InputStreamer streamer(context.file_bytes + bmp_header_size + context.dib_size(), size_of_color_table);
+    InputStreamer streamer(context.file_bytes + header_size + context.dib_size(), size_of_color_table);
     for (u32 i = 0; !streamer.at_end() && i < max_colors; ++i) {
         if (bytes_per_color == 4) {
             if (!streamer.has_u32())
@@ -882,6 +915,7 @@ static bool decode_bmp_color_table(BMPLoadingContext& context)
     }
 
     context.state = BMPLoadingContext::State::ColorTableDecoded;
+
     return true;
 }
 
@@ -1140,6 +1174,14 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
     const u16 bits_per_pixel = context.dib.core.bpp;
 
     BitmapFormat format = [&]() -> BitmapFormat {
+        // NOTE: If this is an BMP included in an ICO, the bitmap format will be converted to BGRA8888.
+        //       This is because images with less than 32 bits of color depth follow a particular format:
+        //       the image is encoded with a color mask (the "XOR mask") together with an opacity mask (the "AND mask") of 1 bit per pixel.
+        //       The height of the encoded image must be exactly twice the real height, before both masks are combined.
+        //       Bitmaps have no knowledge of this format as they do not store extra rows for the AND mask.
+        if (context.is_included_in_ico)
+            return BitmapFormat::BGRA8888;
+
         switch (bits_per_pixel) {
         case 1:
             return BitmapFormat::Indexed1;
@@ -1169,7 +1211,7 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
     }
 
     const u32 width = abs(context.dib.core.width);
-    const u32 height = abs(context.dib.core.height);
+    const u32 height = !context.is_included_in_ico ? context.dib.core.height : (context.dib.core.height / 2);
 
     auto bitmap_or_error = Bitmap::try_create(format, { static_cast<int>(width), static_cast<int>(height) });
     if (bitmap_or_error.is_error()) {
@@ -1191,6 +1233,18 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
 
     InputStreamer streamer(bytes.data(), bytes.size());
 
+    auto process_row_padding = [&](const u8 consumed) -> bool {
+        // Calculate padding
+        u8 remaining = consumed % 4;
+        u8 bytes_to_drop = remaining == 0 ? 0 : 4 - remaining;
+
+        if (streamer.remaining() < bytes_to_drop)
+            return false;
+        streamer.drop_bytes(bytes_to_drop);
+
+        return true;
+    };
+
     auto process_row = [&](u32 row) -> bool {
         u32 space_remaining_before_consuming_row = streamer.remaining();
 
@@ -1203,7 +1257,13 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
                 u8 mask = 8;
                 while (column < width && mask > 0) {
                     mask -= 1;
-                    context.bitmap->scanline_u8(row)[column++] = (byte >> mask) & 0x1;
+                    auto color_idx = (byte >> mask) & 0x1;
+                    if (context.is_included_in_ico) {
+                        auto color = context.color_table[color_idx];
+                        context.bitmap->scanline(row)[column++] = color;
+                    } else {
+                        context.bitmap->scanline_u8(row)[column++] = color_idx;
+                    }
                 }
                 break;
             }
@@ -1214,24 +1274,52 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
                 u8 mask = 8;
                 while (column < width && mask > 0) {
                     mask -= 2;
-                    context.bitmap->scanline_u8(row)[column++] = (byte >> mask) & 0x3;
+                    auto color_idx = (byte >> mask) & 0x3;
+                    if (context.is_included_in_ico) {
+                        auto color = context.color_table[color_idx];
+                        context.bitmap->scanline(row)[column++] = color;
+                    } else {
+                        context.bitmap->scanline_u8(row)[column++] = color_idx;
+                    }
                 }
                 break;
             }
             case 4: {
-                if (!streamer.has_u8())
+                if (!streamer.has_u8()) {
                     return false;
+                }
                 u8 byte = streamer.read_u8();
-                context.bitmap->scanline_u8(row)[column++] = (byte >> 4) & 0xf;
-                if (column < width)
-                    context.bitmap->scanline_u8(row)[column++] = byte & 0xf;
+
+                u32 high_color_idx = (byte >> 4) & 0xf;
+                u32 low_color_idx = byte & 0xf;
+
+                if (context.is_included_in_ico) {
+                    auto high_color = context.color_table[high_color_idx];
+                    auto low_color = context.color_table[low_color_idx];
+                    context.bitmap->scanline(row)[column++] = high_color;
+                    if (column < width) {
+                        context.bitmap->scanline(row)[column++] = low_color;
+                    }
+                } else {
+                    context.bitmap->scanline_u8(row)[column++] = high_color_idx;
+                    if (column < width)
+                        context.bitmap->scanline_u8(row)[column++] = low_color_idx;
+                }
                 break;
             }
-            case 8:
+            case 8: {
                 if (!streamer.has_u8())
                     return false;
-                context.bitmap->scanline_u8(row)[column++] = streamer.read_u8();
+
+                u8 byte = streamer.read_u8();
+                if (context.is_included_in_ico) {
+                    auto color = context.color_table[byte];
+                    context.bitmap->scanline(row)[column++] = color;
+                } else {
+                    context.bitmap->scanline_u8(row)[column++] = byte;
+                }
                 break;
+            }
             case 16: {
                 if (!streamer.has_u16())
                     return false;
@@ -1248,7 +1336,7 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
                 if (!streamer.has_u32())
                     return false;
                 if (context.dib.info.masks.is_empty()) {
-                    context.bitmap->scanline(row)[column++] = streamer.read_u32() | 0xff000000;
+                    context.bitmap->scanline(row)[column++] = streamer.read_u32();
                 } else {
                     context.bitmap->scanline(row)[column++] = int_to_scaled_rgb(context, streamer.read_u32());
                 }
@@ -1258,25 +1346,38 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
 
         auto consumed = space_remaining_before_consuming_row - streamer.remaining();
 
-        // Calculate padding
-        u8 bytes_to_drop = [consumed]() -> u8 {
-            switch (consumed % 4) {
-            case 0:
-                return 0;
-            case 1:
-                return 3;
-            case 2:
-                return 2;
-            case 3:
-                return 1;
-            }
-            VERIFY_NOT_REACHED();
-        }();
-        if (streamer.remaining() < bytes_to_drop)
-            return false;
-        streamer.drop_bytes(bytes_to_drop);
+        return process_row_padding(consumed);
+    };
 
-        return true;
+    auto process_mask_row = [&](u32 row) -> bool {
+        u32 space_remaining_before_consuming_row = streamer.remaining();
+
+        for (u32 column = 0; column < width;) {
+            if (!streamer.has_u8())
+                return false;
+
+            u8 byte = streamer.read_u8();
+            u8 mask = 8;
+            while (column < width && mask > 0) {
+                mask -= 1;
+                // apply transparency mask
+                // AND mask = 0 -> fully opaque
+                // AND mask = 1 -> fully transparent
+                u8 and_byte = (byte >> (mask)) & 0x1;
+                auto pixel = context.bitmap->scanline(row)[column];
+
+                if (and_byte) {
+                    pixel &= 0x00ffffff;
+                } else if (context.dib.core.bpp < 32) {
+                    pixel |= 0xff000000;
+                }
+
+                context.bitmap->scanline(row)[column++] = pixel;
+            }
+        }
+
+        auto consumed = space_remaining_before_consuming_row - streamer.remaining();
+        return process_row_padding(consumed);
     };
 
     if (context.dib.core.height < 0) {
@@ -1285,26 +1386,45 @@ static bool decode_bmp_pixel_data(BMPLoadingContext& context)
             if (!process_row(row))
                 return false;
         }
+
+        if (context.is_included_in_ico) {
+            for (u32 row = 0; row < height; ++row) {
+                if (!process_mask_row(row))
+                    return false;
+            }
+        }
     } else {
+        // BMP is stored bottom-up
         for (i32 row = height - 1; row >= 0; --row) {
             if (!process_row(row))
                 return false;
         }
+
+        if (context.is_included_in_ico) {
+            for (i32 row = height - 1; row >= 0; --row) {
+                if (!process_mask_row(row))
+                    return false;
+            }
+        }
     }
 
-    for (size_t i = 0; i < context.color_table.size(); ++i)
-        context.bitmap->set_palette_color(i, Color::from_rgb(context.color_table[i]));
+    if (!context.is_included_in_ico) {
+        for (size_t i = 0; i < context.color_table.size(); ++i) {
+            context.bitmap->set_palette_color(i, Color::from_rgb(context.color_table[i]));
+        }
+    }
 
     context.state = BMPLoadingContext::State::PixelDataDecoded;
 
     return true;
 }
 
-BMPImageDecoderPlugin::BMPImageDecoderPlugin(u8 const* data, size_t data_size)
+BMPImageDecoderPlugin::BMPImageDecoderPlugin(u8 const* data, size_t data_size, bool is_included_in_ico)
 {
     m_context = make<BMPLoadingContext>();
     m_context->file_bytes = data;
     m_context->file_size = data_size;
+    m_context->is_included_in_ico = is_included_in_ico;
 }
 
 BMPImageDecoderPlugin::~BMPImageDecoderPlugin() = default;
@@ -1338,6 +1458,11 @@ bool BMPImageDecoderPlugin::sniff()
     return decode_bmp_header(*m_context);
 }
 
+bool BMPImageDecoderPlugin::sniff_dib()
+{
+    return decode_bmp_dib(*m_context);
+}
+
 bool BMPImageDecoderPlugin::is_animated()
 {
     return false;

Userland/Libraries/LibGfx/BMPLoader.h

@@ -15,12 +15,13 @@ struct BMPLoadingContext;
 class BMPImageDecoderPlugin final : public ImageDecoderPlugin {
 public:
     virtual ~BMPImageDecoderPlugin() override;
-    BMPImageDecoderPlugin(u8 const*, size_t);
+    BMPImageDecoderPlugin(u8 const*, size_t, bool is_included_in_ico = false);
 
     virtual IntSize size() override;
     virtual void set_volatile() override;
     [[nodiscard]] virtual bool set_nonvolatile(bool& was_purged) override;
     virtual bool sniff() override;
+    bool sniff_dib();
     virtual bool is_animated() override;
     virtual size_t loop_count() override;
     virtual size_t frame_count() override;

Userland/Libraries/LibGfx/ICOLoader.cpp

@@ -9,6 +9,7 @@
 #include <AK/MemoryStream.h>
 #include <AK/NonnullOwnPtrVector.h>
 #include <AK/Types.h>
+#include <LibGfx/BMPLoader.h>
 #include <LibGfx/ICOLoader.h>
 #include <LibGfx/PNGLoader.h>
 #include <string.h>
@@ -35,38 +36,6 @@ struct ICONDIRENTRY {
 };
 static_assert(AssertSize<ICONDIRENTRY, 16>());
 
-struct [[gnu::packed]] BMPFILEHEADER {
-    u8 signature[2];
-    u32 size;
-    u16 reserved1;
-    u16 reserved2;
-    u32 offset;
-};
-static_assert(sizeof(BMPFILEHEADER) == 14);
-
-struct BITMAPINFOHEADER {
-    u32 size;
-    i32 width;
-    i32 height;
-    u16 planes;
-    u16 bpp;
-    u32 compression;
-    u32 size_image;
-    u32 vres;
-    u32 hres;
-    u32 palette_size;
-    u32 important_colors;
-};
-static_assert(sizeof(BITMAPINFOHEADER) == 40);
-
-struct [[gnu::packed]] BMP_ARGB {
-    u8 b;
-    u8 g;
-    u8 r;
-    u8 a;
-};
-static_assert(sizeof(BMP_ARGB) == 4);
-
 struct ICOImageDescriptor {
     u16 width;
     u16 height;
@@ -162,87 +131,6 @@ static bool load_ico_directory(ICOLoadingContext& context)
     return true;
 }
 
-static bool load_ico_bmp(ICOLoadingContext& context, ICOImageDescriptor& desc)
-{
-    BITMAPINFOHEADER info;
-    if (desc.size < sizeof(info))
-        return false;
-
-    memcpy(&info, context.data + desc.offset, sizeof(info));
-    if (info.size != sizeof(info)) {
-        dbgln_if(ICO_DEBUG, "load_ico_bmp: info size: {}, expected: {}", info.size, sizeof(info));
-        return false;
-    }
-
-    if (info.width < 0) {
-        dbgln_if(ICO_DEBUG, "load_ico_bmp: width {} < 0", info.width);
-        return false;
-    }
-
-    if (info.height == NumericLimits<i32>::min()) {
-        dbgln_if(ICO_DEBUG, "load_ico_bmp: height == NumericLimits<i32>::min()");
-        return false;
-    }
-
-    bool topdown = false;
-    if (info.height < 0) {
-        topdown = true;
-        info.height = -info.height;
-    }
-
-    if (info.planes != 1) {
-        dbgln_if(ICO_DEBUG, "load_ico_bmp: planes: {} != 1", info.planes);
-        return false;
-    }
-
-    if (info.bpp != 32) {
-        dbgln_if(ICO_DEBUG, "load_ico_bmp: unsupported bpp: {}", info.bpp);
-        return false;
-    }
-
-    dbgln_if(ICO_DEBUG, "load_ico_bmp: width: {} height: {} direction: {} bpp: {} size_image: {}",
-        info.width, info.height, topdown ? "TopDown" : "BottomUp", info.bpp, info.size_image);
-
-    if (info.compression != 0 || info.palette_size != 0 || info.important_colors != 0) {
-        dbgln_if(ICO_DEBUG, "load_ico_bmp: following fields must be 0: compression: {} palette_size: {} important_colors: {}", info.compression, info.palette_size, info.important_colors);
-        return false;
-    }
-
-    if (info.width != desc.width || info.height != 2 * desc.height) {
-        dbgln_if(ICO_DEBUG, "load_ico_bmp: size mismatch: ico {}x{}, bmp {}x{}", desc.width, desc.height, info.width, info.height);
-        return false;
-    }
-
-    // Mask is 1bpp, and each row must be 4-byte aligned
-    size_t mask_row_len = align_up_to(align_up_to(desc.width, 8) / 8, 4);
-    size_t required_len = desc.height * (desc.width * sizeof(BMP_ARGB) + mask_row_len);
-    size_t available_len = desc.size - sizeof(info);
-    if (required_len > available_len) {
-        dbgln_if(ICO_DEBUG, "load_ico_bmp: required_len: {} > available_len: {}", required_len, available_len);
-        return false;
-    }
-
-    auto bitmap_or_error = Bitmap::try_create(BitmapFormat::BGRA8888, { desc.width, desc.height });
-    if (bitmap_or_error.is_error())
-        return false;
-    desc.bitmap = bitmap_or_error.release_value_but_fixme_should_propagate_errors();
-    Bitmap& bitmap = *desc.bitmap;
-    u8 const* image_base = context.data + desc.offset + sizeof(info);
-    const BMP_ARGB* data_base = (const BMP_ARGB*)image_base;
-    u8 const* mask_base = image_base + desc.width * desc.height * sizeof(BMP_ARGB);
-    for (int y = 0; y < desc.height; y++) {
-        u8 const* row_mask = mask_base + mask_row_len * y;
-        const BMP_ARGB* row_data = data_base + desc.width * y;
-        for (int x = 0; x < desc.width; x++) {
-            u8 mask = !!(row_mask[x / 8] & (0x80 >> (x % 8)));
-            BMP_ARGB data = row_data[x];
-            bitmap.set_pixel(x, topdown ? y : desc.height - y - 1,
-                Color(data.r, data.g, data.b, mask ? 0 : data.a));
-        }
-    }
-    return true;
-}
-
 static bool load_ico_bitmap(ICOLoadingContext& context, Optional<size_t> index)
 {
     if (context.state < ICOLoadingContext::State::DirectoryDecoded) {
@@ -271,8 +159,16 @@ static bool load_ico_bitmap(ICOLoadingContext& context, Optional<size_t> index)
         desc.bitmap = decoded_png_frame.value().image;
         return true;
     } else {
-        if (!load_ico_bmp(context, desc)) {
-            dbgln_if(ICO_DEBUG, "load_ico_bitmap: failed to load BMP encoded image index: {}", real_index);
+        BMPImageDecoderPlugin bmp_decoder(context.data + desc.offset, desc.size, true);
+        if (bmp_decoder.sniff_dib()) {
+            auto decoded_bmp_frame = bmp_decoder.frame(0);
+            if (decoded_bmp_frame.is_error() || !decoded_bmp_frame.value().image) {
+                dbgln_if(ICO_DEBUG, "load_ico_bitmap: failed to load BMP encoded image index: {}", real_index);
+                return false;
+            }
+            desc.bitmap = decoded_bmp_frame.value().image;
+        } else {
+            dbgln_if(ICO_DEBUG, "load_ico_bitmap: encoded image not supported at index: {}", real_index);
             return false;
         }
         return true;