diff --git a/src/windows/include/display_device/windows/settings_utils.h b/src/windows/include/display_device/windows/settings_utils.h index f77aba5..82abb7b 100644 --- a/src/windows/include/display_device/windows/settings_utils.h +++ b/src/windows/include/display_device/windows/settings_utils.h @@ -26,6 +26,22 @@ namespace display_device::win_utils { std::set flattenTopology(const ActiveTopology &topology); + /** + * @brief Remove all unavailable devices from the topology. + * @param win_dd Interface for interacting with the OS. + * @param topology Topology to be stripped. + * @return Topology with only available devices. + * + * EXAMPLES: + * ```cpp + * const WinDisplayDeviceInterface* iface = getIface(...); + * const ActiveTopology topology { { "DeviceId1", "DeviceId2" }, { "DeviceId3" } }; + * const auto stripped_topology { stripTopologyOfUnavailableDevices(*iface, topology) }; + * ``` + */ + ActiveTopology + stripTopologyOfUnavailableDevices(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology); + /** * @brief Get one primary device from the provided topology. * @param win_dd Interface for interacting with the OS. diff --git a/src/windows/settings_manager_revert.cpp b/src/windows/settings_manager_revert.cpp index 61dce97..34d636a 100644 --- a/src/windows/settings_manager_revert.cpp +++ b/src/windows/settings_manager_revert.cpp @@ -46,7 +46,19 @@ namespace display_device { 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) }; + boost::scope::scope_exit topology_prep_guard { [this, &cached_state, ¤t_topology, &system_settings_touched]() { + auto topology_to_restore { win_utils::stripTopologyOfUnavailableDevices(*m_dd_api, cached_state->m_initial.m_topology) }; + if (!m_dd_api->isTopologyValid(topology_to_restore)) { + topology_to_restore = current_topology; + } + + const bool is_topology_the_same { m_dd_api->isTopologyTheSame(current_topology, topology_to_restore) }; + system_settings_touched = system_settings_touched || !is_topology_the_same; + if (!is_topology_the_same && !m_dd_api->setTopology(topology_to_restore)) { + DD_LOG(error) << "failed to revert topology in revertSettings topology guard! Used the following topology:\n" + << toJson(topology_to_restore); + } + } }; // We can revert the modified setting independently before playing around with initial topology. if (!revertModifiedSettings(current_topology, system_settings_touched)) { diff --git a/src/windows/settings_utils.cpp b/src/windows/settings_utils.cpp index 287656e..40d12d3 100644 --- a/src/windows/settings_utils.cpp +++ b/src/windows/settings_utils.cpp @@ -144,6 +144,17 @@ namespace display_device::win_utils { return flattened_topology; } + ActiveTopology + stripTopologyOfUnavailableDevices(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology) { + const auto devices { win_dd.enumAvailableDevices() }; + if (devices.empty()) { + DD_LOG(error) << "Failed to enumerate available devices for stripping topology!"; + return {}; + } + + return stripTopology(topology, devices); + } + std::string getPrimaryDevice(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology) { const auto flat_topology { flattenTopology(topology) }; diff --git a/tests/unit/windows/test_settings_manager_revert.cpp b/tests/unit/windows/test_settings_manager_revert.cpp index 37e7713..c0783ca 100644 --- a/tests/unit/windows/test_settings_manager_revert.cpp +++ b/tests/unit/windows/test_settings_manager_revert.cpp @@ -27,6 +27,11 @@ namespace { { "DeviceId3", { { 456, 123 }, { 60, 1 } } } }; const std::string CURRENT_MODIFIED_PRIMARY_DEVICE { "DeviceId3" }; + const display_device::EnumeratedDeviceList CURRENT_DEVICES { + { .m_device_id = "DeviceId1" }, + { .m_device_id = "DeviceId3" }, + { .m_device_id = "DeviceId4" } + }; // Test fixture(s) for this file class SettingsManagerRevertMocked: public BaseTest { @@ -219,7 +224,19 @@ namespace { void expectedDefaultTopologyGuardCall(InSequence & /* To ensure that sequence is created outside this scope */) { - EXPECT_CALL(*m_dd_api, setTopology(CURRENT_TOPOLOGY)) + EXPECT_CALL(*m_dd_api, enumAvailableDevices()) + .Times(1) + .WillOnce(Return(CURRENT_DEVICES)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + .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(CURRENT_TOPOLOGY == ut_consts::SDCS_FULL->m_initial.m_topology)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, setTopology(ut_consts::SDCS_FULL->m_initial.m_topology)) .Times(1) .WillOnce(Return(true)) .RetiresOnSaturation(); @@ -297,6 +314,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, InvalidModifiedTopology) { .Times(1) .WillOnce(Return(false)); expectedDefaultTopologyGuardCall(sequence); + expectedHdrWorkaroundCalls(sequence); EXPECT_FALSE(getImpl().revertSettings()); } @@ -431,6 +449,82 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToSetPersistence) { EXPECT_FALSE(getImpl().revertSettings()); } +TEST_F_S_MOCKED(TopologyGuard, CurrentTopologyUsedAsFallback) { + InSequence sequence; + expectedDefaultCallsUntilModifiedSettings(sequence); + expectedDefaultRevertModifiedSettingsCall(sequence); + EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, enumAvailableDevices()) + .Times(1) + .WillOnce(Return(CURRENT_DEVICES)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, CURRENT_TOPOLOGY)) + .Times(1) + .WillOnce(Return(true)) + .RetiresOnSaturation(); + expectedHdrWorkaroundCalls(sequence); + + EXPECT_FALSE(getImpl().revertSettings()); +} + +TEST_F_S_MOCKED(TopologyGuard, SystemSettingsUntouched) { + InSequence sequence; + expectedDefaultCallsUntilModifiedSettings(sequence); + EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_modified.m_topology)) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, enumAvailableDevices()) + .Times(1) + .WillOnce(Return(CURRENT_DEVICES)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, CURRENT_TOPOLOGY)) + .Times(1) + .WillOnce(Return(true)) + .RetiresOnSaturation(); + + EXPECT_FALSE(getImpl().revertSettings()); +} + +TEST_F_S_MOCKED(TopologyGuard, FailedToSetTopology) { + InSequence sequence; + expectedDefaultCallsUntilModifiedSettings(sequence); + EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_modified.m_topology)) + .Times(1) + .WillOnce(Return(false)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, enumAvailableDevices()) + .Times(1) + .WillOnce(Return(CURRENT_DEVICES)) + .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + .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(); + expectedHdrWorkaroundCalls(sequence); + + EXPECT_FALSE(getImpl().revertSettings()); +} + TEST_F_S_MOCKED(InvalidInitialTopology) { InSequence sequence; expectedDefaultCallsUntilModifiedSettings(sequence); diff --git a/tests/unit/windows/test_settings_utils.cpp b/tests/unit/windows/test_settings_utils.cpp index 06464c3..4ab207b 100644 --- a/tests/unit/windows/test_settings_utils.cpp +++ b/tests/unit/windows/test_settings_utils.cpp @@ -40,6 +40,40 @@ TEST_F_S_MOCKED(FlattenTopology) { EXPECT_EQ(display_device::win_utils::flattenTopology({}), std::set {}); } +TEST_F_S_MOCKED(StripTopologyOfUnavailableDevices, NoDevicesAreAvailable) { + const display_device::ActiveTopology input_topology { DEFAULT_INITIAL_TOPOLOGY }; + const display_device::ActiveTopology expected_topology {}; + const display_device::EnumeratedDeviceList devices {}; + + EXPECT_CALL(m_dd_api, enumAvailableDevices()).Times(1).WillOnce(Return(devices)); + EXPECT_EQ(display_device::win_utils::stripTopologyOfUnavailableDevices(m_dd_api, input_topology), expected_topology); +} + +TEST_F_S_MOCKED(StripTopologyOfUnavailableDevices, DeviceStripped) { + const display_device::ActiveTopology input_topology { DEFAULT_INITIAL_TOPOLOGY }; + const display_device::ActiveTopology expected_topology { { "DeviceId1" }, { "DeviceId3" } }; + const display_device::EnumeratedDeviceList devices { + { .m_device_id = "DeviceId1" }, + { .m_device_id = "DeviceId3" } + }; + + EXPECT_CALL(m_dd_api, enumAvailableDevices()).Times(1).WillOnce(Return(devices)); + EXPECT_EQ(display_device::win_utils::stripTopologyOfUnavailableDevices(m_dd_api, input_topology), expected_topology); +} + +TEST_F_S_MOCKED(StripTopologyOfUnavailableDevices, NothingStripped) { + const display_device::ActiveTopology input_topology { DEFAULT_INITIAL_TOPOLOGY }; + const display_device::ActiveTopology expected_topology { DEFAULT_INITIAL_TOPOLOGY }; + const display_device::EnumeratedDeviceList devices { + { .m_device_id = "DeviceId1" }, + { .m_device_id = "DeviceId2" }, + { .m_device_id = "DeviceId3" } + }; + + EXPECT_CALL(m_dd_api, enumAvailableDevices()).Times(1).WillOnce(Return(devices)); + EXPECT_EQ(display_device::win_utils::stripTopologyOfUnavailableDevices(m_dd_api, input_topology), expected_topology); +} + TEST_F_S_MOCKED(ComputeInitialState, PreviousStateIsUsed) { const display_device::SingleDisplayConfigState::Initial prev_state { DEFAULT_INITIAL_TOPOLOGY }; EXPECT_EQ(display_device::win_utils::computeInitialState(prev_state, {}, {}), std::make_optional(prev_state));