diff --git a/src/windows/include/displaydevice/windows/settingsmanager.h b/src/windows/include/displaydevice/windows/settingsmanager.h index 98fda2b..f4893e0 100644 --- a/src/windows/include/displaydevice/windows/settingsmanager.h +++ b/src/windows/include/displaydevice/windows/settingsmanager.h @@ -1,6 +1,7 @@ #pragma once // system includes +#include #include // local includes @@ -52,10 +53,11 @@ namespace display_device { * @param config Configuration to be used for preparing topology. * @param topology_before_changes The current topology before any changes. * @param release_context Specifies whether the audio context should be released at the very end IF everything else has succeeded. + * @param system_settings_touched Inticates whether a "write" operation could have been performed on the OS. * @return A tuple of (new_state that is to be updated/persisted, device_to_configure, additional_devices_to_configure). */ [[nodiscard]] std::optional>> - prepareTopology(const SingleDisplayConfiguration &config, const ActiveTopology &topology_before_changes, bool &release_context); + prepareTopology(const SingleDisplayConfiguration &config, const ActiveTopology &topology_before_changes, bool &release_context, bool &system_settings_touched); /** * @brief Changes or restores the primary device based on the cached state, new state and configuration. @@ -63,10 +65,11 @@ namespace display_device { * @param device_to_configure The main device to be used for preparation. * @param guard_fn Reference to the guard function which will be set to restore original state (if needed) in case something else fails down the line. * @param new_state Reference to the new state which is to be updated accordingly. + * @param system_settings_touched Inticates whether a "write" operation could have been performed on the OS. * @return True if no errors have occured, false otherwise. */ [[nodiscard]] bool - preparePrimaryDevice(const SingleDisplayConfiguration &config, const std::string &device_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state); + preparePrimaryDevice(const SingleDisplayConfiguration &config, const std::string &device_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state, bool &system_settings_touched); /** * @brief Changes or restores the display modes based on the cached state, new state and configuration. @@ -75,10 +78,11 @@ namespace display_device { * @param additional_devices_to_configure Additional devices that should be configured. * @param guard_fn Reference to the guard function which will be set to restore original state (if needed) in case something else fails down the line. * @param new_state Reference to the new state which is to be updated accordingly. + * @param system_settings_touched Inticates whether a "write" operation could have been performed on the OS. * @return True if no errors have occured, false otherwise. */ [[nodiscard]] bool - prepareDisplayModes(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state); + prepareDisplayModes(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state, bool &system_settings_touched); /** * @brief Changes or restores the HDR states based on the cached state, new state and configuration. @@ -87,22 +91,29 @@ namespace display_device { * @param additional_devices_to_configure Additional devices that should be configured. * @param guard_fn Reference to the guard function which will be set to restore original state (if needed) in case something else fails down the line. * @param new_state Reference to the new state which is to be updated accordingly. + * @param system_settings_touched Inticates whether a "write" operation could have been performed on the OS. * @return True if no errors have occured, false otherwise. */ [[nodiscard]] bool - prepareHdrStates(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state); + prepareHdrStates(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state, bool &system_settings_touched); /** * @brief Try to revert the modified settings. + * @param current_topology Topology before this method is called. + * @param system_settings_touched Inticates whether a "write" operation could have been performed on the OS. * @returns True on success, false otherwise. * @warning The method assumes that the caller will ensure restoring the topology * in case of a failure! */ [[nodiscard]] bool - revertModifiedSettings(); + revertModifiedSettings(const ActiveTopology ¤t_topology, bool &system_settings_touched); std::shared_ptr m_dd_api; std::shared_ptr m_audio_context_api; std::unique_ptr m_persistence_state; + + private: + /** @see win_utils::blankHdrStates for more details. */ + std::chrono::milliseconds m_hdr_blank_delay { 500 }; // 500ms should be more than enough... }; } // namespace display_device diff --git a/src/windows/include/displaydevice/windows/settingsutils.h b/src/windows/include/displaydevice/windows/settingsutils.h index b88a610..53e017e 100644 --- a/src/windows/include/displaydevice/windows/settingsutils.h +++ b/src/windows/include/displaydevice/windows/settingsutils.h @@ -1,6 +1,7 @@ #pragma once // system includes +#include #include // local includes @@ -133,6 +134,29 @@ namespace display_device::win_utils { const std::set &additional_devices_to_configure, const HdrStateMap &original_states); + /** + * @brief Toggle enabled HDR states off and on again if quick succession. + * + * This is a workaround for a HDR highcontrast color bug which prominently + * happens for IDD HDR displays, but also sometimes (very VERY rarely) for + * dongles. + * + * The bug is cause my more or less any change to the display settings, such as + * enabling HDR display, enabling HDR state, changing display mode of OTHER + * device and so on. + * + * The workaround is to simply turn of HDR, wait a little and then turn it back + * on. + * + * This is what this function does, but only if there are HDR enabled displays + * at the moment. + * + * @param win_dd Interface for interacting with the OS. + * @param delay Delay between OFF and ON states (ON -> OFF -> DELAY -> ON). + */ + void + blankHdrStates(WinDisplayDeviceInterface &win_dd, std::chrono::milliseconds delay); + /** * @brief Make guard function for the topology. * @param win_dd Interface for interacting with the OS. diff --git a/src/windows/settingsmanagerapply.cpp b/src/windows/settingsmanagerapply.cpp index 024c9b1..2ea9e71 100644 --- a/src/windows/settingsmanagerapply.cpp +++ b/src/windows/settingsmanagerapply.cpp @@ -40,6 +40,13 @@ namespace display_device { DD_LOG(info) << "Active topology before any changes:\n" << toJson(topology_before_changes); + bool system_settings_touched { false }; + boost::scope::scope_exit hdr_blank_always_executed_guard { [this, &system_settings_touched]() { + if (system_settings_touched) { + win_utils::blankHdrStates(*m_dd_api, m_hdr_blank_delay); + } + } }; + bool release_context { false }; boost::scope::scope_exit topology_prep_guard { [this, topology = topology_before_changes, was_captured = m_audio_context_api->isCaptured(), &release_context]() { // It is possible that during topology preparation, some settings will be reverted for the modified topology. @@ -61,7 +68,7 @@ namespace display_device { } } }; - const auto &prepped_topology_data { prepareTopology(config, topology_before_changes, release_context) }; + const auto &prepped_topology_data { prepareTopology(config, topology_before_changes, release_context, system_settings_touched) }; if (!prepped_topology_data) { // Error already logged return ApplyResult::DevicePrepFailed; @@ -70,21 +77,21 @@ namespace display_device { DdGuardFn primary_guard_fn { noopFn }; boost::scope::scope_exit primary_guard { primary_guard_fn }; - if (!preparePrimaryDevice(config, device_to_configure, primary_guard_fn, new_state)) { + if (!preparePrimaryDevice(config, device_to_configure, primary_guard_fn, new_state, system_settings_touched)) { // Error already logged return ApplyResult::PrimaryDevicePrepFailed; } DdGuardFn mode_guard_fn { noopFn }; boost::scope::scope_exit mode_guard { mode_guard_fn }; - if (!prepareDisplayModes(config, device_to_configure, additional_devices_to_configure, mode_guard_fn, new_state)) { + if (!prepareDisplayModes(config, device_to_configure, additional_devices_to_configure, mode_guard_fn, new_state, system_settings_touched)) { // Error already logged return ApplyResult::DisplayModePrepFailed; } DdGuardFn hdr_state_guard_fn { noopFn }; boost::scope::scope_exit hdr_state_guard { hdr_state_guard_fn }; - if (!prepareHdrStates(config, device_to_configure, additional_devices_to_configure, hdr_state_guard_fn, new_state)) { + if (!prepareHdrStates(config, device_to_configure, additional_devices_to_configure, hdr_state_guard_fn, new_state, system_settings_touched)) { // Error already logged return ApplyResult::HdrStatePrepFailed; } @@ -110,7 +117,7 @@ namespace display_device { } std::optional>> - SettingsManager::prepareTopology(const SingleDisplayConfiguration &config, const ActiveTopology &topology_before_changes, bool &release_context) { + SettingsManager::prepareTopology(const SingleDisplayConfiguration &config, const ActiveTopology &topology_before_changes, bool &release_context, bool &system_settings_touched) { const EnumeratedDeviceList devices { m_dd_api->enumAvailableDevices() }; if (devices.empty()) { DD_LOG(error) << "Failed to enumerate display devices!"; @@ -161,7 +168,7 @@ namespace display_device { if (change_is_needed) { if (cached_state && !m_dd_api->isTopologyTheSame(cached_state->m_modified.m_topology, new_topology)) { DD_LOG(warning) << "To apply new display device settings, previous modifications must be undone! Trying to undo them now."; - if (!revertModifiedSettings()) { + if (!revertModifiedSettings(topology_before_changes, system_settings_touched)) { DD_LOG(error) << "Failed to apply new configuration, because the previous settings could not be reverted!"; return std::nullopt; } @@ -182,6 +189,7 @@ namespace display_device { } } + system_settings_touched = true; if (!m_dd_api->setTopology(new_topology)) { DD_LOG(error) << "Failed to apply new configuration, because a new topology could not be set!"; return std::nullopt; @@ -196,7 +204,7 @@ namespace display_device { } bool - SettingsManager::preparePrimaryDevice(const SingleDisplayConfiguration &config, const std::string &device_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state) { + SettingsManager::preparePrimaryDevice(const SingleDisplayConfiguration &config, const std::string &device_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state, bool &system_settings_touched) { const auto &cached_state { m_persistence_state->getState() }; const auto cached_primary_device { cached_state ? cached_state->m_modified.m_original_primary_device : std::string {} }; const bool ensure_primary { config.m_device_prep == SingleDisplayConfiguration::DevicePreparation::EnsurePrimary }; @@ -214,6 +222,8 @@ namespace display_device { const auto try_change { [&](const std::string &new_device, const auto info_preamble, const auto error_log) { if (current_primary_device != new_device) { + system_settings_touched = true; + DD_LOG(info) << info_preamble << toJson(new_device); if (!m_dd_api->setAsPrimary(new_device)) { DD_LOG(error) << error_log; @@ -251,7 +261,7 @@ namespace display_device { } bool - SettingsManager::prepareDisplayModes(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state) { + SettingsManager::prepareDisplayModes(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state, bool &system_settings_touched) { const auto &cached_state { m_persistence_state->getState() }; const auto cached_display_modes { cached_state ? cached_state->m_modified.m_original_modes : DeviceDisplayModeMap {} }; const bool change_required { config.m_resolution || config.m_refresh_rate }; @@ -270,11 +280,18 @@ namespace display_device { if (current_display_modes != new_modes) { DD_LOG(info) << info_preamble << toJson(new_modes); if (!m_dd_api->setDisplayModes(new_modes)) { + system_settings_touched = true; DD_LOG(error) << error_log; return false; } - guard_fn = win_utils::modeGuardFn(*m_dd_api, current_display_modes); + // It is possible that the display modes will not actually change even though the "current != new" condition is true. + // This is because of some additional internal checks that determine whether the change is actually needed. + // Therefore we should check the current display modes after the fact! + if (current_display_modes != m_dd_api->getCurrentDisplayModes(win_utils::flattenTopology(new_state.m_modified.m_topology))) { + system_settings_touched = true; + guard_fn = win_utils::modeGuardFn(*m_dd_api, current_display_modes); + } } return true; @@ -307,7 +324,7 @@ namespace display_device { } [[nodiscard]] bool - SettingsManager::prepareHdrStates(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state) { + SettingsManager::prepareHdrStates(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const std::set &additional_devices_to_configure, DdGuardFn &guard_fn, SingleDisplayConfigState &new_state, bool &system_settings_touched) { const auto &cached_state { m_persistence_state->getState() }; const auto cached_hdr_states { cached_state ? cached_state->m_modified.m_original_hdr_states : HdrStateMap {} }; const bool change_required { config.m_hdr_state }; @@ -324,6 +341,8 @@ namespace display_device { const auto try_change { [&](const HdrStateMap &new_states, const auto info_preamble, const auto error_log) { if (current_hdr_states != new_states) { + system_settings_touched = true; + DD_LOG(info) << info_preamble << toJson(new_states); if (!m_dd_api->setHdrStates(new_states)) { DD_LOG(error) << error_log; diff --git a/src/windows/settingsmanagerrevert.cpp b/src/windows/settingsmanagerrevert.cpp index 14b3eb0..2b42b95 100644 --- a/src/windows/settingsmanagerrevert.cpp +++ b/src/windows/settingsmanagerrevert.cpp @@ -40,10 +40,16 @@ namespace display_device { return false; } + bool system_settings_touched { false }; + boost::scope::scope_exit hdr_blank_always_executed_guard { [this, &system_settings_touched]() { + if (system_settings_touched) { + win_utils::blankHdrStates(*m_dd_api, m_hdr_blank_delay); + } + } }; boost::scope::scope_exit topology_prep_guard { win_utils::topologyGuardFn(*m_dd_api, current_topology) }; // We can revert the modified setting independently before playing around with initial topology. - if (!revertModifiedSettings()) { + if (!revertModifiedSettings(current_topology, system_settings_touched)) { // Error already logged return false; } @@ -54,7 +60,9 @@ namespace display_device { return false; } - if (!m_dd_api->setTopology(cached_state->m_initial.m_topology)) { + const bool is_topology_the_same { m_dd_api->isTopologyTheSame(current_topology, cached_state->m_initial.m_topology) }; + system_settings_touched = system_settings_touched || !is_topology_the_same; + if (!is_topology_the_same && !m_dd_api->setTopology(cached_state->m_initial.m_topology)) { DD_LOG(error) << "Failed to change topology to:\n" << toJson(cached_state->m_initial.m_topology); return false; @@ -75,7 +83,7 @@ namespace display_device { } bool - SettingsManager::revertModifiedSettings() { + SettingsManager::revertModifiedSettings(const ActiveTopology ¤t_topology, bool &system_settings_touched) { const auto &cached_state { m_persistence_state->getState() }; if (!cached_state || !cached_state->m_modified.hasModifications()) { return true; @@ -87,7 +95,9 @@ namespace display_device { return false; } - if (!m_dd_api->setTopology(cached_state->m_modified.m_topology)) { + const bool is_topology_the_same { m_dd_api->isTopologyTheSame(current_topology, cached_state->m_modified.m_topology) }; + system_settings_touched = !is_topology_the_same; + if (!is_topology_the_same && !m_dd_api->setTopology(cached_state->m_modified.m_topology)) { DD_LOG(error) << "Failed to change topology to:\n" << toJson(cached_state->m_modified.m_topology); return false; @@ -96,38 +106,58 @@ namespace display_device { DdGuardFn hdr_guard_fn { noopFn }; boost::scope::scope_exit hdr_guard { hdr_guard_fn }; if (!cached_state->m_modified.m_original_hdr_states.empty()) { - hdr_guard_fn = win_utils::hdrStateGuardFn(*m_dd_api, cached_state->m_modified.m_topology); - DD_LOG(info) << "Trying to change back the HDR states to:\n" - << toJson(cached_state->m_modified.m_original_hdr_states); - if (!m_dd_api->setHdrStates(cached_state->m_modified.m_original_hdr_states)) { - // Error already logged - hdr_guard.set_active(false); - return false; + const auto current_states { m_dd_api->getCurrentHdrStates(win_utils::flattenTopology(cached_state->m_modified.m_topology)) }; + if (current_states != cached_state->m_modified.m_original_hdr_states) { + system_settings_touched = true; + + DD_LOG(info) << "Trying to change back the HDR states to:\n" + << toJson(cached_state->m_modified.m_original_hdr_states); + if (!m_dd_api->setHdrStates(cached_state->m_modified.m_original_hdr_states)) { + // Error already logged + return false; + } + + hdr_guard_fn = win_utils::hdrStateGuardFn(*m_dd_api, current_states); } } DdGuardFn mode_guard_fn { noopFn }; boost::scope::scope_exit mode_guard { mode_guard_fn }; if (!cached_state->m_modified.m_original_modes.empty()) { - mode_guard_fn = win_utils::modeGuardFn(*m_dd_api, cached_state->m_modified.m_topology); - DD_LOG(info) << "Trying to change back the display modes to:\n" - << toJson(cached_state->m_modified.m_original_modes); - if (!m_dd_api->setDisplayModes(cached_state->m_modified.m_original_modes)) { - // Error already logged - mode_guard.set_active(false); - return false; + const auto current_modes { m_dd_api->getCurrentDisplayModes(win_utils::flattenTopology(cached_state->m_modified.m_topology)) }; + if (current_modes != cached_state->m_modified.m_original_modes) { + DD_LOG(info) << "Trying to change back the display modes to:\n" + << toJson(cached_state->m_modified.m_original_modes); + if (!m_dd_api->setDisplayModes(cached_state->m_modified.m_original_modes)) { + system_settings_touched = true; + // Error already logged + return false; + } + + // It is possible that the display modes will not actually change even though the "current != new" condition is true. + // This is because of some additional internal checks that determine whether the change is actually needed. + // Therefore we should check the current display modes after the fact! + if (current_modes != m_dd_api->getCurrentDisplayModes(win_utils::flattenTopology(cached_state->m_modified.m_topology))) { + system_settings_touched = true; + mode_guard_fn = win_utils::modeGuardFn(*m_dd_api, current_modes); + } } } DdGuardFn primary_guard_fn { noopFn }; boost::scope::scope_exit primary_guard { primary_guard_fn }; if (!cached_state->m_modified.m_original_primary_device.empty()) { - primary_guard_fn = win_utils::primaryGuardFn(*m_dd_api, cached_state->m_modified.m_topology); - DD_LOG(info) << "Trying to change back the original primary device to: " << toJson(cached_state->m_modified.m_original_primary_device); - if (!m_dd_api->setAsPrimary(cached_state->m_modified.m_original_primary_device)) { - // Error already logged - primary_guard.set_active(false); - return false; + const auto current_primary_device { win_utils::getPrimaryDevice(*m_dd_api, cached_state->m_modified.m_topology) }; + if (current_primary_device != cached_state->m_modified.m_original_primary_device) { + system_settings_touched = true; + + DD_LOG(info) << "Trying to change back the original primary device to: " << toJson(cached_state->m_modified.m_original_primary_device); + if (!m_dd_api->setAsPrimary(cached_state->m_modified.m_original_primary_device)) { + // Error already logged + return false; + } + + primary_guard_fn = win_utils::primaryGuardFn(*m_dd_api, current_primary_device); } } diff --git a/src/windows/settingsutils.cpp b/src/windows/settingsutils.cpp index 71a9720..af089ca 100644 --- a/src/windows/settingsutils.cpp +++ b/src/windows/settingsutils.cpp @@ -3,6 +3,7 @@ // system includes #include +#include // local includes #include "displaydevice/logging.h" @@ -329,6 +330,51 @@ namespace display_device::win_utils { return new_states; } + void + blankHdrStates(WinDisplayDeviceInterface &win_dd, const std::chrono::milliseconds delay) { + const auto topology { win_dd.getCurrentTopology() }; + if (!win_dd.isTopologyValid(topology)) { + DD_LOG(error) << "Got an invalid topology while trying to blank HDR states!"; + return; + } + + const auto current_states { win_dd.getCurrentHdrStates(flattenTopology(topology)) }; + if (current_states.empty()) { + DD_LOG(error) << "Failed to get current HDR states! Topology:\n" + << toJson(topology); + return; + } + + std::set device_ids; + HdrStateMap original_states; + HdrStateMap inverse_states; + for (const auto &[device_id, state] : current_states) { + if (!state || *state != HdrState::Enabled) { + continue; + } + + device_ids.insert(device_id); + original_states[device_id] = HdrState::Enabled; + inverse_states[device_id] = HdrState::Disabled; + } + + if (device_ids.empty()) { + // Nothing to do + return; + } + + DD_LOG(info) << "Applying HDR state \"blank\" workaround (" << delay.count() << "ms) to devices: " << toJson(device_ids, JSON_COMPACT); + if (!win_dd.setHdrStates(inverse_states)) { + DD_LOG(error) << "Failed to apply inverse HDR states during \"blank\"!"; + return; + } + + std::this_thread::sleep_for(delay); + if (!win_dd.setHdrStates(original_states)) { + DD_LOG(error) << "Failed to apply original HDR states during \"blank\"!"; + } + } + DdGuardFn topologyGuardFn(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology) { DD_LOG(debug) << "Got topology in topologyGuardFn:\n" diff --git a/tests/unit/windows/test_settingsmanagerapply.cpp b/tests/unit/windows/test_settingsmanagerapply.cpp index 4447053..b7cfd81 100644 --- a/tests/unit/windows/test_settingsmanagerapply.cpp +++ b/tests/unit/windows/test_settingsmanagerapply.cpp @@ -222,6 +222,19 @@ namespace { .RetiresOnSaturation(); } + void + expectedHdrWorkaroundCalls(InSequence &sequence /* To ensure that sequence is created outside this scope */) { + // Using the "failure" path, to keep it simple + EXPECT_CALL(*m_dd_api, getCurrentTopology()) + .Times(1) + .WillOnce(Return(display_device::ActiveTopology {})) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyValid(display_device::ActiveTopology {})) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + } + std::shared_ptr> m_dd_api { std::make_shared>() }; std::shared_ptr> m_settings_persistence_api { std::make_shared>() }; std::shared_ptr> m_audio_context_api { std::make_shared>() }; @@ -398,6 +411,7 @@ TEST_F_S_MOCKED(PrepareTopology, TopologyChangeFailed) { expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, true); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_device_prep = DevicePrep::EnsureOnlyDisplay }), display_device::SettingsManager::ApplyResult::DevicePrepFailed); } @@ -418,6 +432,7 @@ TEST_F_S_MOCKED(PrepareTopology, AudioContextCaptured) { expectedSetTopologyCall(sequence, { { "DeviceId1" } }); expectedIsTopologyTheSameCall(sequence, DEFAULT_CURRENT_TOPOLOGY, { { "DeviceId1" } }); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_device_prep = DevicePrep::EnsureOnlyDisplay }), display_device::SettingsManager::ApplyResult::Ok); } @@ -439,6 +454,7 @@ TEST_F_S_MOCKED(PrepareTopology, AudioContextCaptureSkipped, NotInitialTopologyS expectedSetTopologyCall(sequence, { { "DeviceId1" } }); expectedIsTopologyTheSameCall(sequence, ut_consts::SDCS_FULL->m_initial.m_topology, { { "DeviceId1" } }); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_device_prep = DevicePrep::EnsureOnlyDisplay }), display_device::SettingsManager::ApplyResult::Ok); } @@ -458,6 +474,7 @@ TEST_F_S_MOCKED(PrepareTopology, AudioContextCaptureSkipped, NoDevicesAreGone) { expectedSetTopologyCall(sequence, persistence_input.m_modified.m_topology); expectedIsTopologyTheSameCall(sequence, DEFAULT_CURRENT_TOPOLOGY, persistence_input.m_modified.m_topology); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId4", .m_device_prep = DevicePrep::EnsureActive }), display_device::SettingsManager::ApplyResult::Ok); } @@ -483,6 +500,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, FailedToGetPrimaryDevice) { expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId4", .m_device_prep = DevicePrep::EnsurePrimary }), display_device::SettingsManager::ApplyResult::PrimaryDevicePrepFailed); } @@ -506,6 +524,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, FailedToSetPrimaryDevice) { expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId4", .m_device_prep = DevicePrep::EnsurePrimary }), display_device::SettingsManager::ApplyResult::PrimaryDevicePrepFailed); } @@ -529,6 +548,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, PrimaryDeviceSet) { expectedIsPrimaryCall(sequence, "DeviceId1"); expectedSetAsPrimaryCall(sequence, "DeviceId4"); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId4", .m_device_prep = DevicePrep::EnsurePrimary }), display_device::SettingsManager::ApplyResult::Ok); } @@ -549,6 +569,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, PrimaryDeviceSet, CachedDeviceReused) { expectedIsPrimaryCall(sequence, "DeviceId2", false); expectedIsPrimaryCall(sequence, "DeviceId3"); expectedSetAsPrimaryCall(sequence, "DeviceId4"); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId4", .m_device_prep = DevicePrep::EnsurePrimary }), display_device::SettingsManager::ApplyResult::Ok); } @@ -576,6 +597,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, PrimaryDeviceSet, GuardInvoked) { expectedPrimaryGuardCall(sequence, "DeviceId1"); expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId4", .m_device_prep = DevicePrep::EnsurePrimary }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); } @@ -604,6 +626,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, PrimaryDeviceSetSkipped) { expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId4", .m_device_prep = DevicePrep::EnsurePrimary }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); } @@ -625,6 +648,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, FailedToRestorePrimaryDevice) { expectedTopologyGuardTopologyCall(sequence, intial_state->m_modified.m_topology); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId3", .m_device_prep = DevicePrep::EnsureActive }), display_device::SettingsManager::ApplyResult::PrimaryDevicePrepFailed); } @@ -647,6 +671,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, PrimaryDeviceRestored) { expectedIsPrimaryCall(sequence, "DeviceId3"); expectedSetAsPrimaryCall(sequence, "DeviceId1", true); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId3", .m_device_prep = DevicePrep::EnsureActive }), display_device::SettingsManager::ApplyResult::Ok); } @@ -673,6 +698,7 @@ TEST_F_S_MOCKED(PreparePrimaryDevice, PrimaryDeviceRestored, PersistenceFailed) expectedPrimaryGuardCall(sequence, "DeviceId3"); expectedTopologyGuardTopologyCall(sequence, intial_state->m_modified.m_topology); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId3", .m_device_prep = DevicePrep::EnsureActive }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); } @@ -730,6 +756,7 @@ TEST_F_S_MOCKED(PrepareDisplayModes, FailedToSetDisplayModes) { expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_resolution = { { 1920, 1080 } } }), display_device::SettingsManager::ApplyResult::DisplayModePrepFailed); } @@ -750,7 +777,9 @@ TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesSet, ResolutionOnly) { expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); expectedSetDisplayModesCall(sequence, new_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), new_modes); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_resolution = { { 1920, 1080 } } }), display_device::SettingsManager::ApplyResult::Ok); } @@ -771,7 +800,9 @@ TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesSet, RefreshRateOnly) { expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); expectedSetDisplayModesCall(sequence, new_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), new_modes); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_refresh_rate = { { 30.85 } } }), display_device::SettingsManager::ApplyResult::Ok); } @@ -793,7 +824,9 @@ TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesSet, ResolutionAndRefreshRate) expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); expectedSetDisplayModesCall(sequence, new_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), new_modes); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_resolution = { { 1920, 1080 } }, .m_refresh_rate = { { 30.85 } } }), display_device::SettingsManager::ApplyResult::Ok); } @@ -817,7 +850,9 @@ TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesSet, ResolutionAndRefreshRate, expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); expectedSetDisplayModesCall(sequence, new_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), new_modes); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_resolution = { { 1920, 1080 } }, .m_refresh_rate = { { 30.85 } } }), display_device::SettingsManager::ApplyResult::Ok); } @@ -838,6 +873,8 @@ TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesSet, CachedModesReused) { expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); expectedSetDisplayModesCall(sequence, new_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), new_modes); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_resolution = { { 1920, 1080 } } }), display_device::SettingsManager::ApplyResult::Ok); } @@ -858,9 +895,36 @@ TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesSet, GuardInvoked) { expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); expectedSetDisplayModesCall(sequence, new_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), new_modes); expectedPersistenceCall(sequence, persistence_input, false); expectedSetDisplayModesGuardCall(sequence, DEFAULT_CURRENT_MODES); + expectedTopologyGuardTopologyCall(sequence); + expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); + + EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_resolution = { { 1920, 1080 } } }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); +} + +TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesSet, GuardNotInvoked) { + auto new_modes { DEFAULT_CURRENT_MODES }; + new_modes["DeviceId1"].m_resolution = { 1920, 1080 }; + + auto persistence_input { DEFAULT_PERSISTENCE_INPUT_BASE }; + persistence_input.m_modified.m_topology = DEFAULT_CURRENT_TOPOLOGY; + persistence_input.m_modified.m_original_modes = DEFAULT_CURRENT_MODES; + + InSequence sequence; + expectedDefaultCallsUntilTopologyPrep(sequence); + expectedIsCapturedCall(sequence, false); + expectedDeviceEnumCall(sequence); + expectedIsTopologyTheSameCall(sequence, DEFAULT_CURRENT_TOPOLOGY, DEFAULT_CURRENT_TOPOLOGY); + + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); + expectedSetDisplayModesCall(sequence, new_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); + expectedPersistenceCall(sequence, persistence_input, false); + expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); @@ -901,6 +965,7 @@ TEST_F_S_MOCKED(PrepareDisplayModes, FailedToRestoreDisplayModes) { expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1" }), display_device::SettingsManager::ApplyResult::DisplayModePrepFailed); } @@ -922,7 +987,9 @@ TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesRestored) { expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); expectedSetDisplayModesCall(sequence, initial_state.m_modified.m_original_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), initial_state.m_modified.m_original_modes); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1" }), display_device::SettingsManager::ApplyResult::Ok); } @@ -944,11 +1011,13 @@ TEST_F_S_MOCKED(PrepareDisplayModes, DisplayModesRestored, PersistenceFailed) { expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_MODES); expectedSetDisplayModesCall(sequence, initial_state.m_modified.m_original_modes); + expectedGetCurrentDisplayModesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), initial_state.m_modified.m_original_modes); expectedPersistenceCall(sequence, persistence_input, false); expectedSetDisplayModesGuardCall(sequence, DEFAULT_CURRENT_MODES); expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1" }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); } @@ -1006,6 +1075,7 @@ TEST_F_S_MOCKED(PrepareHdrStates, FailedToSetHdrStates) { expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_hdr_state = display_device::HdrState::Enabled }), display_device::SettingsManager::ApplyResult::HdrStatePrepFailed); } @@ -1027,6 +1097,7 @@ TEST_F_S_MOCKED(PrepareHdrStates, HdrStatesSet) { expectedGetCurrentHdrStatesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_HDR_STATES); expectedSetHdrStatesCall(sequence, new_states); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_hdr_state = display_device::HdrState::Enabled }), display_device::SettingsManager::ApplyResult::Ok); } @@ -1049,6 +1120,7 @@ TEST_F_S_MOCKED(PrepareHdrStates, HdrStatesSet, PrimaryDeviceSpecified) { expectedGetCurrentHdrStatesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_HDR_STATES); expectedSetHdrStatesCall(sequence, new_states); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_hdr_state = display_device::HdrState::Enabled }), display_device::SettingsManager::ApplyResult::Ok); } @@ -1069,6 +1141,7 @@ TEST_F_S_MOCKED(PrepareHdrStates, HdrStatesSet, CachedModesReused) { expectedGetCurrentHdrStatesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_HDR_STATES); expectedSetHdrStatesCall(sequence, new_states); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_hdr_state = display_device::HdrState::Enabled }), display_device::SettingsManager::ApplyResult::Ok); } @@ -1094,6 +1167,7 @@ TEST_F_S_MOCKED(PrepareHdrStates, HdrStatesSet, GuardInvoked) { expectedSetHdrStatesGuardCall(sequence, DEFAULT_CURRENT_HDR_STATES); expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_hdr_state = display_device::HdrState::Enabled }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); } @@ -1132,6 +1206,7 @@ TEST_F_S_MOCKED(PrepareHdrStates, FailedToRestoreHdrStates) { expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1" }), display_device::SettingsManager::ApplyResult::HdrStatePrepFailed); } @@ -1154,6 +1229,7 @@ TEST_F_S_MOCKED(PrepareHdrStates, HdrStatesRestored) { expectedGetCurrentHdrStatesCall(sequence, display_device::win_utils::flattenTopology(DEFAULT_CURRENT_TOPOLOGY), DEFAULT_CURRENT_HDR_STATES); expectedSetHdrStatesCall(sequence, initial_state.m_modified.m_original_hdr_states); expectedPersistenceCall(sequence, persistence_input); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1" }), display_device::SettingsManager::ApplyResult::Ok); } @@ -1180,6 +1256,7 @@ TEST_F_S_MOCKED(PrepareHdrStates, HdrStatesRestored, PersistenceFailed) { expectedSetHdrStatesGuardCall(sequence, DEFAULT_CURRENT_HDR_STATES); expectedTopologyGuardTopologyCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1" }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); } @@ -1224,6 +1301,7 @@ TEST_F_S_MOCKED(AudioContextDelayedRelease) { expectedIsTopologyTheSameCall(sequence, ut_consts::SDCS_FULL->m_initial.m_topology, { { "DeviceId1" } }); expectedPersistenceCall(sequence, persistence_input); expectedReleaseCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_device_prep = DevicePrep::EnsureOnlyDisplay }), display_device::SettingsManager::ApplyResult::Ok); } @@ -1266,6 +1344,7 @@ TEST_F_S_MOCKED(AudioContextDelayedRelease, ViaGuard) { expectedTopologyGuardTopologyCall(sequence, DEFAULT_CURRENT_TOPOLOGY, false); expectedReleaseCall(sequence); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_device_prep = DevicePrep::EnsureOnlyDisplay }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); } @@ -1290,6 +1369,7 @@ TEST_F_S_MOCKED(AudioContextDelayedRelease, SkippedDueToFailure) { expectedTopologyGuardTopologyCall(sequence, DEFAULT_CURRENT_TOPOLOGY, true); expectedTopologyGuardNewlyCapturedContextCall(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_EQ(getImpl().applySettings({ .m_device_id = "DeviceId1", .m_device_prep = DevicePrep::EnsureOnlyDisplay }), display_device::SettingsManager::ApplyResult::PersistenceSaveFailed); } diff --git a/tests/unit/windows/test_settingsmanagerrevert.cpp b/tests/unit/windows/test_settingsmanagerrevert.cpp index 31b64c3..299601d 100644 --- a/tests/unit/windows/test_settingsmanagerrevert.cpp +++ b/tests/unit/windows/test_settingsmanagerrevert.cpp @@ -26,7 +26,7 @@ namespace { { "DeviceId1", { { 123, 456 }, { 120, 1 } } }, { "DeviceId3", { { 456, 123 }, { 60, 1 } } } }; - const std::string CURRENT_MODIFIED_PRIMARY_DEVICE { "DeviceId1" }; + const std::string CURRENT_MODIFIED_PRIMARY_DEVICE { "DeviceId3" }; // Test fixture(s) for this file class SettingsManagerRevertMocked: public BaseTest { @@ -71,6 +71,10 @@ namespace { .Times(1) .WillOnce(Return(true)) .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, ut_consts::SDCS_FULL->m_modified.m_topology)) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); EXPECT_CALL(*m_dd_api, setTopology(ut_consts::SDCS_FULL->m_modified.m_topology)) .Times(1) .WillOnce(Return(true)) @@ -111,6 +115,10 @@ namespace { void expectedDefaultPrimaryDeviceGuardInitCall(InSequence & /* To ensure that sequence is created outside this scope */) { + EXPECT_CALL(*m_dd_api, isPrimary("DeviceId1")) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); EXPECT_CALL(*m_dd_api, isPrimary(CURRENT_MODIFIED_PRIMARY_DEVICE)) .Times(1) .WillOnce(Return(true)) @@ -134,11 +142,15 @@ namespace { } void - expectedDefaultDisplayModeCall(InSequence & /* To ensure that sequence is created outside this scope */) { + expectedDefaultDisplayModeCall(InSequence & /* To ensure that sequence is created outside this scope */, bool has_changed = true) { EXPECT_CALL(*m_dd_api, setDisplayModes(ut_consts::SDCS_FULL->m_modified.m_original_modes)) .Times(1) .WillOnce(Return(true)) .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, getCurrentDisplayModes(display_device::win_utils::flattenTopology(ut_consts::SDCS_FULL->m_modified.m_topology))) + .Times(1) + .WillOnce(Return(has_changed ? ut_consts::SDCS_FULL->m_modified.m_original_modes : CURRENT_MODIFIED_DISPLAY_MODES)) + .RetiresOnSaturation(); } void @@ -173,6 +185,10 @@ namespace { .Times(1) .WillOnce(Return(true)) .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, ut_consts::SDCS_FULL->m_initial.m_topology)) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); EXPECT_CALL(*m_dd_api, setTopology(ut_consts::SDCS_FULL->m_initial.m_topology)) .Times(1) .WillOnce(Return(true)) @@ -209,6 +225,19 @@ namespace { .RetiresOnSaturation(); } + void + expectedHdrWorkaroundCalls(InSequence &sequence /* To ensure that sequence is created outside this scope */) { + // Using the "failure" path, to keep it simple + EXPECT_CALL(*m_dd_api, getCurrentTopology()) + .Times(1) + .WillOnce(Return(display_device::ActiveTopology {})) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyValid(display_device::ActiveTopology {})) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + } + std::shared_ptr> m_dd_api { std::make_shared>() }; std::shared_ptr> m_settings_persistence_api { std::make_shared>() }; std::shared_ptr> m_audio_context_api { std::make_shared>() }; @@ -279,10 +308,14 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToSetModifiedTopology) { EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_modified.m_topology)) .Times(1) .WillOnce(Return(true)); + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, ut_consts::SDCS_FULL->m_modified.m_topology)) + .Times(1) + .WillOnce(Return(false)); EXPECT_CALL(*m_dd_api, setTopology(ut_consts::SDCS_FULL->m_modified.m_topology)) .Times(1) .WillOnce(Return(false)); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -302,6 +335,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertHdrStates) { .RetiresOnSaturation(); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -321,6 +355,31 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertDisplayModes) { .RetiresOnSaturation(); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); + + EXPECT_FALSE(getImpl().revertSettings()); +} + +TEST_F_S_MOCKED(RevertModifiedSettings, RevertedDisplayModes, PersistenceFailed, GuardNotInvoked) { + auto sdcs_stripped { ut_consts::SDCS_FULL }; + sdcs_stripped->m_modified.m_original_hdr_states.clear(); + sdcs_stripped->m_modified.m_original_primary_device.clear(); + + auto expected_persistent_input { ut_consts::SDCS_FULL }; + expected_persistent_input->m_modified = { expected_persistent_input->m_modified.m_topology }; + + InSequence sequence; + expectedDefaultCallsUntilModifiedSettings(sequence, sdcs_stripped); + expectedDefaultMofifiedTopologyCalls(sequence); + expectedDefaultDisplayModeGuardInitCall(sequence); + expectedDefaultDisplayModeCall(sequence, false); + EXPECT_CALL(*m_settings_persistence_api, store(*serializeState(expected_persistent_input))) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + + expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -340,6 +399,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertPrimaryDevice) { .RetiresOnSaturation(); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -366,6 +426,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToSetPersistence) { expectedDefaultDisplayModeGuardCall(sequence); expectedDefaultHdrStateGuardCall(sequence); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -380,6 +441,7 @@ TEST_F_S_MOCKED(InvalidInitialTopology) { .RetiresOnSaturation(); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -392,12 +454,17 @@ TEST_F_S_MOCKED(FailedToSetInitialTopology) { .Times(1) .WillOnce(Return(true)) .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, ut_consts::SDCS_FULL->m_initial.m_topology)) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); EXPECT_CALL(*m_dd_api, setTopology(ut_consts::SDCS_FULL->m_initial.m_topology)) .Times(1) .WillOnce(Return(false)) .RetiresOnSaturation(); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -413,6 +480,7 @@ TEST_F_S_MOCKED(FailedToClearPersistence) { .RetiresOnSaturation(); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -424,6 +492,7 @@ TEST_F_S_MOCKED(SuccesfullyReverted, WithAudioCapture) { expectedDefaultInitialTopologyCalls(sequence); expectedDefaultFinalPersistenceCalls(sequence); expectedDefaultAudioContextCalls(sequence, true); + expectedHdrWorkaroundCalls(sequence); EXPECT_TRUE(getImpl().revertSettings()); EXPECT_TRUE(getImpl().revertSettings()); // Seconds call after success is NOOP @@ -436,6 +505,7 @@ TEST_F_S_MOCKED(SuccesfullyReverted, NoAudioCapture) { expectedDefaultInitialTopologyCalls(sequence); expectedDefaultFinalPersistenceCalls(sequence); expectedDefaultAudioContextCalls(sequence, false); + expectedHdrWorkaroundCalls(sequence); EXPECT_TRUE(getImpl().revertSettings()); EXPECT_TRUE(getImpl().revertSettings()); // Seconds call after success is NOOP @@ -451,6 +521,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, CachedSettingsAreUpdated) { .RetiresOnSaturation(); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); expectedDefaultCallsUntilModifiedSettingsNoPersistence(sequence); @@ -458,6 +529,36 @@ TEST_F_S_MOCKED(RevertModifiedSettings, CachedSettingsAreUpdated) { expectedDefaultInitialTopologyCalls(sequence); expectedDefaultFinalPersistenceCalls(sequence); expectedDefaultAudioContextCalls(sequence, false); + expectedHdrWorkaroundCalls(sequence); + + EXPECT_TRUE(getImpl().revertSettings()); +} + +TEST_F_S_MOCKED(CurrentSettingsMatchPersistentOnes) { + display_device::SingleDisplayConfigState state { + { CURRENT_TOPOLOGY, + { "DeviceId4" } }, + { CURRENT_TOPOLOGY, + CURRENT_MODIFIED_DISPLAY_MODES, + CURRENT_MODIFIED_HDR_STATES, + "DeviceId4" } + }; + + InSequence sequence; + expectedDefaultCallsUntilModifiedSettings(sequence, state); + EXPECT_CALL(*m_dd_api, isTopologyValid(state.m_modified.m_topology)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, state.m_modified.m_topology)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, getCurrentHdrStates(display_device::win_utils::flattenTopology(state.m_modified.m_topology))).Times(1).WillOnce(Return(CURRENT_MODIFIED_HDR_STATES)).RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, getCurrentDisplayModes(display_device::win_utils::flattenTopology(state.m_modified.m_topology))).Times(1).WillOnce(Return(CURRENT_MODIFIED_DISPLAY_MODES)).RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isPrimary(state.m_modified.m_original_primary_device)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + + auto cleared_modifications { state }; + cleared_modifications.m_modified = { cleared_modifications.m_modified.m_topology }; + EXPECT_CALL(*m_settings_persistence_api, store(*serializeState(cleared_modifications))).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyValid(state.m_initial.m_topology)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, state.m_initial.m_topology)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + expectedDefaultFinalPersistenceCalls(sequence); + expectedDefaultAudioContextCalls(sequence, false); EXPECT_TRUE(getImpl().revertSettings()); } diff --git a/tests/unit/windows/test_settingsutils.cpp b/tests/unit/windows/test_settingsutils.cpp index fa19705..c170a8f 100644 --- a/tests/unit/windows/test_settingsutils.cpp +++ b/tests/unit/windows/test_settingsutils.cpp @@ -8,6 +8,7 @@ namespace { // Convenience keywords for GMock using ::testing::_; using ::testing::Return; + using ::testing::Sequence; using ::testing::StrictMock; // Test fixture(s) for this file @@ -244,6 +245,97 @@ TEST_F_S_MOCKED(TopologyGuardFn, Success) { EXPECT_NO_THROW(guard_fn()); } +TEST_F_S_MOCKED(BlankHdrStates, InvalidTopology) { + Sequence sequence; + EXPECT_CALL(m_dd_api, getCurrentTopology()).Times(1).WillOnce(Return(display_device::ActiveTopology {})).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, isTopologyValid(display_device::ActiveTopology {})).Times(1).WillOnce(Return(false)).RetiresOnSaturation(); + + EXPECT_NO_THROW(display_device::win_utils::blankHdrStates(m_dd_api, std::chrono::milliseconds(0))); +} + +TEST_F_S_MOCKED(BlankHdrStates, FailedToGetHrdStates) { + Sequence sequence; + EXPECT_CALL(m_dd_api, getCurrentTopology()).Times(1).WillOnce(Return(DEFAULT_INITIAL_TOPOLOGY)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, isTopologyValid(DEFAULT_INITIAL_TOPOLOGY)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, getCurrentHdrStates(display_device::win_utils::flattenTopology(DEFAULT_INITIAL_TOPOLOGY))).Times(1).WillOnce(Return(display_device::HdrStateMap {})).RetiresOnSaturation(); + + EXPECT_NO_THROW(display_device::win_utils::blankHdrStates(m_dd_api, std::chrono::milliseconds(0))); +} + +TEST_F_S_MOCKED(BlankHdrStates, NothingToApply) { + Sequence sequence; + EXPECT_CALL(m_dd_api, getCurrentTopology()).Times(1).WillOnce(Return(DEFAULT_INITIAL_TOPOLOGY)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, isTopologyValid(DEFAULT_INITIAL_TOPOLOGY)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, getCurrentHdrStates(display_device::win_utils::flattenTopology(DEFAULT_INITIAL_TOPOLOGY))).Times(1).WillOnce(Return(DEFAULT_CURRENT_HDR_STATES)).RetiresOnSaturation(); + + EXPECT_NO_THROW(display_device::win_utils::blankHdrStates(m_dd_api, std::chrono::milliseconds(0))); +} + +TEST_F_S_MOCKED(BlankHdrStates, FailedToApplyInverseStates) { + const display_device::HdrStateMap initial_states { + { "DeviceId1", { display_device::HdrState::Enabled } }, + { "DeviceId2", { display_device::HdrState::Disabled } }, + { "DeviceId3", std::nullopt } + }; + const display_device::HdrStateMap inverse_states { + { "DeviceId1", { display_device::HdrState::Disabled } } + }; + + Sequence sequence; + EXPECT_CALL(m_dd_api, getCurrentTopology()).Times(1).WillOnce(Return(DEFAULT_INITIAL_TOPOLOGY)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, isTopologyValid(DEFAULT_INITIAL_TOPOLOGY)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, getCurrentHdrStates(display_device::win_utils::flattenTopology(DEFAULT_INITIAL_TOPOLOGY))).Times(1).WillOnce(Return(initial_states)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, setHdrStates(inverse_states)).Times(1).WillOnce(Return(false)).RetiresOnSaturation(); + + EXPECT_NO_THROW(display_device::win_utils::blankHdrStates(m_dd_api, std::chrono::milliseconds(0))); +} + +TEST_F_S_MOCKED(BlankHdrStates, FailedToApplyOriginalStates) { + const display_device::HdrStateMap initial_states { + { "DeviceId1", { display_device::HdrState::Enabled } }, + { "DeviceId2", { display_device::HdrState::Disabled } }, + { "DeviceId3", std::nullopt } + }; + const display_device::HdrStateMap inverse_states { + { "DeviceId1", { display_device::HdrState::Disabled } } + }; + const display_device::HdrStateMap original_states { + { "DeviceId1", { display_device::HdrState::Enabled } } + }; + + Sequence sequence; + EXPECT_CALL(m_dd_api, getCurrentTopology()).Times(1).WillOnce(Return(DEFAULT_INITIAL_TOPOLOGY)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, isTopologyValid(DEFAULT_INITIAL_TOPOLOGY)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, getCurrentHdrStates(display_device::win_utils::flattenTopology(DEFAULT_INITIAL_TOPOLOGY))).Times(1).WillOnce(Return(initial_states)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, setHdrStates(inverse_states)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, setHdrStates(original_states)).Times(1).WillOnce(Return(false)).RetiresOnSaturation(); + + EXPECT_NO_THROW(display_device::win_utils::blankHdrStates(m_dd_api, std::chrono::milliseconds(0))); +} + +TEST_F_S_MOCKED(BlankHdrStates, Success) { + const display_device::HdrStateMap initial_states { + { "DeviceId1", { display_device::HdrState::Enabled } }, + { "DeviceId2", { display_device::HdrState::Disabled } }, + { "DeviceId3", std::nullopt } + }; + const display_device::HdrStateMap inverse_states { + { "DeviceId1", { display_device::HdrState::Disabled } } + }; + const display_device::HdrStateMap original_states { + { "DeviceId1", { display_device::HdrState::Enabled } } + }; + + Sequence sequence; + EXPECT_CALL(m_dd_api, getCurrentTopology()).Times(1).WillOnce(Return(DEFAULT_INITIAL_TOPOLOGY)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, isTopologyValid(DEFAULT_INITIAL_TOPOLOGY)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, getCurrentHdrStates(display_device::win_utils::flattenTopology(DEFAULT_INITIAL_TOPOLOGY))).Times(1).WillOnce(Return(initial_states)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, setHdrStates(inverse_states)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + EXPECT_CALL(m_dd_api, setHdrStates(original_states)).Times(1).WillOnce(Return(true)).RetiresOnSaturation(); + + EXPECT_NO_THROW(display_device::win_utils::blankHdrStates(m_dd_api, std::chrono::milliseconds(0))); +} + TEST_F_S_MOCKED(TopologyGuardFn, Failure) { EXPECT_CALL(m_dd_api, setTopology(display_device::ActiveTopology { { "DeviceId1" } })) .Times(1)