bgfx: Refined configuration handling:

* Sort screen chains by none, default, then collation order (rather than
  whatever order the filesystem yields).
* Correctly persist settings across fullscreen toggle when explicit
  screen chains are configured.
* If chains are specified for a single window only, apply them to all
  windows.
* Treat empty string for screen chain as "default" rather than crashing.
* Changed default setting for bgfx_screen_chains to an empty string so
  chain selection will be saved/restored per system with mame.ini file
  created by -cc with no other settings.

src/osd/modules/lib/osdobj_common.cpp

@@ -173,14 +173,14 @@ const options_entry osd_options::s_option_entries[] =
 	{ nullptr,                                   nullptr,          core_options::option_type::HEADER,    "OSD EMULATED NETWORKING OPTIONS" },
 	{ OSDOPTION_NETWORK_PROVIDER,                OSDOPTVAL_AUTO,   core_options::option_type::STRING,    "Emulated networking provider: " },
 
-	{ nullptr,                                   nullptr,           core_options::option_type::HEADER, "BGFX POST-PROCESSING OPTIONS" },
+	{ nullptr,                                   nullptr,           core_options::option_type::HEADER,   "BGFX POST-PROCESSING OPTIONS" },
-	{ OSDOPTION_BGFX_PATH,                       "bgfx",            core_options::option_type::PATH,   "path to BGFX-related files" },
+	{ OSDOPTION_BGFX_PATH,                       "bgfx",            core_options::option_type::PATH,     "path to BGFX-related files" },
-	{ OSDOPTION_BGFX_BACKEND,                    "auto",            core_options::option_type::STRING, "BGFX backend to use (d3d9, d3d11, d3d12, metal, opengl, gles, vulkan)" },
+	{ OSDOPTION_BGFX_BACKEND,                    "auto",            core_options::option_type::STRING,   "BGFX backend to use (d3d9, d3d11, d3d12, metal, opengl, gles, vulkan)" },
-	{ OSDOPTION_BGFX_DEBUG,                      "0",               core_options::option_type::BOOLEAN, "enable BGFX debugging statistics" },
+	{ OSDOPTION_BGFX_DEBUG,                      "0",               core_options::option_type::BOOLEAN,  "enable BGFX debugging statistics" },
-	{ OSDOPTION_BGFX_SCREEN_CHAINS,              "default",         core_options::option_type::STRING, "comma-delimited list of screen chain JSON names, colon-delimited per-window" },
+	{ OSDOPTION_BGFX_SCREEN_CHAINS,              "",                core_options::option_type::STRING,   "comma-delimited list of screen chain JSON names, colon-delimited per-window" },
-	{ OSDOPTION_BGFX_SHADOW_MASK,                "slot-mask.png",   core_options::option_type::STRING, "shadow mask texture name" },
+	{ OSDOPTION_BGFX_SHADOW_MASK,                "slot-mask.png",   core_options::option_type::STRING,   "shadow mask texture name" },
-	{ OSDOPTION_BGFX_LUT,                        "lut-default.png", core_options::option_type::STRING, "LUT texture name" },
+	{ OSDOPTION_BGFX_LUT,                        "lut-default.png", core_options::option_type::STRING,   "LUT texture name" },
-	{ OSDOPTION_BGFX_AVI_NAME,                   OSDOPTVAL_AUTO,    core_options::option_type::PATH,   "filename for BGFX output logging" },
+	{ OSDOPTION_BGFX_AVI_NAME,                   OSDOPTVAL_AUTO,    core_options::option_type::PATH,     "filename for BGFX output logging" },
 
 	// End of list
 	{ nullptr }

src/osd/modules/render/bgfx/chainmanager.cpp

@@ -35,12 +35,14 @@
 #include "sliderdirtynotifier.h"
 
 #include "util/path.h"
+#include "util/unicode.h"
 #include "util/xmlfile.h"
 
 #include "osdcore.h"
 #include "osdfile.h"
 
 #include <algorithm>
+#include <locale>
 
 
 using namespace rapidjson;
