LibGfx: Use NonnullRefPtr<Bitmap> for frame descriptors
Some checks are pending
CI / macOS, arm64, Sanitizer_CI, Clang (push) Waiting to run
CI / Linux, x86_64, Fuzzers_CI, Clang (push) Waiting to run
CI / Linux, x86_64, Sanitizer_CI, GNU (push) Waiting to run
CI / Linux, x86_64, Sanitizer_CI, Clang (push) Waiting to run
Package the js repl as a binary artifact / Linux, arm64 (push) Waiting to run
Package the js repl as a binary artifact / macOS, arm64 (push) Waiting to run
Package the js repl as a binary artifact / Linux, x86_64 (push) Waiting to run
Run test262 and test-wasm / run_and_update_results (push) Waiting to run
Lint Code / lint (push) Waiting to run
Label PRs with merge conflicts / auto-labeler (push) Waiting to run
Push notes / build (push) Waiting to run

This makes it a bit easier to reason about where bitmaps should be
available.
This commit is contained in:
Jelle Raaijmakers 2025-06-25 10:25:28 +02:00 committed by Shannon Booth
commit 2687246808
Notes: github-actions[bot] 2025-06-25 10:55:58 +00:00
12 changed files with 35 additions and 42 deletions

View file

@ -21,10 +21,10 @@ ErrorOr<NonnullRefPtr<CMYKBitmap>> CMYKBitmap::create_with_size(IntSize const& s
return adopt_ref(*new CMYKBitmap(size, move(data))); return adopt_ref(*new CMYKBitmap(size, move(data)));
} }
ErrorOr<RefPtr<Bitmap>> CMYKBitmap::to_low_quality_rgb() const ErrorOr<NonnullRefPtr<Bitmap>> CMYKBitmap::to_low_quality_rgb() const
{ {
if (!m_rgb_bitmap) { 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 y = 0; y < m_size.height(); ++y) {
for (int x = 0; x < m_size.width(); ++x) { for (int x = 0; x < m_size.width(); ++x) {
@ -35,7 +35,7 @@ ErrorOr<RefPtr<Bitmap>> CMYKBitmap::to_low_quality_rgb() const
} }
} }
return m_rgb_bitmap; return *m_rgb_bitmap;
} }
} }

View file

@ -36,7 +36,7 @@ public:
[[nodiscard]] CMYK* end(); [[nodiscard]] CMYK* end();
[[nodiscard]] size_t data_size() const { return m_data.size(); } [[nodiscard]] size_t data_size() const { return m_data.size(); }
ErrorOr<RefPtr<Bitmap>> to_low_quality_rgb() const; ErrorOr<NonnullRefPtr<Bitmap>> to_low_quality_rgb() const;
private: private:
CMYKBitmap(IntSize const& size, ByteBuffer data) CMYKBitmap(IntSize const& size, ByteBuffer data)

View file

@ -1547,7 +1547,7 @@ ErrorOr<ImageFrameDescriptor> BMPImageDecoderPlugin::frame(size_t index, Optiona
TRY(decode_bmp_pixel_data(*m_context)); TRY(decode_bmp_pixel_data(*m_context));
VERIFY(m_context->bitmap); VERIFY(m_context->bitmap);
return ImageFrameDescriptor { m_context->bitmap, 0 }; return ImageFrameDescriptor { *m_context->bitmap, 0 };
} }
ErrorOr<Optional<ReadonlyBytes>> BMPImageDecoderPlugin::icc_data() ErrorOr<Optional<ReadonlyBytes>> BMPImageDecoderPlugin::icc_data()

View file

@ -488,20 +488,20 @@ ErrorOr<ImageFrameDescriptor> GIFImageDecoderPlugin::frame(size_t index, Optiona
m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame; m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame;
return result.release_error(); 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; m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame;
return result.release_error(); return result.release_error();
} }
m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAllFrames; m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAllFrames;
} }
ImageFrameDescriptor frame {}; ImageFrameDescriptor frame {
frame.image = TRY(m_context->frame_buffer->clone()); .image = TRY(m_context->frame_buffer->clone()),
frame.duration = m_context->images[index]->duration * 10; .duration = m_context->images[index]->duration * 10,
};
if (frame.duration <= 10) { if (frame.duration <= 10)
frame.duration = 100; frame.duration = 100;
}
return frame; return frame;
} }

