From 7cafd7d177fa1aa009a12447f5e6d66b8c4d90ac Mon Sep 17 00:00:00 2001 From: Lucas CHOLLET Date: Sun, 12 Mar 2023 20:08:29 -0400 Subject: [PATCH] LibGfx/PortableFormat: Port to Stream Each one of `[PBM, PGM, PPM]Loader` used yet another stream-like relic. This patch ports all of them to `AK::Stream`. --- .../LibGfx/ImageFormats/PBMLoader.cpp | 18 ++--- .../Libraries/LibGfx/ImageFormats/PBMLoader.h | 2 +- .../LibGfx/ImageFormats/PGMLoader.cpp | 19 +++-- .../Libraries/LibGfx/ImageFormats/PGMLoader.h | 2 +- .../LibGfx/ImageFormats/PPMLoader.cpp | 29 ++++---- .../Libraries/LibGfx/ImageFormats/PPMLoader.h | 2 +- .../ImageFormats/PortableImageLoaderCommon.h | 72 ++++++++++--------- .../ImageFormats/PortableImageMapLoader.h | 23 +++--- 8 files changed, 88 insertions(+), 79 deletions(-) diff --git a/Userland/Libraries/LibGfx/ImageFormats/PBMLoader.cpp b/Userland/Libraries/LibGfx/ImageFormats/PBMLoader.cpp index 666e1e0dd8..959f214d6f 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/PBMLoader.cpp +++ b/Userland/Libraries/LibGfx/ImageFormats/PBMLoader.cpp @@ -6,15 +6,13 @@ */ #include "PBMLoader.h" -#include "AK/Endian.h" #include "PortableImageLoaderCommon.h" -#include "Userland/Libraries/LibGfx/Streamer.h" -#include namespace Gfx { -bool read_image_data(PBMLoadingContext& context, Streamer& streamer) +bool read_image_data(PBMLoadingContext& context) { + auto& stream = *context.stream; Vector color_data; auto const context_size = context.width * context.height; @@ -22,9 +20,10 @@ bool read_image_data(PBMLoadingContext& context, Streamer& streamer) if (context.type == PBMLoadingContext::Type::ASCII) { for (u64 i = 0; i < context_size; ++i) { - u8 byte; - if (!streamer.read(byte)) + auto byte_or_error = stream.read_value(); + if (byte_or_error.is_error()) return false; + auto const byte = byte_or_error.value(); if (byte == '0') color_data[i] = Color::White; else if (byte == '1') @@ -34,11 +33,12 @@ bool read_image_data(PBMLoadingContext& context, Streamer& streamer) } } else if (context.type == PBMLoadingContext::Type::RAWBITS) { for (u64 color_index = 0; color_index < context_size;) { - u8 byte; - if (!streamer.read(byte)) + auto byte_or_error = stream.read_value(); + if (byte_or_error.is_error()) return false; + auto byte = byte_or_error.value(); for (int i = 0; i < 8; i++) { - int val = byte & 0x80; + auto const val = byte & 0x80; if (val == 0) color_data[color_index] = Color::White; diff --git a/Userland/Libraries/LibGfx/ImageFormats/PBMLoader.h b/Userland/Libraries/LibGfx/ImageFormats/PBMLoader.h index 37705432b3..767dda0337 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/PBMLoader.h +++ b/Userland/Libraries/LibGfx/ImageFormats/PBMLoader.h @@ -22,5 +22,5 @@ struct PBM { using PBMLoadingContext = PortableImageMapLoadingContext; using PBMImageDecoderPlugin = PortableImageDecoderPlugin; -bool read_image_data(PBMLoadingContext& context, Streamer& streamer); +bool read_image_data(PBMLoadingContext& context); } diff --git a/Userland/Libraries/LibGfx/ImageFormats/PGMLoader.cpp b/Userland/Libraries/LibGfx/ImageFormats/PGMLoader.cpp index 432ecfe39f..18a3d7fd74 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/PGMLoader.cpp +++ b/Userland/Libraries/LibGfx/ImageFormats/PGMLoader.cpp @@ -5,11 +5,8 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include -#include -#include -#include -#include +#include "PGMLoader.h" +#include "PortableImageLoaderCommon.h" namespace Gfx { @@ -28,8 +25,9 @@ static void set_adjusted_pixels(PGMLoadingContext& context, Vector c } } -bool read_image_data(PGMLoadingContext& context, Streamer& streamer) +bool read_image_data(PGMLoadingContext& context) { + auto& stream = *context.stream; Vector color_data; auto const context_size = context.width * context.height; @@ -37,21 +35,22 @@ bool read_image_data(PGMLoadingContext& context, Streamer& streamer) if (context.type == PGMLoadingContext::Type::ASCII) { for (u64 i = 0; i < context_size; ++i) { - auto number_or_error = read_number(streamer); + auto number_or_error = read_number(stream); if (number_or_error.is_error()) return false; auto value = number_or_error.value(); - if (read_whitespace(context, streamer).is_error()) + if (read_whitespace(context).is_error()) return false; color_data[i] = { (u8)value, (u8)value, (u8)value }; } } else if (context.type == PGMLoadingContext::Type::RAWBITS) { for (u64 i = 0; i < context_size; ++i) { - u8 pixel; - if (!streamer.read(pixel)) + auto pixel_or_error = stream.read_value(); + if (pixel_or_error.is_error()) return false; + auto const pixel = pixel_or_error.value(); color_data[i] = { pixel, pixel, pixel }; } } diff --git a/Userland/Libraries/LibGfx/ImageFormats/PGMLoader.h b/Userland/Libraries/LibGfx/ImageFormats/PGMLoader.h index 087a5a4c6b..c02a921539 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/PGMLoader.h +++ b/Userland/Libraries/LibGfx/ImageFormats/PGMLoader.h @@ -23,5 +23,5 @@ struct PGM { using PGMLoadingContext = PortableImageMapLoadingContext; using PGMImageDecoderPlugin = PortableImageDecoderPlugin; -bool read_image_data(PGMLoadingContext& context, Streamer& streamer); +bool read_image_data(PGMLoadingContext& context); } diff --git a/Userland/Libraries/LibGfx/ImageFormats/PPMLoader.cpp b/Userland/Libraries/LibGfx/ImageFormats/PPMLoader.cpp index 38bd341307..10b442de54 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/PPMLoader.cpp +++ b/Userland/Libraries/LibGfx/ImageFormats/PPMLoader.cpp @@ -7,42 +7,38 @@ #include "PPMLoader.h" #include "PortableImageLoaderCommon.h" -#include -#include -#include -#include -#include -#include namespace Gfx { -bool read_image_data(PPMLoadingContext& context, Streamer& streamer) +bool read_image_data(PPMLoadingContext& context) { Vector color_data; auto const context_size = context.width * context.height; color_data.resize(context_size); + auto& stream = *context.stream; + if (context.type == PPMLoadingContext::Type::ASCII) { for (u64 i = 0; i < context_size; ++i) { - auto const red_or_error = read_number(streamer); + auto const red_or_error = read_number(stream); if (red_or_error.is_error()) return false; - if (read_whitespace(context, streamer).is_error()) + if (read_whitespace(context).is_error()) return false; - auto const green_or_error = read_number(streamer); + auto const green_or_error = read_number(stream); if (green_or_error.is_error()) return false; - if (read_whitespace(context, streamer).is_error()) + if (read_whitespace(context).is_error()) return false; - auto const blue_or_error = read_number(streamer); + auto const blue_or_error = read_number(stream); if (blue_or_error.is_error()) return false; - if (read_whitespace(context, streamer).is_error()) + if (read_whitespace(context).is_error()) return false; Color color { (u8)red_or_error.value(), (u8)green_or_error.value(), (u8)blue_or_error.value() }; @@ -52,8 +48,11 @@ bool read_image_data(PPMLoadingContext& context, Streamer& streamer) } } else if (context.type == PPMLoadingContext::Type::RAWBITS) { for (u64 i = 0; i < context_size; ++i) { - u8 pixel[3]; - if (!streamer.read_bytes(pixel, 3)) + Array pixel; + Bytes buffer { pixel }; + + auto const result = stream.read_until_filled(buffer); + if (result.is_error()) return false; color_data[i] = { pixel[0], pixel[1], pixel[2] }; } diff --git a/Userland/Libraries/LibGfx/ImageFormats/PPMLoader.h b/Userland/Libraries/LibGfx/ImageFormats/PPMLoader.h index 527bddbeae..9803839958 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/PPMLoader.h +++ b/Userland/Libraries/LibGfx/ImageFormats/PPMLoader.h @@ -23,5 +23,5 @@ struct PPM { using PPMLoadingContext = PortableImageMapLoadingContext; using PPMImageDecoderPlugin = PortableImageDecoderPlugin; -bool read_image_data(PPMLoadingContext& context, Streamer& streamer); +bool read_image_data(PPMLoadingContext& context); } diff --git a/Userland/Libraries/LibGfx/ImageFormats/PortableImageLoaderCommon.h b/Userland/Libraries/LibGfx/ImageFormats/PortableImageLoaderCommon.h index 9e64c20927..cf655d123e 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/PortableImageLoaderCommon.h +++ b/Userland/Libraries/LibGfx/ImageFormats/PortableImageLoaderCommon.h @@ -18,7 +18,6 @@ #include #include #include -#include namespace Gfx { @@ -31,14 +30,14 @@ static constexpr Color adjust_color(u16 max_val, Color color) return color; } -static inline ErrorOr read_number(Streamer& streamer) +static inline ErrorOr read_number(SeekableStream& stream) { - u8 byte {}; StringBuilder sb {}; + u8 byte {}; - while (streamer.read(byte)) { + for (auto buffer = TRY(stream.read_some({ &byte, 1 })); !buffer.is_empty(); buffer = TRY(stream.read_some({ &byte, 1 }))) { if (byte == ' ' || byte == '\t' || byte == '\n' || byte == '\r') { - streamer.step_back(); + TRY(stream.seek(-1, SeekMode::FromCurrentPosition)); break; } @@ -53,12 +52,13 @@ static inline ErrorOr read_number(Streamer& streamer) } template -static ErrorOr read_comment([[maybe_unused]] TContext& context, Streamer& streamer) +static ErrorOr read_comment(TContext& context) { + auto& stream = *context.stream; bool is_first_char = true; u8 byte {}; - while (streamer.read(byte)) { + while ((byte = TRY(stream.template read_value()))) { if (is_first_char) { if (byte != '#') return Error::from_string_literal("Can't read comment from stream"); @@ -72,20 +72,20 @@ static ErrorOr read_comment([[maybe_unused]] TContext& context, Streamer& } template -static bool read_magic_number(TContext& context, Streamer& streamer) +static bool read_magic_number(TContext& context) { if (context.state >= TContext::State::MagicNumber) { return true; } - if (!context.data || context.data_size < 2) { + if (context.stream->size().release_value_but_fixme_should_propagate_errors() < 2) { context.state = TContext::State::Error; dbgln_if(PORTABLE_IMAGE_LOADER_DEBUG, "There is no enough data for {}", TContext::FormatDetails::image_type); return false; } - u8 magic_number[2] {}; - if (!streamer.read_bytes(magic_number, 2)) { + Array magic_number {}; + if (context.stream->read_until_filled(Bytes { magic_number }).is_error()) { context.state = TContext::State::Error; dbgln_if(PORTABLE_IMAGE_LOADER_DEBUG, "We can't read magic number for {}", TContext::FormatDetails::image_type); return false; @@ -109,19 +109,25 @@ static bool read_magic_number(TContext& context, Streamer& streamer) } template -static ErrorOr read_whitespace(TContext& context, Streamer& streamer) +static ErrorOr read_whitespace(TContext& context) { + auto& stream = *context.stream; bool is_first_char = true; - u8 byte {}; - while (streamer.read(byte)) { + while (true) { + auto byte_or_error = stream.template read_value(); + // Nothing went wrong if we reached eof while reading a comment. + if (byte_or_error.is_error()) + return {}; + auto const byte = byte_or_error.value(); + if (byte == '#') { - streamer.step_back(); - TRY(read_comment(context, streamer)); + stream.seek(-1, SeekMode::FromCurrentPosition).release_value_but_fixme_should_propagate_errors(); + TRY(read_comment(context)); continue; } if (byte != ' ' && byte != '\t' && byte != '\n' && byte != '\r') { - streamer.step_back(); + stream.seek(-1, SeekMode::FromCurrentPosition).release_value_but_fixme_should_propagate_errors(); if (is_first_char) return Error::from_string_literal("Can't read whitespace from stream"); break; @@ -135,25 +141,25 @@ static ErrorOr read_whitespace(TContext& context, Streamer& streamer) } template -static ErrorOr read_width(TContext& context, Streamer& streamer) +static ErrorOr read_width(TContext& context) { - context.width = TRY(read_number(streamer)); + context.width = TRY(read_number(*context.stream)); context.state = TContext::State::Width; return {}; } template -static ErrorOr read_height(TContext& context, Streamer& streamer) +static ErrorOr read_height(TContext& context) { - context.height = TRY(read_number(streamer)); + context.height = TRY(read_number(*context.stream)); context.state = TContext::State::Height; return {}; } template -static ErrorOr read_max_val(TContext& context, Streamer& streamer) +static ErrorOr read_max_val(TContext& context) { - context.format_details.max_val = TRY(read_number(streamer)); + context.format_details.max_val = TRY(read_number(*context.stream)); if (context.format_details.max_val > 255) { dbgln_if(PORTABLE_IMAGE_LOADER_DEBUG, "We can't parse 2 byte color for {}", TContext::FormatDetails::image_type); @@ -199,21 +205,19 @@ static bool decode(TContext& context) context.state = TContext::State::Error; }); - Streamer streamer(context.data, context.data_size); - - if (!read_magic_number(context, streamer)) + if (!read_magic_number(context)) return false; - if (read_whitespace(context, streamer).is_error()) + if (read_whitespace(context).is_error()) return false; - if (read_width(context, streamer).is_error()) + if (read_width(context).is_error()) return false; - if (read_whitespace(context, streamer).is_error()) + if (read_whitespace(context).is_error()) return false; - if (read_height(context, streamer).is_error()) + if (read_height(context).is_error()) return false; if (context.width > maximum_width_for_decoded_images || context.height > maximum_height_for_decoded_images) { @@ -221,18 +225,18 @@ static bool decode(TContext& context) return false; } - if (read_whitespace(context, streamer).is_error()) + if (read_whitespace(context).is_error()) return false; if constexpr (requires { context.format_details.max_val; }) { - if (read_max_val(context, streamer).is_error()) + if (read_max_val(context).is_error()) return false; - if (read_whitespace(context, streamer).is_error()) + if (read_whitespace(context).is_error()) return false; } - if (!read_image_data(context, streamer)) + if (!read_image_data(context)) return false; error_guard.disarm(); diff --git a/Userland/Libraries/LibGfx/ImageFormats/PortableImageMapLoader.h b/Userland/Libraries/LibGfx/ImageFormats/PortableImageMapLoader.h index df9dda312f..5698df5eb5 100644 --- a/Userland/Libraries/LibGfx/ImageFormats/PortableImageMapLoader.h +++ b/Userland/Libraries/LibGfx/ImageFormats/PortableImageMapLoader.h @@ -7,6 +7,8 @@ #pragma once +#include +#include #include #include #include @@ -38,12 +40,18 @@ struct PortableImageMapLoadingContext { Type type { Type::Unknown }; State state { State::NotDecoded }; - u8 const* data { nullptr }; - size_t data_size { 0 }; + size_t width { 0 }; size_t height { 0 }; FormatDetails format_details {}; RefPtr bitmap; + + NonnullOwnPtr stream; + + PortableImageMapLoadingContext(NonnullOwnPtr stream) + : stream(move(stream)) + { + } }; template @@ -67,17 +75,15 @@ public: virtual ErrorOr> icc_data() override; private: - PortableImageDecoderPlugin(u8 const*, size_t); + PortableImageDecoderPlugin(NonnullOwnPtr stream); OwnPtr m_context; }; template -PortableImageDecoderPlugin::PortableImageDecoderPlugin(u8 const* data, size_t size) +PortableImageDecoderPlugin::PortableImageDecoderPlugin(NonnullOwnPtr stream) { - m_context = make(); - m_context->data = data; - m_context->data_size = size; + m_context = make(move(stream)); } template @@ -114,7 +120,8 @@ bool PortableImageDecoderPlugin::set_nonvolatile(bool& was_purged) template ErrorOr> PortableImageDecoderPlugin::create(ReadonlyBytes data) { - return adopt_nonnull_own_or_enomem(new (nothrow) PortableImageDecoderPlugin(data.data(), data.size())); + auto stream = TRY(try_make(data)); + return adopt_nonnull_own_or_enomem(new (nothrow) PortableImageDecoderPlugin(move(stream))); } template