LibGfx: Make ImageDecoderPlugin::frame() return ErrorOr<>

This is a first step towards better error propagation from image codecs.
This commit is contained in:
Andreas Kling 2021-11-20 14:29:33 +01:00
parent ae7656072a
commit 5a79c69b02
Notes: sideshowbarker 2024-07-18 00:54:14 +09:00
26 changed files with 101 additions and 100 deletions

View file

@ -14,8 +14,9 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
{
Gfx::GIFImageDecoderPlugin gif_decoder(data, size);
auto bitmap = gif_decoder.frame(0).image;
if (bitmap) {
auto bitmap_or_error = gif_decoder.frame(0);
if (!bitmap_or_error.is_error()) {
auto const& bitmap = bitmap_or_error.value().image;
// Looks like a valid GIF. Try to load the other frames:
dbgln_if(GIF_DEBUG, "bitmap size: {}", bitmap->size());
dbgln_if(GIF_DEBUG, "codec size: {}", gif_decoder.size());
@ -24,7 +25,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
dbgln_if(GIF_DEBUG, "loop_count: {}", gif_decoder.loop_count());
dbgln_if(GIF_DEBUG, "frame_count: {}", gif_decoder.frame_count());
for (size_t i = 0; i < gif_decoder.frame_count(); ++i) {
auto ifd = gif_decoder.frame(i);
auto ifd = gif_decoder.frame(i).release_value_but_fixme_should_propagate_errors();
dbgln_if(GIF_DEBUG, "frame #{} size: {}", i, ifd.image->size());
dbgln_if(GIF_DEBUG, "frame #{} duration: {}", i, ifd.duration);
}

View file

@ -31,7 +31,7 @@ TEST_CASE(test_bmp)
EXPECT(!bmp.is_animated());
EXPECT(!bmp.loop_count());
auto frame = bmp.frame(1);
auto frame = bmp.frame(0).release_value_but_fixme_should_propagate_errors();
EXPECT(frame.duration == 0);
}
@ -45,7 +45,7 @@ TEST_CASE(test_gif)
EXPECT(gif.is_animated());
EXPECT(!gif.loop_count());
auto frame = gif.frame(1);
auto frame = gif.frame(1).release_value_but_fixme_should_propagate_errors();
EXPECT(frame.duration == 400);
}
@ -60,8 +60,7 @@ TEST_CASE(test_ico)
EXPECT(!ico.is_animated());
EXPECT(!ico.loop_count());
auto frame = ico.frame(1);
EXPECT(frame.duration == 0);
EXPECT(ico.frame(0).is_error());
}
TEST_CASE(test_jpg)
@ -74,7 +73,7 @@ TEST_CASE(test_jpg)
EXPECT(!jpg.is_animated());
EXPECT(!jpg.loop_count());
auto frame = jpg.frame(1);
auto frame = jpg.frame(0).release_value_but_fixme_should_propagate_errors();
EXPECT(frame.duration == 0);
}
@ -88,7 +87,7 @@ TEST_CASE(test_pbm)
EXPECT(!pbm.is_animated());
EXPECT(!pbm.loop_count());
auto frame = pbm.frame(1);
auto frame = pbm.frame(0).release_value_but_fixme_should_propagate_errors();
EXPECT(frame.duration == 0);
}
@ -102,7 +101,7 @@ TEST_CASE(test_pgm)
EXPECT(!pgm.is_animated());
EXPECT(!pgm.loop_count());
auto frame = pgm.frame(1);
auto frame = pgm.frame(0).release_value_but_fixme_should_propagate_errors();
EXPECT(frame.duration == 0);
}
@ -116,7 +115,7 @@ TEST_CASE(test_png)
EXPECT(!png.is_animated());
EXPECT(!png.loop_count());
auto frame = png.frame(1);
auto frame = png.frame(0).release_value_but_fixme_should_propagate_errors();
EXPECT(frame.duration == 0);
}
@ -130,6 +129,6 @@ TEST_CASE(test_ppm)
EXPECT(!ppm.is_animated());
EXPECT(!ppm.loop_count());
auto frame = ppm.frame(1);
auto frame = ppm.frame(0).release_value_but_fixme_should_propagate_errors();
EXPECT(frame.duration == 0);
}

