From 01a0123c3f9378174b2c38bf1e043844ba0457cd Mon Sep 17 00:00:00 2001 From: Seraphima Zykova Date: Tue, 27 Feb 2024 17:21:56 +0100 Subject: [PATCH] [FancyZones]Fix memory leak on work area change (#31561) * fz memleak fix * rect comparison * filter out unnecessary work area recreation --- .../fancyzones/FancyZonesLib/FancyZones.cpp | 92 ++++++++++++++----- .../fancyzones/FancyZonesLib/ZonesOverlay.cpp | 17 ++-- .../fancyzones/FancyZonesLib/ZonesOverlay.h | 1 - src/modules/fancyzones/FancyZonesLib/util.h | 10 ++ 4 files changed, 86 insertions(+), 34 deletions(-) diff --git a/src/modules/fancyzones/FancyZonesLib/FancyZones.cpp b/src/modules/fancyzones/FancyZonesLib/FancyZones.cpp index d69b1898ac..d5b35de3cf 100644 --- a/src/modules/fancyzones/FancyZonesLib/FancyZones.cpp +++ b/src/modules/fancyzones/FancyZonesLib/FancyZones.cpp @@ -144,13 +144,14 @@ public: LRESULT WndProc(HWND, UINT, WPARAM, LPARAM) noexcept; void OnDisplayChange(DisplayChangeType changeType) noexcept; - bool AddWorkArea(HMONITOR monitor, const FancyZonesDataTypes::WorkAreaId& id) noexcept; + bool AddWorkArea(HMONITOR monitor, const FancyZonesDataTypes::WorkAreaId& id, const FancyZonesUtils::Rect& rect) noexcept; protected: static LRESULT CALLBACK s_WndProc(HWND, UINT, WPARAM, LPARAM) noexcept; private: void UpdateWorkAreas(bool updateWindowPositions) noexcept; + bool ShouldWorkAreasBeRecreated(const std::vector& monitors, const GUID& virtualDesktop, const std::unordered_map>& workAreas) noexcept; void CycleWindows(bool reverse) noexcept; void SyncVirtualDesktops() noexcept; @@ -741,7 +742,7 @@ void FancyZones::OnDisplayChange(DisplayChangeType changeType) noexcept UpdateWorkAreas(updateWindowsPositions); } -bool FancyZones::AddWorkArea(HMONITOR monitor, const FancyZonesDataTypes::WorkAreaId& id) noexcept +bool FancyZones::AddWorkArea(HMONITOR monitor, const FancyZonesDataTypes::WorkAreaId& id, const FancyZonesUtils::Rect& rect) noexcept { auto virtualDesktopIdStr = FancyZonesUtils::GuidToString(id.virtualDesktopId); if (virtualDesktopIdStr) @@ -749,16 +750,6 @@ bool FancyZones::AddWorkArea(HMONITOR monitor, const FancyZonesDataTypes::WorkAr Logger::debug(L"Add new work area on virtual desktop {}", virtualDesktopIdStr.value()); } - FancyZonesUtils::Rect rect{}; - if (monitor) - { - rect = MonitorUtils::GetWorkAreaRect(monitor); - } - else - { - rect = FancyZonesUtils::GetAllMonitorsCombinedRect<&MONITORINFO::rcWork>(); - } - auto parentWorkAreaId = id; parentWorkAreaId.virtualDesktopId = LastUsedVirtualDesktop::instance().GetId(); @@ -791,27 +782,38 @@ void FancyZones::UpdateWorkAreas(bool updateWindowPositions) noexcept { Logger::debug(L"Update work areas, update windows positions: {}", updateWindowPositions); - m_workAreaConfiguration.Clear(); auto currentVirtualDesktop = VirtualDesktop::instance().GetCurrentVirtualDesktopIdFromRegistry(); if (FancyZonesSettings::settings().spanZonesAcrossMonitors) { - FancyZonesDataTypes::WorkAreaId workAreaId; - workAreaId.virtualDesktopId = currentVirtualDesktop; - workAreaId.monitorId = { .deviceId = { .id = ZonedWindowProperties::MultiMonitorName, .instanceId = ZonedWindowProperties::MultiMonitorInstance } }; + std::vector monitors = { FancyZonesDataTypes::MonitorId{ .monitor = nullptr, .deviceId = { .id = ZonedWindowProperties::MultiMonitorName, .instanceId = ZonedWindowProperties::MultiMonitorInstance } } }; + if (ShouldWorkAreasBeRecreated(monitors, currentVirtualDesktop, m_workAreaConfiguration.GetAllWorkAreas())) + { + m_workAreaConfiguration.Clear(); - AddWorkArea(nullptr, workAreaId); + FancyZonesDataTypes::WorkAreaId workAreaId; + workAreaId.virtualDesktopId = currentVirtualDesktop; + workAreaId.monitorId = { .deviceId = { .id = ZonedWindowProperties::MultiMonitorName, .instanceId = ZonedWindowProperties::MultiMonitorInstance } }; + + AddWorkArea(nullptr, workAreaId, FancyZonesUtils::GetAllMonitorsCombinedRect<&MONITORINFO::rcWork>()); + } } else { auto monitors = MonitorUtils::IdentifyMonitors(); - for (const auto& monitor : monitors) + const auto& workAreas = m_workAreaConfiguration.GetAllWorkAreas(); + + if (ShouldWorkAreasBeRecreated(monitors, currentVirtualDesktop, workAreas)) { - FancyZonesDataTypes::WorkAreaId workAreaId; - workAreaId.virtualDesktopId = currentVirtualDesktop; - workAreaId.monitorId = monitor; + m_workAreaConfiguration.Clear(); + for (const auto& monitor : monitors) + { + FancyZonesDataTypes::WorkAreaId workAreaId; + workAreaId.virtualDesktopId = currentVirtualDesktop; + workAreaId.monitorId = monitor; - AddWorkArea(monitor.monitor, workAreaId); + AddWorkArea(monitor.monitor, workAreaId, MonitorUtils::GetWorkAreaRect(monitor.monitor)); + } } } @@ -885,6 +887,52 @@ void FancyZones::UpdateWorkAreas(bool updateWindowPositions) noexcept } } +bool FancyZones::ShouldWorkAreasBeRecreated(const std::vector& monitors, const GUID& virtualDesktop, const std::unordered_map>& workAreas) noexcept +{ + if (monitors.size() != workAreas.size()) + { + Logger::trace(L"Monitor was added or removed"); + return true; + } + + for (const auto& monitor : monitors) + { + auto iter = workAreas.find(monitor.monitor); + if (iter == workAreas.end()) + { + Logger::trace(L"WorkArea not found"); + return true; + } + + if (iter->second->UniqueId().monitorId.deviceId != monitor.deviceId) + { + Logger::trace(L"DeviceId changed"); + return true; + } + + if (iter->second->UniqueId().monitorId.serialNumber != monitor.serialNumber) + { + Logger::trace(L"Serial number changed"); + return true; + } + + if (iter->second->UniqueId().virtualDesktopId != virtualDesktop) + { + Logger::trace(L"Virtual desktop changed"); + return true; + } + + const auto rect = monitor.monitor ? MonitorUtils::GetWorkAreaRect(monitor.monitor) : FancyZonesUtils::Rect(FancyZonesUtils::GetMonitorsCombinedRect<&MONITORINFOEX::rcWork>(FancyZonesUtils::GetAllMonitorRects<&MONITORINFOEX::rcWork>())); + if (iter->second->GetWorkAreaRect() != rect) + { + Logger::trace(L"WorkArea size changed"); + return true; + } + } + + return false; +} + void FancyZones::CycleWindows(bool reverse) noexcept { auto window = GetForegroundWindow(); diff --git a/src/modules/fancyzones/FancyZonesLib/ZonesOverlay.cpp b/src/modules/fancyzones/FancyZonesLib/ZonesOverlay.cpp index d7c9b537fd..bb6896c3bb 100644 --- a/src/modules/fancyzones/FancyZonesLib/ZonesOverlay.cpp +++ b/src/modules/fancyzones/FancyZonesLib/ZonesOverlay.cpp @@ -41,16 +41,6 @@ float ZonesOverlay::GetAnimationAlpha() return std::clamp(millis / FadeInDurationMillis, 0.001f, 1.f); } -ID2D1Factory* ZonesOverlay::GetD2DFactory() -{ - static auto pD2DFactory = [] { - ID2D1Factory* res = nullptr; - D2D1CreateFactory(D2D1_FACTORY_TYPE_MULTI_THREADED, &res); - return res; - }(); - return pD2DFactory; -} - IDWriteFactory* ZonesOverlay::GetWriteFactory() { static auto pDWriteFactory = [] { @@ -99,7 +89,11 @@ ZonesOverlay::ZonesOverlay(HWND window) auto renderTargetSize = D2D1::SizeU(m_clientRect.right - m_clientRect.left, m_clientRect.bottom - m_clientRect.top); auto hwndRenderTargetProperties = D2D1::HwndRenderTargetProperties(window, renderTargetSize); - hr = GetD2DFactory()->CreateHwndRenderTarget(renderTargetProperties, hwndRenderTargetProperties, &m_renderTarget); + ID2D1Factory* factory = nullptr; + D2D1CreateFactory(D2D1_FACTORY_TYPE_MULTI_THREADED, &factory); + hr = factory->CreateHwndRenderTarget(renderTargetProperties, hwndRenderTargetProperties, &m_renderTarget); + factory->Release(); + factory = nullptr; if (!SUCCEEDED(hr)) { @@ -357,5 +351,6 @@ ZonesOverlay::~ZonesOverlay() if (m_renderTarget) { m_renderTarget->Release(); + m_renderTarget = nullptr; } } diff --git a/src/modules/fancyzones/FancyZonesLib/ZonesOverlay.h b/src/modules/fancyzones/FancyZonesLib/ZonesOverlay.h index 1c5eb03a9c..fa20f5edc5 100644 --- a/src/modules/fancyzones/FancyZonesLib/ZonesOverlay.h +++ b/src/modules/fancyzones/FancyZonesLib/ZonesOverlay.h @@ -47,7 +47,6 @@ class ZonesOverlay std::vector m_sceneRects; float GetAnimationAlpha(); - static ID2D1Factory* GetD2DFactory(); static IDWriteFactory* GetWriteFactory(); static D2D1_COLOR_F ConvertColor(COLORREF color); static D2D1_RECT_F ConvertRect(RECT rect); diff --git a/src/modules/fancyzones/FancyZonesLib/util.h b/src/modules/fancyzones/FancyZonesLib/util.h index 98105a1af0..ffcbb9fa76 100644 --- a/src/modules/fancyzones/FancyZonesLib/util.h +++ b/src/modules/fancyzones/FancyZonesLib/util.h @@ -34,6 +34,16 @@ namespace FancyZonesUtils RECT m_rect{}; }; + inline bool operator==(const FancyZonesUtils::Rect& left, const FancyZonesUtils::Rect& right) + { + return left.left() == right.left() && left.right() == right.right() && left.top() == right.top() && left.bottom() == right.bottom(); + } + + inline bool operator!=(const FancyZonesUtils::Rect& left, const FancyZonesUtils::Rect& right) + { + return left.left() != right.left() || left.right() != right.right() || left.top() != right.top() || left.bottom() != right.bottom(); + } + inline void InitRGB(_Out_ RGBQUAD* quad, BYTE alpha, COLORREF color) { ZeroMemory(quad, sizeof(*quad));