LibGfx: Remove home-grown PNG codec in favor of libpng+apng

Tests/LibGfx/TestImageDecoder.cpp

@@ -363,7 +363,10 @@ TEST_CASE(test_png_malformed_frame)
 
     for (auto test_input : test_inputs) {
         auto file = TRY_OR_FAIL(Core::MappedFile::map(test_input));
-        auto plugin_decoder = TRY_OR_FAIL(Gfx::PNGImageDecoderPlugin::create(file->bytes()));
+        auto plugin_decoder_or_error = Gfx::PNGImageDecoderPlugin::create(file->bytes());
+        if (plugin_decoder_or_error.is_error())
+            continue;
+        auto plugin_decoder = plugin_decoder_or_error.release_value();
         auto frame_or_error = plugin_decoder->frame(0);
         EXPECT(frame_or_error.is_error());
     }

Userland/Libraries/LibGfx/CMakeLists.txt

@@ -103,3 +103,8 @@ find_package(JPEG REQUIRED)
 target_include_directories(LibGfx PRIVATE ${JPEG_INCLUDE_DIRS})
 target_link_libraries(LibGfx PRIVATE ${JPEG_LIBRARIES})
 target_link_directories(LibGfx PRIVATE ${JPEG_LIBRARY_DIRS})
+
+find_package(PNG REQUIRED)
+target_include_directories(LibGfx PRIVATE ${PNG_INCLUDE_DIRS})
+target_link_libraries(LibGfx PRIVATE ${PNG_LIBRARIES})
+target_link_directories(LibGfx PRIVATE ${PNG_LIBRARY_DIRS})

Userland/Libraries/LibGfx/ImageFormats/PNGLoader.h

@@ -7,7 +7,6 @@
 #pragma once
 
 #include <LibGfx/ImageFormats/ImageDecoder.h>
-#include <LibGfx/ImageFormats/PNGShared.h>
 
 namespace Gfx {
 
@@ -30,12 +29,10 @@ public:
     virtual Optional<Metadata const&> metadata() override;
     virtual ErrorOr<Optional<ReadonlyBytes>> icc_data() override;
 
-    static void unfilter_scanline(PNG::FilterType filter, Bytes scanline_data, ReadonlyBytes previous_scanlines_data, u8 bytes_per_complete_pixel);
-
 private:
-    PNGImageDecoderPlugin(u8 const*, size_t);
-    bool ensure_image_data_chunk_was_decoded();
-    bool ensure_animation_frame_was_decoded(u32);
+    explicit PNGImageDecoderPlugin(ReadonlyBytes);
+
+    ErrorOr<bool> initialize();
 
     OwnPtr<PNGLoadingContext> m_context;
 };

Userland/Libraries/LibGfx/ImageFormats/PNGShared.h

