From 947ddf865fa9a1d85a75384fe3e4012e2aed7b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Fri, 12 Sep 2025 12:57:52 -0600 Subject: [PATCH] Extract some duplicated code into a pngSave function --- Common/Data/Format/PNGLoad.cpp | 35 +++++++++++++++++++++++ Common/Data/Format/PNGLoad.h | 3 ++ Common/Render/AtlasGen.cpp | 26 +---------------- Common/Render/AtlasGen.h | 1 - Core/Screenshot.cpp | 52 +++------------------------------- GPU/Common/TextureReplacer.cpp | 34 +++------------------- 6 files changed, 47 insertions(+), 104 deletions(-) diff --git a/Common/Data/Format/PNGLoad.cpp b/Common/Data/Format/PNGLoad.cpp index 83128f018e..51ebd5d9ac 100644 --- a/Common/Data/Format/PNGLoad.cpp +++ b/Common/Data/Format/PNGLoad.cpp @@ -5,6 +5,7 @@ #include "Common/Data/Format/PNGLoad.h" #include "Common/Log.h" +#include "Common/File/FileUtil.h" // *image_data_ptr should be deleted with free() // return value of 1 == success. @@ -131,3 +132,37 @@ bool PNGHeaderPeek::IsValidPNGHeader() const { } return true; } + +bool pngSave(const Path &filename, const void *buffer, int w, int h, int bytesPerPixel) { + png_image png{}; + png.version = PNG_IMAGE_VERSION; + png.format = bytesPerPixel == 3 ? PNG_FORMAT_RGB : PNG_FORMAT_RGBA; + png.width = w; + png.height = h; + const int row_stride = w * bytesPerPixel; + + FILE *fp = File::OpenCFile(filename, "wb"); + if (!fp) { + ERROR_LOG(Log::IO, "Unable to open png file for writing: %s", filename.c_str()); + return false; + } + + int result = png_image_write_to_stdio(&png, fp, 0, buffer, row_stride, nullptr); + + if (png.warning_or_error >= 2) { + ERROR_LOG(Log::IO, "Saving image to PNG produced errors."); + } + + png_image_free(&png); + fclose(fp); + + if (!result) { + // Should we even do this? + File::Delete(filename); + + ERROR_LOG(Log::IO, "PNG encode failed."); + return false; + } + + return true; +} diff --git a/Common/Data/Format/PNGLoad.h b/Common/Data/Format/PNGLoad.h index 76ab04ee9e..85057d2278 100644 --- a/Common/Data/Format/PNGLoad.h +++ b/Common/Data/Format/PNGLoad.h @@ -3,6 +3,7 @@ #include #include "Common/BitSet.h" +#include "Common/File/Path.h" // *image_data_ptr should be deleted with free() // return value of 1 == success. @@ -31,3 +32,5 @@ struct PNGHeaderPeek { int Width() const { return swap32(be_width); } int Height() const { return swap32(be_height); } }; + +bool pngSave(const Path &filename, const void *buffer, int w, int h, int bytesPerPixel); diff --git a/Common/Render/AtlasGen.cpp b/Common/Render/AtlasGen.cpp index 071fefd607..085fc01204 100644 --- a/Common/Render/AtlasGen.cpp +++ b/Common/Render/AtlasGen.cpp @@ -65,14 +65,6 @@ void Image::copyfrom(const Image &img, int ox, int oy, Effect effect) { } } -void Image::set(int sx, int sy, int ex, int ey, u32 fil) { - for (int y = sy; y < ey; y++) { - for (int x = sx; x < ex; x++) { - dat[y * w + x] = fil; - } - } -} - bool Image::LoadPNG(const char *png_name) { unsigned char *img_data; int w, h; @@ -89,23 +81,7 @@ bool Image::LoadPNG(const char *png_name) { } void Image::SavePNG(const char *png_name) { - // Save PNG - FILE *fil = fopen(png_name, "wb"); - png_structp png_ptr; - png_infop info_ptr; - png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL); - assert(png_ptr); - info_ptr = png_create_info_struct(png_ptr); - assert(info_ptr); - png_init_io(png_ptr, fil); - //png_set_compression_level(png_ptr, Z_BEST_COMPRESSION); - png_set_IHDR(png_ptr, info_ptr, w, h, 8, PNG_COLOR_TYPE_RGBA, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT); - png_write_info(png_ptr, info_ptr); - for (int y = 0; y < height(); y++) { - png_write_row(png_ptr, (png_byte*)(dat.data() + y * w)); - } - png_write_end(png_ptr, NULL); - png_destroy_write_struct(&png_ptr, &info_ptr); + pngSave(Path(png_name), dat.data(), w, h, 4); } void Image::SaveZIM(const char *zim_name, int zim_format) { diff --git a/Common/Render/AtlasGen.h b/Common/Render/AtlasGen.h index ce2db6503e..9fd7ef5736 100644 --- a/Common/Render/AtlasGen.h +++ b/Common/Render/AtlasGen.h @@ -68,7 +68,6 @@ struct Image { } u32 get1(int x, int y) const { return dat[y * w + x]; } void copyfrom(const Image &img, int ox, int oy, Effect effect); - void set(int sx, int sy, int ex, int ey, u32 fil); bool LoadPNG(const char *png_name); void SavePNG(const char *png_name); void SaveZIM(const char *zim_name, int zim_format); diff --git a/Core/Screenshot.cpp b/Core/Screenshot.cpp index f147b78357..2788c91516 100644 --- a/Core/Screenshot.cpp +++ b/Core/Screenshot.cpp @@ -25,6 +25,7 @@ #include "Common/Data/Convert/ColorConv.h" #include "Common/File/FileUtil.h" #include "Common/File/Path.h" +#include "Common/Data/Format/PNGLoad.h" #include "Common/Log.h" #include "Common/System/System.h" #include "Common/System/Display.h" @@ -38,8 +39,7 @@ // This is used to make non-ASCII paths work for filename. // Technically only needed on Windows. -class JPEGFileStream : public jpge::output_stream -{ +class JPEGFileStream : public jpge::output_stream { public: JPEGFileStream(const Path &filename) { fp_ = File::OpenCFile(filename, "wb"); @@ -101,25 +101,6 @@ static bool WriteScreenshotToJPEG(const Path &filename, int width, int height, i return dst_stream.Valid(); } -static bool WriteScreenshotToPNG(png_imagep image, const Path &filename, int convert_to_8bit, const void *buffer, png_int_32 row_stride, const void *colormap) { - FILE *fp = File::OpenCFile(filename, "wb"); - if (!fp) { - ERROR_LOG(Log::IO, "Unable to open screenshot file for writing."); - return false; - } - - if (png_image_write_to_stdio(image, fp, convert_to_8bit, buffer, row_stride, colormap)) { - fclose(fp); - return true; - } else { - ERROR_LOG(Log::IO, "Screenshot PNG encode failed."); - fclose(fp); - // Should we even do this? - File::Delete(filename); - return false; - } -} - static bool ConvertPixelTo8888RGBA(GPUDebugBufferFormat fmt, u8 &r, u8 &g, u8 &b, u8 &a, const void *buffer, int offset, bool rev) { const u8 *buf8 = (const u8 *)buffer; const u16 *buf16 = (const u16 *)buffer; @@ -383,20 +364,7 @@ ScreenshotResult TakeGameScreenshot(Draw::DrawContext *draw, const Path &filenam bool Save888RGBScreenshot(const Path &filename, ScreenshotFormat fmt, const u8 *bufferRGB888, int w, int h) { if (fmt == ScreenshotFormat::PNG) { - png_image png; - memset(&png, 0, sizeof(png)); - png.version = PNG_IMAGE_VERSION; - png.format = PNG_FORMAT_RGB; - png.width = w; - png.height = h; - bool success = WriteScreenshotToPNG(&png, filename, 0, bufferRGB888, w * 3, nullptr); - png_image_free(&png); - - if (png.warning_or_error >= 2) { - ERROR_LOG(Log::IO, "Saving screenshot to PNG produced errors."); - success = false; - } - return success; + return pngSave(filename, bufferRGB888, w, h, 3); } else if (fmt == ScreenshotFormat::JPG) { jpge::params params; params.m_quality = 90; @@ -407,19 +375,7 @@ bool Save888RGBScreenshot(const Path &filename, ScreenshotFormat fmt, const u8 * } bool Save8888RGBAScreenshot(const Path &filename, const u8 *buffer, int w, int h) { - png_image png{}; - png.version = PNG_IMAGE_VERSION; - png.format = PNG_FORMAT_RGBA; - png.width = w; - png.height = h; - bool success = WriteScreenshotToPNG(&png, filename, 0, buffer, w * 4, nullptr); - png_image_free(&png); - - if (png.warning_or_error >= 2) { - ERROR_LOG(Log::IO, "Saving screenshot to PNG produced errors."); - success = false; - } - return success; + return pngSave(filename, buffer, w, h, 4); } bool Save8888RGBAScreenshot(std::vector &bufferPNG, const u8 *bufferRGBA8888, int w, int h) { diff --git a/GPU/Common/TextureReplacer.cpp b/GPU/Common/TextureReplacer.cpp index b29feeb1e8..ddead376d3 100644 --- a/GPU/Common/TextureReplacer.cpp +++ b/GPU/Common/TextureReplacer.cpp @@ -25,6 +25,7 @@ #include "ext/xxhash.h" #include "Common/Data/Format/IniFile.h" +#include "Common/Data/Format/PNGLoad.h" #include "Common/Data/Text/I18n.h" #include "Common/Data/Text/Parsers.h" #include "Common/File/VFS/DirectoryReader.h" @@ -679,25 +680,6 @@ ReplacedTexture *TextureReplacer::FindReplacement(u64 cachekey, u32 hash, int w, return texture; } -static bool WriteTextureToPNG(png_imagep image, const Path &filename, int convert_to_8bit, const void *buffer, png_int_32 row_stride, const void *colormap) { - FILE *fp = File::OpenCFile(filename, "wb"); - if (!fp) { - ERROR_LOG(Log::TexReplacement, "Save texture: Unable to open texture file '%s' for writing.", filename.c_str()); - return false; - } - - if (png_image_write_to_stdio(image, fp, convert_to_8bit, buffer, row_stride, colormap)) { - fclose(fp); - return true; - } else { - // This shouldn't really happen. - ERROR_LOG(Log::TexReplacement, "Texture PNG encode failed."); - fclose(fp); - remove(filename.c_str()); - return false; - } -} - // We save textures on threadpool tasks since it's a fire-and-forget task, and both I/O and png compression // can be pretty slow. class SaveTextureTask : public Task { @@ -747,19 +729,11 @@ public: // going to write to to .png. saveFilename = saveFilename.WithReplacedExtension(".png"); - png_image png{}; - png.version = PNG_IMAGE_VERSION; - png.format = PNG_FORMAT_RGBA; - png.width = w; - png.height = h; - bool success = WriteTextureToPNG(&png, saveFilename, 0, rgbaData, w * 4, nullptr); - png_image_free(&png); - if (png.warning_or_error >= 2) { + bool success = pngSave(saveFilename, rgbaData, w, h, 4); + if (!success) { ERROR_LOG(Log::TexReplacement, "Saving texture to PNG produced errors."); - } else if (success) { - NOTICE_LOG(Log::TexReplacement, "Saving texture for replacement: %08x / %dx%d in '%s'", replacedInfoHash, w, h, saveFilename.ToVisualString().c_str()); } else { - ERROR_LOG(Log::TexReplacement, "Failed to write '%s'", saveFilename.c_str()); + NOTICE_LOG(Log::TexReplacement, "Saving texture for replacement: %08x / %dx%d in '%s'", replacedInfoHash, w, h, saveFilename.ToVisualString().c_str()); } } };