Improve cellScreenshot (#9851)

* Fix screenshot logging

* Update libpng to 1.6.37

* cellScreenshot: Write text chunks
* cellScreenshot: add overlay image
* screenshot_manager: add /dev_hdd0/photo/

* read_png_file: use deleter instead of manual close

* cellScreenshot: use Qt for overlays
* cellScreenshot: don't apply overlay to regular img
* screenshot_manager: add mount hack for VFS
* cellScreenshot: escape the whole path

3rdparty/libpng

@@ -1 +1 @@
-Subproject commit eddf9023206dc40974c26f589ee2ad63a4227a1e
+Subproject commit a40189cf881e9f0db80511c382292a5604c3c3d1

rpcs3/Emu/Cell/Modules/cellScreenshot.cpp

@@ -26,10 +26,11 @@ void fmt_class_string<CellScreenShotError>::format(std::string& out, u64 arg)
 	});
 }
 
+shared_mutex screenshot_mtx;
 
 std::string screenshot_manager::get_overlay_path() const
 {
-	return vfs::get(overlay_dir_name + overlay_file_name);
+	return vfs::get(overlay_dir_name + "/" + overlay_file_name);
 }
 
 std::string screenshot_manager::get_photo_title() const
@@ -48,11 +49,23 @@ std::string screenshot_manager::get_game_title() const
 	return game;
 }
 
-std::string screenshot_manager::get_screenshot_path() const
+std::string screenshot_manager::get_game_comment() const
 {
-	// TODO: make sure the file can be saved, add suffix and increase counter if file exists
+	return game_comment;
-	// TODO: maybe find a proper home for these
+}
-	return fs::get_config_dir() + "/screenshots/cell/" + get_photo_title() + ".png";
+
+std::string screenshot_manager::get_screenshot_path(const std::string& date_path) const
+{
+	u32 counter = 0;
+	std::string path = vfs::get("/dev_hdd0/photo/" + date_path + "/" + get_photo_title());
+	std::string suffix = ".png";
+
+	while (!Emu.IsStopped() && fs::is_file(path + suffix))
+	{
+		suffix = fmt::format("_%d.png", ++counter);
+	}
+
+	return path + suffix;
 }
 
 
@@ -73,6 +86,7 @@ error_code cellScreenShotSetParameter(vm::cptr<CellScreenShotSetParam> param)
 		return CELL_SCREENSHOT_ERROR_PARAM;
 
 	const auto manager = g_fxo->get<screenshot_manager>();
+	std::lock_guard lock(screenshot_mtx);
 
 	if (param->photo_title && param->photo_title[0] != '\0')
 		manager->photo_title = std::string(param->photo_title.get_ptr());
@@ -110,6 +124,7 @@ error_code cellScreenShotSetOverlayImage(vm::cptr<char> srcDir, vm::cptr<char> s
 	}
 
 	const auto manager = g_fxo->get<screenshot_manager>();
+	std::lock_guard lock(screenshot_mtx);
 
 	manager->overlay_dir_name = std::string(srcDir.get_ptr());
 	manager->overlay_file_name = std::string(srcFile.get_ptr());
@@ -124,6 +139,8 @@ error_code cellScreenShotEnable()
 	cellScreenshot.warning("cellScreenShotEnable()");
 
 	const auto manager = g_fxo->get<screenshot_manager>();
+	std::lock_guard lock(screenshot_mtx);
+
 	manager->is_enabled = true;
 
 	return CELL_OK;
@@ -134,6 +151,8 @@ error_code cellScreenShotDisable()
 	cellScreenshot.warning("cellScreenShotDisable()");
 
 	const auto manager = g_fxo->get<screenshot_manager>();
+	std::lock_guard lock(screenshot_mtx);
+
 	manager->is_enabled = false;
 
 	return CELL_OK;

rpcs3/Emu/Cell/Modules/cellScreenshot.h

