LibGfx: Only include frames with fcTL chunks in the animation

Before this change, IDAT data was mistakenly always included in the
animation. Now we only include frames with explicit fcTL chunks.

As per the PNG spec (third edition):
"The static image may be included as the first frame of the animation
by the presence of a single fcTL chunk before IDAT. Otherwise, the
static image is not part of the animation."

We also fall back to the IDAT data when APNG has acTL but no fcTL
chunks. Test image is 062.png from fDAT-inherits-cICP.html from WPT.

Libraries/LibGfx/ImageFormats/PNGLoader.cpp

@@ -267,6 +267,7 @@ ErrorOr<size_t> PNGLoadingContext::read_frames(png_structp png_ptr, png_infop in
         // Conceptually, at the beginning of each play the output buffer must be completely initialized to a fully transparent black rectangle, with width and height dimensions from the `IHDR` chunk.
         auto output_buffer = TRY(Bitmap::create(BitmapFormat::BGRA8888, AlphaType::Unpremultiplied, size));
         auto painter = Painter::create(output_buffer);
+        size_t animation_frame_count = 0;
 
         for (size_t frame_index = 0; frame_index < frame_count; ++frame_index) {
             png_read_frame_head(png_ptr, info_ptr);
@@ -289,7 +290,8 @@ ErrorOr<size_t> PNGLoadingContext::read_frames(png_structp png_ptr, png_infop in
                 return static_cast<int>(unsigned_duration_ms);
             };
 
-            if (png_get_valid(png_ptr, info_ptr, PNG_INFO_fcTL)) {
+            bool has_fcTL = png_get_valid(png_ptr, info_ptr, PNG_INFO_fcTL);
+            if (has_fcTL) {
                 png_get_next_frame_fcTL(png_ptr, info_ptr, &width, &height, &x, &y, &delay_num, &delay_den, &dispose_op, &blend_op);
             } else {
                 width = png_get_image_width(png_ptr, info_ptr);
@@ -316,7 +318,10 @@ ErrorOr<size_t> PNGLoadingContext::read_frames(png_structp png_ptr, png_infop in
                 VERIFY_NOT_REACHED();
             }
 
-            frame_descriptors.append({ TRY(output_buffer->clone()), duration_ms() });
+            if (has_fcTL) {
+                animation_frame_count++;
+                frame_descriptors.append({ TRY(output_buffer->clone()), duration_ms() });
+            }
 
             switch (dispose_op) {
             case PNG_DISPOSE_OP_NONE:
@@ -334,6 +339,15 @@ ErrorOr<size_t> PNGLoadingContext::read_frames(png_structp png_ptr, png_infop in
                 VERIFY_NOT_REACHED();
             }
         }
+
+        frame_count = animation_frame_count;
+
+        // If we didn't find any valid animation frames with fcTL chunks, fall back to using the base IDAT data as a single frame.
+        if (frame_count == 0) {
+            auto frame_bitmap = TRY(decode_frame(size));
+            frame_descriptors.append({ move(frame_bitmap), 0 });
+            frame_count = 1;
+        }
     } else {
         // This is a single-frame PNG.
         frame_count = 1;
@@ -342,7 +356,7 @@ ErrorOr<size_t> PNGLoadingContext::read_frames(png_structp png_ptr, png_infop in
         auto decoded_frame_bitmap = TRY(decode_frame(size));
         frame_descriptors.append({ move(decoded_frame_bitmap), 0 });
     }
-    return this->frame_count;
+    return frame_count;
 }
 
 PNGImageDecoderPlugin::~PNGImageDecoderPlugin() = default;

Tests/LibGfx/TestImageDecoder.cpp

@@ -363,6 +363,22 @@ TEST_CASE(test_png)
     TRY_OR_FAIL(expect_single_frame(*plugin_decoder));
 }
 
+TEST_CASE(test_apng)
+{
+    auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("png/apng-1-frame.png"sv)));
+    EXPECT(Gfx::PNGImageDecoderPlugin::sniff(file->bytes()));
+    auto plugin_decoder = TRY_OR_FAIL(Gfx::PNGImageDecoderPlugin::create(file->bytes()));
+
+    EXPECT_EQ(plugin_decoder->frame_count(), 1u);
+    EXPECT_EQ(plugin_decoder->loop_count(), 0u);
+
+    auto frame = TRY_OR_FAIL(plugin_decoder->frame(0));
+
+    EXPECT_EQ(frame.duration, 1000);
+    EXPECT_EQ(frame.image->get_pixel(64, 32), Gfx::Color(117, 252, 76));
+    EXPECT_EQ(frame.image->size(), Gfx::IntSize(128, 64));
+}
+
 TEST_CASE(test_exif)
 {
     auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("png/exif.png"sv)));

Tests/LibWeb/TestConfig.ini

@@ -323,7 +323,3 @@ Text/input/WebSocket/echo.html
 
 ; Times out due to us not implementing auto-commit the correct way.
 Text/input/wpt-import/IndexedDB/idbfactory_open.any.html
-
-; Inconsistently fails on CI - we seem to wrongly include the non-animated data in the animation.
-; https://github.com/LadybirdBrowser/ladybird/issues/4519
-Ref/input/wpt-import/png/apng/fDAT-inherits-cICP.html