From 917ef26b39e5560482d5b3e59339fe6e492a3f09 Mon Sep 17 00:00:00 2001 From: marton Date: Sat, 27 Jun 2026 19:36:09 -0400 Subject: [PATCH 1/3] emplace_back fix; display_device interface factory --- src/common/factory.cpp | 20 ++++++++++ src/common/include/display_device/factory.h | 41 +++++++++++++++++++++ src/macos/factory.cpp | 31 ++++++++++++++++ src/macos/mac_display_device_general.cpp | 2 +- src/windows/factory.cpp | 33 +++++++++++++++++ src/windows/win_display_device_general.cpp | 4 +- tests/unit/general/test_factory.cpp | 39 ++++++++++++++++++++ 7 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 src/common/factory.cpp create mode 100644 src/common/include/display_device/factory.h create mode 100644 src/macos/factory.cpp create mode 100644 src/windows/factory.cpp create mode 100644 tests/unit/general/test_factory.cpp diff --git a/src/common/factory.cpp b/src/common/factory.cpp new file mode 100644 index 0000000..aaa004e --- /dev/null +++ b/src/common/factory.cpp @@ -0,0 +1,20 @@ +/** + * @file src/common/factory.cpp + * @brief Factory helpers for unsupported platforms. + */ +// local includes +#include "display_device/factory.h" + +#if !defined(_WIN32) && !defined(__APPLE__) + +namespace display_device { + std::unique_ptr makeSettingsManager(SettingsManagerFactoryConfig) { + return nullptr; + } + + std::unique_ptr makeDisplayPower() { + return nullptr; + } +} // namespace display_device + +#endif diff --git a/src/common/include/display_device/factory.h b/src/common/include/display_device/factory.h new file mode 100644 index 0000000..aeaf6e0 --- /dev/null +++ b/src/common/include/display_device/factory.h @@ -0,0 +1,41 @@ +/** + * @file src/common/include/display_device/factory.h + * @brief Factory helpers for platform display device interfaces. + */ +#pragma once + +// system includes +#include +#include +#include + +// local includes +#include "audio_context_interface.h" +#include "display_power_interface.h" +#include "settings_manager_interface.h" +#include "settings_persistence_interface.h" + +namespace display_device { + /** + * @brief Dependencies and platform-neutral options for creating a settings manager. + */ + struct SettingsManagerFactoryConfig { + std::shared_ptr m_audio_context_api {}; ///< Optional audio context interface. + std::shared_ptr m_settings_persistence_api {}; ///< Optional settings persistence interface. + bool m_throw_on_persistence_load_error {}; ///< Throw when persisted settings cannot be loaded or parsed. + std::optional m_hdr_blank_delay {}; ///< Optional HDR blanking workaround delay on supported platforms. + }; + + /** + * @brief Create the default settings manager for the current platform. + * @param config Dependencies and platform-neutral options. + * @returns A settings manager, or nullptr when the platform is unsupported. + */ + [[nodiscard]] std::unique_ptr makeSettingsManager(SettingsManagerFactoryConfig config = {}); + + /** + * @brief Create the default display power manager for the current platform. + * @returns A display power manager, or nullptr when the platform is unsupported. + */ + [[nodiscard]] std::unique_ptr makeDisplayPower(); +} // namespace display_device diff --git a/src/macos/factory.cpp b/src/macos/factory.cpp new file mode 100644 index 0000000..ca18308 --- /dev/null +++ b/src/macos/factory.cpp @@ -0,0 +1,31 @@ +/** + * @file src/macos/factory.cpp + * @brief Factory helpers for macOS platform interfaces. + */ +// class header include +#include "display_device/factory.h" + +// system includes +#include + +// local includes +#include "display_device/macos/display_power.h" +#include "display_device/macos/mac_api_layer.h" +#include "display_device/macos/mac_display_device.h" +#include "display_device/macos/settings_manager.h" + +namespace display_device { + std::unique_ptr makeSettingsManager(SettingsManagerFactoryConfig config) { + auto api_layer {std::make_shared()}; + return std::make_unique( + std::make_shared(api_layer), + std::move(config.m_audio_context_api), + std::make_unique(std::move(config.m_settings_persistence_api), config.m_throw_on_persistence_load_error), + MacWorkarounds {} + ); + } + + std::unique_ptr makeDisplayPower() { + return std::make_unique(std::make_shared()); + } +} // namespace display_device diff --git a/src/macos/mac_display_device_general.cpp b/src/macos/mac_display_device_general.cpp index 03d27dc..94a5a8f 100644 --- a/src/macos/mac_display_device_general.cpp +++ b/src/macos/mac_display_device_general.cpp @@ -57,7 +57,7 @@ namespace display_device { } } - devices.emplace_back(device_id, display_name, friendly_name, edid, info); + devices.emplace_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); } return devices; diff --git a/src/windows/factory.cpp b/src/windows/factory.cpp new file mode 100644 index 0000000..495f322 --- /dev/null +++ b/src/windows/factory.cpp @@ -0,0 +1,33 @@ +/** + * @file src/windows/factory.cpp + * @brief Factory helpers for Windows platform interfaces. + */ +// class header include +#include "display_device/factory.h" + +// system includes +#include + +// local includes +#include "display_device/windows/display_power.h" +#include "display_device/windows/settings_manager.h" +#include "display_device/windows/win_api_layer.h" +#include "display_device/windows/win_display_device.h" + +namespace display_device { + std::unique_ptr makeSettingsManager(SettingsManagerFactoryConfig config) { + auto api_layer {std::make_shared()}; + return std::make_unique( + std::make_shared(api_layer), + std::move(config.m_audio_context_api), + std::make_unique(std::move(config.m_settings_persistence_api), config.m_throw_on_persistence_load_error), + WinWorkarounds { + .m_hdr_blank_delay = config.m_hdr_blank_delay + } + ); + } + + std::unique_ptr makeDisplayPower() { + return std::make_unique(std::make_shared()); + } +} // namespace display_device diff --git a/src/windows/win_display_device_general.cpp b/src/windows/win_display_device_general.cpp index f4f25fd..bab494a 100644 --- a/src/windows/win_display_device_general.cpp +++ b/src/windows/win_display_device_general.cpp @@ -69,9 +69,9 @@ namespace display_device { m_w_api->getHdrState(best_path) }; - available_devices.emplace_back(device_id, display_name, friendly_name, edid, info); + available_devices.emplace_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); } else { - available_devices.emplace_back(device_id, display_name, friendly_name, edid, std::nullopt); + available_devices.emplace_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, std::nullopt}); } } diff --git a/tests/unit/general/test_factory.cpp b/tests/unit/general/test_factory.cpp new file mode 100644 index 0000000..b5484bb --- /dev/null +++ b/tests/unit/general/test_factory.cpp @@ -0,0 +1,39 @@ +// system includes +#include +#include + +// local includes +#include "display_device/factory.h" +#include "fixtures/fixtures.h" + +namespace { + using namespace std::chrono_literals; + + // Specialized TEST macro(s) for this test file +#define TEST_S(...) DD_MAKE_TEST(TEST, Factory, __VA_ARGS__) +} // namespace + +TEST_S(MakeSettingsManager) { + display_device::SettingsManagerFactoryConfig config { + .m_hdr_blank_delay = 10ms + }; + + const auto settings_manager {display_device::makeSettingsManager(std::move(config))}; + +#if defined(_WIN32) || defined(__APPLE__) + ASSERT_NE(settings_manager, nullptr); + EXPECT_TRUE(settings_manager->resetPersistence()); +#else + EXPECT_EQ(settings_manager, nullptr); +#endif +} + +TEST_S(MakeDisplayPower) { + const auto display_power {display_device::makeDisplayPower()}; + +#if defined(_WIN32) || defined(__APPLE__) + EXPECT_NE(display_power, nullptr); +#else + EXPECT_EQ(display_power, nullptr); +#endif +} From 022dbf23c9aeb2a7e14b5e523e760b9488fd2596 Mon Sep 17 00:00:00 2001 From: marton Date: Sat, 27 Jun 2026 19:54:38 -0400 Subject: [PATCH 2/3] fix sonar findings --- src/common/factory.cpp | 2 +- src/common/include/display_device/factory.h | 2 +- src/macos/factory.cpp | 9 +++------ src/macos/mac_display_device_general.cpp | 2 +- src/windows/factory.cpp | 9 +++------ src/windows/win_display_device_general.cpp | 4 ++-- tests/unit/general/test_factory.cpp | 3 +-- 7 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/common/factory.cpp b/src/common/factory.cpp index aaa004e..3956add 100644 --- a/src/common/factory.cpp +++ b/src/common/factory.cpp @@ -8,7 +8,7 @@ #if !defined(_WIN32) && !defined(__APPLE__) namespace display_device { - std::unique_ptr makeSettingsManager(SettingsManagerFactoryConfig) { + std::unique_ptr makeSettingsManager(const SettingsManagerFactoryConfig &) { return nullptr; } diff --git a/src/common/include/display_device/factory.h b/src/common/include/display_device/factory.h index aeaf6e0..6819051 100644 --- a/src/common/include/display_device/factory.h +++ b/src/common/include/display_device/factory.h @@ -31,7 +31,7 @@ namespace display_device { * @param config Dependencies and platform-neutral options. * @returns A settings manager, or nullptr when the platform is unsupported. */ - [[nodiscard]] std::unique_ptr makeSettingsManager(SettingsManagerFactoryConfig config = {}); + [[nodiscard]] std::unique_ptr makeSettingsManager(const SettingsManagerFactoryConfig &config = {}); /** * @brief Create the default display power manager for the current platform. diff --git a/src/macos/factory.cpp b/src/macos/factory.cpp index ca18308..44f7c4f 100644 --- a/src/macos/factory.cpp +++ b/src/macos/factory.cpp @@ -5,9 +5,6 @@ // class header include #include "display_device/factory.h" -// system includes -#include - // local includes #include "display_device/macos/display_power.h" #include "display_device/macos/mac_api_layer.h" @@ -15,12 +12,12 @@ #include "display_device/macos/settings_manager.h" namespace display_device { - std::unique_ptr makeSettingsManager(SettingsManagerFactoryConfig config) { + std::unique_ptr makeSettingsManager(const SettingsManagerFactoryConfig &config) { auto api_layer {std::make_shared()}; return std::make_unique( std::make_shared(api_layer), - std::move(config.m_audio_context_api), - std::make_unique(std::move(config.m_settings_persistence_api), config.m_throw_on_persistence_load_error), + config.m_audio_context_api, + std::make_unique(config.m_settings_persistence_api, config.m_throw_on_persistence_load_error), MacWorkarounds {} ); } diff --git a/src/macos/mac_display_device_general.cpp b/src/macos/mac_display_device_general.cpp index 94a5a8f..6e87003 100644 --- a/src/macos/mac_display_device_general.cpp +++ b/src/macos/mac_display_device_general.cpp @@ -57,7 +57,7 @@ namespace display_device { } } - devices.emplace_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); + devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); } return devices; diff --git a/src/windows/factory.cpp b/src/windows/factory.cpp index 495f322..7ecd799 100644 --- a/src/windows/factory.cpp +++ b/src/windows/factory.cpp @@ -5,9 +5,6 @@ // class header include #include "display_device/factory.h" -// system includes -#include - // local includes #include "display_device/windows/display_power.h" #include "display_device/windows/settings_manager.h" @@ -15,12 +12,12 @@ #include "display_device/windows/win_display_device.h" namespace display_device { - std::unique_ptr makeSettingsManager(SettingsManagerFactoryConfig config) { + std::unique_ptr makeSettingsManager(const SettingsManagerFactoryConfig &config) { auto api_layer {std::make_shared()}; return std::make_unique( std::make_shared(api_layer), - std::move(config.m_audio_context_api), - std::make_unique(std::move(config.m_settings_persistence_api), config.m_throw_on_persistence_load_error), + config.m_audio_context_api, + std::make_unique(config.m_settings_persistence_api, config.m_throw_on_persistence_load_error), WinWorkarounds { .m_hdr_blank_delay = config.m_hdr_blank_delay } diff --git a/src/windows/win_display_device_general.cpp b/src/windows/win_display_device_general.cpp index bab494a..af9342d 100644 --- a/src/windows/win_display_device_general.cpp +++ b/src/windows/win_display_device_general.cpp @@ -69,9 +69,9 @@ namespace display_device { m_w_api->getHdrState(best_path) }; - available_devices.emplace_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); + available_devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); } else { - available_devices.emplace_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, std::nullopt}); + available_devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, std::nullopt}); } } diff --git a/tests/unit/general/test_factory.cpp b/tests/unit/general/test_factory.cpp index b5484bb..eec4a9a 100644 --- a/tests/unit/general/test_factory.cpp +++ b/tests/unit/general/test_factory.cpp @@ -1,6 +1,5 @@ // system includes #include -#include // local includes #include "display_device/factory.h" @@ -18,7 +17,7 @@ TEST_S(MakeSettingsManager) { .m_hdr_blank_delay = 10ms }; - const auto settings_manager {display_device::makeSettingsManager(std::move(config))}; + const auto settings_manager {display_device::makeSettingsManager(config)}; #if defined(_WIN32) || defined(__APPLE__) ASSERT_NE(settings_manager, nullptr); From 80105218577cf62633de5d4cd291d7034dc14f4b Mon Sep 17 00:00:00 2001 From: marton Date: Sat, 27 Jun 2026 20:05:44 -0400 Subject: [PATCH 3/3] override sonar --- src/macos/mac_display_device_general.cpp | 3 ++- src/windows/win_display_device_general.cpp | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/macos/mac_display_device_general.cpp b/src/macos/mac_display_device_general.cpp index 6e87003..f0dcc23 100644 --- a/src/macos/mac_display_device_general.cpp +++ b/src/macos/mac_display_device_general.cpp @@ -57,7 +57,8 @@ namespace display_device { } } - devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); + // Keep braced aggregate construction; emplace_back(args...) relies on parenthesized aggregate init, which older libc++ rejects. + devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); // NOSONAR } return devices; diff --git a/src/windows/win_display_device_general.cpp b/src/windows/win_display_device_general.cpp index af9342d..cf636b1 100644 --- a/src/windows/win_display_device_general.cpp +++ b/src/windows/win_display_device_general.cpp @@ -69,9 +69,11 @@ namespace display_device { m_w_api->getHdrState(best_path) }; - available_devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); + // Keep braced aggregate construction; emplace_back(args...) relies on parenthesized aggregate init, which older libc++ rejects. + available_devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info}); // NOSONAR } else { - available_devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, std::nullopt}); + // Keep braced aggregate construction; emplace_back(args...) relies on parenthesized aggregate init, which older libc++ rejects. + available_devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, std::nullopt}); // NOSONAR } }