diff --git a/Tests/LibGfx/TestImageWriter.cpp b/Tests/LibGfx/TestImageWriter.cpp index 01f793ce37f..0d7b5e52fa8 100644 --- a/Tests/LibGfx/TestImageWriter.cpp +++ b/Tests/LibGfx/TestImageWriter.cpp @@ -6,6 +6,9 @@ #include #include +#include +#include +#include #include #include #include @@ -37,31 +40,38 @@ static ErrorOr> expect_single_frame_of_size(Gfx::Imag return frame; } +template +static ErrorOr encode_bitmap(Gfx::Bitmap const& bitmap, ExtraArgs... extra_args) +{ + if constexpr (requires(AllocatingMemoryStream stream) { Writer::encode(stream, bitmap, extra_args...); }) { + AllocatingMemoryStream stream; + TRY(Writer::encode(stream, bitmap, extra_args...)); + return stream.read_until_eof(); + } else { + return Writer::encode(bitmap, extra_args...); + } +} + template static ErrorOr> get_roundtrip_bitmap(Gfx::Bitmap const& bitmap) { - ByteBuffer encoded_data; - if constexpr (requires(AllocatingMemoryStream stream) { Writer::encode(stream, bitmap); }) { - AllocatingMemoryStream stream; - TRY(Writer::encode(stream, bitmap)); - encoded_data = TRY(stream.read_until_eof()); - } else { - encoded_data = TRY(Writer::encode(bitmap)); - } + auto encoded_data = TRY(encode_bitmap(bitmap)); + return expect_single_frame_of_size(*TRY(Loader::create(encoded_data)), bitmap.size()); +} - auto plugin = TRY(Loader::create(encoded_data)); - return expect_single_frame_of_size(*plugin, bitmap.size()); +static void expect_bitmaps_equal(Gfx::Bitmap const& a, Gfx::Bitmap const& b) +{ + VERIFY(a.size() == b.size()); + for (int y = 0; y < a.height(); ++y) + for (int x = 0; x < a.width(); ++x) + EXPECT_EQ(a.get_pixel(x, y), b.get_pixel(x, y)); } template static ErrorOr test_roundtrip(Gfx::Bitmap const& bitmap) { auto decoded = TRY((get_roundtrip_bitmap(bitmap))); - - for (int y = 0; y < bitmap.height(); ++y) - for (int x = 0; x < bitmap.width(); ++x) - EXPECT_EQ(decoded->get_pixel(x, y), bitmap.get_pixel(x, y)); - + expect_bitmaps_equal(*decoded, bitmap); return {}; } @@ -120,3 +130,18 @@ TEST_CASE(test_webp) TRY_OR_FAIL((test_roundtrip(TRY_OR_FAIL(create_test_rgb_bitmap())))); TRY_OR_FAIL((test_roundtrip(TRY_OR_FAIL(create_test_rgba_bitmap())))); } + +TEST_CASE(test_webp_icc) +{ + auto sRGB_icc_profile = MUST(Gfx::ICC::sRGB()); + auto sRGB_icc_data = MUST(Gfx::ICC::encode(sRGB_icc_profile)); + + auto rgba_bitmap = TRY_OR_FAIL(create_test_rgba_bitmap()); + auto encoded_rgba_bitmap = TRY_OR_FAIL((encode_bitmap(rgba_bitmap, Gfx::WebPEncoderOptions { .icc_data = sRGB_icc_data }))); + + auto decoded_rgba_plugin = TRY_OR_FAIL(Gfx::WebPImageDecoderPlugin::create(encoded_rgba_bitmap)); + expect_bitmaps_equal(*TRY_OR_FAIL(expect_single_frame_of_size(*decoded_rgba_plugin, rgba_bitmap->size())), rgba_bitmap); + auto decoded_rgba_profile = TRY_OR_FAIL(Gfx::ICC::Profile::try_load_from_externally_owned_memory(TRY_OR_FAIL(decoded_rgba_plugin->icc_data()).value())); + auto reencoded_icc_data = TRY_OR_FAIL(Gfx::ICC::encode(decoded_rgba_profile)); + EXPECT_EQ(sRGB_icc_data, reencoded_icc_data); +} diff --git a/Userland/Libraries/LibGfx/ImageFormats/WebPWriter.cpp b/Userland/Libraries/LibGfx/ImageFormats/WebPWriter.cpp index bb869e6c2f6..08b4f593e41 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/WebPWriter.cpp +++ b/Userland/Libraries/LibGfx/ImageFormats/WebPWriter.cpp @@ -215,33 +215,41 @@ static ErrorOr write_VP8X_header(Stream& stream, VP8XHeader const& header) LittleEndianOutputBitStream bit_stream { MaybeOwned(stream) }; + // Don't use bit_stream.write_bits() to write individual flags here: + // The spec describes bit flags in MSB to LSB order, but write_bits() writes LSB to MSB. + u8 flags = 0; // "Reserved (Rsv): 2 bits // MUST be 0. Readers MUST ignore this field." - TRY(bit_stream.write_bits(0u, 2u)); // "ICC profile (I): 1 bit // Set if the file contains an 'ICCP' Chunk." - TRY(bit_stream.write_bits(header.has_icc, 1u)); + if (header.has_icc) + flags |= 0x20; // "Alpha (L): 1 bit // Set if any of the frames of the image contain transparency information ("alpha")." - TRY(bit_stream.write_bits(header.has_alpha, 1u)); + if (header.has_alpha) + flags |= 0x10; // "Exif metadata (E): 1 bit // Set if the file contains Exif metadata." - TRY(bit_stream.write_bits(header.has_exif, 1u)); + if (header.has_exif) + flags |= 0x8; // "XMP metadata (X): 1 bit // Set if the file contains XMP metadata." - TRY(bit_stream.write_bits(header.has_xmp, 1u)); + if (header.has_xmp) + flags |= 0x4; // "Animation (A): 1 bit // Set if this is an animated image. Data in 'ANIM' and 'ANMF' Chunks should be used to control the animation." - TRY(bit_stream.write_bits(header.has_animation, 1u)); + if (header.has_animation) + flags |= 0x2; // "Reserved (R): 1 bit // MUST be 0. Readers MUST ignore this field." - TRY(bit_stream.write_bits(0u, 1u)); + + TRY(bit_stream.write_bits(flags, 8u)); // "Reserved: 24 bits // MUST be 0. Readers MUST ignore this field." @@ -304,7 +312,7 @@ ErrorOr WebPWriter::encode(Stream& stream, Bitmap const& bitmap, Options c iccp_chunk_bytes = TRY(iccp_chunk_stream.read_until_eof()); AllocatingMemoryStream vp8x_header_stream; - TRY(write_VP8X_header(vp8x_header_stream, { .has_icc = true, .width = (u32)bitmap.width(), .height = (u32)bitmap.height() })); + TRY(write_VP8X_header(vp8x_header_stream, { .has_icc = true, .has_alpha = alpha_is_used_hint, .width = (u32)bitmap.width(), .height = (u32)bitmap.height() })); auto vp8x_header_bytes = TRY(vp8x_header_stream.read_until_eof()); AllocatingMemoryStream vp8x_chunk_stream;