@@ -119,10 +121,28 @@ void chain_manager::get_default_chain_info(std::string &out_chain_name, int32_t
 void chain_manager::refresh_available_chains()
 {
 	m_available_chains.clear();
-	m_available_chains.push_back(chain_desc("none", ""));
+	m_available_chains.emplace_back("none", "");
+
+	find_available_chains(util::path_concat(m_options.bgfx_path(), "chains"), "");
+	std::collate<wchar_t> const &coll = std::use_facet<std::collate<wchar_t> >(std::locale());
+	std::sort(
+			m_available_chains.begin(),
+			m_available_chains.end(),
+			[&coll] (chain_desc const &x, chain_desc const &y) -> bool
+			{
+				if (x.m_name == "none")
+					return y.m_name != "none";
+				else if (y.m_name == "none")
+					return false;
+				else if (x.m_name == "default")
+					return y.m_name != "default";
+				else if (y.m_name == "default")
+					return false;
+				std::wstring const xstr = wstring_from_utf8(x.m_name);
+				std::wstring const ystr = wstring_from_utf8(y.m_name);
+				return coll.compare(xstr.data(), xstr.data() + xstr.size(), ystr.data(), ystr.data() + ystr.size()) < 0;
+			});
 
-	const std::string chains_path  = util::string_format("%s" PATH_SEPARATOR "chains", m_options.bgfx_path());
-	find_available_chains(chains_path, "");
 	if (m_default_chain_index == -1)
 	{
 		for (size_t i = 0; i < m_available_chains.size(); i++)
@@ -159,42 +179,40 @@ void chain_manager::destroy_unloaded_chains()
 	}
 }
 
-void chain_manager::find_available_chains(std::string root, std::string path)
+void chain_manager::find_available_chains(std::string_view root, std::string_view path)
 {
-	osd::directory::ptr directory = osd::directory::open(root + path);
+	osd::directory::ptr directory = osd::directory::open(path.empty() ? std::string(root) : util::path_concat(root, path));
-	if (directory != nullptr)
+	if (directory)
 	{
-		for (const osd::directory::entry *entry = directory->read(); entry != nullptr; entry = directory->read())
+		for (const osd::directory::entry *entry = directory->read(); entry; entry = directory->read())
 		{
 			if (entry->type == osd::directory::entry::entry_type::FILE)
 			{
-				std::string name(entry->name);
+				const std::string_view name(entry->name);
-				std::string extension(".json");
+				const std::string_view extension(".json");
 
 				// Does the name has at least one character in addition to ".json"?
 				if (name.length() > extension.length())
 				{
 					size_t start = name.length() - extension.length();
-					std::string test_segment = name.substr(start, extension.length());
+					const std::string_view test_segment = name.substr(start, extension.length());
 
 					// Does it end in .json?
 					if (test_segment == extension)
 					{
-						m_available_chains.push_back(chain_desc(name.substr(0, start), path));
+						m_available_chains.emplace_back(std::string(name.substr(0, start)), std::string(path));
 					}
 				}
 			}
 			else if (entry->type == osd::directory::entry::entry_type::DIR)
 			{
-				std::string name = entry->name;
+				const std::string_view name = entry->name;
-				if (!(name == "." || name == ".."))
+				if ((name != ".") && (name != ".."))
 				{
-					std::string appended_path = path + "/" + name;
+					if (path.empty())
-					if (path.length() == 0)
+						find_available_chains(root, name);
-					{
+					else
-						appended_path = name;
+						find_available_chains(root, util::path_concat(path, name));
-					}
-					find_available_chains(root, path + "/" + name);
 				}
 			}
 		}
@@ -253,17 +271,17 @@ std::unique_ptr<bgfx_chain> chain_manager::load_chain(std::string name, uint32_t
 	return chain;
 }
 
-void chain_manager::parse_chain_selections(std::string chain_str)
+void chain_manager::parse_chain_selections(std::string_view chain_str)
 {
-	std::vector<std::string> chain_names = split_option_string(chain_str);
+	std::vector<std::string_view> chain_names = split_option_string(chain_str);
 
 	if (chain_names.empty())
 		chain_names.push_back("default");
 
 	while (m_current_chain.size() != chain_names.size())
 	{
-		m_screen_chains.push_back(nullptr);
+		m_screen_chains.emplace_back(nullptr);
-		m_chain_names.push_back("");
+		m_chain_names.emplace_back();
 		m_current_chain.push_back(CHAIN_NONE);
 	}
 
@@ -273,9 +291,7 @@ void chain_manager::parse_chain_selections(std::string chain_str)
 		for (chain_index = 0; chain_index < m_available_chains.size(); chain_index++)
 		{
 			if (m_available_chains[chain_index].m_name == chain_names[index])
-			{
 				break;
-			}
 		}
 
 		if (chain_index < m_available_chains.size())
@@ -291,24 +307,35 @@ void chain_manager::parse_chain_selections(std::string chain_str)
 	}
 }
 
-std::vector<std::string> chain_manager::split_option_string(std::string chain_str) const
+std::vector<std::string_view> chain_manager::split_option_string(std::string_view chain_str) const
 {
-	std::vector<std::string> chain_names;
+	std::vector<std::string_view> chain_names;
 
-	uint32_t length = chain_str.length();
+	const uint32_t length = chain_str.length();
 	uint32_t win = 0;
 	uint32_t last_start = 0;
-	for (uint32_t i = 0; i < length + 1; i++)
+	for (uint32_t i = 0; i <= length; i++)
 	{
-		if (i == length || chain_str[i] == ',' || chain_str[i] == ':')
+		if (i == length || (chain_str[i] == ',') || (chain_str[i] == ':'))
 		{
-			if (win == m_window_index)
+			if ((win == 0) || (win == m_window_index))
 			{
-				chain_names.push_back(chain_str.substr(last_start, i - last_start));
+				// treat an empty string as equivalent to "default"
+				if (i > last_start)
+					chain_names.push_back(chain_str.substr(last_start, i - last_start));
+				else
+					chain_names.push_back("default");
 			}
+
 			last_start = i + 1;
-			if (chain_str[i] == ':')
+			if ((i < length) && (chain_str[i] == ':'))
 			{
+				// no point walking the rest of the string if this was our window
+				if (win == m_window_index)
+					break;
+
+				// don't use first for all if more than one window is specified
+				chain_names.clear();
 				win++;
 			}
 		}
@@ -701,10 +728,18 @@ void chain_manager::restore_slider_settings(int32_t id, std::vector<std::vector<
 
 void chain_manager::load_config(util::xml::data_node const &windownode)
 {
-	bool const explicit_chains = OPTION_PRIORITY_NORMAL <= m_options.get_entry(OSDOPTION_BGFX_SCREEN_CHAINS)->priority();
+	bool const persist = windownode.get_attribute_int("persist", 1) != 0;
+	bool const default_chains = OPTION_PRIORITY_NORMAL > m_options.get_entry(OSDOPTION_BGFX_SCREEN_CHAINS)->priority();
+	bool const explicit_chains = !persist && !default_chains && *m_options.bgfx_screen_chains();
 
 	// if chains weren't explicitly specified, restore the chains from the config file
-	if (!explicit_chains)
+	if (explicit_chains)
+	{
+		osd_printf_verbose(
+				"BGFX: Ignoring chain selection from window 0 configuration due to explicitly specified chains\n",
+				m_window_index);
+	}
+	else
 	{
 		bool changed = false;
 		util::xml::data_node const *screennode = windownode.get_child("screen");

src/osd/modules/render/bgfx/chainmanager.h

@@ -22,6 +22,8 @@
 
 #include <map>
 #include <string>
+#include <string_view>
+#include <utility>
 #include <vector>
 
 
@@ -36,19 +38,6 @@ namespace ui { class menu_item; }
 class bgfx_chain;
 class bgfx_slider;
 
-class chain_desc
-{
-public:
-	chain_desc(std::string name, std::string path)
-		: m_name(name)
-		, m_path(path)
-	{
-	}
-
-	const std::string m_name;
-	const std::string m_path;
-};
-
 class chain_manager
 {
 public:
@@ -99,6 +88,24 @@ public:
 	void save_config(util::xml::data_node &parentnode);
 
 private:
+	class chain_desc
+	{
+	public:
+		chain_desc(const chain_desc &) = default;
+		chain_desc(chain_desc &&) = default;
+		chain_desc &operator=(const chain_desc &) = default;
+		chain_desc &operator=(chain_desc &&) = default;
+
+		chain_desc(std::string &&name, std::string &&path)
+			: m_name(std::move(name))
+			, m_path(std::move(path))
+		{
+		}
+
+		std::string m_name;
+		std::string m_path;
+	};
+
 	void load_chains();
 	void destroy_chains();
 	void reload_chains();
@@ -108,9 +115,9 @@ private:
 	void get_default_chain_info(std::string &out_chain_name, int32_t &out_chain_index);
 	void refresh_available_chains();
 	void destroy_unloaded_chains();
-	void find_available_chains(std::string root, std::string path);
+	void find_available_chains(std::string_view root, std::string_view path);
-	void parse_chain_selections(std::string chain_str);
+	void parse_chain_selections(std::string_view chain_str);
-	std::vector<std::string> split_option_string(std::string chain_str) const;
+	std::vector<std::string_view> split_option_string(std::string_view chain_str) const;
 
 	void update_screen_count(uint32_t screen_count);
 

src/osd/modules/render/drawbgfx.cpp

@@ -537,10 +537,10 @@ renderer_bgfx::renderer_bgfx(osd_window &window, parent_module &parent)
 renderer_bgfx::~renderer_bgfx()
 {
 	// persist settings across fullscreen toggle
-	if (m_chains)
+	if (m_config)
-		m_chains->save_config(m_module().persistent_settings());
-	else if (m_config)
 		m_config->get_first_child()->copy_into(m_module().persistent_settings());
+	else if (m_chains)
+		m_chains->save_config(m_module().persistent_settings());
 
 	bgfx::reset(0, 0, BGFX_RESET_NONE);
 
@@ -1620,6 +1620,7 @@ void renderer_bgfx::load_config(util::xml::data_node const &parentnode)
 		else
 			m_config->get_first_child()->delete_node();
 		windownode->copy_into(*m_config);
+		m_config->get_first_child()->set_attribute("persist", "0");
 		osd_printf_verbose("BGFX: Found configuration for window %d\n", window().index());
 		break;
 	}