From 2687246808bf6c00eb85608377646c2c24e9a14a Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Wed, 25 Jun 2025 10:25:28 +0200 Subject: [PATCH] LibGfx: Use NonnullRefPtr for frame descriptors This makes it a bit easier to reason about where bitmaps should be available. --- Libraries/LibGfx/CMYKBitmap.cpp | 6 ++--- Libraries/LibGfx/CMYKBitmap.h | 2 +- Libraries/LibGfx/ImageFormats/BMPLoader.cpp | 2 +- Libraries/LibGfx/ImageFormats/GIFLoader.cpp | 12 +++++----- Libraries/LibGfx/ImageFormats/ICOLoader.cpp | 10 +-------- Libraries/LibGfx/ImageFormats/ImageDecoder.h | 4 ++-- Libraries/LibGfx/ImageFormats/JPEGLoader.cpp | 2 +- Libraries/LibGfx/ImageFormats/TIFFLoader.cpp | 2 +- .../LibGfx/ImageFormats/TinyVGLoader.cpp | 4 ++-- .../ImageDecoder/ConnectionFromClient.cpp | 10 +++++---- Tests/LibGfx/TestImageDecoder.cpp | 1 - Utilities/image.cpp | 22 +++++++++---------- 12 files changed, 35 insertions(+), 42 deletions(-) diff --git a/Libraries/LibGfx/CMYKBitmap.cpp b/Libraries/LibGfx/CMYKBitmap.cpp index b052b3c9351..2ea6b798f55 100644 --- a/Libraries/LibGfx/CMYKBitmap.cpp +++ b/Libraries/LibGfx/CMYKBitmap.cpp @@ -21,10 +21,10 @@ ErrorOr> CMYKBitmap::create_with_size(IntSize const& s return adopt_ref(*new CMYKBitmap(size, move(data))); } -ErrorOr> CMYKBitmap::to_low_quality_rgb() const +ErrorOr> CMYKBitmap::to_low_quality_rgb() const { if (!m_rgb_bitmap) { - m_rgb_bitmap = TRY(Bitmap::create(BitmapFormat::BGRx8888, { m_size.width(), m_size.height() })); + m_rgb_bitmap = TRY(Bitmap::create(BitmapFormat::BGRx8888, m_size)); for (int y = 0; y < m_size.height(); ++y) { for (int x = 0; x < m_size.width(); ++x) { @@ -35,7 +35,7 @@ ErrorOr> CMYKBitmap::to_low_quality_rgb() const } } - return m_rgb_bitmap; + return *m_rgb_bitmap; } } diff --git a/Libraries/LibGfx/CMYKBitmap.h b/Libraries/LibGfx/CMYKBitmap.h index b92f591b1ad..cf0b7fe4829 100644 --- a/Libraries/LibGfx/CMYKBitmap.h +++ b/Libraries/LibGfx/CMYKBitmap.h @@ -36,7 +36,7 @@ public: [[nodiscard]] CMYK* end(); [[nodiscard]] size_t data_size() const { return m_data.size(); } - ErrorOr> to_low_quality_rgb() const; + ErrorOr> to_low_quality_rgb() const; private: CMYKBitmap(IntSize const& size, ByteBuffer data) diff --git a/Libraries/LibGfx/ImageFormats/BMPLoader.cpp b/Libraries/LibGfx/ImageFormats/BMPLoader.cpp index a3c08be99de..6598a201098 100644 --- a/Libraries/LibGfx/ImageFormats/BMPLoader.cpp +++ b/Libraries/LibGfx/ImageFormats/BMPLoader.cpp @@ -1547,7 +1547,7 @@ ErrorOr BMPImageDecoderPlugin::frame(size_t index, Optiona TRY(decode_bmp_pixel_data(*m_context)); VERIFY(m_context->bitmap); - return ImageFrameDescriptor { m_context->bitmap, 0 }; + return ImageFrameDescriptor { *m_context->bitmap, 0 }; } ErrorOr> BMPImageDecoderPlugin::icc_data() diff --git a/Libraries/LibGfx/ImageFormats/GIFLoader.cpp b/Libraries/LibGfx/ImageFormats/GIFLoader.cpp index 2370e45d37c..bd5144667a3 100644 --- a/Libraries/LibGfx/ImageFormats/GIFLoader.cpp +++ b/Libraries/LibGfx/ImageFormats/GIFLoader.cpp @@ -488,20 +488,20 @@ ErrorOr GIFImageDecoderPlugin::frame(size_t index, Optiona m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame; return result.release_error(); } - if (auto result = decode_frame(*m_context, 0); result.is_error()) { + if (result = decode_frame(*m_context, 0); result.is_error()) { m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame; return result.release_error(); } m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAllFrames; } - ImageFrameDescriptor frame {}; - frame.image = TRY(m_context->frame_buffer->clone()); - frame.duration = m_context->images[index]->duration * 10; + ImageFrameDescriptor frame { + .image = TRY(m_context->frame_buffer->clone()), + .duration = m_context->images[index]->duration * 10, + }; - if (frame.duration <= 10) { + if (frame.duration <= 10) frame.duration = 100; - } return frame; } diff --git a/Libraries/LibGfx/ImageFormats/ICOLoader.cpp b/Libraries/LibGfx/ImageFormats/ICOLoader.cpp index 7084ee38fa6..071b22b893e 100644 --- a/Libraries/LibGfx/ImageFormats/ICOLoader.cpp +++ b/Libraries/LibGfx/ImageFormats/ICOLoader.cpp @@ -171,10 +171,6 @@ ErrorOr ICOImageDecoderPlugin::load_ico_bitmap(ICOLoadingContext& context) if (PNGImageDecoderPlugin::sniff({ context.data + desc.offset, desc.size })) { auto png_decoder = TRY(PNGImageDecoderPlugin::create({ context.data + desc.offset, desc.size })); auto decoded_png_frame = TRY(png_decoder->frame(0)); - if (!decoded_png_frame.image) { - dbgln_if(ICO_DEBUG, "load_ico_bitmap: failed to load PNG encoded image index: {}", real_index); - return Error::from_string_literal("Encoded image not null"); - } desc.bitmap = decoded_png_frame.image; return {}; } else { @@ -184,10 +180,6 @@ ErrorOr ICOImageDecoderPlugin::load_ico_bitmap(ICOLoadingContext& context) // inside an ICO image. if (bmp_decoder->sniff_dib()) { auto decoded_bmp_frame = TRY(bmp_decoder->frame(0)); - if (!decoded_bmp_frame.image) { - dbgln_if(ICO_DEBUG, "load_ico_bitmap: failed to load BMP encoded image index: {}", real_index); - return Error::from_string_literal("Encoded image not null"); - } desc.bitmap = decoded_bmp_frame.image; } else { dbgln_if(ICO_DEBUG, "load_ico_bitmap: encoded image not supported at index: {}", real_index); @@ -244,7 +236,7 @@ ErrorOr ICOImageDecoderPlugin::frame(size_t index, Optiona } VERIFY(m_context->images[m_context->largest_index].bitmap); - return ImageFrameDescriptor { m_context->images[m_context->largest_index].bitmap, 0 }; + return ImageFrameDescriptor { *m_context->images[m_context->largest_index].bitmap, 0 }; } } diff --git a/Libraries/LibGfx/ImageFormats/ImageDecoder.h b/Libraries/LibGfx/ImageFormats/ImageDecoder.h index 4dd2f8e3c80..f0d3f73ff7b 100644 --- a/Libraries/LibGfx/ImageFormats/ImageDecoder.h +++ b/Libraries/LibGfx/ImageFormats/ImageDecoder.h @@ -23,12 +23,12 @@ namespace Gfx { class Bitmap; struct ImageFrameDescriptor { - RefPtr image; + NonnullRefPtr image; int duration { 0 }; }; struct VectorImageFrameDescriptor { - RefPtr image; + NonnullRefPtr image; int duration { 0 }; }; diff --git a/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp b/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp index 4118a4b88b0..886ec6d259c 100644 --- a/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp +++ b/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp @@ -217,7 +217,7 @@ ErrorOr JPEGImageDecoderPlugin::frame(size_t index, Option m_context->state = JPEGLoadingContext::State::Decoded; } - return ImageFrameDescriptor { m_context->rgb_bitmap, 0 }; + return ImageFrameDescriptor { *m_context->rgb_bitmap, 0 }; } Optional JPEGImageDecoderPlugin::metadata() diff --git a/Libraries/LibGfx/ImageFormats/TIFFLoader.cpp b/Libraries/LibGfx/ImageFormats/TIFFLoader.cpp index 1abfd7d0cbe..24c095ec1d1 100644 --- a/Libraries/LibGfx/ImageFormats/TIFFLoader.cpp +++ b/Libraries/LibGfx/ImageFormats/TIFFLoader.cpp @@ -786,7 +786,7 @@ ErrorOr TIFFImageDecoderPlugin::frame(size_t index, Option if (m_context->cmyk_bitmap()) return ImageFrameDescriptor { TRY(m_context->cmyk_bitmap()->to_low_quality_rgb()), 0 }; - return ImageFrameDescriptor { m_context->bitmap(), 0 }; + return ImageFrameDescriptor { *m_context->bitmap(), 0 }; } Optional TIFFImageDecoderPlugin::metadata() diff --git a/Libraries/LibGfx/ImageFormats/TinyVGLoader.cpp b/Libraries/LibGfx/ImageFormats/TinyVGLoader.cpp index 0f53759d78d..5d2fab9a12e 100644 --- a/Libraries/LibGfx/ImageFormats/TinyVGLoader.cpp +++ b/Libraries/LibGfx/ImageFormats/TinyVGLoader.cpp @@ -571,13 +571,13 @@ ErrorOr TinyVGImageDecoderPlugin::frame(size_t, Optionaldecoded_image->size()); if (!m_context->bitmap || m_context->bitmap->size() != target_size) m_context->bitmap = TRY(m_context->decoded_image->bitmap(target_size)); - return ImageFrameDescriptor { m_context->bitmap }; + return ImageFrameDescriptor { *m_context->bitmap }; } ErrorOr TinyVGImageDecoderPlugin::vector_frame(size_t) { TRY(ensure_fully_decoded(*m_context)); - return VectorImageFrameDescriptor { m_context->decoded_image, 0 }; + return VectorImageFrameDescriptor { *m_context->decoded_image, 0 }; } } diff --git a/Services/ImageDecoder/ConnectionFromClient.cpp b/Services/ImageDecoder/ConnectionFromClient.cpp index ef275d2d487..a2e140fec2b 100644 --- a/Services/ImageDecoder/ConnectionFromClient.cpp +++ b/Services/ImageDecoder/ConnectionFromClient.cpp @@ -86,15 +86,17 @@ Messages::ImageDecoderServer::ConnectNewClientsResponse ConnectionFromClient::co static void decode_image_to_bitmaps_and_durations_with_decoder(Gfx::ImageDecoder const& decoder, Optional ideal_size, Vector>& bitmaps, Vector& durations) { + bitmaps.ensure_capacity(decoder.frame_count()); + durations.ensure_capacity(decoder.frame_count()); for (size_t i = 0; i < decoder.frame_count(); ++i) { auto frame_or_error = decoder.frame(i, ideal_size); if (frame_or_error.is_error()) { - bitmaps.append({}); - durations.append(0); + bitmaps.unchecked_append({}); + durations.unchecked_append(0); } else { auto frame = frame_or_error.release_value(); - bitmaps.append(frame.image.release_nonnull()); - durations.append(frame.duration); + bitmaps.unchecked_append(frame.image); + durations.unchecked_append(frame.duration); } } } diff --git a/Tests/LibGfx/TestImageDecoder.cpp b/Tests/LibGfx/TestImageDecoder.cpp index c8b56a6c60d..7ee35d68d2a 100644 --- a/Tests/LibGfx/TestImageDecoder.cpp +++ b/Tests/LibGfx/TestImageDecoder.cpp @@ -211,7 +211,6 @@ TEST_CASE(test_gif_without_global_color_table) auto plugin_decoder = TRY_OR_FAIL(Gfx::GIFImageDecoderPlugin::create(gif_data)); EXPECT_EQ(plugin_decoder->frame_count(), 1u); auto frame = TRY_OR_FAIL(plugin_decoder->frame(0)); - EXPECT(frame.image); EXPECT_EQ(frame.image->size(), Gfx::IntSize(1, 1)); EXPECT_EQ(frame.image->get_pixel(0, 0), Gfx::Color::NamedColor::Red); } diff --git a/Utilities/image.cpp b/Utilities/image.cpp index 71a224e3dba..ef5f5a02651 100644 --- a/Utilities/image.cpp +++ b/Utilities/image.cpp @@ -14,7 +14,7 @@ #include #include -using AnyBitmap = Variant, RefPtr>; +using AnyBitmap = Variant, NonnullRefPtr>; struct LoadedImage { Gfx::NaturalFrameFormat internal_format; AnyBitmap bitmap; @@ -33,7 +33,7 @@ static ErrorOr load_image(RefPtr const& decoder, case Gfx::NaturalFrameFormat::Vector: return TRY(decoder->frame(frame_index)).image; case Gfx::NaturalFrameFormat::CMYK: - return RefPtr(TRY(decoder->cmyk_frame())); + return TRY(decoder->cmyk_frame()); } VERIFY_NOT_REACHED(); }()); @@ -43,9 +43,9 @@ static ErrorOr load_image(RefPtr const& decoder, static ErrorOr invert_cmyk(LoadedImage& image) { - if (!image.bitmap.has>()) + if (!image.bitmap.has>()) return Error::from_string_literal("Can't --invert-cmyk with RGB bitmaps"); - auto& frame = image.bitmap.get>(); + auto& frame = image.bitmap.get>(); for (auto& pixel : *frame) { pixel.c = ~pixel.c; @@ -58,18 +58,18 @@ static ErrorOr invert_cmyk(LoadedImage& image) static ErrorOr crop_image(LoadedImage& image, Gfx::IntRect const& rect) { - if (!image.bitmap.has>()) + if (!image.bitmap.has>()) return Error::from_string_literal("Can't --crop CMYK bitmaps yet"); - auto& frame = image.bitmap.get>(); + auto& frame = image.bitmap.get>(); frame = TRY(frame->cropped(rect)); return {}; } static ErrorOr move_alpha_to_rgb(LoadedImage& image) { - if (!image.bitmap.has>()) + if (!image.bitmap.has>()) return Error::from_string_literal("Can't --move-alpha-to-rgb with CMYK bitmaps"); - auto& frame = image.bitmap.get>(); + auto& frame = image.bitmap.get>(); switch (frame->format()) { case Gfx::BitmapFormat::Invalid: @@ -96,9 +96,9 @@ static ErrorOr move_alpha_to_rgb(LoadedImage& image) static ErrorOr strip_alpha(LoadedImage& image) { - if (!image.bitmap.has>()) + if (!image.bitmap.has>()) return Error::from_string_literal("Can't --strip-alpha with CMYK bitmaps"); - auto& frame = image.bitmap.get>(); + auto& frame = image.bitmap.get>(); switch (frame->format()) { case Gfx::BitmapFormat::Invalid: @@ -128,7 +128,7 @@ static ErrorOr save_image(LoadedImage& image, StringView out_path, u8 jpeg return Core::OutputBufferedFile::create(move(output_stream)); }; - auto& frame = image.bitmap.get>(); + auto& frame = image.bitmap.get>(); if (out_path.ends_with(".jpg"sv, CaseSensitivity::CaseInsensitive) || out_path.ends_with(".jpeg"sv, CaseSensitivity::CaseInsensitive)) { TRY(Gfx::JPEGWriter::encode(*TRY(stream()), *frame, { .icc_data = image.icc_data, .quality = jpeg_quality }));