LibGfx: Return descriptive errors when decoding PNGs

We had an interesting tri-state `ErrorOr<bool>` going on when decoding PNGs; replace it with `ErrorOr<void>` and add some descriptive errors.
gmta · Nov 4, 2024 · 85f1ccd · 85f1ccd
1 parent 10f0f74
commit 85f1ccd
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 18 deletions.
diff --git a/Userland/Libraries/LibGfx/ImageFormats/PNGLoader.cpp b/Userland/Libraries/LibGfx/ImageFormats/PNGLoader.cpp
@@ -75,9 +75,7 @@ struct PNGLoadingContext {
 ErrorOr<NonnullOwnPtr<ImageDecoderPlugin>> PNGImageDecoderPlugin::create(ReadonlyBytes bytes)
 {
     auto decoder = adopt_own(*new PNGImageDecoderPlugin(bytes));
-    if (!TRY(decoder->initialize()))
-        return Error::from_string_literal("PNG load error");
-
+    TRY(decoder->initialize());
     return decoder;
 }
 
@@ -127,19 +125,19 @@ ErrorOr<Optional<ReadonlyBytes>> PNGImageDecoderPlugin::icc_data()
     return OptionalNone {};
 }
 
-ErrorOr<bool> PNGImageDecoderPlugin::initialize()
+ErrorOr<void> PNGImageDecoderPlugin::initialize()
 {
     png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr, nullptr, nullptr);
     if (!png_ptr)
-        return false;
+        return Error::from_string_view("Failed to allocate read struct"sv);
     AK::ScopeGuard png_cleanup = [&] { png_destroy_read_struct(&png_ptr, nullptr, nullptr); };
 
     png_infop info_ptr = png_create_info_struct(png_ptr);
     if (!info_ptr)
-        return false;
+        return Error::from_string_view("Failed to allocate info struct"sv);
 
-    if (setjmp(png_jmpbuf(png_ptr)))
-        return false;
+    if (auto error_value = setjmp(png_jmpbuf(png_ptr)); error_value)
+        return Error::from_errno(error_value);
 
     png_set_read_fn(png_ptr, &m_context->data, [](png_structp png_ptr, png_bytep data, png_size_t length) {
         auto* read_data = reinterpret_cast<ReadonlyBytes*>(png_get_io_ptr(png_ptr));
@@ -182,26 +180,22 @@ ErrorOr<bool> PNGImageDecoderPlugin::initialize()
     int compression_type = 0;
     u8* profile_data = nullptr;
     u32 profile_len = 0;
-    if (png_get_iCCP(png_ptr, info_ptr, &profile_name, &compression_type, &profile_data, &profile_len)) {
+    if (png_get_iCCP(png_ptr, info_ptr, &profile_name, &compression_type, &profile_data, &profile_len))
         m_context->icc_profile = TRY(ByteBuffer::copy(profile_data, profile_len));
-    }
 
     png_read_update_info(png_ptr, info_ptr);
     m_context->frame_count = TRY(m_context->read_frames(png_ptr, info_ptr));
 
     u8* exif_data = nullptr;
     u32 exif_length = 0;
     int const num_exif_chunks = png_get_eXIf_1(png_ptr, info_ptr, &exif_length, &exif_data);
-    if (num_exif_chunks > 0) {
+    if (num_exif_chunks > 0)
         m_context->exif_metadata = TRY(TIFFImageDecoderPlugin::read_exif_metadata({ exif_data, exif_length }));
-    }
 
-    if (m_context->exif_metadata) {
-        if (auto result = m_context->apply_exif_orientation(); result.is_error())
-            dbgln("Could not apply eXIf chunk orientation for PNG: {}", result.error());
-    }
+    if (m_context->exif_metadata)
+        TRY(m_context->apply_exif_orientation());
 
-    return true;
+    return {};
 }
 
 ErrorOr<void> PNGLoadingContext::apply_exif_orientation()

diff --git a/Userland/Libraries/LibGfx/ImageFormats/PNGLoader.h b/Userland/Libraries/LibGfx/ImageFormats/PNGLoader.h
@@ -32,7 +32,7 @@ class PNGImageDecoderPlugin final : public ImageDecoderPlugin {
 private:
     explicit PNGImageDecoderPlugin(ReadonlyBytes);
 
-    ErrorOr<bool> initialize();
+    ErrorOr<void> initialize();
 
     OwnPtr<PNGLoadingContext> m_context;
 };