View file

@ -188,7 +188,11 @@ Icon FileIconProvider::icon_for_executable(const String& path)
if (!section.has_value()) {
bitmap = s_executable_icon.bitmap_for_size(icon_section.image_size);
} else {
bitmap = Gfx::PNGImageDecoderPlugin(reinterpret_cast<u8 const*>(section->raw_data()), section->size()).frame(0).image;
// FIXME: Use the ImageDecoder service.
auto frame_or_error = Gfx::PNGImageDecoderPlugin(reinterpret_cast<u8 const*>(section->raw_data()), section->size()).frame(0);
if (!frame_or_error.is_error()) {
bitmap = frame_or_error.value().image;
}
}
if (!bitmap) {

View file

@ -56,7 +56,7 @@ void ImageWidget::animate()
{
m_current_frame_index = (m_current_frame_index + 1) % m_image_decoder->frame_count();
const auto& current_frame = m_image_decoder->frame(m_current_frame_index);
auto current_frame = m_image_decoder->frame(m_current_frame_index).release_value_but_fixme_should_propagate_errors();
set_bitmap(current_frame.image);
if (current_frame.duration != m_timer->interval()) {
@ -81,14 +81,14 @@ void ImageWidget::load_from_file(StringView path)
m_image_decoder = Gfx::ImageDecoder::try_create(mapped_file.bytes());
VERIFY(m_image_decoder);
auto frame = m_image_decoder->frame(0);
auto frame = m_image_decoder->frame(0).release_value_but_fixme_should_propagate_errors();
auto bitmap = frame.image;
VERIFY(bitmap);
set_bitmap(bitmap);
if (m_image_decoder->is_animated() && m_image_decoder->frame_count() > 1) {
const auto& first_frame = m_image_decoder->frame(0);
auto first_frame = m_image_decoder->frame(0).release_value_but_fixme_should_propagate_errors();
m_timer->set_interval(first_frame.duration);
m_timer->on_timeout = [this] { animate(); };
m_timer->start();

View file

@ -1354,19 +1354,19 @@ size_t BMPImageDecoderPlugin::frame_count()
return 1;
}
ImageFrameDescriptor BMPImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> BMPImageDecoderPlugin::frame(size_t index)
{
if (i > 0)
return {};
if (index > 0)
return Error::from_string_literal("BMPImageDecoderPlugin: Invalid frame index"sv);
if (m_context->state == BMPLoadingContext::State::Error)
return {};
return Error::from_string_literal("BMPImageDecoderPlugin: Decoding failed"sv);
if (m_context->state < BMPLoadingContext::State::PixelDataDecoded && !decode_bmp_pixel_data(*m_context))
return {};
return Error::from_string_literal("BMPImageDecoderPlugin: Decoding failed"sv);
VERIFY(m_context->bitmap);
return { m_context->bitmap, 0 };
return ImageFrameDescriptor { m_context->bitmap, 0 };
}
}

View file

@ -24,7 +24,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<BMPLoadingContext> m_context;

View file

@ -136,7 +136,8 @@ ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::try_load_from_fd_and_close(int fd, String
{
auto file = TRY(MappedFile::map_from_fd_and_close(fd, path));
if (auto decoder = ImageDecoder::try_create(file->bytes())) {
if (auto bitmap = decoder->frame(0).image)
auto frame = TRY(decoder->frame(0));
if (auto& bitmap = frame.image)
return bitmap.release_nonnull();
}

View file

@ -997,22 +997,22 @@ size_t DDSImageDecoderPlugin::frame_count()
return 1;
}
ImageFrameDescriptor DDSImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> DDSImageDecoderPlugin::frame(size_t index)
{
if (i > 0)
return {};
if (index > 0)
return Error::from_string_literal("DDSImageDecoderPlugin: Invalid frame index"sv);
if (m_context->state == DDSLoadingContext::State::Error)
return {};
return Error::from_string_literal("DDSImageDecoderPlugin: Decoding failed"sv);
if (m_context->state < DDSLoadingContext::State::BitmapDecoded) {
bool success = decode_dds(*m_context);
if (!success)
return {};
return Error::from_string_literal("DDSImageDecoderPlugin: Decoding failed"sv);
}
VERIFY(m_context->bitmap);
return { m_context->bitmap, 0 };
return ImageFrameDescriptor { m_context->bitmap, 0 };
}
}

View file

@ -245,7 +245,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<DDSLoadingContext> m_context;

View file

@ -691,36 +691,30 @@ size_t GIFImageDecoderPlugin::frame_count()
return m_context->images.size();
}
ImageFrameDescriptor GIFImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> GIFImageDecoderPlugin::frame(size_t index)
{
if (m_context->error_state >= GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame) {
return {};
return Error::from_string_literal("GIFImageDecoderPlugin: Decoding failed"sv);
}
if (m_context->state < GIFLoadingContext::State::FrameDescriptorsLoaded) {
if (!load_gif_frame_descriptors(*m_context)) {
m_context->error_state = GIFLoadingContext::ErrorState::FailedToLoadFrameDescriptors;
return {};
return Error::from_string_literal("GIFImageDecoderPlugin: Decoding failed"sv);
}
}
if (m_context->error_state == GIFLoadingContext::ErrorState::NoError && !decode_frame(*m_context, i)) {
if (m_context->error_state == GIFLoadingContext::ErrorState::NoError && !decode_frame(*m_context, index)) {
if (m_context->state < GIFLoadingContext::State::FrameComplete || !decode_frame(*m_context, 0)) {
m_context->error_state = GIFLoadingContext::ErrorState::FailedToDecodeAnyFrame;
return {};
return Error::from_string_literal("GIFImageDecoderPlugin: Decoding failed"sv);
}
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 = image_or_error.release_value_but_fixme_should_propagate_errors();
frame.duration = m_context->images.at(i).duration * 10;
frame.image = TRY(m_context->frame_buffer->clone());
frame.duration = m_context->images.at(index).duration * 10;
if (frame.duration <= 10) {
frame.duration = 100;

View file

@ -25,7 +25,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<GIFLoadingContext> m_context;

View file

@ -263,11 +263,12 @@ static bool load_ico_bitmap(ICOLoadingContext& context, Optional<size_t> index)
PNGImageDecoderPlugin png_decoder(context.data + desc.offset, desc.size);
if (png_decoder.sniff()) {
desc.bitmap = png_decoder.frame(0).image;
if (!desc.bitmap) {
auto decoded_png_frame = png_decoder.frame(0);
if (!decoded_png_frame.is_error() || !decoded_png_frame.value().image) {
dbgln_if(ICO_DEBUG, "load_ico_bitmap: failed to load PNG encoded image index: {}", real_index);
return false;
}
desc.bitmap = decoded_png_frame.value().image;
return true;
} else {
if (!load_ico_bmp(context, desc)) {
@ -338,26 +339,26 @@ size_t ICOImageDecoderPlugin::frame_count()
return 1;
}
ImageFrameDescriptor ICOImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> ICOImageDecoderPlugin::frame(size_t index)
{
if (i > 0)
return {};
if (index > 0)
return Error::from_string_literal("ICOImageDecoderPlugin: Invalid frame index"sv);
if (m_context->state == ICOLoadingContext::State::Error)
return {};
return Error::from_string_literal("ICOImageDecoderPlugin: Decoding failed"sv);
if (m_context->state < ICOLoadingContext::State::BitmapDecoded) {
// NOTE: This forces the chunk decoding to happen.
bool success = load_ico_bitmap(*m_context, {});
if (!success) {
m_context->state = ICOLoadingContext::State::Error;
return {};
return Error::from_string_literal("ICOImageDecoderPlugin: Decoding failed"sv);
}
m_context->state = ICOLoadingContext::State::BitmapDecoded;
}
VERIFY(m_context->images[m_context->largest_index].bitmap);
return { m_context->images[m_context->largest_index].bitmap, 0 };
return ImageFrameDescriptor { m_context->images[m_context->largest_index].bitmap, 0 };
}
}

View file

@ -24,7 +24,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<ICOLoadingContext> m_context;

View file

@ -39,7 +39,7 @@ public:
virtual bool is_animated() = 0;
virtual size_t loop_count() = 0;
virtual size_t frame_count() = 0;
virtual ImageFrameDescriptor frame(size_t i) = 0;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) = 0;
protected:
ImageDecoderPlugin() { }
@ -59,7 +59,7 @@ public:
bool is_animated() const { return m_plugin->is_animated(); }
size_t loop_count() const { return m_plugin->loop_count(); }
size_t frame_count() const { return m_plugin->frame_count(); }
ImageFrameDescriptor frame(size_t i) const { return m_plugin->frame(i); }
ErrorOr<ImageFrameDescriptor> frame(size_t index) const { return m_plugin->frame(index); }
private:
explicit ImageDecoder(NonnullOwnPtr<ImageDecoderPlugin>);

View file

@ -1276,23 +1276,23 @@ size_t JPGImageDecoderPlugin::frame_count()
return 1;
}
ImageFrameDescriptor JPGImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> JPGImageDecoderPlugin::frame(size_t index)
{
if (i > 0)
return {};
if (index > 0)
return Error::from_string_literal("JPGImageDecoderPlugin: Invalid frame index"sv);
if (m_context->state == JPGLoadingContext::State::Error)
return {};
return Error::from_string_literal("JPGImageDecoderPlugin: Decoding failed"sv);
if (m_context->state < JPGLoadingContext::State::BitmapDecoded) {
if (!decode_jpg(*m_context)) {
m_context->state = JPGLoadingContext::State::Error;
return {};
return Error::from_string_literal("JPGImageDecoderPlugin: Decoding failed"sv);
}
m_context->state = JPGLoadingContext::State::BitmapDecoded;
}
return { m_context->bitmap, 0 };
return ImageFrameDescriptor { m_context->bitmap, 0 };
}
}

View file

@ -23,7 +23,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<JPGLoadingContext> m_context;

View file

@ -162,22 +162,22 @@ size_t PBMImageDecoderPlugin::frame_count()
return 1;
}
ImageFrameDescriptor PBMImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> PBMImageDecoderPlugin::frame(size_t index)
{
if (i > 0)
return {};
if (index > 0)
return Error::from_string_literal("PBMImageDecoderPlugin: Invalid frame index"sv);
if (m_context->state == PBMLoadingContext::State::Error)
return {};
return Error::from_string_literal("PBMImageDecoderPlugin: Decoding failed"sv);
if (m_context->state < PBMLoadingContext::State::Decoded) {
bool success = decode(*m_context);
if (!success)
return {};
return Error::from_string_literal("PBMImageDecoderPlugin: Decoding failed"sv);
}
VERIFY(m_context->bitmap);
return { m_context->bitmap, 0 };
return ImageFrameDescriptor { m_context->bitmap, 0 };
}
}

View file

@ -27,7 +27,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<PBMLoadingContext> m_context;

View file

@ -165,22 +165,22 @@ size_t PGMImageDecoderPlugin::frame_count()
return 1;
}
ImageFrameDescriptor PGMImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> PGMImageDecoderPlugin::frame(size_t index)
{
if (i > 0)
return {};
if (index > 0)
return Error::from_string_literal("PGMImageDecoderPlugin: Invalid frame index"sv);
if (m_context->state == PGMLoadingContext::State::Error)
return {};
return Error::from_string_literal("PGMImageDecoderPlugin: Decoding failed"sv);
if (m_context->state < PGMLoadingContext::State::Decoded) {
bool success = decode(*m_context);
if (!success)
return {};
return Error::from_string_literal("PGMImageDecoderPlugin: Decoding failed"sv);
}
VERIFY(m_context->bitmap);
return { m_context->bitmap, 0 };
return ImageFrameDescriptor { m_context->bitmap, 0 };
}
}

