From d88c7fee32b0f5a045967436f250ad8265fb9edf Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 5 Dec 2022 20:34:27 +0100 Subject: [PATCH] LibGfx+Userland: Make PNGWriter::encode() return ErrorOr This is a first step towards handling PNG encoding failures instead of just falling over and crashing the program. This initial step will cause encode() to return an error if the final ByteBuffer copy fails to allocate. There are more potential failures that will be surfaced by subsequent commits. Two FIXMEs were killed in the making of this patch. :^) --- Userland/Applications/Browser/BrowserWindow.cpp | 2 +- Userland/Applications/PixelPaint/Image.cpp | 2 +- Userland/Demos/Mandelbrot/Mandelbrot.cpp | 2 +- Userland/Libraries/LibGfx/PNGWriter.cpp | 5 ++--- Userland/Libraries/LibGfx/PNGWriter.h | 2 +- Userland/Libraries/LibWeb/HTML/HTMLCanvasElement.cpp | 8 ++++++-- Userland/Services/SpiceAgent/SpiceAgent.cpp | 2 +- Userland/Utilities/headless-browser.cpp | 2 +- Userland/Utilities/shot.cpp | 6 ++++-- 9 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Userland/Applications/Browser/BrowserWindow.cpp b/Userland/Applications/Browser/BrowserWindow.cpp index 56c55727a5..1f1fe0846e 100644 --- a/Userland/Applications/Browser/BrowserWindow.cpp +++ b/Userland/Applications/Browser/BrowserWindow.cpp @@ -744,7 +744,7 @@ ErrorOr BrowserWindow::take_screenshot(ScreenshotType type) LexicalPath path { Core::StandardPaths::downloads_directory() }; path = path.append(Core::DateTime::now().to_deprecated_string("screenshot-%Y-%m-%d-%H-%M-%S.png"sv)); - auto encoded = Gfx::PNGWriter::encode(*bitmap.bitmap()); + auto encoded = TRY(Gfx::PNGWriter::encode(*bitmap.bitmap())); auto screenshot_file = TRY(Core::Stream::File::open(path.string(), Core::Stream::OpenMode::Write)); TRY(screenshot_file->write(encoded)); diff --git a/Userland/Applications/PixelPaint/Image.cpp b/Userland/Applications/PixelPaint/Image.cpp index 9d7f971d59..c95c717401 100644 --- a/Userland/Applications/PixelPaint/Image.cpp +++ b/Userland/Applications/PixelPaint/Image.cpp @@ -193,7 +193,7 @@ ErrorOr Image::export_png_to_file(Core::File& file, bool preserve_alpha_ch auto bitmap_format = preserve_alpha_channel ? Gfx::BitmapFormat::BGRA8888 : Gfx::BitmapFormat::BGRx8888; auto bitmap = TRY(try_compose_bitmap(bitmap_format)); - auto encoded_data = Gfx::PNGWriter::encode(*bitmap); + auto encoded_data = TRY(Gfx::PNGWriter::encode(*bitmap)); if (!file.write(encoded_data.data(), encoded_data.size())) return Error::from_errno(file.error()); diff --git a/Userland/Demos/Mandelbrot/Mandelbrot.cpp b/Userland/Demos/Mandelbrot/Mandelbrot.cpp index 2683956617..41a7a2cef7 100644 --- a/Userland/Demos/Mandelbrot/Mandelbrot.cpp +++ b/Userland/Demos/Mandelbrot/Mandelbrot.cpp @@ -377,7 +377,7 @@ ErrorOr Mandelbrot::export_image(DeprecatedString const& export_path, Imag break; } case ImageType::PNG: - encoded_data = Gfx::PNGWriter::encode(m_set.bitmap()); + encoded_data = TRY(Gfx::PNGWriter::encode(m_set.bitmap())); break; case ImageType::QOI: encoded_data = Gfx::QOIWriter::encode(m_set.bitmap()); diff --git a/Userland/Libraries/LibGfx/PNGWriter.cpp b/Userland/Libraries/LibGfx/PNGWriter.cpp index 073ef48488..f5cb20f0c1 100644 --- a/Userland/Libraries/LibGfx/PNGWriter.cpp +++ b/Userland/Libraries/LibGfx/PNGWriter.cpp @@ -261,15 +261,14 @@ void PNGWriter::add_IDAT_chunk(Gfx::Bitmap const& bitmap) add_chunk(png_chunk); } -ByteBuffer PNGWriter::encode(Gfx::Bitmap const& bitmap) +ErrorOr PNGWriter::encode(Gfx::Bitmap const& bitmap) { PNGWriter writer; writer.add_png_header(); writer.add_IHDR_chunk(bitmap.width(), bitmap.height(), 8, PNG::ColorType::TruecolorWithAlpha, 0, 0, 0); writer.add_IDAT_chunk(bitmap); writer.add_IEND_chunk(); - // FIXME: Handle OOM failure. - return ByteBuffer::copy(writer.m_data).release_value_but_fixme_should_propagate_errors(); + return ByteBuffer::copy(writer.m_data); } } diff --git a/Userland/Libraries/LibGfx/PNGWriter.h b/Userland/Libraries/LibGfx/PNGWriter.h index 62a0ef0304..09d116fdd5 100644 --- a/Userland/Libraries/LibGfx/PNGWriter.h +++ b/Userland/Libraries/LibGfx/PNGWriter.h @@ -17,7 +17,7 @@ class PNGChunk; class PNGWriter { public: - static ByteBuffer encode(Gfx::Bitmap const&); + static ErrorOr encode(Gfx::Bitmap const&); private: PNGWriter() = default; diff --git a/Userland/Libraries/LibWeb/HTML/HTMLCanvasElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLCanvasElement.cpp index 587eddd683..faad688c1c 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLCanvasElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLCanvasElement.cpp @@ -176,8 +176,12 @@ DeprecatedString HTMLCanvasElement::to_data_url(DeprecatedString const& type, [[ return {}; if (type != "image/png") return {}; - auto encoded_bitmap = Gfx::PNGWriter::encode(*m_bitmap); - return AK::URL::create_with_data(type, encode_base64(encoded_bitmap), true).to_deprecated_string(); + auto encoded_bitmap_or_error = Gfx::PNGWriter::encode(*m_bitmap); + if (encoded_bitmap_or_error.is_error()) { + dbgln("Gfx::PNGWriter failed to encode the HTMLCanvasElement: {}", encoded_bitmap_or_error.error()); + return {}; + } + return AK::URL::create_with_data(type, encode_base64(encoded_bitmap_or_error.value()), true).to_deprecated_string(); } void HTMLCanvasElement::present() diff --git a/Userland/Services/SpiceAgent/SpiceAgent.cpp b/Userland/Services/SpiceAgent/SpiceAgent.cpp index c9bb4a1eeb..da0f97a92b 100644 --- a/Userland/Services/SpiceAgent/SpiceAgent.cpp +++ b/Userland/Services/SpiceAgent/SpiceAgent.cpp @@ -79,7 +79,7 @@ void SpiceAgent::on_message_received() ReadonlyBytes bytes; if (mime == "image/x-serenityos") { auto bitmap = m_clipboard_connection.get_bitmap(); - backing_byte_buffer = Gfx::PNGWriter::encode(*bitmap); + backing_byte_buffer = MUST(Gfx::PNGWriter::encode(*bitmap)); bytes = backing_byte_buffer; } else { auto data = clipboard.data(); diff --git a/Userland/Utilities/headless-browser.cpp b/Userland/Utilities/headless-browser.cpp index b13d87e5d7..bed46ac996 100644 --- a/Userland/Utilities/headless-browser.cpp +++ b/Userland/Utilities/headless-browser.cpp @@ -703,7 +703,7 @@ static void load_page_for_screenshot_and_exit(HeadlessBrowserPageClient& page_cl page_client.paint(output_rect, output_bitmap); - auto image_buffer = Gfx::PNGWriter::encode(output_bitmap); + auto image_buffer = MUST(Gfx::PNGWriter::encode(output_bitmap)); MUST(output_file->write(image_buffer.bytes())); exit(0); diff --git a/Userland/Utilities/shot.cpp b/Userland/Utilities/shot.cpp index 394d65d2c3..b266005abc 100644 --- a/Userland/Utilities/shot.cpp +++ b/Userland/Utilities/shot.cpp @@ -151,11 +151,13 @@ ErrorOr serenity_main(Main::Arguments arguments) return 0; } - auto encoded_bitmap = Gfx::PNGWriter::encode(*bitmap); - if (encoded_bitmap.is_empty()) { + auto encoded_bitmap_or_error = Gfx::PNGWriter::encode(*bitmap); + if (encoded_bitmap_or_error.is_error()) { warnln("Failed to encode PNG"); return 1; } + auto encoded_bitmap = encoded_bitmap_or_error.release_value(); + if (edit_image) output_path = Core::DateTime::now().to_deprecated_string("/tmp/screenshot-%Y-%m-%d-%H-%M-%S.png"sv);