From dc612231f5879f406818be6711cf2b9ada7e2910 Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 22 Jul 2023 00:55:50 +0300 Subject: [PATCH] LibGfx/TGA: Simplify the code by converting it to use AK::Stream The conversion to AK::Stream makes everything much more straightforward and understandable now, because we remove the custom reader we had. Because AK::Stream is more tested, it means that the code should be now more robust against bugs as well. --- .../LibGfx/ImageFormats/TGALoader.cpp | 245 ++++++------------ .../Libraries/LibGfx/ImageFormats/TGALoader.h | 5 +- 2 files changed, 79 insertions(+), 171 deletions(-) diff --git a/Userland/Libraries/LibGfx/ImageFormats/TGALoader.cpp b/Userland/Libraries/LibGfx/ImageFormats/TGALoader.cpp index 888efe7c0a..686a99311c 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/TGALoader.cpp +++ b/Userland/Libraries/LibGfx/ImageFormats/TGALoader.cpp @@ -4,9 +4,11 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include +#include #include namespace Gfx { @@ -37,134 +39,30 @@ struct [[gnu::packed]] TGAHeader { u8 bits_per_pixel; u8 image_descriptor; }; - static_assert(sizeof(TGAHeader) == 18); -union [[gnu::packed]] TGAPixel { - struct TGAColor { - u8 blue; - u8 green; - u8 red; - u8 alpha; - } components; +} - u32 data; +template<> +struct AK::Traits : public GenericTraits { + static constexpr bool is_trivially_serializable() { return true; } }; -struct TGAPixelPacket { - bool raw; - u8 pixels_count; -}; - -static_assert(AssertSize()); - -class TGAReader { -public: - TGAReader(ReadonlyBytes data) - : m_data(move(data)) - { - } - - TGAReader(ReadonlyBytes data, size_t index) - : m_data(move(data)) - , m_index(index) - { - } - - ALWAYS_INLINE u8 read_u8() - { - u8 value = m_data[m_index]; - m_index++; - return value; - } - - ALWAYS_INLINE i8 read_i8() - { - return static_cast(read_u8()); - } - - ALWAYS_INLINE u16 read_u16() - { - return read_u8() | read_u8() << 8; - } - - ALWAYS_INLINE i16 read_i16() - { - return read_i8() | read_i8() << 8; - } - - ALWAYS_INLINE u32 read_u32() - { - return read_u16() | read_u16() << 16; - } - - ALWAYS_INLINE i32 read_i32() - { - return read_i16() | read_i16() << 16; - } - - ALWAYS_INLINE TGAPixelPacket read_packet_type() - { - auto pixel_packet_type = read_u8(); - auto pixel_packet = TGAPixelPacket(); - pixel_packet.raw = !(pixel_packet_type & 0x80); - pixel_packet.pixels_count = (pixel_packet_type & 0x7f); - - // NOTE: Run-length-encoded/Raw pixel packets cannot encode zero pixels, - // so value 0 stands for 1 pixel, 1 stands for 2, etc... - pixel_packet.pixels_count++; - return pixel_packet; - } - - ALWAYS_INLINE TGAPixel read_pixel(u8 bits_per_pixel) - { - auto pixel = TGAPixel(); - - switch (bits_per_pixel) { - case 24: - pixel.components.blue = read_u8(); - pixel.components.green = read_u8(); - pixel.components.red = read_u8(); - pixel.components.alpha = 0xFF; - return pixel; - - case 32: - pixel.components.blue = read_u8(); - pixel.components.green = read_u8(); - pixel.components.red = read_u8(); - pixel.components.alpha = read_u8(); - return pixel; - - default: - VERIFY_NOT_REACHED(); - } - } - - size_t index() const - { - return m_index; - } - - ReadonlyBytes data() const - { - return m_data; - } - -private: - ReadonlyBytes m_data; - size_t m_index { 0 }; -}; +namespace Gfx { struct TGALoadingContext { + TGALoadingContext(FixedMemoryStream stream) + : stream(move(stream)) + { + } + FixedMemoryStream stream; TGAHeader header {}; - OwnPtr reader = { nullptr }; RefPtr bitmap; }; -TGAImageDecoderPlugin::TGAImageDecoderPlugin(u8 const* file_data, size_t file_size) +TGAImageDecoderPlugin::TGAImageDecoderPlugin(NonnullOwnPtr context) + : m_context(move(context)) { - m_context = make(); - m_context->reader = make(ReadonlyBytes { file_data, file_size }); } TGAImageDecoderPlugin::~TGAImageDecoderPlugin() = default; @@ -174,55 +72,68 @@ IntSize TGAImageDecoderPlugin::size() return IntSize { m_context->header.width, m_context->header.height }; } +static ErrorOr ensure_header_validity(TGAHeader const& header, size_t whole_image_stream_size) +{ + auto bytes_remaining = whole_image_stream_size - sizeof(TGAHeader); + if ((header.bits_per_pixel % 8) != 0 || header.bits_per_pixel < 8 || header.bits_per_pixel > 32) + return Error::from_string_literal("Invalid bit depth"); + if (header.data_type_code == TGADataType::UncompressedRGB && bytes_remaining < static_cast(header.width) * header.height * (header.bits_per_pixel / 8)) + return Error::from_string_literal("Not enough data to read an image with the expected size"); + return {}; +} + ErrorOr TGAImageDecoderPlugin::decode_tga_header() { - auto& reader = m_context->reader; - if (reader->data().size() < sizeof(TGAHeader)) - return Error::from_string_literal("Not enough data to be a TGA image"); - - m_context->header.id_length = reader->read_u8(); - m_context->header.color_map_type = reader->read_u8(); - m_context->header.data_type_code = static_cast(reader->read_u8()); - m_context->header.color_map_origin = reader->read_i16(); - m_context->header.color_map_length = reader->read_i16(); - m_context->header.color_map_depth = reader->read_u8(); - m_context->header.x_origin = reader->read_i16(); - m_context->header.y_origin = reader->read_i16(); - m_context->header.width = reader->read_u16(); - m_context->header.height = reader->read_u16(); - m_context->header.bits_per_pixel = reader->read_u8(); - m_context->header.image_descriptor = reader->read_u8(); - - auto bytes_remaining = reader->data().size() - reader->index(); - - if (m_context->header.data_type_code == TGADataType::UncompressedRGB && bytes_remaining < static_cast(m_context->header.width) * m_context->header.height * (m_context->header.bits_per_pixel / 8)) - return Error::from_string_literal("Not enough data to read an image with the expected size"); - - if (m_context->header.bits_per_pixel < 8 || m_context->header.bits_per_pixel > 32) - return Error::from_string_literal("Invalid bit depth"); - + m_context->header = TRY(m_context->stream.read_value()); + TRY(ensure_header_validity(m_context->header, m_context->stream.readonly_bytes().size())); return {}; } ErrorOr TGAImageDecoderPlugin::validate_before_create(ReadonlyBytes data) { - if (data.size() < sizeof(TGAHeader)) - return false; - TGAHeader const& header = *reinterpret_cast(data.data()); - if (header.data_type_code == TGADataType::UncompressedRGB && data.size() < static_cast(header.width) * header.height * (header.bits_per_pixel / 8)) - return false; - if (header.bits_per_pixel < 8 || header.bits_per_pixel > 32) - return false; - return true; + FixedMemoryStream stream { data }; + auto header = TRY(stream.read_value()); + return !ensure_header_validity(header, data.size()).is_error(); } ErrorOr> TGAImageDecoderPlugin::create(ReadonlyBytes data) { - auto plugin = TRY(adopt_nonnull_own_or_enomem(new (nothrow) TGAImageDecoderPlugin(data.data(), data.size()))); + FixedMemoryStream stream { data }; + auto context = TRY(adopt_nonnull_own_or_enomem(new (nothrow) TGALoadingContext(move(stream)))); + auto plugin = TRY(adopt_nonnull_own_or_enomem(new (nothrow) TGAImageDecoderPlugin(move(context)))); TRY(plugin->decode_tga_header()); return plugin; } +static ErrorOr read_pixel_from_stream(Stream& stream, size_t bytes_size) +{ + // NOTE: We support 24-bit color pixels and 32-bit color pixels + VERIFY(bytes_size == 3 || bytes_size == 4); + if (bytes_size == 3) { + Array raw; + TRY(stream.read_until_filled(raw.span())); + return Color(raw[2], raw[1], raw[0]).value(); + } + return stream.read_value(); +} + +struct TGAPixelPacketHeader { + bool raw { false }; + u8 pixels_count { 0 }; +}; + +static ErrorOr read_pixel_packet_header(Stream& stream) +{ + auto const pixel_packet_header = TRY(stream.read_value()); + bool pixels_raw_in_packet = !(pixel_packet_header & 0x80); + u8 pixels_count_in_packet = (pixel_packet_header & 0x7f); + // NOTE: Run-length-encoded/Raw pixel packets cannot encode zero pixels, + // so value 0 stands for 1 pixel, 1 stands for 2, etc... + pixels_count_in_packet++; + VERIFY(pixels_count_in_packet > 0); + return TGAPixelPacketHeader { pixels_raw_in_packet, pixels_count_in_packet }; +} + ErrorOr TGAImageDecoderPlugin::frame(size_t index, Optional) { auto bits_per_pixel = m_context->header.bits_per_pixel; @@ -239,16 +150,8 @@ ErrorOr TGAImageDecoderPlugin::frame(size_t index, Optiona if (color_map > 1) return Error::from_string_literal("TGAImageDecoderPlugin: Invalid color map type"); - if (m_context->bitmap) { + if (m_context->bitmap) return ImageFrameDescriptor { m_context->bitmap, 0 }; - } else { - // NOTE: Just to be on the safe side, if m_context->bitmap is nullptr, then - // just re-construct the reader object. This will ensure that if the bitmap - // was set as volatile and therefore it is gone, we can always re-generate it - // with a new call to this method! - VERIFY(m_context->reader); - m_context->reader = make(m_context->reader->data(), sizeof(TGAHeader)); - } RefPtr bitmap; switch (bits_per_pixel) { @@ -278,30 +181,34 @@ ErrorOr TGAImageDecoderPlugin::frame(size_t index, Optiona if (x_origin != 0 && x_origin != width) return Error::from_string_literal("TGAImageDecoderPlugin: Can only handle X origin which is 0 or the entire width"); + VERIFY((bits_per_pixel % 8) == 0); + auto bytes_per_pixel = bits_per_pixel / 8; + switch (data_type) { case TGADataType::UncompressedRGB: { for (int row = 0; row < height; ++row) { for (int col = 0; col < width; ++col) { - auto pixel = m_context->reader->read_pixel(bits_per_pixel); auto actual_row = row; if (y_origin < height) actual_row = height - 1 - row; auto actual_col = col; if (x_origin > width) actual_col = width - 1 - col; - bitmap->scanline(actual_row)[actual_col] = pixel.data; + bitmap->scanline(actual_row)[actual_col] = TRY(read_pixel_from_stream(m_context->stream, bytes_per_pixel)); } } break; } + case TGADataType::RunLengthEncodedRGB: { size_t pixel_index = 0; size_t pixel_count = height * width; while (pixel_index < pixel_count) { - auto packet_type = m_context->reader->read_packet_type(); - VERIFY(packet_type.pixels_count > 0); - TGAPixel pixel = m_context->reader->read_pixel(bits_per_pixel); - auto max_pixel_index = min(pixel_index + packet_type.pixels_count, pixel_count); + auto pixel_packet_header = TRY(read_pixel_packet_header(m_context->stream)); + VERIFY(pixel_packet_header.pixels_count > 0); + + auto pixel = TRY(read_pixel_from_stream(m_context->stream, bytes_per_pixel)); + auto max_pixel_index = min(pixel_index + pixel_packet_header.pixels_count, pixel_count); for (size_t current_pixel_index = pixel_index; current_pixel_index < max_pixel_index; ++current_pixel_index) { int row = current_pixel_index / width; int col = current_pixel_index % width; @@ -311,11 +218,11 @@ ErrorOr TGAImageDecoderPlugin::frame(size_t index, Optiona auto actual_col = col; if (x_origin > width) actual_col = width - 1 - col; - bitmap->scanline(actual_row)[actual_col] = pixel.data; - if (packet_type.raw && (current_pixel_index + 1) < max_pixel_index) - pixel = m_context->reader->read_pixel(bits_per_pixel); + bitmap->scanline(actual_row)[actual_col] = pixel; + if (pixel_packet_header.raw && (current_pixel_index + 1) < max_pixel_index) + pixel = TRY(read_pixel_from_stream(m_context->stream, bytes_per_pixel)); } - pixel_index += packet_type.pixels_count; + pixel_index += pixel_packet_header.pixels_count; } break; } diff --git a/Userland/Libraries/LibGfx/ImageFormats/TGALoader.h b/Userland/Libraries/LibGfx/ImageFormats/TGALoader.h index 7995e1dd05..129a3e848f 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/TGALoader.h +++ b/Userland/Libraries/LibGfx/ImageFormats/TGALoader.h @@ -18,7 +18,6 @@ public: static ErrorOr> create(ReadonlyBytes); virtual ~TGAImageDecoderPlugin() override; - TGAImageDecoderPlugin(u8 const*, size_t); virtual IntSize size() override; @@ -26,8 +25,10 @@ public: virtual ErrorOr> icc_data() override; private: + TGAImageDecoderPlugin(NonnullOwnPtr); + ErrorOr decode_tga_header(); - OwnPtr m_context; + NonnullOwnPtr m_context; }; }