View file

@ -27,7 +27,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<PGMLoadingContext> m_context;

View file

@ -964,23 +964,23 @@ size_t PNGImageDecoderPlugin::frame_count()
return 1;
}
ImageFrameDescriptor PNGImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> PNGImageDecoderPlugin::frame(size_t index)
{
if (i > 0)
return {};
if (index > 0)
return Error::from_string_literal("PNGImageDecoderPlugin: Invalid frame index"sv);
if (m_context->state == PNGLoadingContext::State::Error)
return {};
return Error::from_string_literal("PNGImageDecoderPlugin: Decoding failed"sv);
if (m_context->state < PNGLoadingContext::State::BitmapDecoded) {
// NOTE: This forces the chunk decoding to happen.
bool success = decode_png_bitmap(*m_context);
if (!success)
return {};
return Error::from_string_literal("PNGImageDecoderPlugin: Decoding failed"sv);
}
VERIFY(m_context->bitmap);
return { m_context->bitmap, 0 };
return ImageFrameDescriptor { m_context->bitmap, 0 };
}
}

View file

@ -24,7 +24,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<PNGLoadingContext> m_context;

View file

@ -169,22 +169,22 @@ size_t PPMImageDecoderPlugin::frame_count()
return 1;
}
ImageFrameDescriptor PPMImageDecoderPlugin::frame(size_t i)
ErrorOr<ImageFrameDescriptor> PPMImageDecoderPlugin::frame(size_t index)
{
if (i > 0)
return {};
if (index > 0)
return Error::from_string_literal("PPMImageDecoderPlugin: Invalid frame index"sv);
if (m_context->state == PPMLoadingContext::State::Error)
return {};
return Error::from_string_literal("PGMImageDecoderPlugin: Decoding failed"sv);
if (m_context->state < PPMLoadingContext::State::Decoded) {
bool success = decode(*m_context);
if (!success)
return {};
return Error::from_string_literal("PGMImageDecoderPlugin: Decoding failed"sv);
}
VERIFY(m_context->bitmap);
return { m_context->bitmap, 0 };
return ImageFrameDescriptor { m_context->bitmap, 0 };
}
}