View file

@ -171,10 +171,6 @@ ErrorOr<void> ICOImageDecoderPlugin::load_ico_bitmap(ICOLoadingContext& context)
if (PNGImageDecoderPlugin::sniff({ context.data + desc.offset, desc.size })) { if (PNGImageDecoderPlugin::sniff({ context.data + desc.offset, desc.size })) {
auto png_decoder = TRY(PNGImageDecoderPlugin::create({ 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)); 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; desc.bitmap = decoded_png_frame.image;
return {}; return {};
} else { } else {
@ -184,10 +180,6 @@ ErrorOr<void> ICOImageDecoderPlugin::load_ico_bitmap(ICOLoadingContext& context)
// inside an ICO image. // inside an ICO image.
if (bmp_decoder->sniff_dib()) { if (bmp_decoder->sniff_dib()) {
auto decoded_bmp_frame = TRY(bmp_decoder->frame(0)); 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; desc.bitmap = decoded_bmp_frame.image;
} else { } else {
dbgln_if(ICO_DEBUG, "load_ico_bitmap: encoded image not supported at index: {}", real_index); dbgln_if(ICO_DEBUG, "load_ico_bitmap: encoded image not supported at index: {}", real_index);
@ -244,7 +236,7 @@ ErrorOr<ImageFrameDescriptor> ICOImageDecoderPlugin::frame(size_t index, Optiona
} }
VERIFY(m_context->images[m_context->largest_index].bitmap); 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 };
} }
} }

View file

@ -23,12 +23,12 @@ namespace Gfx {
class Bitmap; class Bitmap;
struct ImageFrameDescriptor { struct ImageFrameDescriptor {
RefPtr<Bitmap> image; NonnullRefPtr<Bitmap> image;
int duration { 0 }; int duration { 0 };
}; };
struct VectorImageFrameDescriptor { struct VectorImageFrameDescriptor {
RefPtr<VectorGraphic> image; NonnullRefPtr<VectorGraphic> image;
int duration { 0 }; int duration { 0 };
}; };

View file

@ -217,7 +217,7 @@ ErrorOr<ImageFrameDescriptor> JPEGImageDecoderPlugin::frame(size_t index, Option
m_context->state = JPEGLoadingContext::State::Decoded; m_context->state = JPEGLoadingContext::State::Decoded;
} }
return ImageFrameDescriptor { m_context->rgb_bitmap, 0 }; return ImageFrameDescriptor { *m_context->rgb_bitmap, 0 };
} }
Optional<Metadata const&> JPEGImageDecoderPlugin::metadata() Optional<Metadata const&> JPEGImageDecoderPlugin::metadata()

View file

@ -786,7 +786,7 @@ ErrorOr<ImageFrameDescriptor> TIFFImageDecoderPlugin::frame(size_t index, Option
if (m_context->cmyk_bitmap()) if (m_context->cmyk_bitmap())
return ImageFrameDescriptor { TRY(m_context->cmyk_bitmap()->to_low_quality_rgb()), 0 }; 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<Metadata const&> TIFFImageDecoderPlugin::metadata() Optional<Metadata const&> TIFFImageDecoderPlugin::metadata()

View file

@ -571,13 +571,13 @@ ErrorOr<ImageFrameDescriptor> TinyVGImageDecoderPlugin::frame(size_t, Optional<I
auto target_size = ideal_size.value_or(m_context->decoded_image->size()); auto target_size = ideal_size.value_or(m_context->decoded_image->size());
if (!m_context->bitmap || m_context->bitmap->size() != target_size) if (!m_context->bitmap || m_context->bitmap->size() != target_size)
m_context->bitmap = TRY(m_context->decoded_image->bitmap(target_size)); m_context->bitmap = TRY(m_context->decoded_image->bitmap(target_size));
return ImageFrameDescriptor { m_context->bitmap }; return ImageFrameDescriptor { *m_context->bitmap };
} }
ErrorOr<VectorImageFrameDescriptor> TinyVGImageDecoderPlugin::vector_frame(size_t) ErrorOr<VectorImageFrameDescriptor> TinyVGImageDecoderPlugin::vector_frame(size_t)
{ {
TRY(ensure_fully_decoded(*m_context)); TRY(ensure_fully_decoded(*m_context));
return VectorImageFrameDescriptor { m_context->decoded_image, 0 }; return VectorImageFrameDescriptor { *m_context->decoded_image, 0 };
} }
} }

