From f408dd83e27fb106ed60b66eafebc4015040bf26 Mon Sep 17 00:00:00 2001 From: Vas Crabb Date: Thu, 2 Feb 2023 03:23:36 +1100 Subject: osd: Fixed BGFX crash on toggling fullscreen on Linux. Also fixed draw13.cpp upsetting older versions of clang. --- src/osd/mac/video.cpp | 3 + src/osd/modules/lib/osdobj_common.cpp | 4 +- src/osd/modules/render/draw13.cpp | 14 ++- src/osd/modules/render/drawbgfx.cpp | 180 ++++++++++++++++++++++++---------- src/osd/modules/render/drawbgfx.h | 24 +++-- src/osd/sdl/osdsdl.cpp | 3 + 6 files changed, 155 insertions(+), 73 deletions(-) diff --git a/src/osd/mac/video.cpp b/src/osd/mac/video.cpp index 4ae04c4e132..5fc8111f3db 100644 --- a/src/osd/mac/video.cpp +++ b/src/osd/mac/video.cpp @@ -137,6 +137,9 @@ static void check_osd_inputs(running_machine &machine) // check for toggling fullscreen mode if (machine.ui_input().pressed(IPT_OSD_1)) { + // destroy the renderers first so that the render module can bounce if it depends on having a window handle + for (auto it = osd_common_t::window_list().rbegin(); osd_common_t::window_list().rend() != it; ++it) + (*it)->renderer_reset(); for (auto const &curwin : osd_common_t::window_list()) dynamic_cast(*curwin).toggle_full_screen(); } diff --git a/src/osd/modules/lib/osdobj_common.cpp b/src/osd/modules/lib/osdobj_common.cpp index 92eacddc739..0cc2f6a3b61 100644 --- a/src/osd/modules/lib/osdobj_common.cpp +++ b/src/osd/modules/lib/osdobj_common.cpp @@ -691,8 +691,8 @@ void osd_common_t::window_exit() void osd_common_t::osd_exit() { // destroy the renderers before shutting down their parent module - for (auto const &window : s_window_list) - window->renderer_reset(); + for (auto it = window_list().rbegin(); window_list().rend() != it; ++it) + (*it)->renderer_reset(); m_mod_man.exit(); diff --git a/src/osd/modules/render/draw13.cpp b/src/osd/modules/render/draw13.cpp index 9b426c6c4f2..aea9b3f2b3f 100644 --- a/src/osd/modules/render/draw13.cpp +++ b/src/osd/modules/render/draw13.cpp @@ -149,7 +149,7 @@ struct copy_info_t mutable int samples; mutable int perf; /* list */ - copy_info_t *next; + copy_info_t const *next; }; /* renderer_sdl2 is the information about SDL for the current screen */ @@ -990,9 +990,9 @@ private: void expand_copy_info(); void free_copy_info(); - static void add_list(copy_info_t *&head, copy_info_t const &element, Uint32 bm); + static void add_list(copy_info_t const *&head, copy_info_t const &element, Uint32 bm); - copy_info_t *m_blit_info[SDL_TEXFORMAT_LAST + 1]; + copy_info_t const *m_blit_info[SDL_TEXFORMAT_LAST + 1]; bool m_blit_info_initialized; bool m_gllib_loaded; @@ -1065,7 +1065,7 @@ void video_sdl2::free_copy_info() { for (int i = 0; i <= SDL_TEXFORMAT_LAST; i++) { - for (copy_info_t *bi = m_blit_info[i]; bi != nullptr; ) + for (copy_info_t const *bi = m_blit_info[i]; bi != nullptr; ) { if (bi->pixel_count) { @@ -1078,9 +1078,7 @@ void video_sdl2::free_copy_info() bi->samples, bi->perf); } - copy_info_t *freeme = bi; - bi = bi->next; - delete freeme; + delete std::exchange(bi, bi->next); } m_blit_info[i] = nullptr; } @@ -1088,7 +1086,7 @@ void video_sdl2::free_copy_info() } } -void video_sdl2::add_list(copy_info_t *&head, copy_info_t const &element, Uint32 bm) +void video_sdl2::add_list(copy_info_t const *&head, copy_info_t const &element, Uint32 bm) { copy_info_t *const newci = new copy_info_t(element); diff --git a/src/osd/modules/render/drawbgfx.cpp b/src/osd/modules/render/drawbgfx.cpp index 3edc3cfd465..f212ad46b9e 100644 --- a/src/osd/modules/render/drawbgfx.cpp +++ b/src/osd/modules/render/drawbgfx.cpp @@ -64,6 +64,87 @@ extern void *GetOSWindow(void *wincontroller); #include +//============================================================ +// Renderer interface to parent module +//============================================================ + +class renderer_bgfx::parent_module +{ +public: + util::xml::data_node &persistent_settings() { return *m_persistent_settings; } + osd_options const &options() const { return *m_options; } + uint32_t max_texture_size() const { return m_max_texture_size; } + + template + util::notifier_subscription subscribe_load(void (T::*func)(util::xml::data_node const &), T *obj) + { + return m_load_notifier.subscribe(delegate(func, obj)); + } + + template + util::notifier_subscription subscribe_save(void (T::*func)(util::xml::data_node &), T *obj) + { + return m_save_notifier.subscribe(delegate(func, obj)); + } + +protected: + parent_module() + : m_options(nullptr) + , m_max_texture_size(0) + , m_renderer_count(0) + { + } + virtual ~parent_module() + { + assert(!m_renderer_count); + } + + bool has_active_renderers() const + { + return 0 < m_renderer_count; + } + + util::notifier m_load_notifier; + util::notifier m_save_notifier; + util::xml::file::ptr m_persistent_settings; + osd_options const *m_options; + uint32_t m_max_texture_size; + +private: + friend class parent_module_holder; + + virtual void last_renderer_destroyed() = 0; + + void renderer_created() + { + ++m_renderer_count; + } + + void renderer_destroyed() + { + assert(m_renderer_count); + if (!--m_renderer_count) + last_renderer_destroyed(); + } + + unsigned m_renderer_count; +}; + + +inline renderer_bgfx::parent_module_holder::parent_module_holder(parent_module &parent) + : m_parent(parent) +{ + m_parent.renderer_created(); +} + + +inline renderer_bgfx::parent_module_holder::~parent_module_holder() +{ + m_parent.renderer_destroyed(); +} + + + //============================================================ // OSD MODULE //============================================================ @@ -72,13 +153,11 @@ namespace osd { namespace { -class video_bgfx : public osd_module, public render_module +class video_bgfx : public osd_module, public render_module, protected renderer_bgfx::parent_module { public: video_bgfx() : osd_module(OSD_RENDERER_PROVIDER, "bgfx") - , m_options(nullptr) - , m_max_texture_size(0) , m_bgfx_library_initialized(false) { } @@ -93,6 +172,8 @@ protected: virtual unsigned flags() const override { return FLAG_INTERACTIVE; } private: + virtual void last_renderer_destroyed() override; + void load_config(config_type cfg_type, config_level cfg_level, util::xml::data_node const *parentnode); void save_config(config_type cfg_type, util::xml::data_node *parentnode); @@ -100,11 +181,6 @@ private: static bool set_platform_data(bgfx::PlatformData &platform_data, osd_window const &window); - util::notifier m_load_notifier; - util::notifier m_save_notifier; - util::xml::file::ptr m_persistent_settings; - osd_options const *m_options; - uint32_t m_max_texture_size; bool m_bgfx_library_initialized; }; @@ -165,6 +241,8 @@ int video_bgfx::init(osd_interface &osd, osd_options const &options) void video_bgfx::exit() { + assert(!has_active_renderers()); + if (m_bgfx_library_initialized) { osd_printf_verbose("Shutting down BGFX library\n"); @@ -184,22 +262,13 @@ void video_bgfx::exit() std::unique_ptr video_bgfx::create(osd_window &window) { - // hacky - reinitialise when window 0 is created - // this is the cause of the fullscreen toggle crash on Linux - if ((window.index() == 0) && m_bgfx_library_initialized) - { - osd_printf_verbose("Shutting down BGFX library to recreate window 0\n"); - imguiDestroy(); - bgfx::shutdown(); - m_bgfx_library_initialized = false; - m_max_texture_size = 0; - } - // start BGFX if this is the first window if (!m_bgfx_library_initialized) { - osd_printf_verbose("Initializing BGFX library\n"); assert(window.index() == 0); // bad things will happen otherwise + assert(!has_active_renderers()); + + osd_printf_verbose("Initializing BGFX library\n"); if (!init_bgfx_library(window)) { osd_printf_error("BGFX library initialization failed\n"); @@ -208,13 +277,24 @@ std::unique_ptr video_bgfx::create(osd_window &window) m_bgfx_library_initialized = true; } - return std::make_unique( - window, - *m_options, - m_load_notifier, - m_save_notifier, - *m_persistent_settings, - m_max_texture_size); + return std::make_unique(window, static_cast(*this)); +} + + +//============================================================ +// video_bgfx::last_renderer_destroyed +//============================================================ + +void video_bgfx::last_renderer_destroyed() +{ + if (m_bgfx_library_initialized) + { + osd_printf_verbose("No more renderers - shutting down BGFX library\n"); + imguiDestroy(); + bgfx::shutdown(); + m_bgfx_library_initialized = false; + m_max_texture_size = 0; + } } @@ -413,16 +493,9 @@ static void *sdlNativeWindowHandle(SDL_Window *window) // renderer_bgfx - constructor //============================================================ -renderer_bgfx::renderer_bgfx( - osd_window &window, - const osd_options &options, - util::notifier &load_notifier, - util::notifier &save_notifier, - util::xml::data_node &persistent_settings, - uint32_t max_texsize) +renderer_bgfx::renderer_bgfx(osd_window &window, parent_module &parent) : osd_renderer(window) - , m_options(options) - , m_max_texture_size(max_texsize) + , m_module(parent) , m_framebuffer(nullptr) , m_texture_cache(nullptr) , m_dimensions(0, 0) @@ -432,12 +505,11 @@ renderer_bgfx::renderer_bgfx( , m_avi_view(nullptr) , m_avi_writer(nullptr) , m_avi_target(nullptr) - , m_load_sub(load_notifier.subscribe(delegate(&renderer_bgfx::load_config, this))) - , m_save_sub(save_notifier.subscribe(delegate(&renderer_bgfx::save_config, this))) - , m_persistent_settings(persistent_settings) + , m_load_sub(parent.subscribe_load(&renderer_bgfx::load_config, this)) + , m_save_sub(parent.subscribe_save(&renderer_bgfx::save_config, this)) { // load settings if recreated after fullscreen toggle - util::xml::data_node *windownode = m_persistent_settings.get_child("window"); + util::xml::data_node *windownode = m_module().persistent_settings().get_child("window"); while (windownode) { if (windownode->get_attribute_int("index", -1) != window.index()) @@ -466,9 +538,9 @@ renderer_bgfx::~renderer_bgfx() { // persist settings across fullscreen toggle if (m_chains) - m_chains->save_config(m_persistent_settings); + m_chains->save_config(m_module().persistent_settings()); else if (m_config) - m_config->get_first_child()->copy_into(m_persistent_settings); + m_config->get_first_child()->copy_into(m_module().persistent_settings()); bgfx::reset(0, 0, BGFX_RESET_NONE); @@ -508,15 +580,15 @@ int renderer_bgfx::create() m_effects = std::make_unique(*m_shaders); // Create program from shaders. - m_gui_effect[0] = m_effects->get_or_load_effect(m_options, "gui_opaque"); - m_gui_effect[1] = m_effects->get_or_load_effect(m_options, "gui_blend"); - m_gui_effect[2] = m_effects->get_or_load_effect(m_options, "gui_multiply"); - m_gui_effect[3] = m_effects->get_or_load_effect(m_options, "gui_add"); + m_gui_effect[0] = m_effects->get_or_load_effect(m_module().options(), "gui_opaque"); + m_gui_effect[1] = m_effects->get_or_load_effect(m_module().options(), "gui_blend"); + m_gui_effect[2] = m_effects->get_or_load_effect(m_module().options(), "gui_multiply"); + m_gui_effect[3] = m_effects->get_or_load_effect(m_module().options(), "gui_add"); - m_screen_effect[0] = m_effects->get_or_load_effect(m_options, "screen_opaque"); - m_screen_effect[1] = m_effects->get_or_load_effect(m_options, "screen_blend"); - m_screen_effect[2] = m_effects->get_or_load_effect(m_options, "screen_multiply"); - m_screen_effect[3] = m_effects->get_or_load_effect(m_options, "screen_add"); + m_screen_effect[0] = m_effects->get_or_load_effect(m_module().options(), "screen_opaque"); + m_screen_effect[1] = m_effects->get_or_load_effect(m_module().options(), "screen_blend"); + m_screen_effect[2] = m_effects->get_or_load_effect(m_module().options(), "screen_multiply"); + m_screen_effect[3] = m_effects->get_or_load_effect(m_module().options(), "screen_add"); if (window().index() != 0) { @@ -533,10 +605,10 @@ int renderer_bgfx::create() m_ortho_view->set_backbuffer(m_framebuffer); } - const uint32_t max_prescale_size = std::min(2u * std::max(wdim.width(), wdim.height()), m_max_texture_size); + const uint32_t max_prescale_size = std::min(2u * std::max(wdim.width(), wdim.height()), m_module().max_texture_size()); m_chains = std::make_unique( window().machine(), - m_options, + m_module().options(), *m_textures, *m_targets, *m_effects, @@ -584,7 +656,7 @@ void renderer_bgfx::record() } else { - m_avi_writer->record(m_options.bgfx_avi_name()); + m_avi_writer->record(m_module().options().bgfx_avi_name()); m_avi_target = m_targets->create_target("avibuffer", bgfx::TextureFormat::BGRA8, s_width[0], s_height[0], 1, 1, TARGET_STYLE_CUSTOM, false, true, 1, 0); m_avi_texture = bgfx::createTexture2D(s_width[0], s_height[0], false, 1, bgfx::TextureFormat::BGRA8, BGFX_TEXTURE_BLIT_DST | BGFX_TEXTURE_READ_BACK); @@ -1061,6 +1133,7 @@ int renderer_bgfx::draw(int update) // screen). if (m_config) { + osd_printf_verbose("BGFX: Applying configuration for window %d\n", window().index()); m_chains->load_config(*m_config->get_first_child()); m_config.reset(); } @@ -1547,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); + osd_printf_verbose("BGFX: Found configuration for window %d\n", window().index()); break; } } diff --git a/src/osd/modules/render/drawbgfx.h b/src/osd/modules/render/drawbgfx.h index 14e076843a7..0ea3504964c 100644 --- a/src/osd/modules/render/drawbgfx.h +++ b/src/osd/modules/render/drawbgfx.h @@ -37,13 +37,9 @@ class avi_write; class renderer_bgfx : public osd_renderer, public slider_dirty_notifier { public: - renderer_bgfx( - osd_window &window, - const osd_options &options, - util::notifier &load_notifier, - util::notifier &save_notifier, - util::xml::data_node &persistent_settings, - uint32_t max_texsize); + class parent_module; + + renderer_bgfx(osd_window &window, parent_module &parent_module); virtual ~renderer_bgfx(); virtual int create() override; @@ -78,6 +74,16 @@ private: BUFFER_DONE }; + class parent_module_holder + { + public: + parent_module_holder(parent_module &parent); + ~parent_module_holder(); + parent_module &operator()() const { return m_parent; } + private: + parent_module &m_parent; + }; + void vertex(ScreenVertex* vertex, float x, float y, float z, uint32_t rgba, float u, float v); void render_avi_quad(); void update_recording(); @@ -109,8 +115,7 @@ private: void load_config(util::xml::data_node const &parentnode); void save_config(util::xml::data_node &parentnode); - const osd_options& m_options; - const uint32_t m_max_texture_size; + parent_module_holder m_module; // keep this where it will be destructed last bgfx_target *m_framebuffer; bgfx_texture *m_texture_cache; @@ -148,7 +153,6 @@ private: std::unique_ptr m_config; const util::notifier_subscription m_load_sub; const util::notifier_subscription m_save_sub; - util::xml::data_node &m_persistent_settings; static const uint16_t CACHE_SIZE; static const uint32_t PACKABLE_SIZE; diff --git a/src/osd/sdl/osdsdl.cpp b/src/osd/sdl/osdsdl.cpp index 633abff3a3d..260fd74891a 100644 --- a/src/osd/sdl/osdsdl.cpp +++ b/src/osd/sdl/osdsdl.cpp @@ -703,6 +703,9 @@ void sdl_osd_interface::check_osd_inputs() // check for toggling fullscreen mode if (machine().ui_input().pressed(IPT_OSD_1)) { + // destroy the renderers first so that the render module can bounce if it depends on having a window handle + for (auto it = osd_common_t::window_list().rbegin(); osd_common_t::window_list().rend() != it; ++it) + (*it)->renderer_reset(); for (auto const &curwin : osd_common_t::window_list()) dynamic_cast(*curwin).toggle_full_screen(); } -- cgit v1.2.3