Skip to content

Commit

Permalink
LibGfx/GIF: Correctly write frames with a non-null position
Browse files Browse the repository at this point in the history
  • Loading branch information
LucasChollet authored and AtkinsSJ committed May 25, 2024
1 parent 934516d commit fb79aa6
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 8 deletions.
10 changes: 9 additions & 1 deletion Tests/LibGfx/TestImageWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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)
Expand Down
11 changes: 4 additions & 7 deletions Userland/Libraries/LibGfx/ImageFormats/GIFWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand Down

0 comments on commit fb79aa6

Please sign in to comment.