View file

@ -86,15 +86,17 @@ Messages::ImageDecoderServer::ConnectNewClientsResponse ConnectionFromClient::co
static void decode_image_to_bitmaps_and_durations_with_decoder(Gfx::ImageDecoder const& decoder, Optional<Gfx::IntSize> ideal_size, Vector<RefPtr<Gfx::Bitmap>>& bitmaps, Vector<u32>& durations) static void decode_image_to_bitmaps_and_durations_with_decoder(Gfx::ImageDecoder const& decoder, Optional<Gfx::IntSize> ideal_size, Vector<RefPtr<Gfx::Bitmap>>& bitmaps, Vector<u32>& durations)
{ {
bitmaps.ensure_capacity(decoder.frame_count());
durations.ensure_capacity(decoder.frame_count());
for (size_t i = 0; i < decoder.frame_count(); ++i) { for (size_t i = 0; i < decoder.frame_count(); ++i) {
auto frame_or_error = decoder.frame(i, ideal_size); auto frame_or_error = decoder.frame(i, ideal_size);
if (frame_or_error.is_error()) { if (frame_or_error.is_error()) {
bitmaps.append({}); bitmaps.unchecked_append({});
durations.append(0); durations.unchecked_append(0);
} else { } else {
auto frame = frame_or_error.release_value(); auto frame = frame_or_error.release_value();
bitmaps.append(frame.image.release_nonnull()); bitmaps.unchecked_append(frame.image);
durations.append(frame.duration); durations.unchecked_append(frame.duration);
} }
} }
} }

View file

@ -211,7 +211,6 @@ TEST_CASE(test_gif_without_global_color_table)
auto plugin_decoder = TRY_OR_FAIL(Gfx::GIFImageDecoderPlugin::create(gif_data)); auto plugin_decoder = TRY_OR_FAIL(Gfx::GIFImageDecoderPlugin::create(gif_data));
EXPECT_EQ(plugin_decoder->frame_count(), 1u); EXPECT_EQ(plugin_decoder->frame_count(), 1u);
auto frame = TRY_OR_FAIL(plugin_decoder->frame(0)); 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->size(), Gfx::IntSize(1, 1));
EXPECT_EQ(frame.image->get_pixel(0, 0), Gfx::Color::NamedColor::Red); EXPECT_EQ(frame.image->get_pixel(0, 0), Gfx::Color::NamedColor::Red);
} }

View file

