diff --git a/src/modules/fancyzones/FancyZonesLib/FancyZonesData.cpp b/src/modules/fancyzones/FancyZonesLib/FancyZonesData.cpp index ee143fb771ed..72d195be9702 100644 --- a/src/modules/fancyzones/FancyZonesLib/FancyZonesData.cpp +++ b/src/modules/fancyzones/FancyZonesLib/FancyZonesData.cpp @@ -180,12 +180,12 @@ const std::unordered_map FancyZonesData::FindDeviceInfo(const FancyZonesDataTypes::DeviceIdData& zoneWindowId) const +std::optional FancyZonesData::FindDeviceInfo(const FancyZonesDataTypes::DeviceIdData& id) const { std::scoped_lock lock{ dataLock }; for (const auto& [deviceId, deviceInfo] : deviceInfoMap) { - if (zoneWindowId.isEqualWithNullVirtualDesktopId(deviceId)) + if (id.isEqualWithNullVirtualDesktopId(deviceId)) { return deviceInfo; } @@ -206,8 +206,11 @@ bool FancyZonesData::AddDevice(const FancyZonesDataTypes::DeviceIdData& deviceId _TRACER_; using namespace FancyZonesDataTypes; + auto deviceInfo = FindDeviceInfo(deviceId); + std::scoped_lock lock{ dataLock }; - if (!deviceInfoMap.contains(deviceId)) + + if (!deviceInfo.has_value()) { wil::unique_cotaskmem_string virtualDesktopId; if (SUCCEEDED(StringFromCLSID(deviceId.virtualDesktopId, &virtualDesktopId))) @@ -224,13 +227,12 @@ bool FancyZonesData::AddDevice(const FancyZonesDataTypes::DeviceIdData& deviceId const ZoneSetData zoneSetData{ guidString.get(), ZoneSetLayoutType::PriorityGrid }; DeviceInfoData defaultDeviceInfoData{ zoneSetData, DefaultValues::ShowSpacing, DefaultValues::Spacing, DefaultValues::ZoneCount, DefaultValues::SensitivityRadius }; deviceInfoMap[deviceId] = std::move(defaultDeviceInfoData); + return true; } else { - deviceInfoMap[deviceId] = DeviceInfoData{ ZoneSetData{ NonLocalizable::NullStr, ZoneSetLayoutType::Blank } }; + Logger::error("Failed to create an ID for the new layout"); } - - return true; } return false; @@ -245,7 +247,7 @@ void FancyZonesData::CloneDeviceInfo(const FancyZonesDataTypes::DeviceIdData& so std::scoped_lock lock{ dataLock }; // The source virtual desktop is deleted, simply ignore it. - if (!deviceInfoMap.contains(source)) + if (!FindDeviceInfo(source).has_value()) { return; } diff --git a/src/modules/fancyzones/FancyZonesLib/FancyZonesData.h b/src/modules/fancyzones/FancyZonesLib/FancyZonesData.h index 49d3d28ee62b..d26f39c9fcd4 100644 --- a/src/modules/fancyzones/FancyZonesLib/FancyZonesData.h +++ b/src/modules/fancyzones/FancyZonesLib/FancyZonesData.h @@ -46,7 +46,7 @@ class FancyZonesData void SetVirtualDesktopCheckCallback(std::function callback); - std::optional FindDeviceInfo(const FancyZonesDataTypes::DeviceIdData& zoneWindowId) const; + std::optional FindDeviceInfo(const FancyZonesDataTypes::DeviceIdData& id) const; std::optional FindCustomZoneSet(const std::wstring& guid) const; const JSONHelpers::TDeviceInfoMap& GetDeviceInfoMap() const; diff --git a/src/modules/fancyzones/FancyZonesLib/FancyZonesDataTypes.h b/src/modules/fancyzones/FancyZonesLib/FancyZonesDataTypes.h index 7d3de1b6e447..a0d4eeeef150 100644 --- a/src/modules/fancyzones/FancyZonesLib/FancyZonesDataTypes.h +++ b/src/modules/fancyzones/FancyZonesLib/FancyZonesDataTypes.h @@ -146,6 +146,11 @@ namespace FancyZonesDataTypes int sensitivityRadius; }; + inline bool operator==(const ZoneSetData& lhs, const ZoneSetData& rhs) + { + return lhs.type == rhs.type && lhs.uuid == rhs.uuid; + } + inline bool operator==(const DeviceIdData& lhs, const DeviceIdData& rhs) { return lhs.deviceName.compare(rhs.deviceName) == 0 && lhs.width == rhs.width && lhs.height == rhs.height && lhs.virtualDesktopId == rhs.virtualDesktopId && lhs.monitorId.compare(rhs.monitorId) == 0; @@ -160,6 +165,11 @@ namespace FancyZonesDataTypes { return lhs.deviceName.compare(rhs.deviceName) < 0 || lhs.width < rhs.width || lhs.height < rhs.height || lhs.monitorId.compare(rhs.monitorId) < 0; } + + inline bool operator==(const DeviceInfoData& lhs, const DeviceInfoData& rhs) + { + return lhs.activeZoneSet == rhs.activeZoneSet && lhs.showSpacing == rhs.showSpacing && lhs.spacing == rhs.spacing && lhs.zoneCount == rhs.zoneCount && lhs.sensitivityRadius == rhs.sensitivityRadius; + } } namespace std diff --git a/src/modules/fancyzones/FancyZonesTests/UnitTests/JsonHelpers.Tests.cpp b/src/modules/fancyzones/FancyZonesTests/UnitTests/JsonHelpers.Tests.cpp index 153905e91e8f..d9d781a21167 100644 --- a/src/modules/fancyzones/FancyZonesTests/UnitTests/JsonHelpers.Tests.cpp +++ b/src/modules/fancyzones/FancyZonesTests/UnitTests/JsonHelpers.Tests.cpp @@ -2042,6 +2042,258 @@ namespace FancyZonesUnitTests Assert::IsFalse(data.RemoveAppLastZone(nullptr, deviceId, zoneSetId)); } + + TEST_METHOD (AddDevice) + { + FancyZonesDataTypes::DeviceIdData expected{ + .deviceName = L"Device", + .width = 200, + .height = 100, + .virtualDesktopId = m_defaultVDId + }; + + auto result = m_fzData.AddDevice(expected); + Assert::IsTrue(result); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + + Assert::IsFalse(actualMap.find(expected) == actualMap.end()); + } + + TEST_METHOD (AddDeviceWithNullVirtualDesktopId) + { + FancyZonesDataTypes::DeviceIdData expected{ + .deviceName = L"Device", + .width = 200, + .height = 100, + .virtualDesktopId = GUID_NULL + }; + + auto result = m_fzData.AddDevice(expected); + Assert::IsTrue(result); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + + Assert::IsFalse(actualMap.find(expected) == actualMap.end()); + } + + TEST_METHOD (AddDeviceDuplicate) + { + FancyZonesDataTypes::DeviceIdData expected{ + .deviceName = L"Device", + .width = 200, + .height = 100, + .virtualDesktopId = m_defaultVDId + }; + + auto result = m_fzData.AddDevice(expected); + Assert::IsTrue(result); + + auto result2 = m_fzData.AddDevice(expected); + Assert::IsFalse(result2); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + + Assert::IsFalse(actualMap.find(expected) == actualMap.end()); + } + + TEST_METHOD (AddDeviceWithNullVirtualDesktopIdDuplicated) + { + FancyZonesDataTypes::DeviceIdData expected{ + .deviceName = L"Device", + .width = 200, + .height = 100, + .virtualDesktopId = GUID_NULL + }; + + auto result = m_fzData.AddDevice(expected); + Assert::IsTrue(result); + + auto result2 = m_fzData.AddDevice(expected); + Assert::IsFalse(result2); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + + Assert::IsFalse(actualMap.find(expected) == actualMap.end()); + } + + TEST_METHOD (AddDeviceDuplicatedComparedWithNillVirtualDesktopId) + { + FancyZonesDataTypes::DeviceIdData device1{ + .deviceName = L"Device", + .width = 200, + .height = 100, + .virtualDesktopId = m_defaultVDId + }; + + FancyZonesDataTypes::DeviceIdData device2{ + .deviceName = L"Device", + .width = 200, + .height = 100, + .virtualDesktopId = GUID_NULL + }; + + auto result = m_fzData.AddDevice(device1); + Assert::IsTrue(result); + + auto result2 = m_fzData.AddDevice(device2); + Assert::IsFalse(result2); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + + Assert::IsFalse(actualMap.find(device1) == actualMap.end()); + Assert::IsTrue(actualMap.find(device2) == actualMap.end()); + } + + TEST_METHOD (AddDeviceDuplicatedComparedWithNillVirtualDesktopId2) + { + FancyZonesDataTypes::DeviceIdData device1{ + .deviceName = L"Device", + .width = 200, + .height = 100, + .virtualDesktopId = m_defaultVDId + }; + + FancyZonesDataTypes::DeviceIdData device2{ + .deviceName = L"Device", + .width = 200, + .height = 100, + .virtualDesktopId = GUID_NULL + }; + + auto result2 = m_fzData.AddDevice(device2); + Assert::IsTrue(result2); + + auto result1 = m_fzData.AddDevice(device1); + Assert::IsFalse(result1); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + + Assert::IsFalse(actualMap.find(device2) == actualMap.end()); + Assert::IsTrue(actualMap.find(device1) == actualMap.end()); + } + + TEST_METHOD(CloneDeviceInfo) + { + FancyZonesDataTypes::DeviceIdData deviceSrc{ + .deviceName = L"Device1", + .width = 200, + .height = 100, + .virtualDesktopId = m_defaultVDId + }; + FancyZonesDataTypes::DeviceIdData deviceDst{ + .deviceName = L"Device2", + .width = 300, + .height = 400, + .virtualDesktopId = m_defaultVDId + }; + + Assert::IsTrue(m_fzData.AddDevice(deviceSrc)); + Assert::IsTrue(m_fzData.AddDevice(deviceDst)); + + m_fzData.CloneDeviceInfo(deviceSrc, deviceDst); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + Assert::IsFalse(actualMap.find(deviceSrc) == actualMap.end()); + Assert::IsFalse(actualMap.find(deviceDst) == actualMap.end()); + + auto expected = m_fzData.FindDeviceInfo(deviceSrc); + auto actual = m_fzData.FindDeviceInfo(deviceDst); + + Assert::IsTrue(expected.has_value()); + Assert::IsTrue(actual.has_value()); + Assert::IsTrue(expected.value() == actual.value()); + } + + TEST_METHOD (CloneDeviceInfoIntoUnknownDevice) + { + FancyZonesDataTypes::DeviceIdData deviceSrc{ + .deviceName = L"Device1", + .width = 200, + .height = 100, + .virtualDesktopId = m_defaultVDId + }; + FancyZonesDataTypes::DeviceIdData deviceDst{ + .deviceName = L"Device2", + .width = 300, + .height = 400, + .virtualDesktopId = m_defaultVDId + }; + + Assert::IsTrue(m_fzData.AddDevice(deviceSrc)); + + m_fzData.CloneDeviceInfo(deviceSrc, deviceDst); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + Assert::IsFalse(actualMap.find(deviceSrc) == actualMap.end()); + Assert::IsFalse(actualMap.find(deviceDst) == actualMap.end()); + + auto expected = m_fzData.FindDeviceInfo(deviceSrc); + auto actual = m_fzData.FindDeviceInfo(deviceDst); + + Assert::IsTrue(expected.has_value()); + Assert::IsTrue(actual.has_value()); + Assert::IsTrue(expected.value() == actual.value()); + } + + TEST_METHOD (CloneDeviceInfoFromUnknownDevice) + { + FancyZonesDataTypes::DeviceIdData deviceSrc{ + .deviceName = L"Device1", + .width = 200, + .height = 100, + .virtualDesktopId = m_defaultVDId + }; + FancyZonesDataTypes::DeviceIdData deviceDst{ + .deviceName = L"Device2", + .width = 300, + .height = 400, + .virtualDesktopId = m_defaultVDId + }; + + Assert::IsTrue(m_fzData.AddDevice(deviceDst)); + + m_fzData.CloneDeviceInfo(deviceSrc, deviceDst); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + Assert::IsTrue(actualMap.find(deviceSrc) == actualMap.end()); + Assert::IsFalse(actualMap.find(deviceDst) == actualMap.end()); + + Assert::IsFalse(m_fzData.FindDeviceInfo(deviceSrc).has_value()); + Assert::IsTrue(m_fzData.FindDeviceInfo(deviceDst).has_value()); + } + + TEST_METHOD(CloneDeviceInfoNullVirtualDesktopId) + { + FancyZonesDataTypes::DeviceIdData deviceSrc{ + .deviceName = L"Device1", + .width = 200, + .height = 100, + .virtualDesktopId = GUID_NULL + }; + FancyZonesDataTypes::DeviceIdData deviceDst{ + .deviceName = L"Device2", + .width = 300, + .height = 400, + .virtualDesktopId = m_defaultVDId + }; + + Assert::IsTrue(m_fzData.AddDevice(deviceSrc)); + Assert::IsTrue(m_fzData.AddDevice(deviceDst)); + + m_fzData.CloneDeviceInfo(deviceSrc, deviceDst); + + auto actualMap = m_fzData.GetDeviceInfoMap(); + Assert::IsFalse(actualMap.find(deviceSrc) == actualMap.end()); + Assert::IsFalse(actualMap.find(deviceDst) == actualMap.end()); + + auto expected = m_fzData.FindDeviceInfo(deviceSrc); + auto actual = m_fzData.FindDeviceInfo(deviceDst); + + Assert::IsTrue(expected.has_value()); + Assert::IsTrue(actual.has_value()); + Assert::IsTrue(expected.value() == actual.value()); + } }; TEST_CLASS(EditorArgsUnitTests)