@@ -27,21 +27,24 @@ struct CellScreenShotSetParam
 	vm::bptr<void> reserved;
 };
 
+extern shared_mutex screenshot_mtx;
+
 struct screenshot_manager
 {
-	atomic_t<bool> is_enabled{ false };
+	bool is_enabled{false};
 
 	std::string photo_title;
 	std::string game_title;
 	std::string game_comment;
 
-	atomic_t<s32> overlay_offset_x{ 0 };
+	s32 overlay_offset_x{0};
-	atomic_t<s32> overlay_offset_y{ 0 };
+	s32 overlay_offset_y{0};
 	std::string overlay_dir_name;
 	std::string overlay_file_name;
 
 	std::string get_overlay_path() const;
 	std::string get_photo_title() const;
 	std::string get_game_title() const;
-	std::string get_screenshot_path() const;
+	std::string get_game_comment() const;
+	std::string get_screenshot_path(const std::string& date_path) const;
 };

rpcs3/Emu/System.cpp

@@ -240,6 +240,7 @@ void Emulator::Init(bool add_only)
 		make_path_verbose(dev_hdd0 + "disc/");
 		make_path_verbose(dev_hdd0 + "savedata/");
 		make_path_verbose(dev_hdd0 + "savedata/vmc/");
+		make_path_verbose(dev_hdd0 + "photo/");
 		make_path_verbose(dev_hdd1 + "caches/");
 	}
 

rpcs3/rpcs3qt/gs_frame.cpp

@@ -11,8 +11,11 @@
 #include "Emu/Cell/Modules/cellScreenshot.h"
 
 #include <QCoreApplication>
+#include <QDateTime>
 #include <QKeyEvent>
 #include <QMessageBox>
+#include <QPainter>
+
 #include <string>
 #include <thread>
 
@@ -40,7 +43,8 @@
 #include <QtDBus/QDBusConnection>
 #endif
 
-LOG_CHANNEL(screenshot);
+LOG_CHANNEL(screenshot_log, "SCREENSHOT");
+LOG_CHANNEL(mark_log, "MARK");
 
 extern atomic_t<bool> g_user_asked_for_frame_capture;
 
@@ -147,7 +151,7 @@ void gs_frame::keyPressEvent(QKeyEvent *keyEvent)
 		if (keyEvent->modifiers() == Qt::AltModifier)
 		{
 			static int count = 0;
-			screenshot.success("Made forced mark %d in log", ++count);
+			mark_log.success("Made forced mark %d in log", ++count);
 			return;
 		}
 		else if (keyEvent->modifiers() == Qt::ControlModifier)
@@ -442,16 +446,16 @@ void gs_frame::take_screenshot(const std::vector<u8> sshot_data, const u32 sshot
 
 			if (!fs::create_dir(screen_path) && fs::g_tls_error != fs::error::exist)
 			{
-				screenshot.error("Failed to create screenshot path \"%s\" : %s", screen_path, fs::g_tls_error);
+				screenshot_log.error("Failed to create screenshot path \"%s\" : %s", screen_path, fs::g_tls_error);
 				return;
 			}
 
-			std::string filename = screen_path + "screenshot-" + date_time::current_time_narrow<'_'>() + ".png";
+			const std::string filename = screen_path + "screenshot-" + date_time::current_time_narrow<'_'>() + ".png";
 
 			fs::file sshot_file(filename, fs::open_mode::create + fs::open_mode::write + fs::open_mode::excl);
 			if (!sshot_file)
 			{
-				screenshot.error("[Screenshot] Failed to save screenshot \"%s\" : %s", filename, fs::g_tls_error);
+				screenshot_log.error("Failed to save screenshot \"%s\" : %s", filename, fs::g_tls_error);
 				return;
 			}
 
@@ -474,69 +478,139 @@ void gs_frame::take_screenshot(const std::vector<u8> sshot_data, const u32 sshot
 				}
 			}
 