@ -14,7 +14,7 @@
#include <LibGfx/ImageFormats/WebPSharedLossless.h> #include <LibGfx/ImageFormats/WebPSharedLossless.h>
#include <LibGfx/ImageFormats/WebPWriter.h> #include <LibGfx/ImageFormats/WebPWriter.h>
using AnyBitmap = Variant<RefPtr<Gfx::Bitmap>, RefPtr<Gfx::CMYKBitmap>>; using AnyBitmap = Variant<NonnullRefPtr<Gfx::Bitmap>, NonnullRefPtr<Gfx::CMYKBitmap>>;
struct LoadedImage { struct LoadedImage {
Gfx::NaturalFrameFormat internal_format; Gfx::NaturalFrameFormat internal_format;
AnyBitmap bitmap; AnyBitmap bitmap;
@ -33,7 +33,7 @@ static ErrorOr<LoadedImage> load_image(RefPtr<Gfx::ImageDecoder> const& decoder,
case Gfx::NaturalFrameFormat::Vector: case Gfx::NaturalFrameFormat::Vector:
return TRY(decoder->frame(frame_index)).image; return TRY(decoder->frame(frame_index)).image;
case Gfx::NaturalFrameFormat::CMYK: case Gfx::NaturalFrameFormat::CMYK:
return RefPtr(TRY(decoder->cmyk_frame())); return TRY(decoder->cmyk_frame());
} }
VERIFY_NOT_REACHED(); VERIFY_NOT_REACHED();
}()); }());
@ -43,9 +43,9 @@ static ErrorOr<LoadedImage> load_image(RefPtr<Gfx::ImageDecoder> const& decoder,
static ErrorOr<void> invert_cmyk(LoadedImage& image) static ErrorOr<void> invert_cmyk(LoadedImage& image)
{ {
if (!image.bitmap.has<RefPtr<Gfx::CMYKBitmap>>()) if (!image.bitmap.has<NonnullRefPtr<Gfx::CMYKBitmap>>())
return Error::from_string_literal("Can't --invert-cmyk with RGB bitmaps"); return Error::from_string_literal("Can't --invert-cmyk with RGB bitmaps");
auto& frame = image.bitmap.get<RefPtr<Gfx::CMYKBitmap>>(); auto& frame = image.bitmap.get<NonnullRefPtr<Gfx::CMYKBitmap>>();
for (auto& pixel : *frame) { for (auto& pixel : *frame) {
pixel.c = ~pixel.c; pixel.c = ~pixel.c;
@ -58,18 +58,18 @@ static ErrorOr<void> invert_cmyk(LoadedImage& image)
static ErrorOr<void> crop_image(LoadedImage& image, Gfx::IntRect const& rect) static ErrorOr<void> crop_image(LoadedImage& image, Gfx::IntRect const& rect)
{ {
if (!image.bitmap.has<RefPtr<Gfx::Bitmap>>()) if (!image.bitmap.has<NonnullRefPtr<Gfx::Bitmap>>())
return Error::from_string_literal("Can't --crop CMYK bitmaps yet"); return Error::from_string_literal("Can't --crop CMYK bitmaps yet");
auto& frame = image.bitmap.get<RefPtr<Gfx::Bitmap>>(); auto& frame = image.bitmap.get<NonnullRefPtr<Gfx::Bitmap>>();
frame = TRY(frame->cropped(rect)); frame = TRY(frame->cropped(rect));
return {}; return {};
} }
static ErrorOr<void> move_alpha_to_rgb(LoadedImage& image) static ErrorOr<void> move_alpha_to_rgb(LoadedImage& image)
{ {
if (!image.bitmap.has<RefPtr<Gfx::Bitmap>>()) if (!image.bitmap.has<NonnullRefPtr<Gfx::Bitmap>>())
return Error::from_string_literal("Can't --move-alpha-to-rgb with CMYK bitmaps"); return Error::from_string_literal("Can't --move-alpha-to-rgb with CMYK bitmaps");
auto& frame = image.bitmap.get<RefPtr<Gfx::Bitmap>>(); auto& frame = image.bitmap.get<NonnullRefPtr<Gfx::Bitmap>>();
switch (frame->format()) { switch (frame->format()) {
case Gfx::BitmapFormat::Invalid: case Gfx::BitmapFormat::Invalid:
@ -96,9 +96,9 @@ static ErrorOr<void> move_alpha_to_rgb(LoadedImage& image)
static ErrorOr<void> strip_alpha(LoadedImage& image) static ErrorOr<void> strip_alpha(LoadedImage& image)
{ {
if (!image.bitmap.has<RefPtr<Gfx::Bitmap>>()) if (!image.bitmap.has<NonnullRefPtr<Gfx::Bitmap>>())
return Error::from_string_literal("Can't --strip-alpha with CMYK bitmaps"); return Error::from_string_literal("Can't --strip-alpha with CMYK bitmaps");
auto& frame = image.bitmap.get<RefPtr<Gfx::Bitmap>>(); auto& frame = image.bitmap.get<NonnullRefPtr<Gfx::Bitmap>>();
switch (frame->format()) { switch (frame->format()) {
case Gfx::BitmapFormat::Invalid: case Gfx::BitmapFormat::Invalid:
@ -128,7 +128,7 @@ static ErrorOr<void> save_image(LoadedImage& image, StringView out_path, u8 jpeg
return Core::OutputBufferedFile::create(move(output_stream)); return Core::OutputBufferedFile::create(move(output_stream));
}; };
auto& frame = image.bitmap.get<RefPtr<Gfx::Bitmap>>(); auto& frame = image.bitmap.get<NonnullRefPtr<Gfx::Bitmap>>();
if (out_path.ends_with(".jpg"sv, CaseSensitivity::CaseInsensitive) || out_path.ends_with(".jpeg"sv, CaseSensitivity::CaseInsensitive)) { 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 })); TRY(Gfx::JPEGWriter::encode(*TRY(stream()), *frame, { .icc_data = image.icc_data, .quality = jpeg_quality }));