From 1c3f11a5a6b95a6d7bf2df705dac2b47e4406338 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Fri, 19 Apr 2024 16:10:57 -0600 Subject: [PATCH] Userland: Remove remaining callers of synchronous ImageDecoder API Most of these now just await the image decoding, equivalent (ish) to the old behavior. A more async-aware refactor should happen some time in the future. --- .../Applications/ImageViewer/ViewWidget.cpp | 19 +++++++++---------- Userland/Applications/PixelPaint/Image.cpp | 11 +++-------- .../SoundPlayer/SoundPlayerWidget.cpp | 5 +++-- Userland/Libraries/LibGUI/FileSystemModel.cpp | 6 +++--- .../LibImageDecoderClient/Client.cpp | 10 ---------- .../Libraries/LibImageDecoderClient/Client.h | 3 --- Userland/Utilities/pixelflut.cpp | 8 +++----- 7 files changed, 21 insertions(+), 41 deletions(-) diff --git a/Userland/Applications/ImageViewer/ViewWidget.cpp b/Userland/Applications/ImageViewer/ViewWidget.cpp index a699cacf6d7..58f249d85ec 100644 --- a/Userland/Applications/ImageViewer/ViewWidget.cpp +++ b/Userland/Applications/ImageViewer/ViewWidget.cpp @@ -237,16 +237,15 @@ ErrorOr ViewWidget::try_open_file(String const& path, Core::File& file) // Use out-of-process decoding for raster formats. auto client = TRY(ImageDecoderClient::Client::try_create()); auto mime_type = Core::guess_mime_type_based_on_filename(path); - auto decoded_image = client->decode_image(file_data, OptionalNone {}, mime_type); - if (!decoded_image.has_value()) { - return Error::from_string_literal("Failed to decode image"); - } - is_animated = decoded_image->is_animated; - loop_count = decoded_image->loop_count; - frames.ensure_capacity(decoded_image->frames.size()); - for (u32 i = 0; i < decoded_image->frames.size(); i++) { - auto& frame_data = decoded_image->frames[i]; - frames.unchecked_append({ BitmapImage::create(frame_data.bitmap, decoded_image->scale), int(frame_data.duration) }); + + // FIXME: Refactor file opening to be more async-aware, and don't await this promise + auto decoded_image = TRY(client->decode_image(file_data, {}, {}, OptionalNone {}, mime_type)->await()); + is_animated = decoded_image.is_animated; + loop_count = decoded_image.loop_count; + frames.ensure_capacity(decoded_image.frames.size()); + for (u32 i = 0; i < decoded_image.frames.size(); i++) { + auto& frame_data = decoded_image.frames[i]; + frames.unchecked_append({ BitmapImage::create(frame_data.bitmap, decoded_image.scale), int(frame_data.duration) }); } } diff --git a/Userland/Applications/PixelPaint/Image.cpp b/Userland/Applications/PixelPaint/Image.cpp index 94b3ac34345..8e891a70461 100644 --- a/Userland/Applications/PixelPaint/Image.cpp +++ b/Userland/Applications/PixelPaint/Image.cpp @@ -57,14 +57,9 @@ ErrorOr> Image::decode_bitmap(ReadonlyBytes bitmap_da auto optional_mime_type = guessed_mime_type.map([](auto mime_type) { return mime_type.to_byte_string(); }); // FIXME: Find a way to avoid the memory copying here. - auto maybe_decoded_image = client->decode_image(bitmap_data, OptionalNone {}, optional_mime_type); - if (!maybe_decoded_image.has_value()) - return Error::from_string_literal("Image decode failed"); - - // FIXME: Support multi-frame images? - auto decoded_image = maybe_decoded_image.release_value(); - if (decoded_image.frames.is_empty()) - return Error::from_string_literal("Image decode failed (no frames)"); + // FIXME: Support multi-frame images + // FIXME: Refactor image decoding to be more async-aware, and don't await this promise + auto decoded_image = TRY(client->decode_image(bitmap_data, {}, {}, OptionalNone {}, optional_mime_type)->await()); return decoded_image.frames.first().bitmap; } diff --git a/Userland/Applications/SoundPlayer/SoundPlayerWidget.cpp b/Userland/Applications/SoundPlayer/SoundPlayerWidget.cpp index d30e53a8314..598322c4c5b 100644 --- a/Userland/Applications/SoundPlayer/SoundPlayerWidget.cpp +++ b/Userland/Applications/SoundPlayer/SoundPlayerWidget.cpp @@ -195,8 +195,9 @@ RefPtr SoundPlayerWidget::get_image_from_music_file() // FIXME: We randomly select the first picture available for the track, // We might want to hardcode or let the user set a preference. - auto decoded_image_or_error = m_image_decoder_client.decode_image(pictures[0].data); - if (!decoded_image_or_error.has_value()) + // FIXME: Refactor image decoding to be more async-aware, and don't await this promise + auto decoded_image_or_error = m_image_decoder_client.decode_image(pictures[0].data, {}, {})->await(); + if (decoded_image_or_error.is_error()) return {}; auto const decoded_image = decoded_image_or_error.release_value(); diff --git a/Userland/Libraries/LibGUI/FileSystemModel.cpp b/Userland/Libraries/LibGUI/FileSystemModel.cpp index 5d40e942d41..9a72b584359 100644 --- a/Userland/Libraries/LibGUI/FileSystemModel.cpp +++ b/Userland/Libraries/LibGUI/FileSystemModel.cpp @@ -717,9 +717,9 @@ static ErrorOr> render_thumbnail(StringView path) } auto mime_type = Core::guess_mime_type_based_on_filename(path); - auto decoded_image = maybe_client->decode_image(file->bytes(), thumbnail_size, mime_type); - if (!decoded_image.has_value()) - return Error::from_string_literal("Unable to decode the image."); + + // FIXME: Refactor thumbnail rendering to be more async-aware. Possibly return this promise to the caller. + auto decoded_image = TRY(maybe_client->decode_image(file->bytes(), {}, {}, thumbnail_size, mime_type)->await()); return decoded_image; })); diff --git a/Userland/Libraries/LibImageDecoderClient/Client.cpp b/Userland/Libraries/LibImageDecoderClient/Client.cpp index ff7508040af..b204b149828 100644 --- a/Userland/Libraries/LibImageDecoderClient/Client.cpp +++ b/Userland/Libraries/LibImageDecoderClient/Client.cpp @@ -60,16 +60,6 @@ NonnullRefPtr> Client::decode_image(ReadonlyBytes en return promise; } -Optional Client::decode_image(ReadonlyBytes encoded_data, Optional ideal_size, Optional mime_type) -{ - auto promise = decode_image( - encoded_data, [](auto) -> ErrorOr { return {}; }, [](Error&) -> void {}, ideal_size, mime_type); - auto result = promise->await(); - if (result.is_error()) - return {}; - return result.release_value(); -} - void Client::did_decode_image(i64 image_id, bool is_animated, u32 loop_count, Vector const& bitmaps, Vector const& durations, Gfx::FloatPoint scale) { VERIFY(!bitmaps.is_empty()); diff --git a/Userland/Libraries/LibImageDecoderClient/Client.h b/Userland/Libraries/LibImageDecoderClient/Client.h index b8a13fb3425..f4c8ab42fbb 100644 --- a/Userland/Libraries/LibImageDecoderClient/Client.h +++ b/Userland/Libraries/LibImageDecoderClient/Client.h @@ -36,9 +36,6 @@ public: NonnullRefPtr> decode_image(ReadonlyBytes, Function(DecodedImage&)> on_resolved, Function on_rejected, Optional ideal_size = {}, Optional mime_type = {}); - // FIXME: Move all clients to the promise-based API and get rid of this synchronous (i.e. EventLoop-spinning) one. - Optional decode_image(ReadonlyBytes, Optional ideal_size = {}, Optional mime_type = {}); - Function on_death; private: diff --git a/Userland/Utilities/pixelflut.cpp b/Userland/Utilities/pixelflut.cpp index ee12868a61a..cf3ba90e58f 100644 --- a/Userland/Utilities/pixelflut.cpp +++ b/Userland/Utilities/pixelflut.cpp @@ -100,12 +100,10 @@ ErrorOr> Client::create(StringView image_path, StringView ScopeGuard guard = [&] { image_decoder->shutdown(); }; auto byte_buffer = TRY(image_file->read_until_eof(16 * KiB)); - auto maybe_image = image_decoder->decode_image(byte_buffer); + auto image_promise = image_decoder->decode_image(byte_buffer, {}, {}); + auto image_result = TRY(image_promise->await()); - if (!maybe_image.has_value()) - return Error::from_string_view("Image could not be read"sv); - - auto image = maybe_image->frames.take_first().bitmap; + auto image = image_result.frames.take_first().bitmap; // Make sure to not draw out of bounds; some servers will disconnect us for that! if (image->width() > canvas_size.width()) {