From fb79aa6159d950e0c0559f2c658d6e322c4c13ed Mon Sep 17 00:00:00 2001
From: Lucas CHOLLET <lucas.chollet@free.fr>
Date: Wed, 22 May 2024 17:43:23 -0400
Subject: [PATCH] LibGfx/GIF: Correctly write frames with a non-null position

---
 Tests/LibGfx/TestImageWriter.cpp                     | 10 +++++++++-
 Userland/Libraries/LibGfx/ImageFormats/GIFWriter.cpp | 11 ++++-------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/Tests/LibGfx/TestImageWriter.cpp b/Tests/LibGfx/TestImageWriter.cpp
index 3c5528a0d53a06..1de90d334c723b 100644
--- a/Tests/LibGfx/TestImageWriter.cpp
+++ b/Tests/LibGfx/TestImageWriter.cpp
@@ -129,19 +129,23 @@ TEST_CASE(test_gif_animated)
 {
     auto bitmap_1 = TRY_OR_FAIL(TRY_OR_FAIL(create_test_rgb_bitmap())->cropped({ 0, 0, 16, 16 }));
     auto bitmap_2 = TRY_OR_FAIL(TRY_OR_FAIL(create_test_rgb_bitmap())->cropped({ 16, 16, 16, 16 }));
+    auto bitmap_3 = TRY_OR_FAIL(bitmap_2->clone());
+
+    bitmap_3->scanline(3)[3] = Color(Color::NamedColor::Red).value();
 
     auto stream_buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(3072));
     FixedMemoryStream stream { Bytes { stream_buffer } };
     auto animation_writer = TRY_OR_FAIL(Gfx::GIFWriter::start_encoding_animation(stream, bitmap_1->size(), 0));
     TRY_OR_FAIL(animation_writer->add_frame(*bitmap_1, 100));
     TRY_OR_FAIL(animation_writer->add_frame(*bitmap_2, 200));
+    TRY_OR_FAIL(animation_writer->add_frame_relative_to_last_frame(*bitmap_3, 200, *bitmap_2));
 
     auto encoded_animation = ReadonlyBytes { stream_buffer.data(), stream.offset() };
 
     auto decoder = TRY_OR_FAIL(Gfx::GIFImageDecoderPlugin::create(encoded_animation));
 
     EXPECT_EQ(decoder->size(), bitmap_1->size());
-    EXPECT_EQ(decoder->frame_count(), 2u);
+    EXPECT_EQ(decoder->frame_count(), 3u);
     EXPECT_EQ(decoder->loop_count(), 0u);
     EXPECT(decoder->is_animated());
 
@@ -152,6 +156,10 @@ TEST_CASE(test_gif_animated)
     auto const frame_2 = TRY_OR_FAIL(decoder->frame(1));
     EXPECT_EQ(frame_2.duration, 200);
     expect_bitmaps_equal(*frame_2.image, bitmap_2);
+
+    auto const frame_3 = TRY_OR_FAIL(decoder->frame(2));
+    EXPECT_EQ(frame_3.duration, 200);
+    expect_bitmaps_equal(*frame_3.image, bitmap_3);
 }
 
 TEST_CASE(test_jpeg)
diff --git a/Userland/Libraries/LibGfx/ImageFormats/GIFWriter.cpp b/Userland/Libraries/LibGfx/ImageFormats/GIFWriter.cpp
index a50e04669da4c2..632791c3b762bd 100644
--- a/Userland/Libraries/LibGfx/ImageFormats/GIFWriter.cpp
+++ b/Userland/Libraries/LibGfx/ImageFormats/GIFWriter.cpp
@@ -93,16 +93,16 @@ ErrorOr<void> write_image_data(Stream& stream, Bitmap const& bitmap, ColorPalett
     return {};
 }
 
-ErrorOr<void> write_image_descriptor(BigEndianOutputBitStream& stream, Bitmap const& bitmap)
+ErrorOr<void> write_image_descriptor(BigEndianOutputBitStream& stream, Bitmap const& bitmap, IntPoint at = {})
 {
     // 20. Image Descriptor
 
     // Image Separator
     TRY(stream.write_value<u8>(0x2c));
     // Image Left Position
-    TRY(stream.write_value<u16>(0));
+    TRY(stream.write_value<u16>(at.x()));
     // Image Top Position
-    TRY(stream.write_value<u16>(0));
+    TRY(stream.write_value<u16>(at.y()));
     // Image Width
     TRY(stream.write_value<u16>(bitmap.width()));
     // Image Height
@@ -177,9 +177,6 @@ class GIFAnimationWriter : public AnimationWriter {
 
 ErrorOr<void> GIFAnimationWriter::add_frame(Bitmap& bitmap, int duration_ms, IntPoint at = {})
 {
-    // FIXME: Consider frame's position
-    (void)at;
-
     // Let's get rid of the previously written trailer
     if (!m_is_first_frame)
         TRY(m_stream.seek(-1, SeekMode::FromCurrentPosition));
@@ -189,7 +186,7 @@ ErrorOr<void> GIFAnimationWriter::add_frame(Bitmap& bitmap, int duration_ms, Int
     // Write a Table-Based Image
     BigEndianOutputBitStream bit_stream { MaybeOwned { m_stream } };
     TRY(write_graphic_control_extension(bit_stream, duration_ms));
-    TRY(write_image_descriptor(bit_stream, bitmap));
+    TRY(write_image_descriptor(bit_stream, bitmap, at));
 
     auto const palette = TRY(median_cut(bitmap, 256));
     TRY(write_color_table(m_stream, palette));