diff --git a/src/common/include/display_device/types.h b/src/common/include/display_device/types.h index dd49d4a..0d411e3 100644 --- a/src/common/include/display_device/types.h +++ b/src/common/include/display_device/types.h @@ -195,6 +195,7 @@ namespace display_device { std::string m_friendly_name {}; ///< A human-readable name for the device. std::optional m_edid {}; ///< Some basic parsed EDID data. std::optional m_info {}; ///< Additional information about an active display device. + bool m_is_internal {}; ///< Positively identified built-in panel; false also covers unknown connectors. /** * @brief Comparator for strict equality. diff --git a/src/common/json_serializer.cpp b/src/common/json_serializer.cpp index ff56800..4d043fd 100644 --- a/src/common/json_serializer.cpp +++ b/src/common/json_serializer.cpp @@ -20,6 +20,33 @@ namespace display_device { DD_JSON_DEFINE_SERIALIZE_STRUCT(Point, x, y) DD_JSON_DEFINE_SERIALIZE_STRUCT(EdidData, manufacturer_id, product_code, serial_number) DD_JSON_DEFINE_SERIALIZE_STRUCT(EnumeratedDevice::Info, resolution, resolution_scale, refresh_rate, primary, origin_point, hdr_state) - DD_JSON_DEFINE_SERIALIZE_STRUCT(EnumeratedDevice, device_id, display_name, friendly_name, edid, info) + + /** + * @brief Serialize an enumerated display, including its internal-panel classification. + * @param nlohmann_json_j JSON output object. + * @param nlohmann_json_t Device to serialize. + */ + void to_json(nlohmann::json &nlohmann_json_j, const EnumeratedDevice &nlohmann_json_t) { + DD_JSON_TO(device_id) + DD_JSON_TO(display_name) + DD_JSON_TO(friendly_name) + DD_JSON_TO(edid) + DD_JSON_TO(info) + DD_JSON_TO(is_internal) + } + + /** + * @brief Deserialize a display, treating missing internal-panel information as unknown. + * @param nlohmann_json_j JSON input object. + * @param nlohmann_json_t Device to populate. + */ + void from_json(const nlohmann::json &nlohmann_json_j, EnumeratedDevice &nlohmann_json_t) { + DD_JSON_FROM(device_id) + DD_JSON_FROM(display_name) + DD_JSON_FROM(friendly_name) + DD_JSON_FROM(edid) + DD_JSON_FROM(info) + nlohmann_json_t.m_is_internal = nlohmann_json_j.value("is_internal", false); + } DD_JSON_DEFINE_SERIALIZE_STRUCT(SingleDisplayConfiguration, device_id, device_prep, resolution, refresh_rate, hdr_state) } // namespace display_device diff --git a/src/windows/include/display_device/windows/settings_manager.h b/src/windows/include/display_device/windows/settings_manager.h index 3c6971c..d9caca6 100644 --- a/src/windows/include/display_device/windows/settings_manager.h +++ b/src/windows/include/display_device/windows/settings_manager.h @@ -112,6 +112,8 @@ namespace display_device { /** * @brief Try to revert the modified settings. + * If a device from the modified topology is no longer available, only the settings + * of the remaining devices are reverted and the persisted record is left unchanged. * @param current_topology Topology before this method is called. * @param system_settings_touched Indicates whether a "write" operation could have been performed on the OS. * @param switched_topology [Optional] Indicates whether the current topology was switched to revert settings. @@ -148,6 +150,15 @@ namespace display_device { */ [[nodiscard]] RevertResult revertModifiedPrimaryDevice(const SingleDisplayConfigState::Modified &modified_state, DdGuardFn &guard_fn, bool &system_settings_touched); + /** + * @brief Recover to surviving original displays or a built-in panel after an undock. + * Only devices activated by this session may be removed. The original recovery + * record remains intact on failure; a visible replacement is verified first. + * @param current_topology Active topology before attempting to restore the saved layout. + * @return True when the replacement topology has been applied and verified. + */ + [[nodiscard]] bool recoverMissingTopology(const ActiveTopology ¤t_topology); + private: std::shared_ptr m_dd_api; std::shared_ptr m_audio_context_api; diff --git a/src/windows/include/display_device/windows/settings_utils.h b/src/windows/include/display_device/windows/settings_utils.h index 36e63bb..2bcd5f2 100644 --- a/src/windows/include/display_device/windows/settings_utils.h +++ b/src/windows/include/display_device/windows/settings_utils.h @@ -27,6 +27,16 @@ namespace display_device::win_utils { */ StringSet flattenTopology(const ActiveTopology &topology); + /** + * @brief Get the ids of the enumerated devices. + * @param devices Enumerated devices. + * @return Device ids of the enumerated devices. + * @examples + * const auto device_ids { getDeviceIds(win_dd.enumAvailableDevices()) }; + * @examples_end + */ + StringSet getDeviceIds(const EnumeratedDeviceList &devices); + /** * @brief Create extended topology from all the available devices. * @param win_dd Interface for interacting with the OS. diff --git a/src/windows/include/display_device/windows/types.h b/src/windows/include/display_device/windows/types.h index 1a07202..038b0cb 100644 --- a/src/windows/include/display_device/windows/types.h +++ b/src/windows/include/display_device/windows/types.h @@ -146,6 +146,7 @@ namespace display_device { DeviceDisplayModeMap m_original_modes {}; ///< Original display modes before modification. HdrStateMap m_original_hdr_states {}; ///< Original HDR states before modification. std::string m_original_primary_device {}; ///< Original primary device before modification. + std::string m_interim_primary_device {}; ///< Primary device to restore while the original one is unavailable. /** * @brief Check if the changed topology has any other modifications. diff --git a/src/windows/include/display_device/windows/win_display_device.h b/src/windows/include/display_device/windows/win_display_device.h index b12e76b..7d37f06 100644 --- a/src/windows/include/display_device/windows/win_display_device.h +++ b/src/windows/include/display_device/windows/win_display_device.h @@ -19,9 +19,10 @@ namespace display_device { public: /** * Default constructor for the class. + * @param save_to_database Whether CCD changes update Windows' saved display layout. * @param w_api A pointer to the Windows API layer. Will throw on nullptr! */ - explicit WinDisplayDevice(std::shared_ptr w_api); + explicit WinDisplayDevice(std::shared_ptr w_api, bool save_to_database = true); /** * @copydoc WinDisplayDeviceInterface::isApiAccessAvailable @@ -90,5 +91,6 @@ namespace display_device { private: std::shared_ptr m_w_api; + bool m_save_to_database; }; } // namespace display_device diff --git a/src/windows/json_serializer.cpp b/src/windows/json_serializer.cpp index c553dca..e65a7c8 100644 --- a/src/windows/json_serializer.cpp +++ b/src/windows/json_serializer.cpp @@ -13,7 +13,35 @@ namespace display_device { // Structs DD_JSON_DEFINE_SERIALIZE_STRUCT(DisplayMode, resolution, refresh_rate) DD_JSON_DEFINE_SERIALIZE_STRUCT(SingleDisplayConfigState::Initial, topology, primary_devices) - DD_JSON_DEFINE_SERIALIZE_STRUCT(SingleDisplayConfigState::Modified, topology, original_modes, original_hdr_states, original_primary_device) + + /** + * @brief Serialize the modified state, including the interim primary device when there is one. + * @param nlohmann_json_j JSON output object. + * @param nlohmann_json_t Modified state to serialize. + */ + void to_json(nlohmann::json &nlohmann_json_j, const SingleDisplayConfigState::Modified &nlohmann_json_t) { + DD_JSON_TO(topology) + DD_JSON_TO(original_modes) + DD_JSON_TO(original_hdr_states) + DD_JSON_TO(original_primary_device) + if (!nlohmann_json_t.m_interim_primary_device.empty()) { + DD_JSON_TO(interim_primary_device) + } + } + + /** + * @brief Deserialize the modified state, treating a missing interim primary device as none. + * @param nlohmann_json_j JSON input object. + * @param nlohmann_json_t Modified state to populate. + */ + void from_json(const nlohmann::json &nlohmann_json_j, SingleDisplayConfigState::Modified &nlohmann_json_t) { + DD_JSON_FROM(topology) + DD_JSON_FROM(original_modes) + DD_JSON_FROM(original_hdr_states) + DD_JSON_FROM(original_primary_device) + nlohmann_json_t.m_interim_primary_device = nlohmann_json_j.value("interim_primary_device", std::string {}); + } + DD_JSON_DEFINE_SERIALIZE_STRUCT(SingleDisplayConfigState, initial, modified) DD_JSON_DEFINE_SERIALIZE_STRUCT(WinWorkarounds, hdr_blank_delay) } // namespace display_device diff --git a/src/windows/settings_manager_apply.cpp b/src/windows/settings_manager_apply.cpp index 1bddf63..d68d2b0 100644 --- a/src/windows/settings_manager_apply.cpp +++ b/src/windows/settings_manager_apply.cpp @@ -22,6 +22,27 @@ namespace display_device { void noopFn() { // Intentionally empty guard callback. } + + /** + * @brief Split off the cached entries of devices that are not part of the topology being applied. + * + * Such devices, for example an unplugged display, cannot be restored now, so their entries must be carried over untouched. + * @param entries Cached per-device entries. The entries of inactive devices are removed. + * @param active Device IDs of the topology being applied. + * @return The removed entries. + */ + template + Entries takeInactive(Entries &entries, const StringSet &active) { + Entries inactive; + std::erase_if(entries, [&active, &inactive](const auto &entry) { + if (active.contains(entry.first)) { + return false; + } + inactive.insert(entry); + return true; + }); + return inactive; + } } // namespace SettingsManager::ApplyResult SettingsManager::applySettings(const SingleDisplayConfiguration &config) { @@ -206,7 +227,18 @@ namespace display_device { bool 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 {}}; + auto cached_primary_device {cached_state ? cached_state->m_modified.m_original_primary_device : std::string {}}; + const auto cached_interim_device {cached_state ? cached_state->m_modified.m_interim_primary_device : std::string {}}; + std::string pending_primary_device; + if (const auto active_devices {win_utils::flattenTopology(new_state.m_modified.m_topology)}; !cached_primary_device.empty() && !active_devices.contains(cached_primary_device)) { + // The original primary device is inactive (e.g. unplugged), so it can only be restored once it returns. + // Until then, the device that was primary before this happened is the one to go back to. + pending_primary_device = std::move(cached_primary_device); + cached_primary_device = cached_interim_device; + if (!active_devices.contains(cached_primary_device)) { + cached_primary_device.clear(); + } + } const bool ensure_primary {config.m_device_prep == SingleDisplayConfiguration::DevicePreparation::EnsurePrimary}; const bool might_need_to_restore {!cached_primary_device.empty()}; @@ -246,7 +278,12 @@ namespace display_device { } // Here we preserve the data from persistence (unless there's none) as in the end that is what we want to go back to. - new_state.m_modified.m_original_primary_device = original_primary_device; + if (pending_primary_device.empty()) { + new_state.m_modified.m_original_primary_device = original_primary_device; + } else { + new_state.m_modified.m_original_primary_device = pending_primary_device; + new_state.m_modified.m_interim_primary_device = original_primary_device; + } return true; } @@ -255,12 +292,14 @@ namespace display_device { return false; } + new_state.m_modified.m_original_primary_device = pending_primary_device; return true; } bool SettingsManager::prepareDisplayModes(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const StringSet &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 {}}; + auto cached_display_modes {cached_state ? cached_state->m_modified.m_original_modes : DeviceDisplayModeMap {}}; + const auto pending_display_modes {takeInactive(cached_display_modes, win_utils::flattenTopology(new_state.m_modified.m_topology))}; const bool change_required {config.m_resolution || config.m_refresh_rate}; const bool might_need_to_restore {!cached_display_modes.empty()}; @@ -306,6 +345,7 @@ namespace display_device { // Here we preserve the data from persistence (unless there's none) as in the end that is what we want to go back to. new_state.m_modified.m_original_modes = original_display_modes; + new_state.m_modified.m_original_modes.insert(pending_display_modes.begin(), pending_display_modes.end()); return true; } @@ -314,12 +354,14 @@ namespace display_device { return false; } + new_state.m_modified.m_original_modes = pending_display_modes; return true; } [[nodiscard]] bool SettingsManager::prepareHdrStates(const SingleDisplayConfiguration &config, const std::string &device_to_configure, const StringSet &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 {}}; + auto cached_hdr_states {cached_state ? cached_state->m_modified.m_original_hdr_states : HdrStateMap {}}; + const auto pending_hdr_states {takeInactive(cached_hdr_states, win_utils::flattenTopology(new_state.m_modified.m_topology))}; const bool change_required {config.m_hdr_state}; const bool might_need_to_restore {!cached_hdr_states.empty()}; @@ -360,6 +402,7 @@ namespace display_device { // Here we preserve the data from persistence (unless there's none) as in the end that is what we want to go back to. new_state.m_modified.m_original_hdr_states = original_hdr_states; + new_state.m_modified.m_original_hdr_states.insert(pending_hdr_states.begin(), pending_hdr_states.end()); return true; } @@ -368,6 +411,7 @@ namespace display_device { return false; } + new_state.m_modified.m_original_hdr_states = pending_hdr_states; return true; } } // namespace display_device diff --git a/src/windows/settings_manager_revert.cpp b/src/windows/settings_manager_revert.cpp index e9acea0..e28bf9b 100644 --- a/src/windows/settings_manager_revert.cpp +++ b/src/windows/settings_manager_revert.cpp @@ -6,7 +6,10 @@ #include "display_device/windows/settings_manager.h" // system includes +#include #include +#include +#include // local includes #include "display_device/logging.h" @@ -21,6 +24,226 @@ namespace display_device { void noopFn() { // Intentionally empty guard callback. } + + /** + * @brief Append available groups without reactivating stream-only outputs or duplicating devices. + * @param topology Groups to consider, in priority order. + * @param available Currently enumerated device IDs. + * @param activated Device IDs activated only for the stream. + * @param target Recovery topology to extend. + * @param included IDs already included in the recovery topology. + */ + void appendRecoveryGroups(const ActiveTopology &topology, const StringSet &available, const StringSet &activated, ActiveTopology &target, StringSet &included) { + for (const auto &group : topology) { + std::vector remaining; + for (const auto &id : group) { + if (available.contains(id) && !activated.contains(id) && included.insert(id).second) { + remaining.push_back(id); + } + } + if (!remaining.empty()) { + target.push_back(std::move(remaining)); + } + } + } + + /** + * @brief Add the groups of a target topology to a staged topology, keeping clone groups intact. + * + * A target group that shares a device with a staged group extends that group in place instead of + * being added separately. This way the staged topology needs no more sources than necessary. + * Devices that are already staged stay where they are. + * @param target Groups that must be active once staging is done. + * @param staged Topology to extend. + */ + void stageRecoveryGroups(const ActiveTopology &target, ActiveTopology &staged) { + for (const auto &group : target) { + const auto in_group {[&group](const auto &id) { + return std::ranges::find(group, id) != group.end(); + }}; + const auto host {std::ranges::find_if(staged, [&in_group](const auto &staged_group) { + return std::ranges::any_of(staged_group, in_group); + })}; + if (host == staged.end()) { + staged.push_back(group); + continue; + } + + std::vector missing; + for (const auto &id : group) { + if (std::ranges::none_of(staged, [&id](const auto &staged_group) { + return std::ranges::find(staged_group, id) != staged_group.end(); + })) { + missing.push_back(id); + } + } + if (host->size() + missing.size() > 2) { + // Windows cannot clone more than two displays, so the extra members are staged on their own. + if (!missing.empty()) { + staged.push_back(std::move(missing)); + } + continue; + } + + host->insert(host->end(), missing.begin(), missing.end()); + } + } + + /** + * @brief Check whether an original device is away, so that its settings have to wait for its return. + * @param id Device ID to check. + * @param initial IDs of the devices in the initial topology. + * @param available Currently enumerated device IDs. + * @return True if the device is part of the initial topology and is unavailable. + */ + bool isPendingDevice(const std::string &id, const StringSet &initial, const StringSet &available) { + return initial.contains(id) && !available.contains(id); + } + + /** + * @brief Keep only the pending settings of original devices that are currently unavailable. + * + * These settings cannot be restored until the devices return, so they must outlive a topology recovery. + * The modified topology is reduced to those devices, as they are all that is needed to restore the settings. + * The initial topology is extended with the recovered one, so that new settings can still be applied while the devices are away. + * @param state State to filter. + * @param available Currently enumerated device IDs. + * @param recovered_topology Topology that replaced the original one. + * @return State with only the settings of unavailable original devices, or nothing if there are none. + */ + std::optional keepUnavailableSettings(const SingleDisplayConfigState &state, const StringSet &available, const ActiveTopology &recovered_topology) { + const auto initial {win_utils::flattenTopology(state.m_initial.m_topology)}; + + SingleDisplayConfigState result {state}; + std::erase_if(result.m_modified.m_original_modes, [&initial, &available](const auto &entry) { + return !isPendingDevice(entry.first, initial, available); + }); + std::erase_if(result.m_modified.m_original_hdr_states, [&initial, &available](const auto &entry) { + return !isPendingDevice(entry.first, initial, available); + }); + if (!isPendingDevice(result.m_modified.m_original_primary_device, initial, available)) { + result.m_modified.m_original_primary_device.clear(); + } + result.m_modified.m_interim_primary_device.clear(); // Already restored by the revert that led here. + if (!result.m_modified.hasModifications()) { + return std::nullopt; + } + + // Only the returning devices need to be active to restore their settings. + result.m_modified.m_topology.clear(); + StringSet included; + for (const auto &group : state.m_initial.m_topology) { + std::vector pending_group; + for (const auto &id : group) { + if (isPendingDevice(id, initial, available) && included.insert(id).second) { + pending_group.push_back(id); + } + } + if (!pending_group.empty()) { + result.m_modified.m_topology.push_back(std::move(pending_group)); + } + } + + // The original devices alone would leave nothing to build on until one of them returns. + StringSet known {initial}; + for (const auto &group : recovered_topology) { + std::vector recovered_group; + for (const auto &id : group) { + if (known.insert(id).second) { + recovered_group.push_back(id); + } + } + if (!recovered_group.empty()) { + result.m_initial.m_topology.push_back(std::move(recovered_group)); + } + } + return result; + } + + /** + * @brief Find the original devices that have settings recorded but are left out of the modified topology. + * + * These are carried over from an earlier session, e.g. the settings of a display that was unplugged at the time. + * @param state State to inspect. + * @return IDs of the devices that are in the initial topology only. + */ + StringSet findCarriedDevices(const SingleDisplayConfigState &state) { + const auto &modified {state.m_modified}; + StringSet recorded; + for (const auto &id : modified.m_original_modes | std::views::keys) { + recorded.insert(id); + } + for (const auto &id : modified.m_original_hdr_states | std::views::keys) { + recorded.insert(id); + } + recorded.insert(modified.m_original_primary_device); + + const auto initial_ids {win_utils::flattenTopology(state.m_initial.m_topology)}; + const auto topology_ids {win_utils::flattenTopology(modified.m_topology)}; + StringSet carried; + for (const auto &id : recorded) { + if (initial_ids.contains(id) && !topology_ids.contains(id)) { + carried.insert(id); + } + } + return carried; + } + + /** + * @brief Restrict modified settings to the devices that are still available. + * @param modified Modified settings to filter. + * @param available Currently enumerated device IDs. + * @return Modified settings without the unavailable devices. + */ + SingleDisplayConfigState::Modified keepAvailableDevices(const SingleDisplayConfigState::Modified &modified, const StringSet &available) { + SingleDisplayConfigState::Modified result {modified}; + result.m_topology.clear(); + StringSet included; + appendRecoveryGroups(modified.m_topology, available, {}, result.m_topology, included); + std::erase_if(result.m_original_modes, [&available](const auto &entry) { + return !available.contains(entry.first); + }); + std::erase_if(result.m_original_hdr_states, [&available](const auto &entry) { + return !available.contains(entry.first); + }); + if (!available.contains(result.m_original_primary_device)) { + // The interim device is what was primary before the original one went away. + result.m_original_primary_device = available.contains(result.m_interim_primary_device) ? result.m_interim_primary_device : std::string {}; + } + result.m_interim_primary_device.clear(); + return result; + } + + /** + * @brief Activate the carried devices that are back and drop the settings of those that are not. + * @param dd_api Display device API. + * @param state Persisted state that carries the settings. + * @param modified_state Modified settings to prepare for the revert. + * @return True if some carried devices are still unavailable, so their settings have to be kept. + */ + bool prepareCarriedDevices(const WinDisplayDeviceInterface &dd_api, const SingleDisplayConfigState &state, SingleDisplayConfigState::Modified &modified_state) { + const auto carried {findCarriedDevices(state)}; + if (carried.empty()) { + return false; + } + + // Settings carried over from an earlier session can only be restored once their devices are active. + const auto available {win_utils::getDeviceIds(dd_api.enumAvailableDevices())}; + if (available.empty()) { + return false; + } + + bool some_unavailable {false}; + for (const auto &id : carried) { + if (available.contains(id)) { + modified_state.m_topology.push_back({id}); + } else { + some_unavailable = true; + } + } + modified_state = keepAvailableDevices(modified_state, available); + return some_unavailable; + } } // namespace SettingsManager::RevertResult SettingsManager::revertSettings() { @@ -79,13 +302,19 @@ namespace display_device { const bool is_topology_the_same {m_dd_api->isTopologyTheSame(current_topology, cached_state->m_initial.m_topology)}; const bool need_to_switch_topology {!is_topology_the_same || switched_to_modified_topology}; system_settings_touched = system_settings_touched || !is_topology_the_same; + std::optional pending_state; if (need_to_switch_topology && !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 RevertResult::SwitchingTopologyFailed; + if (!recoverMissingTopology(current_topology)) { + return RevertResult::SwitchingTopologyFailed; + } + + // Original devices that are still unplugged keep their settings until they return. + pending_state = keepUnavailableSettings(*cached_state, win_utils::getDeviceIds(m_dd_api->enumAvailableDevices()), m_dd_api->getCurrentTopology()); } - if (!m_persistence_state->persistState(std::nullopt)) { + if (!m_persistence_state->persistState(pending_state)) { DD_LOG(error) << "Failed to save reverted settings! Undoing initial topology changes..."; return RevertResult::PersistenceSaveFailed; } @@ -99,6 +328,59 @@ namespace display_device { return RevertResult::Ok; } + bool SettingsManager::recoverMissingTopology(const ActiveTopology ¤t_topology) { + const auto &state {*m_persistence_state->getState()}; + const auto initial {win_utils::flattenTopology(state.m_initial.m_topology)}; + const auto modified {win_utils::flattenTopology(state.m_modified.m_topology)}; + const auto devices {m_dd_api->enumAvailableDevices()}; + const auto available {win_utils::getDeviceIds(devices)}; + if (available.empty() || std::ranges::all_of(initial, [&available](const auto &id) { + return available.contains(id); + })) { + // An API failure with unchanged hardware must not discard the original state. + return false; + } + + StringSet activated; + std::ranges::set_difference(modified, initial, std::inserter(activated, activated.end())); + ActiveTopology target; + StringSet included; + appendRecoveryGroups(state.m_initial.m_topology, available, activated, target, included); + if (target.empty()) { + // An arbitrary virtual/unknown output is not proof of a usable laptop screen. + for (const auto &device : devices) { + if (device.m_is_internal && !activated.contains(device.m_device_id)) { + appendRecoveryGroups({{device.m_device_id}}, available, activated, target, included); + break; + } + } + } + if (target.empty()) { + return false; // Closed lid and no original display: retain pending recovery. + } + const auto replacements {win_utils::flattenTopology(target)}; + appendRecoveryGroups(current_topology, available, activated, target, included); // Preserve unrelated displays activated by the user. + + // First activate the replacement without switching off the current output. + // Keep the target's groups intact, even when a member is already active, since cloned displays share a source. + ActiveTopology staged {current_topology}; + stageRecoveryGroups(target, staged); + if (!m_dd_api->setTopology(staged)) { + return false; + } + if (const auto active {win_utils::flattenTopology(m_dd_api->getCurrentTopology())}; !std::ranges::all_of(replacements, [&active](const auto &id) { + return active.contains(id); + })) { + return false; + } + if (!m_dd_api->setTopology(target) || !m_dd_api->isTopologyTheSame(m_dd_api->getCurrentTopology(), target)) { + return false; + } + DD_LOG(info) << "Recovered display topology after original devices disappeared:\n" + << toJson(target); + return true; + } + SettingsManager::RevertResult SettingsManager::revertModifiedHdrStates(const SingleDisplayConfigState::Modified &modified_state, DdGuardFn &guard_fn, bool &system_settings_touched) { using enum RevertResult; @@ -191,12 +473,39 @@ namespace display_device { return RevertResult::TopologyIsInvalid; } - const bool is_topology_the_same {m_dd_api->isTopologyTheSame(current_topology, cached_state->m_modified.m_topology)}; + auto modified_state {cached_state->m_modified}; + bool keep_record {prepareCarriedDevices(*m_dd_api, *cached_state, modified_state)}; + bool is_topology_the_same {m_dd_api->isTopologyTheSame(current_topology, modified_state.m_topology)}; system_settings_touched = !is_topology_the_same; - if (!is_topology_the_same && !m_dd_api->setTopology(cached_state->m_modified.m_topology)) { + if (!is_topology_the_same && !m_dd_api->setTopology(modified_state.m_topology)) { DD_LOG(error) << "Failed to change topology to:\n" - << toJson(cached_state->m_modified.m_topology); - return RevertResult::SwitchingTopologyFailed; + << toJson(modified_state.m_topology); + + const auto available {win_utils::getDeviceIds(m_dd_api->enumAvailableDevices())}; + const auto modified_ids {win_utils::flattenTopology(modified_state.m_topology)}; + if (const bool devices_missing {!available.empty() && std::ranges::any_of(modified_ids, [&available](const auto &id) { + return !available.contains(id); + })}; + !devices_missing) { + return RevertResult::SwitchingTopologyFailed; + } + keep_record = true; + + // A display from the modified topology has been unplugged. Revert the settings of the remaining + // displays so that the initial topology, or its recovery, can still be applied. + modified_state = keepAvailableDevices(modified_state, available); + DD_LOG(warning) << "Some modified devices are unavailable, reverting settings for the remaining topology:\n" + << toJson(modified_state.m_topology); + if (modified_state.m_topology.empty()) { + return RevertResult::Ok; + } + + is_topology_the_same = m_dd_api->isTopologyTheSame(current_topology, modified_state.m_topology); + if (!is_topology_the_same && !m_dd_api->setTopology(modified_state.m_topology)) { + DD_LOG(error) << "Failed to change topology to:\n" + << toJson(modified_state.m_topology); + return RevertResult::SwitchingTopologyFailed; + } } if (switched_topology) { *switched_topology = !is_topology_the_same; @@ -204,25 +513,33 @@ namespace display_device { DdGuardFn hdr_guard_fn {noopFn}; boost::scope::scope_exit hdr_guard {hdr_guard_fn}; - if (const auto result {revertModifiedHdrStates(cached_state->m_modified, hdr_guard_fn, system_settings_touched)}; result != RevertResult::Ok) { + if (const auto result {revertModifiedHdrStates(modified_state, hdr_guard_fn, system_settings_touched)}; result != RevertResult::Ok) { // Error already logged return result; } DdGuardFn mode_guard_fn {noopFn}; boost::scope::scope_exit mode_guard {mode_guard_fn}; - if (const auto result {revertModifiedDisplayModes(cached_state->m_modified, mode_guard_fn, system_settings_touched)}; result != RevertResult::Ok) { + if (const auto result {revertModifiedDisplayModes(modified_state, mode_guard_fn, system_settings_touched)}; result != RevertResult::Ok) { // Error already logged return result; } DdGuardFn primary_guard_fn {noopFn}; boost::scope::scope_exit primary_guard {primary_guard_fn}; - if (const auto result {revertModifiedPrimaryDevice(cached_state->m_modified, primary_guard_fn, system_settings_touched)}; result != RevertResult::Ok) { + if (const auto result {revertModifiedPrimaryDevice(modified_state, primary_guard_fn, system_settings_touched)}; result != RevertResult::Ok) { // Error already logged return result; } + if (keep_record) { + // Keep the full record so that the unplugged displays can still be reverted if they return. + hdr_guard.set_active(false); + mode_guard.set_active(false); + primary_guard.set_active(false); + return RevertResult::Ok; + } + auto cleared_data {*cached_state}; cleared_data.m_modified = {cleared_data.m_modified.m_topology}; if (!m_persistence_state->persistState(cleared_data)) { diff --git a/src/windows/settings_utils.cpp b/src/windows/settings_utils.cpp index a2798bc..e978f03 100644 --- a/src/windows/settings_utils.cpp +++ b/src/windows/settings_utils.cpp @@ -98,6 +98,15 @@ namespace display_device::win_utils { return flattened_topology; } + StringSet getDeviceIds(const EnumeratedDeviceList &devices) { + StringSet ids; + for (const auto &device : devices) { + ids.insert(device.m_device_id); + } + + return ids; + } + ActiveTopology createFullExtendedTopology(const WinDisplayDeviceInterface &win_dd) { const auto devices {win_dd.enumAvailableDevices()}; if (devices.empty()) { diff --git a/src/windows/win_display_device_general.cpp b/src/windows/win_display_device_general.cpp index e75ad59..afa9984 100644 --- a/src/windows/win_display_device_general.cpp +++ b/src/windows/win_display_device_general.cpp @@ -13,8 +13,9 @@ #include "display_device/windows/win_api_utils.h" namespace display_device { - WinDisplayDevice::WinDisplayDevice(std::shared_ptr w_api): - m_w_api {std::move(w_api)} { + WinDisplayDevice::WinDisplayDevice(std::shared_ptr w_api, bool save_to_database): + m_w_api {std::move(w_api)}, + m_save_to_database {save_to_database} { if (!m_w_api) { throw std::invalid_argument {"Nullptr provided for WinApiLayerInterface in WinDisplayDevice!"}; } @@ -53,6 +54,8 @@ namespace display_device { const auto source_mode {is_active ? win_utils::getSourceMode(win_utils::getSourceIndex(best_path, display_data->m_modes), display_data->m_modes) : nullptr}; const auto display_name {is_active ? m_w_api->getDisplayName(best_path) : std::string {}}; // Inactive devices can have multiple display names, so it's just meaningless use any const auto edid {EdidData::parse(m_w_api->getEdid(best_path))}; + const auto technology {best_path.targetInfo.outputTechnology}; + const bool is_internal {technology == DISPLAYCONFIG_OUTPUT_TECHNOLOGY_INTERNAL || technology == DISPLAYCONFIG_OUTPUT_TECHNOLOGY_LVDS || technology == DISPLAYCONFIG_OUTPUT_TECHNOLOGY_DISPLAYPORT_EMBEDDED || technology == DISPLAYCONFIG_OUTPUT_TECHNOLOGY_UDI_EMBEDDED}; if (is_active && !source_mode) { DD_LOG(warning) << "Device " << device_id << " is missing source mode!"; @@ -70,10 +73,10 @@ namespace display_device { }; // 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(cpp:S6003): Direct emplace_back args fail on older toolchains + available_devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, info, is_internal}); // NOSONAR(cpp:S6003): Direct emplace_back args fail on older toolchains } else { // 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(cpp:S6003): Direct emplace_back args fail on older toolchains + available_devices.push_back(EnumeratedDevice {device_id, display_name, friendly_name, edid, std::nullopt, is_internal}); // NOSONAR(cpp:S6003): Direct emplace_back args fail on older toolchains } } diff --git a/src/windows/win_display_device_modes.cpp b/src/windows/win_display_device_modes.cpp index 9c71f58..ec9a7ad 100644 --- a/src/windows/win_display_device_modes.cpp +++ b/src/windows/win_display_device_modes.cpp @@ -26,7 +26,7 @@ namespace display_device { /** * @see set_display_modes for a description as this was split off to reduce cognitive complexity. */ - bool doSetModes(WinApiLayerInterface &w_api, const DeviceDisplayModeMap &modes, const Strategy strategy) { + bool doSetModes(WinApiLayerInterface &w_api, const DeviceDisplayModeMap &modes, const Strategy strategy, const bool save_to_database) { auto display_data {w_api.queryDisplayConfig(QueryType::Active)}; if (!display_data) { // Error already logged @@ -85,7 +85,7 @@ namespace display_device { return true; } - UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_SAVE_TO_DATABASE | SDC_VIRTUAL_MODE_AWARE}; + UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | (save_to_database ? SDC_SAVE_TO_DATABASE : 0u) | SDC_VIRTUAL_MODE_AWARE}; if (strategy == Strategy::Relaxed) { // It's probably best for Windows to select the "best" display settings for us. However, in case we // have custom resolution set in nvidia control panel for example, this flag will prevent successfully applying @@ -182,7 +182,7 @@ namespace display_device { return false; } - if (!doSetModes(*m_w_api, modes, Strategy::Relaxed)) { + if (!doSetModes(*m_w_api, modes, Strategy::Relaxed, m_save_to_database)) { // Error already logged return false; } @@ -219,7 +219,7 @@ namespace display_device { // resolution to be selected, we actually need to omit SDC_ALLOW_CHANGES // flag. DD_LOG(info) << "Failed to change display modes using Windows recommended modes, trying to set modes more strictly!"; - if (doSetModes(*m_w_api, modes, Strategy::Strict)) { + if (doSetModes(*m_w_api, modes, Strategy::Strict, m_save_to_database)) { current_modes = getCurrentDisplayModes(device_ids); if (!current_modes.empty() && all_modes_match(current_modes)) { return true; @@ -227,7 +227,7 @@ namespace display_device { } } - const UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_SAVE_TO_DATABASE | SDC_VIRTUAL_MODE_AWARE}; + const UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | (m_save_to_database ? SDC_SAVE_TO_DATABASE : 0u) | SDC_VIRTUAL_MODE_AWARE}; static_cast(m_w_api->setDisplayConfig(original_data->m_paths, original_data->m_modes, flags)); // Return value does not matter as we are trying out best to undo DD_LOG(error) << "Failed to set display mode(-s) completely!"; return false; diff --git a/src/windows/win_display_device_primary.cpp b/src/windows/win_display_device_primary.cpp index 92ee964..78bf874 100644 --- a/src/windows/win_display_device_primary.cpp +++ b/src/windows/win_display_device_primary.cpp @@ -97,7 +97,7 @@ namespace display_device { modified_modes.insert(*source_index); } - const UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_SAVE_TO_DATABASE | SDC_VIRTUAL_MODE_AWARE}; + const UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | (m_save_to_database ? SDC_SAVE_TO_DATABASE : 0u) | SDC_VIRTUAL_MODE_AWARE}; if (const LONG result {m_w_api->setDisplayConfig(display_data->m_paths, display_data->m_modes, flags)}; result != ERROR_SUCCESS) { DD_LOG(error) << m_w_api->getErrorString(result) << " failed to set primary mode for " << device_id << "!"; return false; diff --git a/src/windows/win_display_device_topology.cpp b/src/windows/win_display_device_topology.cpp index 9d938f2..e530ff0 100644 --- a/src/windows/win_display_device_topology.cpp +++ b/src/windows/win_display_device_topology.cpp @@ -19,7 +19,7 @@ namespace display_device { /** * @see set_topology for a description as this was split off to reduce cognitive complexity. */ - bool doSetTopology(WinApiLayerInterface &w_api, const ActiveTopology &new_topology, const PathAndModeData &display_data) { + bool doSetTopology(WinApiLayerInterface &w_api, const ActiveTopology &new_topology, const PathAndModeData &display_data, const bool save_to_database) { const auto path_data {win_utils::collectSourceDataForMatchingPaths(w_api, display_data.m_paths)}; if (path_data.empty()) { // Error already logged @@ -32,6 +32,17 @@ namespace display_device { return false; } + if (!save_to_database) { + // TOPOLOGY_SUPPLIED also changes the remembered topology. Session-only + // layouts must use the explicitly temporary supplied-config API path. + const UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_ALLOW_CHANGES | SDC_VIRTUAL_MODE_AWARE}; + if (const LONG result {w_api.setDisplayConfig(paths, {}, flags)}; result != ERROR_SUCCESS) { + DD_LOG(error) << w_api.getErrorString(result) << " failed to set temporary topology!"; + return false; + } + return true; + } + UINT32 flags {SDC_APPLY | SDC_TOPOLOGY_SUPPLIED | SDC_ALLOW_PATH_ORDER_CHANGES | SDC_VIRTUAL_MODE_AWARE}; if (LONG result {w_api.setDisplayConfig(paths, {}, flags)}; result == ERROR_GEN_FAILURE) { DD_LOG(warning) << w_api.getErrorString(result) << " failed to change topology using the topology from Windows DB! Asking Windows to create the topology."; @@ -158,7 +169,7 @@ namespace display_device { return false; } - if (doSetTopology(*m_w_api, new_topology, *original_data)) { + if (doSetTopology(*m_w_api, new_topology, *original_data, m_save_to_database)) { if (const auto updated_topology {getCurrentTopology()}; isTopologyValid(updated_topology)) { if (isTopologyTheSame(new_topology, updated_topology)) { return true; @@ -194,7 +205,7 @@ namespace display_device { } // Revert back to the original topology - const UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_SAVE_TO_DATABASE | SDC_VIRTUAL_MODE_AWARE}; + const UINT32 flags {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | (m_save_to_database ? SDC_SAVE_TO_DATABASE : 0u) | SDC_VIRTUAL_MODE_AWARE}; static_cast(m_w_api->setDisplayConfig(original_data->m_paths, original_data->m_modes, flags)); // Return value does not matter as we are trying out best to undo } diff --git a/tests/unit/general/test_json_converter.cpp b/tests/unit/general/test_json_converter.cpp index 8c7d8dc..4861566 100644 --- a/tests/unit/general/test_json_converter.cpp +++ b/tests/unit/general/test_json_converter.cpp @@ -58,9 +58,9 @@ TEST_F_S(EnumeratedDevice) { const auto item_1 {makeEnumeratedDeviceOne()}; const auto item_2 {makeEnumeratedDeviceTwo()}; - executeTestCase(display_device::EnumeratedDevice {}, R"({"device_id":"","display_name":"","edid":null,"friendly_name":"","info":null})"); - executeTestCase(item_1, R"({"device_id":"ID_1","display_name":"NAME_2","edid":null,"friendly_name":"FU_NAME_3","info":{"hdr_state":"Enabled","origin_point":{"x":1,"y":2},"primary":false,"refresh_rate":{"type":"double","value":119.9554},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"rational","value":{"denominator":100,"numerator":175}}}})"); - executeTestCase(item_2, R"({"device_id":"ID_2","display_name":"NAME_2","edid":{"manufacturer_id":"","product_code":"","serial_number":0},"friendly_name":"FU_NAME_2","info":{"hdr_state":"Disabled","origin_point":{"x":0,"y":0},"primary":true,"refresh_rate":{"type":"rational","value":{"denominator":10000,"numerator":1199554}},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"double","value":1.75}}})"); + executeTestCase(display_device::EnumeratedDevice {}, R"({"device_id":"","display_name":"","edid":null,"friendly_name":"","info":null,"is_internal":false})"); + executeTestCase(item_1, R"({"device_id":"ID_1","display_name":"NAME_2","edid":null,"friendly_name":"FU_NAME_3","info":{"hdr_state":"Enabled","origin_point":{"x":1,"y":2},"primary":false,"refresh_rate":{"type":"double","value":119.9554},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"rational","value":{"denominator":100,"numerator":175}}},"is_internal":false})"); + executeTestCase(item_2, R"({"device_id":"ID_2","display_name":"NAME_2","edid":{"manufacturer_id":"","product_code":"","serial_number":0},"friendly_name":"FU_NAME_2","info":{"hdr_state":"Disabled","origin_point":{"x":0,"y":0},"primary":true,"refresh_rate":{"type":"rational","value":{"denominator":10000,"numerator":1199554}},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"double","value":1.75}},"is_internal":false})"); executeInvalidJsonTestCase(); executeFromJsonFailureTestCase(R"({})"); executeFromJsonFailureTestCase(R"({"device_id":"","display_name":"","edid":null,"friendly_name":"","info":{"hdr_state":null,"origin_point":{"x":0,"y":0},"primary":false,"refresh_rate":{"type":"unknown","value":0},"resolution":{"height":0,"width":0},"resolution_scale":{"type":"double","value":1.0}}})"); @@ -73,9 +73,9 @@ TEST_F_S(EnumeratedDeviceList) { display_device::EnumeratedDevice item_3 {}; executeTestCase(display_device::EnumeratedDeviceList {}, R"([])"); - executeTestCase(display_device::EnumeratedDeviceList {item_1, item_2, item_3}, R"([{"device_id":"ID_1","display_name":"NAME_2","edid":null,"friendly_name":"FU_NAME_3","info":{"hdr_state":"Enabled","origin_point":{"x":1,"y":2},"primary":false,"refresh_rate":{"type":"double","value":119.9554},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"rational","value":{"denominator":100,"numerator":175}}}},)" - R"({"device_id":"ID_2","display_name":"NAME_2","edid":{"manufacturer_id":"","product_code":"","serial_number":0},"friendly_name":"FU_NAME_2","info":{"hdr_state":"Disabled","origin_point":{"x":0,"y":0},"primary":true,"refresh_rate":{"type":"rational","value":{"denominator":10000,"numerator":1199554}},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"double","value":1.75}}},)" - R"({"device_id":"","display_name":"","edid":null,"friendly_name":"","info":null}])"); + executeTestCase(display_device::EnumeratedDeviceList {item_1, item_2, item_3}, R"([{"device_id":"ID_1","display_name":"NAME_2","edid":null,"friendly_name":"FU_NAME_3","info":{"hdr_state":"Enabled","origin_point":{"x":1,"y":2},"primary":false,"refresh_rate":{"type":"double","value":119.9554},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"rational","value":{"denominator":100,"numerator":175}}},"is_internal":false},)" + R"({"device_id":"ID_2","display_name":"NAME_2","edid":{"manufacturer_id":"","product_code":"","serial_number":0},"friendly_name":"FU_NAME_2","info":{"hdr_state":"Disabled","origin_point":{"x":0,"y":0},"primary":true,"refresh_rate":{"type":"rational","value":{"denominator":10000,"numerator":1199554}},"resolution":{"height":1080,"width":1920},"resolution_scale":{"type":"double","value":1.75}},"is_internal":false},)" + R"({"device_id":"","display_name":"","edid":null,"friendly_name":"","info":null,"is_internal":false}])"); executeInvalidJsonTestCase(); executeFromJsonFailureTestCase(R"([{}])"); executeToJsonFailureTestCase(display_device::EnumeratedDeviceList {display_device::EnumeratedDevice {.m_device_id = "ID\xC2"}}); @@ -125,3 +125,11 @@ TEST_F_S(Bool) { executeInvalidJsonTestCase(); executeFromJsonFailureTestCase(R"("not-bool")"); } + +TEST_F_S(EnumeratedDeviceInternalPanelMetadata) { + display_device::EnumeratedDevice device {.m_is_internal = true}; + EXPECT_TRUE(display_device::fromJson(R"({"device_id":"","display_name":"","edid":null,"friendly_name":"","info":null})", device)); + EXPECT_FALSE(device.m_is_internal); + device.m_is_internal = true; + executeTestCase(device, R"({"device_id":"","display_name":"","edid":null,"friendly_name":"","info":null,"is_internal":true})"); +} diff --git a/tests/unit/windows/test_json_converter.cpp b/tests/unit/windows/test_json_converter.cpp index 5f2afd4..67812e7 100644 --- a/tests/unit/windows/test_json_converter.cpp +++ b/tests/unit/windows/test_json_converter.cpp @@ -45,6 +45,7 @@ TEST_F_S(SingleDisplayConfigState) { executeTestCase(display_device::SingleDisplayConfigState {}, R"({"initial":{"primary_devices":[],"topology":[]},"modified":{"original_hdr_states":{},"original_modes":{},"original_primary_device":"","topology":[]}})"); executeTestCase(valid_input, R"({"initial":{"primary_devices":["DeviceId1"],"topology":[["DeviceId1"]]},"modified":{"original_hdr_states":{"DeviceId2":"Disabled"},"original_modes":{"DeviceId2":{"refresh_rate":{"denominator":1,"numerator":120},"resolution":{"height":1080,"width":1920}}},"original_primary_device":"DeviceId2","topology":[["DeviceId2"]]}})"); + executeTestCase(display_device::SingleDisplayConfigState {.m_modified = {.m_original_primary_device = "DeviceId2", .m_interim_primary_device = "DeviceId3"}}, R"({"initial":{"primary_devices":[],"topology":[]},"modified":{"interim_primary_device":"DeviceId3","original_hdr_states":{},"original_modes":{},"original_primary_device":"DeviceId2","topology":[]}})"); executeInvalidJsonTestCase(); executeFromJsonFailureTestCase(R"({})"); executeToJsonFailureTestCase(display_device::SingleDisplayConfigState { diff --git a/tests/unit/windows/test_settings_manager_revert.cpp b/tests/unit/windows/test_settings_manager_revert.cpp index 50e9cb1..fc30ff7 100644 --- a/tests/unit/windows/test_settings_manager_revert.cpp +++ b/tests/unit/windows/test_settings_manager_revert.cpp @@ -340,6 +340,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToSetModifiedTopology) { EXPECT_CALL(*m_dd_api, setTopology(ut_consts::SDCS_FULL->m_modified.m_topology)) .Times(1) .WillOnce(Return(false)); + EXPECT_CALL(*m_dd_api, enumAvailableDevices()).WillOnce(Return(CURRENT_DEVICES)); expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); @@ -529,6 +530,7 @@ TEST_F_S_MOCKED(FailedToSetInitialTopology) { .WillOnce(Return(false)) .RetiresOnSaturation(); + EXPECT_CALL(*m_dd_api, enumAvailableDevices()).WillOnce(Return(CURRENT_DEVICES)); expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); diff --git a/tests/unit/windows/test_settings_manager_undock.cpp b/tests/unit/windows/test_settings_manager_undock.cpp new file mode 100644 index 0000000..dd71ddc --- /dev/null +++ b/tests/unit/windows/test_settings_manager_undock.cpp @@ -0,0 +1,378 @@ +/** + * @file tests/unit/windows/test_settings_manager_undock.cpp + * @brief Tests for display recovery across laptop lid and docking changes. + */ +#include "display_device/windows/settings_manager.h" +#include "display_device/windows/settings_utils.h" +#include "fixtures/fixtures.h" +#include "fixtures/mock_audio_context.h" +#include "fixtures/mock_settings_persistence.h" +#include "utils/helpers.h" +#include "utils/mock_win_display_device.h" + +#include +#include + +namespace { + using namespace display_device; + using ::testing::_; + using ::testing::NiceMock; + using ::testing::Return; + + class UndockRecovery: public BaseTest { + public: + ActiveTopology active {{"stream"}}; + EnumeratedDeviceList devices {{.m_device_id = "stream"}}; + SingleDisplayConfigState state {{{{"dock"}}, {"dock"}}, {{{"stream"}}, {}, {}, {}}}; + std::vector writes; + bool fail_activation {}; + bool lie_about_activation {}; + bool fail_cleanup {}; + bool fail_clear {}; + bool lie_about_cleanup {}; + bool split_clones {}; + int clears {}; + std::shared_ptr> api = std::make_shared>(); + std::shared_ptr> persistence = std::make_shared>(); + std::shared_ptr> audio = std::make_shared>(); + std::unique_ptr manager; + + /** + * @brief Simulate topology writes, including unavailable devices and unreliable API results. + * @param target Requested active display groups. + * @return Whether the simulated API reports success. + */ + bool applyTopology(const ActiveTopology &target) { + writes.push_back(target); + const auto ids = win_utils::flattenTopology(target); + if (ids.empty()) { + ADD_FAILURE() << "Attempted to blank every output"; + return false; + } + for (const auto &id : ids) { + if (std::ranges::none_of(devices, [&id](const auto &d) { + return d.m_device_id == id; + })) { + return false; + } + } + if (fail_activation && ids.contains("panel")) { + return false; + } + if (split_clones && ids.contains("left") && ids.contains("right") && std::ranges::none_of(target, [](const auto &group) { + return std::ranges::find(group, "left") != group.end() && std::ranges::find(group, "right") != group.end(); + })) { + return false; // The adapter cannot drive both members of the clone group from separate sources. + } + if (fail_cleanup && !ids.contains("stream")) { + return false; + } + if (!lie_about_activation && !(lie_about_cleanup && !ids.contains("stream"))) { + active = target; + } + return true; + } + + /** @brief Connect the stateful display and persistence mocks to the manager. */ + void init() { + ON_CALL(*api, isApiAccessAvailable()).WillByDefault(Return(true)); + ON_CALL(*api, enumAvailableDevices()).WillByDefault([this] { + return devices; + }); + ON_CALL(*api, getCurrentTopology()).WillByDefault([this] { + return active; + }); + ON_CALL(*api, isTopologyValid(_)).WillByDefault([](const auto &t) { + return !t.empty(); + }); + ON_CALL(*api, isTopologyTheSame(_, _)).WillByDefault([](const auto &a, const auto &b) { + return win_utils::flattenTopology(a) == win_utils::flattenTopology(b); + }); + ON_CALL(*api, setTopology(_)).WillByDefault([this](const ActiveTopology &target) { + return applyTopology(target); + }); + ON_CALL(*persistence, load()).WillByDefault([this] { + return serializeState(state); + }); + ON_CALL(*persistence, store(_)).WillByDefault(Return(true)); + ON_CALL(*persistence, clear()).WillByDefault([this] { + if (fail_clear) { + return false; + } + ++clears; + return true; + }); + manager = std::make_unique(api, audio, std::make_unique(persistence), WinWorkarounds {}); + } + + /** @brief Make the built-in panel available after opening the lid. */ + void openLid() { + devices.push_back({.m_device_id = "panel", .m_is_internal = true}); + } + }; +} // namespace + +TEST_F(UndockRecovery, ClosedLidRetainsRecoveryUntilPanelAppears) { + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + EXPECT_EQ(clears, 0); + EXPECT_EQ(active, (ActiveTopology {{"stream"}})); + openLid(); + writes.clear(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"panel"}})); + ASSERT_GE(writes.size(), 3u); + EXPECT_EQ(win_utils::flattenTopology(writes[writes.size() - 2]), (StringSet {"panel", "stream"})); + EXPECT_EQ(clears, 1); + writes.clear(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_TRUE(writes.empty()); +} + +TEST_F(UndockRecovery, RecoverySurvivesControllerRestart) { + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + manager.reset(); + openLid(); + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"panel"}})); +} + +TEST_F(UndockRecovery, RedockRestoresOriginalOutput) { + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + devices.push_back({.m_device_id = "dock"}); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"dock"}})); + EXPECT_EQ(clears, 1); +} + +TEST_F(UndockRecovery, PreservesUnrelatedActiveOutput) { + openLid(); + devices.push_back({.m_device_id = "other"}); + active.push_back({"other"}); + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(win_utils::flattenTopology(active), (StringSet {"panel", "other"})); +} + +TEST_F(UndockRecovery, RestoresSurvivingOriginalDisplay) { + state.m_initial.m_topology.push_back({"survivor"}); + devices.push_back({.m_device_id = "survivor"}); + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"survivor"}})); +} + +TEST_F(UndockRecovery, DoesNotMistakeUnknownOutputForLaptopPanel) { + devices.push_back({.m_device_id = "unknown"}); + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + EXPECT_EQ(clears, 0); +} + +TEST_F(UndockRecovery, ActivationFailureDoesNotRemoveStreamingOutput) { + openLid(); + fail_activation = true; + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + EXPECT_EQ(clears, 0); + EXPECT_TRUE(win_utils::flattenTopology(active).contains("stream")); +} + +TEST_F(UndockRecovery, ActivationRequiresReadback) { + openLid(); + lie_about_activation = true; + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + EXPECT_EQ(clears, 0); + for (const auto &target : writes) { + if (!win_utils::flattenTopology(target).contains("dock")) { + EXPECT_TRUE(win_utils::flattenTopology(target).contains("stream")); + } + } +} + +TEST_F(UndockRecovery, CleanupFailureRetainsPersistence) { + openLid(); + fail_cleanup = true; + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + EXPECT_EQ(clears, 0); + EXPECT_TRUE(win_utils::flattenTopology(active).contains("panel")); +} + +TEST_F(UndockRecovery, FailedPersistenceCanRetry) { + openLid(); + fail_clear = true; + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::PersistenceSaveFailed); + EXPECT_EQ(clears, 0); + fail_clear = false; + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"panel"}})); + EXPECT_EQ(clears, 1); +} + +TEST_F(UndockRecovery, CleanupRequiresReadback) { + openLid(); + lie_about_cleanup = true; + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + EXPECT_EQ(clears, 0); + EXPECT_TRUE(win_utils::flattenTopology(active).contains("stream")); +} + +TEST_F(UndockRecovery, UnpluggedDisplayInModifiedTopologyDoesNotBlockRecovery) { + const DisplayMode dock_mode {{3840, 2160}, {60, 1}}; + const DisplayMode stream_mode {{1920, 1080}, {60, 1}}; + state.m_modified = {{{"dock"}, {"stream"}}, {{"dock", dock_mode}, {"stream", stream_mode}}, {{"dock", HdrState::Enabled}, {"stream", HdrState::Disabled}}, "dock"}; + std::vector mode_writes; + std::vector hdr_writes; + init(); + ON_CALL(*api, setDisplayModes(_)).WillByDefault([&mode_writes](const DeviceDisplayModeMap &modes) { + mode_writes.push_back(modes); + return true; + }); + ON_CALL(*api, setHdrStates(_)).WillByDefault([&hdr_writes](const HdrStateMap &states) { + hdr_writes.push_back(states); + return true; + }); + EXPECT_CALL(*api, setAsPrimary(_)).Times(0); + + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::SwitchingTopologyFailed); + EXPECT_EQ(clears, 0); + EXPECT_EQ(active, (ActiveTopology {{"stream"}})); + ASSERT_FALSE(mode_writes.empty()); + EXPECT_EQ(mode_writes.front(), (DeviceDisplayModeMap {{"stream", stream_mode}})); + ASSERT_FALSE(hdr_writes.empty()); + EXPECT_EQ(hdr_writes.front(), (HdrStateMap {{"stream", HdrState::Disabled}})); + + openLid(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"panel"}})); + EXPECT_EQ(clears, 0); // The dock display still has settings to restore. + + // Redocking restores what the unplugged display had before the stream. + devices.push_back({.m_device_id = "dock"}); + mode_writes.clear(); + hdr_writes.clear(); + ::testing::Mock::VerifyAndClearExpectations(api.get()); + EXPECT_CALL(*api, setAsPrimary("dock")).WillOnce(Return(true)); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + ASSERT_FALSE(mode_writes.empty()); + EXPECT_EQ(mode_writes.front(), (DeviceDisplayModeMap {{"dock", dock_mode}})); + ASSERT_FALSE(hdr_writes.empty()); + EXPECT_EQ(hdr_writes.front(), (HdrStateMap {{"dock", HdrState::Enabled}})); + EXPECT_EQ(win_utils::flattenTopology(active), (StringSet {"dock", "panel"})); + EXPECT_EQ(clears, 1); +} + +TEST_F(UndockRecovery, DoesNotRetainSettingsOfStreamOnlyDisplay) { + const DisplayMode stream_mode {{1920, 1080}, {60, 1}}; + state.m_modified = {{{"stream"}}, {{"stream", stream_mode}}, {}, {}}; + openLid(); + init(); + ON_CALL(*api, setDisplayModes(_)).WillByDefault(Return(true)); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"panel"}})); + EXPECT_EQ(clears, 1); +} + +TEST_F(UndockRecovery, PendingSettingsSurviveApplyWhileStillUndocked) { + const DisplayMode dock_mode {{3840, 2160}, {60, 1}}; + state.m_initial = {{{"dock"}, {"panel"}}, {"dock"}}; // As retained after a recovery to the panel. + state.m_modified = {{{"dock"}}, {{"dock", dock_mode}}, {{"dock", HdrState::Enabled}}, "dock"}; + active = {{"panel"}}; + devices = {{.m_device_id = "panel", .m_info = EnumeratedDevice::Info {.m_primary = true}, .m_is_internal = true}}; + std::vector mode_writes; + init(); + ON_CALL(*api, setDisplayModes(_)).WillByDefault([&mode_writes](const DeviceDisplayModeMap &modes) { + mode_writes.push_back(modes); + return true; + }); + ON_CALL(*api, setHdrStates(_)).WillByDefault(Return(true)); + ON_CALL(*api, setAsPrimary(_)).WillByDefault(Return(true)); + ON_CALL(*api, isPrimary("panel")).WillByDefault(Return(true)); + ON_CALL(*api, getCurrentDisplayModes(_)).WillByDefault(Return(DeviceDisplayModeMap {{"panel", {{1920, 1080}, {60, 1}}}})); + ON_CALL(*api, getCurrentHdrStates(_)).WillByDefault(Return(HdrStateMap {{"panel", HdrState::Disabled}})); + + SingleDisplayConfiguration config; + config.m_device_id = "panel"; + config.m_device_prep = SingleDisplayConfiguration::DevicePreparation::VerifyOnly; + EXPECT_CALL(*api, setAsPrimary("dock")).Times(0); + EXPECT_EQ(manager->applySettings(config), SettingsManager::ApplyResult::Ok); + EXPECT_TRUE(mode_writes.empty()); + + // The dock returns and gets its own settings back. + devices.push_back({.m_device_id = "dock"}); + ::testing::Mock::VerifyAndClearExpectations(api.get()); + EXPECT_CALL(*api, setAsPrimary("dock")).WillOnce(Return(true)); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + ASSERT_FALSE(mode_writes.empty()); + EXPECT_EQ(mode_writes.front(), (DeviceDisplayModeMap {{"dock", dock_mode}})); + EXPECT_EQ(clears, 1); +} + +TEST_F(UndockRecovery, EnsurePrimaryWhileOriginalPrimaryIsAway) { + // The dock was primary. After it left, "A" took over and the dock's primary status is pending. + state.m_initial = {{{"dock"}, {"A"}, {"B"}}, {"dock"}}; + state.m_modified = {{{"dock"}}, {}, {}, "dock"}; + active = {{"A"}, {"B"}}; + devices = { + {.m_device_id = "A", .m_info = EnumeratedDevice::Info {.m_primary = true}}, + {.m_device_id = "B", .m_info = EnumeratedDevice::Info {}}, + }; + std::string primary {"A"}; + init(); + ON_CALL(*api, isPrimary(_)).WillByDefault([&primary](std::string_view id) { + return id == primary; + }); + ON_CALL(*api, setAsPrimary(_)).WillByDefault([&primary](std::string_view id) { + primary = std::string {id}; + return true; + }); + + SingleDisplayConfiguration config; + config.m_device_id = "B"; + config.m_device_prep = SingleDisplayConfiguration::DevicePreparation::EnsurePrimary; + EXPECT_EQ(manager->applySettings(config), SettingsManager::ApplyResult::Ok); + EXPECT_EQ(primary, "B"); + + // Reverting while the dock is still away brings back the display that was primary before the session. + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(primary, "A"); + EXPECT_EQ(clears, 0); + + // The dock's own primary status is still pending and is restored once it returns. + devices.push_back({.m_device_id = "dock"}); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(primary, "dock"); + EXPECT_EQ(clears, 1); +} + +TEST_F(UndockRecovery, StagingKeepsCloneGroupWithActiveMember) { + state.m_initial.m_topology.push_back({"left", "right"}); + devices.push_back({.m_device_id = "left"}); + devices.push_back({.m_device_id = "right"}); + active = {{"left"}, {"stream"}}; + split_clones = true; + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"left", "right"}})); + ASSERT_GE(writes.size(), 3u); + EXPECT_EQ(writes[writes.size() - 2], (ActiveTopology {{"left", "right"}, {"stream"}})); +} + +TEST_F(UndockRecovery, StagingKeepsSurvivingCloneGroup) { + state.m_initial.m_topology.push_back({"left", "right"}); + devices.push_back({.m_device_id = "left"}); + devices.push_back({.m_device_id = "right"}); + split_clones = true; + init(); + EXPECT_EQ(manager->revertSettings(), SettingsManager::RevertResult::Ok); + EXPECT_EQ(active, (ActiveTopology {{"left", "right"}})); + ASSERT_GE(writes.size(), 3u); + EXPECT_EQ(writes[writes.size() - 2], (ActiveTopology {{"stream"}, {"left", "right"}})); +} diff --git a/tests/unit/windows/test_win_display_device_general.cpp b/tests/unit/windows/test_win_display_device_general.cpp index 3adfdc2..50275a6 100644 --- a/tests/unit/windows/test_win_display_device_general.cpp +++ b/tests/unit/windows/test_win_display_device_general.cpp @@ -151,6 +151,9 @@ TEST_F_S_MOCKED(EnumAvailableDevices) { const auto pam_active_and_inactive {[]() { auto pam {ut_consts::PAM_3_ACTIVE}; pam->m_paths.at(0).targetInfo.refreshRate.Denominator = 0; + pam->m_paths.at(0).targetInfo.outputTechnology = DISPLAYCONFIG_OUTPUT_TECHNOLOGY_DISPLAYPORT_EMBEDDED; + pam->m_paths.at(1).targetInfo.outputTechnology = DISPLAYCONFIG_OUTPUT_TECHNOLOGY_INDIRECT_WIRED; + pam->m_paths.at(2).targetInfo.outputTechnology = DISPLAYCONFIG_OUTPUT_TECHNOLOGY_LVDS; pam->m_paths.at(2).flags &= ~DISPLAYCONFIG_PATH_ACTIVE; return pam; }()}; @@ -179,7 +182,8 @@ TEST_F_S_MOCKED(EnumAvailableDevices) { true, {0, 0}, std::nullopt - }}, + }, + true}, {"DeviceId2", "DisplayName2", "FriendlyName2", @@ -196,7 +200,8 @@ TEST_F_S_MOCKED(EnumAvailableDevices) { "", "FriendlyName3", std::nullopt, - std::nullopt} + std::nullopt, + true} }; EXPECT_EQ(m_win_dd.enumAvailableDevices(), expected_list); } diff --git a/tests/unit/windows/test_win_display_device_modes.cpp b/tests/unit/windows/test_win_display_device_modes.cpp index 543361b..0ed204d 100644 --- a/tests/unit/windows/test_win_display_device_modes.cpp +++ b/tests/unit/windows/test_win_display_device_modes.cpp @@ -102,8 +102,8 @@ namespace { } } - void setupExpectedDisplayModeUndo(const std::optional &pam_initial) const { - EXPECT_CALL(*m_layer, setDisplayConfig(pam_initial->m_paths, pam_initial->m_modes, UNDO_FLAGS)) + void setupExpectedDisplayModeUndo(const std::optional &pam_initial, const UINT32 flags = UNDO_FLAGS) const { + EXPECT_CALL(*m_layer, setDisplayConfig(pam_initial->m_paths, pam_initial->m_modes, flags)) .Times(1) .WillOnce(Return(ERROR_SUCCESS)) .RetiresOnSaturation(); @@ -113,6 +113,16 @@ namespace { display_device::WinDisplayDevice m_win_dd {m_layer}; }; + /** @brief Exercise saved and session-only display changes with the same assertions. */ + class WinDisplayDeviceModesPersistence: public WinDisplayDeviceModesMocked, public ::testing::WithParamInterface { + public: + WinDisplayDeviceModesPersistence() { + m_win_dd = display_device::WinDisplayDevice {m_layer, GetParam()}; + } + }; + + INSTANTIATE_TEST_SUITE_P(SaveToDatabase, WinDisplayDeviceModesPersistence, ::testing::Bool()); + // Specialized TEST macro(s) for this test file #define TEST_F_S(...) DD_MAKE_TEST(TEST_F, WinDisplayDeviceModes, __VA_ARGS__) #define TEST_F_S_MOCKED(...) DD_MAKE_TEST(TEST_F, WinDisplayDeviceModesMocked, __VA_ARGS__) @@ -339,7 +349,7 @@ TEST_F_S_MOCKED(SetDisplayModes, Relaxed) { EXPECT_TRUE(m_win_dd.setDisplayModes(new_modes)); } -TEST_F_S_MOCKED(SetDisplayModes, Strict) { +TEST_P(WinDisplayDeviceModesPersistence, Strict) { const auto new_modes {makeStrictDisplayModes()}; const auto &pam_initial {ut_consts::PAM_4_ACTIVE_WITH_2_DUPLICATES}; const auto pam_submitted {applyExpectedModesOntoInput(pam_initial, new_modes, {"DeviceId4"})}; @@ -349,13 +359,13 @@ TEST_F_S_MOCKED(SetDisplayModes, Strict) { // Relaxed try { - setupExpectedDisplayModeSubmitAttempt(sequence, pam_initial, pam_submitted, RELAXED_FLAGS); + setupExpectedDisplayModeSubmitAttempt(sequence, pam_initial, pam_submitted, (GetParam() ? RELAXED_FLAGS : RELAXED_FLAGS & ~SDC_SAVE_TO_DATABASE)); setupExpectedGetCurrentDisplayModesCall(sequence, pam_initial); } // Strict try { - setupExpectedDisplayModeSubmitAttempt(sequence, pam_initial, pam_submitted, STRICT_FLAGS); + setupExpectedDisplayModeSubmitAttempt(sequence, pam_initial, pam_submitted, (GetParam() ? STRICT_FLAGS : STRICT_FLAGS & ~SDC_SAVE_TO_DATABASE)); setupExpectedGetCurrentDisplayModesCall(sequence, pam_submitted); } @@ -517,7 +527,7 @@ TEST_F_S_MOCKED(SetDisplayModes, Relaxed, FailedToGetCurrentDisplayModes) { EXPECT_FALSE(m_win_dd.setDisplayModes(new_modes)); } -TEST_F_S_MOCKED(SetDisplayModes, Strict, FailedToSetDisplayConfig) { +TEST_P(WinDisplayDeviceModesPersistence, StrictFailureRestoresOriginalModes) { const auto new_modes {makeStrictDisplayModes()}; const auto &pam_initial {ut_consts::PAM_4_ACTIVE_WITH_2_DUPLICATES}; const auto pam_submitted {applyExpectedModesOntoInput(pam_initial, new_modes, {"DeviceId4"})}; @@ -527,14 +537,14 @@ TEST_F_S_MOCKED(SetDisplayModes, Strict, FailedToSetDisplayConfig) { // Relaxed try { - setupExpectedDisplayModeSubmitAttempt(sequence, pam_initial, pam_submitted, RELAXED_FLAGS); + setupExpectedDisplayModeSubmitAttempt(sequence, pam_initial, pam_submitted, (GetParam() ? RELAXED_FLAGS : RELAXED_FLAGS & ~SDC_SAVE_TO_DATABASE)); setupExpectedGetCurrentDisplayModesCall(sequence, pam_initial); } // Strict try { - setupExpectedDisplayModeSubmitAttempt(sequence, pam_initial, pam_submitted, STRICT_FLAGS, ERROR_ACCESS_DENIED); - setupExpectedDisplayModeUndo(pam_initial); + setupExpectedDisplayModeSubmitAttempt(sequence, pam_initial, pam_submitted, (GetParam() ? STRICT_FLAGS : STRICT_FLAGS & ~SDC_SAVE_TO_DATABASE), ERROR_ACCESS_DENIED); + setupExpectedDisplayModeUndo(pam_initial, GetParam() ? UNDO_FLAGS : UNDO_FLAGS & ~SDC_SAVE_TO_DATABASE); } EXPECT_FALSE(m_win_dd.setDisplayModes(new_modes)); diff --git a/tests/unit/windows/test_win_display_device_primary.cpp b/tests/unit/windows/test_win_display_device_primary.cpp index ca720c9..f8de40b 100644 --- a/tests/unit/windows/test_win_display_device_primary.cpp +++ b/tests/unit/windows/test_win_display_device_primary.cpp @@ -39,14 +39,14 @@ namespace { expectActivePathLookup(m_layer, id_number); } - void setupExpectedSetAsPrimaryConfigCall(InSequence &sequence /* To ensure that sequence is created outside this scope */, const std::optional &initial_pam, const std::optional &expected_pam, const int active_path_id, const LONG result = ERROR_SUCCESS) const { + void setupExpectedSetAsPrimaryConfigCall(InSequence &sequence /* To ensure that sequence is created outside this scope */, const std::optional &initial_pam, const std::optional &expected_pam, const int active_path_id, const LONG result = ERROR_SUCCESS, const UINT32 flags = FLAGS) const { EXPECT_CALL(*m_layer, queryDisplayConfig(display_device::QueryType::Active)) .Times(1) .WillOnce(Return(initial_pam)); setupExpectedGetActivePathCall(active_path_id, sequence); expectDeviceIdLookups(m_layer, 4); - EXPECT_CALL(*m_layer, setDisplayConfig(expected_pam->m_paths, expected_pam->m_modes, FLAGS)) + EXPECT_CALL(*m_layer, setDisplayConfig(expected_pam->m_paths, expected_pam->m_modes, flags)) .Times(1) .WillOnce(Return(result)) .RetiresOnSaturation(); @@ -63,6 +63,16 @@ namespace { display_device::WinDisplayDevice m_win_dd {m_layer}; }; + /** @brief Exercise saved and session-only display changes with the same assertions. */ + class WinDisplayDevicePrimaryPersistence: public WinDisplayDevicePrimaryMocked, public ::testing::WithParamInterface { + public: + WinDisplayDevicePrimaryPersistence() { + m_win_dd = display_device::WinDisplayDevice {m_layer, GetParam()}; + } + }; + + INSTANTIATE_TEST_SUITE_P(SaveToDatabase, WinDisplayDevicePrimaryPersistence, ::testing::Bool()); + // Specialized TEST macro(s) for this test file #define TEST_F_S(...) DD_MAKE_TEST(TEST_F, WinDisplayDevicePrimary, __VA_ARGS__) #define TEST_F_S_MOCKED(...) DD_MAKE_TEST(TEST_F, WinDisplayDevicePrimaryMocked, __VA_ARGS__) @@ -221,12 +231,12 @@ TEST_F_S_MOCKED(SetAsPrimary, DuplicatePrimaryDevicesSet) { EXPECT_TRUE(m_win_dd.setAsPrimary("DeviceId2")); } -TEST_F_S_MOCKED(SetAsPrimary, NonDuplicatePrimaryDeviceSet) { +TEST_P(WinDisplayDevicePrimaryPersistence, NonDuplicatePrimaryDeviceSet) { const auto &initial_pam {ut_consts::PAM_4_ACTIVE_WITH_2_DUPLICATES}; const auto expected_pam {makeShiftedPrimaryPam(initial_pam, 3, {0, 1, 2, 3})}; InSequence sequence; - setupExpectedSetAsPrimaryConfigCall(sequence, initial_pam, expected_pam, 4); + setupExpectedSetAsPrimaryConfigCall(sequence, initial_pam, expected_pam, 4, ERROR_SUCCESS, GetParam() ? FLAGS : FLAGS & ~SDC_SAVE_TO_DATABASE); EXPECT_TRUE(m_win_dd.setAsPrimary("DeviceId4")); } diff --git a/tests/unit/windows/test_win_display_device_topology.cpp b/tests/unit/windows/test_win_display_device_topology.cpp index e747cf1..73caa88 100644 --- a/tests/unit/windows/test_win_display_device_topology.cpp +++ b/tests/unit/windows/test_win_display_device_topology.cpp @@ -16,6 +16,9 @@ namespace { using ::testing::Return; using ::testing::StrictMock; + const UINT32 SAVED_TOPOLOGY_FLAGS {SDC_APPLY | SDC_TOPOLOGY_SUPPLIED | SDC_ALLOW_PATH_ORDER_CHANGES | SDC_VIRTUAL_MODE_AWARE}; + const UINT32 TEMPORARY_TOPOLOGY_FLAGS {SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_ALLOW_CHANGES | SDC_VIRTUAL_MODE_AWARE}; + // Test fixture(s) for this file class WinDisplayDeviceTopology: public BaseTest { public: @@ -54,6 +57,16 @@ namespace { display_device::WinDisplayDevice m_win_dd {m_layer}; }; + /** @brief Exercise saved and session-only display changes with the same assertions. */ + class WinDisplayDeviceTopologyPersistence: public WinDisplayDeviceTopologyMocked, public ::testing::WithParamInterface { + public: + WinDisplayDeviceTopologyPersistence() { + m_win_dd = display_device::WinDisplayDevice {m_layer, GetParam()}; + } + }; + + INSTANTIATE_TEST_SUITE_P(SaveToDatabase, WinDisplayDeviceTopologyPersistence, ::testing::Bool()); + // Specialized TEST macro(s) for this test file #define TEST_F_S(...) DD_MAKE_TEST(TEST_F, WinDisplayDeviceTopology, __VA_ARGS__) #define TEST_F_S_MOCKED(...) DD_MAKE_TEST(TEST_F, WinDisplayDeviceTopologyMocked, __VA_ARGS__) @@ -252,7 +265,7 @@ TEST_F_S_MOCKED(isTopologyTheSame) { EXPECT_EQ(m_win_dd.isTopologyTheSame({{"ID_3"}, {"ID_1", "ID_2"}}, {{"ID_2", "ID_1"}, {"ID_3"}}), true); } -TEST_F_S_MOCKED(SetCurrentTopology) { +TEST_P(WinDisplayDeviceTopologyPersistence, SetCurrentTopology) { InSequence sequence; setupExpectCallFor3ActivePathsAndModes(display_device::QueryType::Active, sequence); setupExpectCallFor3ActivePathsAndModes(display_device::QueryType::All, sequence); @@ -264,7 +277,7 @@ TEST_F_S_MOCKED(SetCurrentTopology) { display_device::win_utils::setDesktopIndex(expected_path, std::nullopt); display_device::win_utils::setActive(expected_path); - UINT32 expected_flags {SDC_APPLY | SDC_TOPOLOGY_SUPPLIED | SDC_ALLOW_PATH_ORDER_CHANGES | SDC_VIRTUAL_MODE_AWARE}; + UINT32 expected_flags {GetParam() ? SAVED_TOPOLOGY_FLAGS : TEMPORARY_TOPOLOGY_FLAGS}; EXPECT_CALL(*m_layer, setDisplayConfig(std::vector {expected_path}, std::vector {}, expected_flags)) .Times(1) .WillOnce(Return(ERROR_SUCCESS)); @@ -364,12 +377,12 @@ TEST_F_S_MOCKED(SetCurrentTopology, WindowsDoesNotKnowAboutTheTopology, FailedTo EXPECT_FALSE(m_win_dd.setTopology({{"DeviceId1"}})); } -TEST_F_S_MOCKED(SetCurrentTopology, FailedToSetTopology, NoRecovery) { +TEST_P(WinDisplayDeviceTopologyPersistence, FailedToSetTopology) { InSequence sequence; setupExpectCallFor3ActivePathsAndModes(display_device::QueryType::Active, sequence); setupExpectCallFor3ActivePathsAndModes(display_device::QueryType::All, sequence); - UINT32 expected_flags {SDC_APPLY | SDC_TOPOLOGY_SUPPLIED | SDC_ALLOW_PATH_ORDER_CHANGES | SDC_VIRTUAL_MODE_AWARE}; + UINT32 expected_flags {GetParam() ? SAVED_TOPOLOGY_FLAGS : TEMPORARY_TOPOLOGY_FLAGS}; EXPECT_CALL(*m_layer, setDisplayConfig(getExpectedPathToBeSet(), std::vector {}, expected_flags)) .Times(1) .WillOnce(Return(ERROR_INVALID_PARAMETER)); @@ -405,12 +418,12 @@ TEST_F_S_MOCKED(SetCurrentTopology, TopologyWasSetAccordingToWinApi, CouldNotGet EXPECT_FALSE(m_win_dd.setTopology({{"DeviceId1"}})); } -TEST_F_S_MOCKED(SetCurrentTopology, TopologyWasSetAccordingToWinApi, WinApiLied) { +TEST_P(WinDisplayDeviceTopologyPersistence, ReadbackFailureRestoresOriginalTopology) { InSequence sequence; setupExpectCallFor3ActivePathsAndModes(display_device::QueryType::Active, sequence); setupExpectCallFor3ActivePathsAndModes(display_device::QueryType::All, sequence); - UINT32 expected_flags {SDC_APPLY | SDC_TOPOLOGY_SUPPLIED | SDC_ALLOW_PATH_ORDER_CHANGES | SDC_VIRTUAL_MODE_AWARE}; + UINT32 expected_flags {GetParam() ? SAVED_TOPOLOGY_FLAGS : TEMPORARY_TOPOLOGY_FLAGS}; EXPECT_CALL(*m_layer, setDisplayConfig(getExpectedPathToBeSet(), std::vector {}, expected_flags)) .Times(1) .WillOnce(Return(ERROR_SUCCESS)); @@ -419,7 +432,7 @@ TEST_F_S_MOCKED(SetCurrentTopology, TopologyWasSetAccordingToWinApi, WinApiLied) setupExpectCallFor3ActivePathsAndModes(display_device::QueryType::Active, sequence); // Called when doing the undo - expected_flags = SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_SAVE_TO_DATABASE | SDC_VIRTUAL_MODE_AWARE; + expected_flags = SDC_APPLY | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_VIRTUAL_MODE_AWARE | (GetParam() ? SDC_SAVE_TO_DATABASE : 0); EXPECT_CALL(*m_layer, setDisplayConfig(ut_consts::PAM_3_ACTIVE->m_paths, ut_consts::PAM_3_ACTIVE->m_modes, expected_flags)) .Times(1) .WillOnce(Return(ERROR_SUCCESS));