View file

@ -27,7 +27,7 @@ public:
virtual bool is_animated() override;
virtual size_t loop_count() override;
virtual size_t frame_count() override;
virtual ImageFrameDescriptor frame(size_t i) override;
virtual ErrorOr<ImageFrameDescriptor> frame(size_t index) override;
private:
OwnPtr<PPMLoadingContext> m_context;

View file

@ -52,12 +52,12 @@ Messages::ImageDecoderServer::DecodeImageResponse ClientConnection::decode_image
Vector<Gfx::ShareableBitmap> bitmaps;
Vector<u32> durations;
for (size_t i = 0; i < decoder->frame_count(); ++i) {
auto frame = decoder->frame(i);
if (frame.image)
bitmaps.append(frame.image->to_shareable_bitmap());
else
auto frame_or_error = decoder->frame(i);
if (frame_or_error.is_error() || !frame_or_error.value().image)
bitmaps.append(Gfx::ShareableBitmap {});
durations.append(frame.duration);
else
bitmaps.append(frame_or_error.value().image->to_shareable_bitmap());
durations.append(frame_or_error.value().duration);
}
return { decoder->is_animated(), static_cast<u32>(decoder->loop_count()), bitmaps, durations };

View file

@ -140,17 +140,18 @@ void SpiceAgent::on_message_received()
m_clipboard_connection.async_set_clipboard_data(anon_buffer, "text/plain", {});
return;
} else {
RefPtr<Gfx::Bitmap> bitmap;
ErrorOr<Gfx::ImageFrameDescriptor> frame_or_error = Gfx::ImageFrameDescriptor {};
if (type == ClipboardType::PNG) {
bitmap = Gfx::PNGImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0).image;
frame_or_error = Gfx::PNGImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0);
} else if (type == ClipboardType::BMP) {
bitmap = Gfx::BMPImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0).image;
frame_or_error = Gfx::BMPImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0);
} else if (type == ClipboardType::JPG) {
bitmap = Gfx::JPGImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0).image;
frame_or_error = Gfx::JPGImageDecoderPlugin(data_buffer.data(), data_buffer.size()).frame(0);
} else {
dbgln("Unknown clipboard type: {}", (u32)type);
return;
}
auto const& bitmap = frame_or_error.value().image;
m_clipboard_connection.set_bitmap(*bitmap);
}
break;