-			std::vector<u8> encoded_png;
+			screenshot_manager manager;
+			{
+				const auto fxo = g_fxo->get<screenshot_manager>();
+				std::lock_guard lock(screenshot_mtx);
+				manager = *fxo;
+			}
 
-			png_structp write_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, nullptr, nullptr, nullptr);
+			struct scoped_png_ptrs
-			png_infop info_ptr    = png_create_info_struct(write_ptr);
+			{
-			png_set_IHDR(write_ptr, info_ptr, sshot_width, sshot_height, 8, PNG_COLOR_TYPE_RGBA, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
+				png_structp write_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, nullptr, nullptr, nullptr);
+				png_infop info_ptr    = png_create_info_struct(write_ptr);
+
+				~scoped_png_ptrs()
+				{
+					png_free_data(write_ptr, info_ptr, PNG_FREE_ALL, -1);
+					png_destroy_write_struct(&write_ptr, &info_ptr);
+				}
+			};
+
+			png_text text[6] = {};
+			int num_text = 0;
+
+			const QDateTime date_time       = QDateTime::currentDateTime();
+			const std::string creation_time = date_time.toString("yyyy:MM:dd hh:mm:ss").toStdString();
+			const std::string title_id      = Emu.GetTitleID();
+			const std::string photo_title   = manager.get_photo_title();
+			const std::string game_title    = manager.get_game_title();
+			const std::string game_comment  = manager.get_game_comment();
+
+			// Write tEXt chunk
+			text[num_text].compression = PNG_TEXT_COMPRESSION_NONE;
+			text[num_text].key         = const_cast<char*>("Creation Time");
+			text[num_text].text        = const_cast<char*>(creation_time.c_str());
+			++num_text;
+			text[num_text].compression = PNG_TEXT_COMPRESSION_NONE;
+			text[num_text].key         = const_cast<char*>("Source");
+			text[num_text].text        = const_cast<char*>("RPCS3"); // Originally PlayStation(R)3
+			++num_text;
+			text[num_text].compression = PNG_TEXT_COMPRESSION_NONE;
+			text[num_text].key         = const_cast<char*>("Title ID");
+			text[num_text].text        = const_cast<char*>(title_id.c_str());
+			++num_text;
+
+			// Write tTXt chunk (they probably meant zTXt)
+			text[num_text].compression = PNG_TEXT_COMPRESSION_zTXt;
+			text[num_text].key         = const_cast<char*>("Title");
+			text[num_text].text        = const_cast<char*>(photo_title.c_str());
+			++num_text;
+			text[num_text].compression = PNG_TEXT_COMPRESSION_zTXt;
+			text[num_text].key         = const_cast<char*>("Game Title");
+			text[num_text].text        = const_cast<char*>(game_title.c_str());
+			++num_text;
+			text[num_text].compression = PNG_TEXT_COMPRESSION_zTXt;
+			text[num_text].key         = const_cast<char*>("Comment");
+			text[num_text].text        = const_cast<char*>(game_comment.c_str());
 
 			std::vector<u8*> rows(sshot_height);
 			for (usz y = 0; y < sshot_height; y++)
 				rows[y] = sshot_data_alpha.data() + y * sshot_width * 4;
 
-			png_set_rows(write_ptr, info_ptr, &rows[0]);
+			std::vector<u8> encoded_png;
-			png_set_write_fn(write_ptr, &encoded_png,
-				[](png_structp png_ptr, png_bytep data, png_size_t length)
-				{
-					std::vector<u8>* p = static_cast<std::vector<u8>*>(png_get_io_ptr(png_ptr));
-					p->insert(p->end(), data, data + length);
-				},
-				nullptr);
 
