From 99e589cc980988197bd4f367fae8bc4a5d4c51a1 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Wed, 24 Nov 2021 14:19:40 -0800 Subject: [PATCH 1/2] Log libpng warnings and errors --- Source/Core/Common/Image.cpp | 41 +++++++++++++++++++++++++++++++++++- Source/Core/Common/ImageC.c | 16 ++++++++++++++ Source/Core/Common/ImageC.h | 24 ++++++++++++++++----- 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/Source/Core/Common/Image.cpp b/Source/Core/Common/Image.cpp index 3b5c644c4d..87a2cb7e1f 100644 --- a/Source/Core/Common/Image.cpp +++ b/Source/Core/Common/Image.cpp @@ -8,6 +8,7 @@ #include +#include "Common/Assert.h" #include "Common/CommonTypes.h" #include "Common/IOFile.h" #include "Common/ImageC.h" @@ -48,6 +49,18 @@ static void WriteCallback(png_structp png_ptr, png_bytep data, size_t length) buffer->insert(buffer->end(), data, data + length); } +static void ErrorCallback(ErrorHandler* self, const char* msg) +{ + std::vector* errors = static_cast*>(self->error_list); + errors->emplace_back(msg); +} + +static void WarningCallback(ErrorHandler* self, const char* msg) +{ + std::vector* warnings = static_cast*>(self->warning_list); + warnings->emplace_back(msg); +} + bool SavePNG(const std::string& path, const u8* input, ImageByteFormat format, u32 width, u32 height, int stride, int level) { @@ -75,6 +88,14 @@ bool SavePNG(const std::string& path, const u8* input, ImageByteFormat format, u std::vector buffer; buffer.reserve(byte_per_pixel * width * height); + std::vector warnings; + std::vector errors; + ErrorHandler error_handler; + error_handler.error_list = &errors; + error_handler.warning_list = &warnings; + error_handler.StoreError = ErrorCallback; + error_handler.StoreWarning = WarningCallback; + std::vector rows; rows.reserve(height); for (u32 row = 0; row < height; row++) @@ -82,7 +103,8 @@ bool SavePNG(const std::string& path, const u8* input, ImageByteFormat format, u rows.push_back(&input[row * stride]); } - png_structp png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, 0, 0, 0); + png_structp png_ptr = + png_create_write_struct(PNG_LIBPNG_VER_STRING, &error_handler, PngError, PngWarning); png_infop info_ptr = png_create_info_struct(png_ptr); bool success = false; @@ -103,6 +125,23 @@ bool SavePNG(const std::string& path, const u8* input, ImageByteFormat format, u timer.Stop(); INFO_LOG_FMT(FRAMEDUMP, "{} byte {} by {} image saved to {} at level {} in {}", buffer.size(), width, height, path, level, timer.GetTimeElapsedFormatted()); + ASSERT(errors.size() == 0); + if (warnings.size() != 0) + { + WARN_LOG_FMT(FRAMEDUMP, "Saved with {} warnings:", warnings.size()); + for (auto& warning : warnings) + WARN_LOG_FMT(FRAMEDUMP, "libpng warning: {}", warning); + } + } + else + { + ERROR_LOG_FMT(FRAMEDUMP, + "Failed to save {} by {} image to {} at level {}: {} warnings, {} errors", width, + height, path, level, warnings.size(), errors.size()); + for (auto& error : errors) + ERROR_LOG_FMT(FRAMEDUMP, "libpng error: {}", error); + for (auto& warning : warnings) + WARN_LOG_FMT(FRAMEDUMP, "libpng warning: {}", warning); } return success; diff --git a/Source/Core/Common/ImageC.c b/Source/Core/Common/ImageC.c index daeeca899a..b13f16938d 100644 --- a/Source/Core/Common/ImageC.c +++ b/Source/Core/Common/ImageC.c @@ -25,3 +25,19 @@ bool SavePNG0(png_structp png_ptr, png_infop info_ptr, int png_format, png_uint_ return true; } + +// pngerror.c says: "Note that the error function MUST NOT return to the calling routine or serious +// problems will occur. The return method used in the default routine calls +// longjmp(png_ptr->jmp_buf_ptr, 1)" +void PngError(png_structp png_ptr, png_const_charp msg) +{ + struct ErrorHandler* error_logger = (struct ErrorHandler*)png_get_error_ptr(png_ptr); + error_logger->StoreError(error_logger, msg); + png_longjmp(png_ptr, 1); +} + +void PngWarning(png_structp png_ptr, png_const_charp msg) +{ + struct ErrorHandler* error_logger = (struct ErrorHandler*)png_get_error_ptr(png_ptr); + error_logger->StoreWarning(error_logger, msg); +} diff --git a/Source/Core/Common/ImageC.h b/Source/Core/Common/ImageC.h index 9bca859214..bc350516f8 100644 --- a/Source/Core/Common/ImageC.h +++ b/Source/Core/Common/ImageC.h @@ -7,9 +7,23 @@ #include #ifdef __cplusplus -extern "C" +extern "C" { +#endif +struct ErrorHandler +{ + void* error_list; // std::vector* + void* warning_list; // std::vector* + void (*StoreError)(struct ErrorHandler* self, const char* msg); + void (*StoreWarning)(struct ErrorHandler* self, const char* msg); +}; + +bool SavePNG0(png_structp png_ptr, png_infop info_ptr, int png_format, png_uint_32 width, + png_uint_32 height, int level, png_voidp io_ptr, png_rw_ptr write_fn, + png_bytepp row_pointers); + +void PngError(png_structp png_ptr, png_const_charp msg); +void PngWarning(png_structp png_ptr, png_const_charp msg); + +#ifdef __cplusplus +} #endif - bool - SavePNG0(png_structp png_ptr, png_infop info_ptr, int png_format, png_uint_32 width, - png_uint_32 height, int level, png_voidp io_ptr, png_rw_ptr write_fn, - png_bytepp row_pointers); From 8cf841ecc7f53c20eeda80e5beb0f3430868d28e Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Wed, 24 Nov 2021 14:39:07 -0800 Subject: [PATCH 2/2] Fix saving RGBA images PNG_FORMAT_RGB and PNG_COLOR_TYPE_RGB both evaluate to 2, but PNG_FORMAT_RGBA evaluates to 3 while PNG_COLOR_TYPE_RGBA evaluates to 6; the bit indicating a palette is 1 while the bit indicating alpha is 4. --- Source/Core/Common/Image.cpp | 9 +++++---- Source/Core/Common/ImageC.c | 4 ++-- Source/Core/Common/ImageC.h | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Source/Core/Common/Image.cpp b/Source/Core/Common/Image.cpp index 87a2cb7e1f..54efcf5e6d 100644 --- a/Source/Core/Common/Image.cpp +++ b/Source/Core/Common/Image.cpp @@ -68,18 +68,19 @@ bool SavePNG(const std::string& path, const u8* input, ImageByteFormat format, u timer.Start(); size_t byte_per_pixel; - int png_format; + int color_type; switch (format) { case ImageByteFormat::RGB: - png_format = PNG_FORMAT_RGB; + color_type = PNG_COLOR_TYPE_RGB; byte_per_pixel = 3; break; case ImageByteFormat::RGBA: - png_format = PNG_FORMAT_RGBA; + color_type = PNG_COLOR_TYPE_RGBA; byte_per_pixel = 4; break; default: + ASSERT_MSG(FRAMEDUMP, false, "Invalid format %d", static_cast(format)); return false; } @@ -110,7 +111,7 @@ bool SavePNG(const std::string& path, const u8* input, ImageByteFormat format, u bool success = false; if (png_ptr != nullptr && info_ptr != nullptr) { - success = SavePNG0(png_ptr, info_ptr, png_format, width, height, level, &buffer, WriteCallback, + success = SavePNG0(png_ptr, info_ptr, color_type, width, height, level, &buffer, WriteCallback, const_cast(rows.data())); } png_destroy_write_struct(&png_ptr, &info_ptr); diff --git a/Source/Core/Common/ImageC.c b/Source/Core/Common/ImageC.c index b13f16938d..7316ae5c77 100644 --- a/Source/Core/Common/ImageC.c +++ b/Source/Core/Common/ImageC.c @@ -9,7 +9,7 @@ // The main purpose of this function is to allow specifying the compression level, which // png_image_write_to_memory does not allow. row_pointers is not modified by libpng, but also isn't // const for some reason. -bool SavePNG0(png_structp png_ptr, png_infop info_ptr, int png_format, png_uint_32 width, +bool SavePNG0(png_structp png_ptr, png_infop info_ptr, int color_type, png_uint_32 width, png_uint_32 height, int level, png_voidp io_ptr, png_rw_ptr write_fn, png_bytepp row_pointers) { @@ -17,7 +17,7 @@ bool SavePNG0(png_structp png_ptr, png_infop info_ptr, int png_format, png_uint_ return false; png_set_compression_level(png_ptr, level); - png_set_IHDR(png_ptr, info_ptr, width, height, 8, png_format, PNG_INTERLACE_NONE, + png_set_IHDR(png_ptr, info_ptr, width, height, 8, color_type, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT); png_set_rows(png_ptr, info_ptr, row_pointers); png_set_write_fn(png_ptr, io_ptr, write_fn, NULL); diff --git a/Source/Core/Common/ImageC.h b/Source/Core/Common/ImageC.h index bc350516f8..db9427d2f7 100644 --- a/Source/Core/Common/ImageC.h +++ b/Source/Core/Common/ImageC.h @@ -17,7 +17,7 @@ struct ErrorHandler void (*StoreWarning)(struct ErrorHandler* self, const char* msg); }; -bool SavePNG0(png_structp png_ptr, png_infop info_ptr, int png_format, png_uint_32 width, +bool SavePNG0(png_structp png_ptr, png_infop info_ptr, int color_type, png_uint_32 width, png_uint_32 height, int level, png_voidp io_ptr, png_rw_ptr write_fn, png_bytepp row_pointers);