@@ -1,67 +0,0 @@
-/*
- * Copyright (c) 2022, the SerenityOS developers.
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <AK/Array.h>
-#include <AK/Error.h>
-#include <AK/SIMD.h>
-
-namespace Gfx::PNG {
-
-// https://www.w3.org/TR/PNG/#5PNG-file-signature
-static constexpr Array<u8, 8> header = { 0x89, 'P', 'N', 'G', 13, 10, 26, 10 };
-
-// https://www.w3.org/TR/PNG/#6Colour-values
-enum class ColorType : u8 {
-    Greyscale = 0,
-    Truecolor = 2, // RGB
-    IndexedColor = 3,
-    GreyscaleWithAlpha = 4,
-    TruecolorWithAlpha = 6,
-};
-
-// https://www.w3.org/TR/PNG/#9Filter-types
-enum class FilterType : u8 {
-    None,
-    Sub,
-    Up,
-    Average,
-    Paeth,
-};
-
-inline ErrorOr<FilterType> filter_type(u8 byte)
-{
-    if (byte <= 4)
-        return static_cast<FilterType>(byte);
-    return Error::from_string_literal("PNGImageDecoderPlugin: Invalid PNG filter");
-}
-
-// https://www.w3.org/TR/PNG/#9Filter-type-4-Paeth
-ALWAYS_INLINE u8 paeth_predictor(u8 a, u8 b, u8 c)
-{
-    int p = a + b - c;
-    int pa = AK::abs(p - a);
-    int pb = AK::abs(p - b);
-    int pc = AK::abs(p - c);
-    if (pa <= pb && pa <= pc)
-        return a;
-    if (pb <= pc)
-        return b;
-    return c;
-}
-
-ALWAYS_INLINE AK::SIMD::u8x4 paeth_predictor(AK::SIMD::u8x4 a, AK::SIMD::u8x4 b, AK::SIMD::u8x4 c)
-{
-    return AK::SIMD::u8x4 {
-        paeth_predictor(a[0], b[0], c[0]),
-        paeth_predictor(a[1], b[1], c[1]),
-        paeth_predictor(a[2], b[2], c[2]),
-        paeth_predictor(a[3], b[3], c[3]),
-    };
-}
-
-};

Userland/Libraries/LibGfx/ImageFormats/PNGWriter.cpp

@@ -1,284 +1,69 @@
 /*
- * Copyright (c) 2021, Pierre Hoffmeister
- * Copyright (c) 2021, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2021, Aziz Berkay Yesilyurt <abyesilyurt@gmail.com>
+ * Copyright (c) 2024, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <AK/Concepts.h>
-#include <AK/FixedArray.h>
-#include <AK/SIMDExtras.h>
-#include <AK/String.h>
-#include <LibCompress/Zlib.h>
-#include <LibCrypto/Checksum/CRC32.h>
+#include <AK/NonnullOwnPtr.h>
+#include <AK/Vector.h>
 #include <LibGfx/Bitmap.h>
 #include <LibGfx/ImageFormats/PNGWriter.h>
-
-#pragma GCC diagnostic ignored "-Wpsabi"
+#include <png.h>
 
 namespace Gfx {
 
-class PNGChunk {
-    using data_length_type = u32;
-
-public:
-    explicit PNGChunk(String);
-    auto const& data() const { return m_data; }
-    String const& type() const { return m_type; }
-    ErrorOr<void> reserve(size_t bytes) { return m_data.try_ensure_capacity(bytes); }
-
-    template<typename T>
-    ErrorOr<void> add_as_big_endian(T);
-
-    ErrorOr<void> add_u8(u8);
-
-    ErrorOr<void> compress_and_add(ReadonlyBytes);
-    ErrorOr<void> add(ReadonlyBytes);
-
-    ErrorOr<void> store_type();
-    void store_data_length();
-    u32 crc();
-
-private:
-    ByteBuffer m_data;
-    String m_type;
+struct WriterContext {
+    Vector<u8*> row_pointers;
+    ByteBuffer png_data;
 };
 
-PNGChunk::PNGChunk(String type)
-    : m_type(move(type))
-{
-    VERIFY(m_type.bytes().size() == 4);
-
-    // NOTE: These are MUST() because they should always be able to fit in m_data's inline capacity.
-    MUST(add_as_big_endian<data_length_type>(0));
-    MUST(store_type());
-}
-
-ErrorOr<void> PNGChunk::store_type()
-{
-    TRY(add(type().bytes()));
-    return {};
-}
-
-void PNGChunk::store_data_length()
-{
-    auto data_length = BigEndian<u32>(m_data.size() - sizeof(data_length_type) - m_type.bytes().size());
-    __builtin_memcpy(m_data.offset_pointer(0), &data_length, sizeof(u32));
-}
-
-u32 PNGChunk::crc()
-{
-    u32 crc = Crypto::Checksum::CRC32({ m_data.offset_pointer(sizeof(data_length_type)), m_data.size() - sizeof(data_length_type) }).digest();
-    return crc;
-}
-
-ErrorOr<void> PNGChunk::compress_and_add(ReadonlyBytes uncompressed_bytes)
-{
-    return add(TRY(Compress::ZlibCompressor::compress_all(uncompressed_bytes, Compress::ZlibCompressionLevel::Best)));
-}
-
-ErrorOr<void> PNGChunk::add(ReadonlyBytes bytes)
-{
-    TRY(m_data.try_append(bytes));
-    return {};
-}
-
-template<typename T>
-ErrorOr<void> PNGChunk::add_as_big_endian(T data)
-{
-    auto data_out = AK::convert_between_host_and_big_endian(data);
-    TRY(m_data.try_append(&data_out, sizeof(T)));
-    return {};
-}
-
-ErrorOr<void> PNGChunk::add_u8(u8 data)
-{
-    TRY(m_data.try_append(data));
-    return {};
-}
-
-ErrorOr<void> PNGWriter::add_chunk(PNGChunk& png_chunk)
-{
-    png_chunk.store_data_length();
-    u32 crc = png_chunk.crc();
-    TRY(png_chunk.add_as_big_endian(crc));
-    TRY(m_data.try_append(png_chunk.data().data(), png_chunk.data().size()));
-    return {};
-}
-
-ErrorOr<void> PNGWriter::add_png_header()
-{
-    TRY(m_data.try_append(PNG::header.data(), PNG::header.size()));
-    return {};
-}
-
-ErrorOr<void> PNGWriter::add_IHDR_chunk(u32 width, u32 height, u8 bit_depth, PNG::ColorType color_type, u8 compression_method, u8 filter_method, u8 interlace_method)
-{
-    PNGChunk png_chunk { "IHDR"_string };
-    TRY(png_chunk.add_as_big_endian(width));
-    TRY(png_chunk.add_as_big_endian(height));
-    TRY(png_chunk.add_u8(bit_depth));
-    TRY(png_chunk.add_u8(to_underlying(color_type)));
-    TRY(png_chunk.add_u8(compression_method));
-    TRY(png_chunk.add_u8(filter_method));
-    TRY(png_chunk.add_u8(interlace_method));
-    TRY(add_chunk(png_chunk));
-    return {};
-}
-
-ErrorOr<void> PNGWriter::add_iCCP_chunk(ReadonlyBytes icc_data)
+ErrorOr<ByteBuffer> PNGWriter::encode(Gfx::Bitmap const& bitmap, Options options)
 {
-    // https://www.w3.org/TR/png/#11iCCP
-    PNGChunk chunk { "iCCP"_string };
-
-    TRY(chunk.add("embedded profile"sv.bytes()));
-    TRY(chunk.add_u8(0)); // \0-terminate profile name
+    auto context = make<WriterContext>();
+    int width = bitmap.width();
+    int height = bitmap.height();
 
-    TRY(chunk.add_u8(0)); // compression method deflate
-    TRY(chunk.compress_and_add(icc_data));
-
-    TRY(add_chunk(chunk));
-    return {};
-}
-
-ErrorOr<void> PNGWriter::add_IEND_chunk()
-{
-    PNGChunk png_chunk { "IEND"_string };
-    TRY(add_chunk(png_chunk));
-    return {};
-}
-
-union [[gnu::packed]] Pixel {
-    ARGB32 rgba { 0 };
-    struct {
-        u8 red;
-        u8 green;
-        u8 blue;
-        u8 alpha;
-    };
-    AK::SIMD::u8x4 simd;
-
-    ALWAYS_INLINE static AK::SIMD::u8x4 gfx_to_png(Pixel pixel)
-    {
-        swap(pixel.red, pixel.blue);
-        return pixel.simd;
+    png_structp png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, nullptr, nullptr, nullptr);
+    if (!png_ptr) {
+        return Error::from_string_literal("Failed to create PNG write struct");
     }
-};
-static_assert(AssertSize<Pixel, 4>());
-
-ErrorOr<void> PNGWriter::add_IDAT_chunk(Gfx::Bitmap const& bitmap)
-{
-    PNGChunk png_chunk { "IDAT"_string };
-    TRY(png_chunk.reserve(bitmap.size_in_bytes()));
-
-    ByteBuffer uncompressed_block_data;
-    TRY(uncompressed_block_data.try_ensure_capacity(bitmap.size_in_bytes() + bitmap.height()));
-
-    auto dummy_scanline = TRY(FixedArray<Pixel>::create(bitmap.width()));
-    auto const* scanline_minus_1 = dummy_scanline.data();
-
-    for (int y = 0; y < bitmap.height(); ++y) {
-        auto* scanline = reinterpret_cast<Pixel const*>(bitmap.scanline(y));
-
-        struct Filter {
-            PNG::FilterType type;
-            ByteBuffer buffer {};
-            int sum = 0;
 
-            ErrorOr<void> append(u8 byte)
-            {
-                TRY(buffer.try_append(byte));
-                sum += static_cast<i8>(byte);
-                return {};
-            }
-
-            ErrorOr<void> append(AK::SIMD::u8x4 simd)
-            {
-                TRY(append(simd[0]));
-                TRY(append(simd[1]));
-                TRY(append(simd[2]));
-                TRY(append(simd[3]));
-                return {};
-            }
-        };
-
-        Filter none_filter { .type = PNG::FilterType::None };
-        TRY(none_filter.buffer.try_ensure_capacity(sizeof(Pixel) * bitmap.width()));
-
-        Filter sub_filter { .type = PNG::FilterType::Sub };
-        TRY(sub_filter.buffer.try_ensure_capacity(sizeof(Pixel) * bitmap.width()));
-
-        Filter up_filter { .type = PNG::FilterType::Up };
-        TRY(up_filter.buffer.try_ensure_capacity(sizeof(Pixel) * bitmap.width()));
-
-        Filter average_filter { .type = PNG::FilterType::Average };
-        TRY(average_filter.buffer.try_ensure_capacity(sizeof(ARGB32) * bitmap.width()));
-
-        Filter paeth_filter { .type = PNG::FilterType::Paeth };
-        TRY(paeth_filter.buffer.try_ensure_capacity(sizeof(ARGB32) * bitmap.width()));
-
-        auto pixel_x_minus_1 = Pixel::gfx_to_png(dummy_scanline[0]);
-        auto pixel_xy_minus_1 = Pixel::gfx_to_png(dummy_scanline[0]);
-
-        for (int x = 0; x < bitmap.width(); ++x) {
-            auto pixel = Pixel::gfx_to_png(scanline[x]);
-            auto pixel_y_minus_1 = Pixel::gfx_to_png(scanline_minus_1[x]);
-
-            TRY(none_filter.append(pixel));
-
-            TRY(sub_filter.append(pixel - pixel_x_minus_1));
-
-            TRY(up_filter.append(pixel - pixel_y_minus_1));
+    png_infop info_ptr = png_create_info_struct(png_ptr);
+    if (!info_ptr) {
+        png_destroy_write_struct(&png_ptr, nullptr);
+        return Error::from_string_literal("Failed to create PNG info struct");
+    }
 
-            // The sum Orig(a) + Orig(b) shall be performed without overflow (using at least nine-bit arithmetic).
-            auto sum = AK::SIMD::to_u16x4(pixel_x_minus_1) + AK::SIMD::to_u16x4(pixel_y_minus_1);
-            auto average = AK::SIMD::to_u8x4(sum / 2);
-            TRY(average_filter.append(pixel - average));
+    if (setjmp(png_jmpbuf(png_ptr))) {
+        png_destroy_write_struct(&png_ptr, &info_ptr);
+        return Error::from_string_literal("Error during PNG encoding");
+    }
 
-            TRY(paeth_filter.append(pixel - PNG::paeth_predictor(pixel_x_minus_1, pixel_y_minus_1, pixel_xy_minus_1)));
+    if (options.icc_data.has_value()) {
+        png_set_iCCP(png_ptr, info_ptr, "embedded profile", 0, options.icc_data->data(), options.icc_data->size());
+    }
 
-            pixel_x_minus_1 = pixel;
-            pixel_xy_minus_1 = pixel_y_minus_1;
-        }
+    if (bitmap.format() == BitmapFormat::BGRA8888 || bitmap.format() == BitmapFormat::BGRx8888) {
+        png_set_bgr(png_ptr);
+    }
 
-        scanline_minus_1 = scanline;
+    png_set_write_fn(png_ptr, &context->png_data, [](png_structp png_ptr, u8* data, size_t length) {
+        auto* buffer = reinterpret_cast<ByteBuffer*>(png_get_io_ptr(png_ptr));
+        buffer->append(data, length); }, nullptr);
 
-        // 12.8 Filter selection: https://www.w3.org/TR/PNG/#12Filter-selection
-        // For best compression of truecolour and greyscale images, the recommended approach
-        // is adaptive filtering in which a filter is chosen for each scanline.
-        // The following simple heuristic has performed well in early tests:
-        // compute the output scanline using all five filters, and select the filter that gives the smallest sum of absolute values of outputs.
-        // (Consider the output bytes as signed differences for this test.)
-        Filter& best_filter = none_filter;
-        if (abs(best_filter.sum) > abs(sub_filter.sum))
-            best_filter = sub_filter;
-        if (abs(best_filter.sum) > abs(up_filter.sum))
-            best_filter = up_filter;
-        if (abs(best_filter.sum) > abs(average_filter.sum))
-            best_filter = average_filter;
-        if (abs(best_filter.sum) > abs(paeth_filter.sum))
-            best_filter = paeth_filter;
+    png_set_IHDR(png_ptr, info_ptr, width, height, 8, PNG_COLOR_TYPE_RGBA, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
 
-        TRY(uncompressed_block_data.try_append(to_underlying(best_filter.type)));
-        TRY(uncompressed_block_data.try_append(best_filter.buffer));
+    context->row_pointers.resize(height);
+    for (int y = 0; y < height; ++y) {
+        context->row_pointers[y] = const_cast<u8*>(bitmap.scanline_u8(y));
     }
 
-    TRY(png_chunk.compress_and_add(uncompressed_block_data));
-    TRY(add_chunk(png_chunk));
-    return {};
-}
+    png_set_rows(png_ptr, info_ptr, context->row_pointers.data());
+    png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, nullptr);
 
-ErrorOr<ByteBuffer> PNGWriter::encode(Gfx::Bitmap const& bitmap, Options options)
-{
-    PNGWriter writer;
-    TRY(writer.add_png_header());
-    TRY(writer.add_IHDR_chunk(bitmap.width(), bitmap.height(), 8, PNG::ColorType::TruecolorWithAlpha, 0, 0, 0));
-    if (options.icc_data.has_value())
-        TRY(writer.add_iCCP_chunk(options.icc_data.value()));
-    TRY(writer.add_IDAT_chunk(bitmap));
-    TRY(writer.add_IEND_chunk());
-    return ByteBuffer::copy(writer.m_data);
+    png_destroy_write_struct(&png_ptr, &info_ptr);
+
+    return context->png_data;
 }
 
 }

Userland/Libraries/LibGfx/ImageFormats/PNGWriter.h

@@ -1,21 +1,18 @@
 /*
- * Copyright (c) 2021, Pierre Hoffmeister
- * Copyright (c) 2021, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2024, Andreas Kling <andreas@ladybird.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #pragma once
 
+#include <AK/Forward.h>
 #include <AK/Optional.h>
-#include <AK/Vector.h>
+#include <AK/Span.h>
 #include <LibGfx/Forward.h>
-#include <LibGfx/ImageFormats/PNGShared.h>
 
 namespace Gfx {
 
-class PNGChunk;
-
 // This is not a nested struct to work around https://llvm.org/PR36684
 struct PNGWriterOptions {
     // Data for the iCCP chunk.
@@ -31,14 +28,6 @@ public:
 
 private:
     PNGWriter() = default;
-
-    Vector<u8> m_data;
-    ErrorOr<void> add_chunk(PNGChunk&);
-    ErrorOr<void> add_png_header();
-    ErrorOr<void> add_IHDR_chunk(u32 width, u32 height, u8 bit_depth, PNG::ColorType color_type, u8 compression_method, u8 filter_method, u8 interlace_method);
-    ErrorOr<void> add_iCCP_chunk(ReadonlyBytes icc_data);
-    ErrorOr<void> add_IDAT_chunk(Gfx::Bitmap const&);
-    ErrorOr<void> add_IEND_chunk();
 };
 
 }

vcpkg.json

@@ -7,6 +7,10 @@
     },
     "icu",
     "libjpeg-turbo",
+    {
+      "name": "libpng",
+      "features": [ "apng" ]
+    },
     "skia",
     "sqlite3",
     "woff2"
@@ -24,6 +28,10 @@
       "name": "libjpeg-turbo",
       "version": "3.0.2"
     },
+    {
+      "name": "libpng",
+      "version": "1.6.43#1"
+    },
     {
       "name": "skia",
       "version": "124#0"