-			png_write_png(write_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, nullptr);
+			const auto write_png = [&]()
-
+			{
-			png_free_data(write_ptr, info_ptr, PNG_FREE_ALL, -1);
+				scoped_png_ptrs ptrs;
-			png_destroy_write_struct(&write_ptr, nullptr);
+				png_set_IHDR(ptrs.write_ptr, ptrs.info_ptr, sshot_width, sshot_height, 8, PNG_COLOR_TYPE_RGBA, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
+				png_set_text(ptrs.write_ptr, ptrs.info_ptr, text, 6);
+				png_set_rows(ptrs.write_ptr, ptrs.info_ptr, &rows[0]);
+				png_set_write_fn(ptrs.write_ptr, &encoded_png, [](png_structp png_ptr, png_bytep data, png_size_t length)
+					{
+						std::vector<u8>* p = static_cast<std::vector<u8>*>(png_get_io_ptr(png_ptr));
+						p->insert(p->end(), data, data + length);
+					}, nullptr);
+				png_write_png(ptrs.write_ptr, ptrs.info_ptr, PNG_TRANSFORM_IDENTITY, nullptr);
+			};
 
+			write_png();
 			sshot_file.write(encoded_png.data(), encoded_png.size());
 
-			screenshot.success("[Screenshot] Successfully saved screenshot to %s", filename);
+			screenshot_log.success("Successfully saved screenshot to %s", filename);
 
-			const auto fxo = g_fxo->get<screenshot_manager>();
+			if (manager.is_enabled)
-
-			if (fxo->is_enabled)
 			{
-				const std::string cell_sshot_filename = fxo->get_screenshot_path();
+				const std::string cell_sshot_overlay_path = manager.get_overlay_path();
+				if (fs::is_file(cell_sshot_overlay_path))
+				{
+					screenshot_log.notice("Adding overlay to cell screenshot from %s", cell_sshot_overlay_path);
+
+					QImage overlay_img;
+
+					if (!overlay_img.load(qstr(cell_sshot_overlay_path)))
+					{
+						screenshot_log.error("Failed to read cell screenshot overlay '%s' : %s", cell_sshot_overlay_path, fs::g_tls_error);
+					}
+					// TODO: the overlay and its offset need to be scaled based on image size, resolution scaling and video resolution
+					else if (manager.overlay_offset_x < static_cast<s64>(sshot_width)
+					      && manager.overlay_offset_y < static_cast<s64>(sshot_height)
+					      && manager.overlay_offset_x + overlay_img.width() > 0
+					      && manager.overlay_offset_y + overlay_img.height() > 0)
+					{
+						QImage screenshot_img(rows[0], sshot_width, sshot_height, QImage::Format_RGBA8888);
+						QPainter painter(&screenshot_img);
+						painter.drawImage(manager.overlay_offset_x, manager.overlay_offset_y, overlay_img);
+
+						std::memcpy(rows[0], screenshot_img.constBits(), static_cast<usz>(sshot_height) * screenshot_img.bytesPerLine());
+
+						screenshot_log.success("Applied screenshot overlay '%s'", cell_sshot_overlay_path);
+					}
+				}
+
+				const std::string cell_sshot_filename = manager.get_screenshot_path(date_time.toString("yyyy/MM/dd").toStdString());
 				const std::string cell_sshot_dir      = fs::get_parent_dir(cell_sshot_filename);
 
-				screenshot.notice("[Screenshot] Saving cell screenshot to %s", cell_sshot_filename);
+				screenshot_log.notice("Saving cell screenshot to %s", cell_sshot_filename);
 
 				if (!fs::create_path(cell_sshot_dir) && fs::g_tls_error != fs::error::exist)
 				{
-					screenshot.error("Failed to create cell screenshot dir \"%s\" : %s", cell_sshot_dir, fs::g_tls_error);
+					screenshot_log.error("Failed to create cell screenshot dir \"%s\" : %s", cell_sshot_dir, fs::g_tls_error);
 					return;
 				}
 
 				fs::file cell_sshot_file(cell_sshot_filename, fs::open_mode::create + fs::open_mode::write + fs::open_mode::excl);
 				if (!cell_sshot_file)
 				{
-					screenshot.error("[Screenshot] Failed to save cell screenshot \"%s\" : %s", cell_sshot_filename, fs::g_tls_error);
+					screenshot_log.error("Failed to save cell screenshot \"%s\" : %s", cell_sshot_filename, fs::g_tls_error);
 					return;
 				}
 
-				const std::string cell_sshot_overlay_path = fxo->get_overlay_path();
+				encoded_png.clear();
-				if (fs::is_file(cell_sshot_overlay_path))
+				write_png();
-				{
-					screenshot.notice("[Screenshot] Adding overlay to cell screenshot from %s", cell_sshot_overlay_path);
-					// TODO: add overlay to screenshot
-				}
-
-				// TODO: add tEXt chunk with creation time, source, title id
-				// TODO: add tTXt chunk with data procured from cellScreenShotSetParameter (get_photo_title, get_game_title, game_comment)
-
 				cell_sshot_file.write(encoded_png.data(), encoded_png.size());
 
-				screenshot.success("[Screenshot] Successfully saved cell screenshot to %s", cell_sshot_filename);
+				screenshot_log.success("Successfully saved cell screenshot to %s", cell_sshot_filename);
 			}
 
 			return;

rpcs3/rpcs3qt/screenshot_manager_dialog.cpp

@@ -2,6 +2,8 @@
 #include "screenshot_preview.h"
 #include "qt_utils.h"
 #include "Utilities/File.h"
+#include "Emu/VFS.h"
+#include "Emu/System.h"
 
 #include <QApplication>
 #include <QDir>
@@ -26,25 +28,33 @@ screenshot_manager_dialog::screenshot_manager_dialog(QWidget* parent) : QDialog(
 	m_grid->setIconSize(m_icon_size);
 	m_grid->setGridSize(m_icon_size + QSize(10, 10));
 
-	const std::string screen_path = fs::get_config_dir() + "screenshots/";
+	// HACK: dev_hdd0 must be mounted for vfs to work for loading trophies.
+	vfs::mount("/dev_hdd0", Emulator::GetHddDir());
+
+	const std::string screenshot_path_qt   = fs::get_config_dir() + "screenshots/";
+	const std::string screenshot_path_cell = vfs::get("/dev_hdd0/photo/");
 	const QStringList filter{ QStringLiteral("*.png") };
-	QDirIterator dir_iter(QString::fromStdString(screen_path), filter, QDir::Files | QDir::NoDotAndDotDot, QDirIterator::Subdirectories);
 
 	QPixmap placeholder(m_icon_size);
 	placeholder.fill(Qt::gray);
 	m_placeholder = QIcon(placeholder);
 
-	while (dir_iter.hasNext())
+	for (const std::string& path : { screenshot_path_qt, screenshot_path_cell })
 	{
-		const QString filepath = dir_iter.next();
+		QDirIterator dir_iter(QString::fromStdString(path), filter, QDir::Files | QDir::NoDotAndDotDot, QDirIterator::Subdirectories);
 
-		QListWidgetItem* item = new QListWidgetItem;
+		while (dir_iter.hasNext())
-		item->setData(item_role::source, filepath);
+		{
-		item->setData(item_role::loaded, false);
+			const QString filepath = dir_iter.next();
-		item->setIcon(m_placeholder);
-		item->setToolTip(filepath);
 
-		m_grid->addItem(item);
+			QListWidgetItem* item = new QListWidgetItem;
+			item->setData(item_role::source, filepath);
+			item->setData(item_role::loaded, false);
+			item->setIcon(m_placeholder);
+			item->setToolTip(filepath);
+
+			m_grid->addItem(item);
+		}
 	}
 
 	m_icon_loader = new QFutureWatcher<thumbnail>(this);