From 2da4cfcc80b5fcd322c5194a829b76571616e5b4 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 6 Nov 2021 11:52:35 +0100 Subject: [PATCH] LibGfx: Use ErrorOr for Bitmap::clone() --- Userland/Applications/PixelPaint/Layer.cpp | 6 +++++- Userland/Libraries/LibCards/Card.cpp | 3 +-- Userland/Libraries/LibGUI/FileIconProvider.cpp | 5 +++-- Userland/Libraries/LibGfx/Bitmap.cpp | 10 ++++++---- Userland/Libraries/LibGfx/Bitmap.h | 2 +- Userland/Libraries/LibGfx/GIFLoader.cpp | 8 +++++++- 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/Userland/Applications/PixelPaint/Layer.cpp b/Userland/Applications/PixelPaint/Layer.cpp index c0be4533148..e0ca6ff023b 100644 --- a/Userland/Applications/PixelPaint/Layer.cpp +++ b/Userland/Applications/PixelPaint/Layer.cpp @@ -39,7 +39,11 @@ RefPtr Layer::try_create_with_bitmap(Image& image, NonnullRefPtr Layer::try_create_snapshot(Image& image, Layer const& layer) { - auto snapshot = try_create_with_bitmap(image, *layer.bitmap().clone(), layer.name()); + auto new_bitmap_or_error = layer.bitmap().clone(); + if (new_bitmap_or_error.is_error()) + return nullptr; + + auto snapshot = try_create_with_bitmap(image, new_bitmap_or_error.release_value_but_fixme_should_propagate_errors(), layer.name()); /* We set these properties directly because calling the setters might notify the image of an update on the newly created layer, but this diff --git a/Userland/Libraries/LibCards/Card.cpp b/Userland/Libraries/LibCards/Card.cpp index 2c27912d861..78b53a5361d 100644 --- a/Userland/Libraries/LibCards/Card.cpp +++ b/Userland/Libraries/LibCards/Card.cpp @@ -172,8 +172,7 @@ void Card::clear_and_draw(GUI::Painter& painter, const Color& background_color) NonnullRefPtr Card::invert_bitmap(Gfx::Bitmap& bitmap) { - auto inverted_bitmap = bitmap.clone(); - VERIFY(inverted_bitmap); + auto inverted_bitmap = bitmap.clone().release_value_but_fixme_should_propagate_errors(); for (int y = 0; y < inverted_bitmap->height(); y++) { for (int x = 0; x < inverted_bitmap->width(); x++) { inverted_bitmap->set_pixel(x, y, inverted_bitmap->get_pixel(x, y).inverted()); diff --git a/Userland/Libraries/LibGUI/FileIconProvider.cpp b/Userland/Libraries/LibGUI/FileIconProvider.cpp index 4345ece13bd..302b0f25901 100644 --- a/Userland/Libraries/LibGUI/FileIconProvider.cpp +++ b/Userland/Libraries/LibGUI/FileIconProvider.cpp @@ -239,11 +239,12 @@ Icon FileIconProvider::icon_for_path(const String& path, mode_t mode) auto& emblem = size < 32 ? *s_symlink_emblem_small : *s_symlink_emblem; auto original_bitmap = target_icon.bitmap_for_size(size); VERIFY(original_bitmap); - auto generated_bitmap = original_bitmap->clone(); - if (!generated_bitmap) { + auto generated_bitmap_or_error = original_bitmap->clone(); + if (generated_bitmap_or_error.is_error()) { dbgln("Failed to clone {}x{} icon for symlink variant", size, size); return s_symlink_icon; } + auto generated_bitmap = generated_bitmap_or_error.release_value_but_fixme_should_propagate_errors(); GUI::Painter painter(*generated_bitmap); painter.blit({ size - emblem.width(), size - emblem.height() }, emblem, emblem.rect()); diff --git a/Userland/Libraries/LibGfx/Bitmap.cpp b/Userland/Libraries/LibGfx/Bitmap.cpp index 52fde5c20ca..5090434917e 100644 --- a/Userland/Libraries/LibGfx/Bitmap.cpp +++ b/Userland/Libraries/LibGfx/Bitmap.cpp @@ -332,17 +332,19 @@ Bitmap::Bitmap(BitmapFormat format, Core::AnonymousBuffer buffer, const IntSize& allocate_palette_from_format(m_format, palette); } -RefPtr Bitmap::clone() const +ErrorOr> Bitmap::clone() const { auto new_bitmap = Bitmap::try_create(format(), size(), scale()); - if (!new_bitmap) - return nullptr; + if (!new_bitmap) { + // FIXME: Propagate the *real* error, once we have it. + return Error::from_errno(ENOMEM); + } VERIFY(size_in_bytes() == new_bitmap->size_in_bytes()); memcpy(new_bitmap->scanline(0), scanline(0), size_in_bytes()); - return new_bitmap; + return new_bitmap.release_nonnull(); } RefPtr Bitmap::rotated(Gfx::RotationDirection rotation_direction) const diff --git a/Userland/Libraries/LibGfx/Bitmap.h b/Userland/Libraries/LibGfx/Bitmap.h index 223bd3d5d56..b7c2f25e310 100644 --- a/Userland/Libraries/LibGfx/Bitmap.h +++ b/Userland/Libraries/LibGfx/Bitmap.h @@ -109,7 +109,7 @@ public: return false; } - [[nodiscard]] RefPtr clone() const; + ErrorOr> clone() const; [[nodiscard]] RefPtr rotated(Gfx::RotationDirection) const; [[nodiscard]] RefPtr flipped(Gfx::Orientation) const; diff --git a/Userland/Libraries/LibGfx/GIFLoader.cpp b/Userland/Libraries/LibGfx/GIFLoader.cpp index d17202ee710..f13c25b53e6 100644 --- a/Userland/Libraries/LibGfx/GIFLoader.cpp +++ b/Userland/Libraries/LibGfx/GIFLoader.cpp @@ -733,8 +733,14 @@ ImageFrameDescriptor GIFImageDecoderPlugin::frame(size_t i) m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAllFrames; } + auto image_or_error = m_context->frame_buffer->clone(); + if (image_or_error.is_error()) { + m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAllFrames; + return {}; + } + ImageFrameDescriptor frame {}; - frame.image = m_context->frame_buffer->clone(); + frame.image = image_or_error.release_value_but_fixme_should_propagate_errors(); frame.duration = m_context->images.at(i).duration * 10; if (frame.duration <= 10) {