[FancyZones] Improve code quality (part 2: WorkArea init) (#23030)

* init WorkArea with a rectangle
* keep the highlighted zones state in a separate class

src/modules/fancyzones/FancyZonesLib/FancyZones.cpp

@@ -733,7 +733,17 @@ void FancyZones::AddWorkArea(HMONITOR monitor, const FancyZonesDataTypes::WorkAr
             parentId = parentArea->UniqueId();
         }
 
-        auto workArea = MakeWorkArea(m_hinstance, monitor, id, parentId);
+        FancyZonesUtils::Rect rect{};
+        if (monitor)
+        {
+            rect = MonitorUtils::GetWorkAreaRect(monitor);
+        }
+        else
+        {
+            rect = FancyZonesUtils::GetAllMonitorsCombinedRect<&MONITORINFO::rcWork>();
+        }
+        
+        auto workArea = MakeWorkArea(m_hinstance, id, parentId, rect);
         if (workArea)
         {
             m_workAreaHandler.AddWorkArea(VirtualDesktop::instance().GetCurrentVirtualDesktopId(), monitor, workArea);

src/modules/fancyzones/FancyZonesLib/FancyZonesLib.vcxproj

@@ -75,6 +75,7 @@
     <ClInclude Include="WindowUtils.h" />
     <ClInclude Include="Zone.h" />
     <ClInclude Include="Colors.h" />
+    <ClInclude Include="HighlightedZones.h" />
     <ClInclude Include="ZoneIndexSetBitmask.h" />
     <ClInclude Include="WorkArea.h" />
     <ClInclude Include="ZonesOverlay.h" />
@@ -124,6 +125,7 @@
     <ClCompile Include="WindowUtils.cpp" />
     <ClCompile Include="Zone.cpp" />
     <ClCompile Include="WorkArea.cpp" />
+    <ClCompile Include="HighlightedZones.cpp" />
     <ClCompile Include="ZonesOverlay.cpp" />
   </ItemGroup>
   <ItemGroup>

src/modules/fancyzones/FancyZonesLib/FancyZonesLib.vcxproj.filters

@@ -153,6 +153,9 @@
     <ClInclude Include="FancyZonesData\LayoutData.h">
       <Filter>Header Files\FancyZonesData</Filter>
     </ClInclude>
+    <ClInclude Include="HighlightedZones.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
     <ClInclude Include="NotificationUtil.h">
       <Filter>Header Files</Filter>
     </ClInclude>
@@ -251,6 +254,9 @@
     <ClCompile Include="LayoutAssignedWindows.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="HighlightedZones.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />

src/modules/fancyzones/FancyZonesLib/HighlightedZones.cpp

@@ -0,0 +1,55 @@
+#include "pch.h"
+#include "HighlightedZones.h"
+
+#include <FancyZonesLib/Layout.h>
+
+HighlightedZones::HighlightedZones()
+{
+}
+
+const ZoneIndexSet& HighlightedZones::Zones() const noexcept
+{
+    return m_highlightZone;
+}
+
+bool HighlightedZones::Empty() const noexcept
+{
+    return m_highlightZone.empty();
+}
+
+bool HighlightedZones::Update(const Layout* layout, POINT const& point, bool selectManyZones) noexcept
+{
+    if (!layout)
+    {
+        return false;
+    }
+
+    auto highlightZone = layout->ZonesFromPoint(point);
+
+    if (selectManyZones)
+    {
+        if (m_initialHighlightZone.empty())
+        {
+            // first time
+            m_initialHighlightZone = highlightZone;
+        }
+        else
+        {
+            highlightZone = layout->GetCombinedZoneRange(m_initialHighlightZone, highlightZone);
+        }
+    }
+    else
+    {
+        m_initialHighlightZone = {};
+    }
+
+    const bool updated = (highlightZone != m_highlightZone);
+    m_highlightZone = std::move(highlightZone);
+    return updated;
+}
+
+void HighlightedZones::Reset() noexcept
+{
+    m_highlightZone = {};
+    m_initialHighlightZone = {};
+}

src/modules/fancyzones/FancyZonesLib/HighlightedZones.h

@@ -0,0 +1,22 @@
+#pragma once
+
+#include <FancyZonesLib/Zone.h>
+
+class Layout;
+
+class HighlightedZones
+{
+public:
+    HighlightedZones();
+    ~HighlightedZones() = default;
+
+    const ZoneIndexSet& Zones() const noexcept;
+    bool Empty() const noexcept;
+
+    bool Update(const Layout* layout, POINT const& point, bool selectManyZones) noexcept;
+    void Reset() noexcept;
+
+private:
+    ZoneIndexSet m_initialHighlightZone;
+    ZoneIndexSet m_highlightZone;
+};

src/modules/fancyzones/FancyZonesLib/MonitorUtils.cpp

@@ -383,4 +383,19 @@ namespace MonitorUtils
         
         return displays;
     }
+
+    FancyZonesUtils::Rect GetWorkAreaRect(HMONITOR monitor)
+    {
+        if (monitor)
+        {
+            MONITORINFO mi{};
+            mi.cbSize = sizeof(mi);
+            if (GetMonitorInfoW(monitor, &mi))
+            {
+                return FancyZonesUtils::Rect(mi.rcWork);
+            }
+        }
+
+        return FancyZonesUtils::Rect{};
+    }
 }

src/modules/fancyzones/FancyZonesLib/MonitorUtils.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <FancyZonesLib/FancyZonesDataTypes.h>
+#include <FancyZonesLib/util.h>
 
 namespace MonitorUtils
 {
@@ -19,4 +20,6 @@ namespace MonitorUtils
 
     std::vector<FancyZonesDataTypes::MonitorId> IdentifyMonitors() noexcept;
     void OpenWindowOnActiveMonitor(HWND window, HMONITOR monitor) noexcept;
+
+    FancyZonesUtils::Rect GetWorkAreaRect(HMONITOR monitor);
 };

src/modules/fancyzones/FancyZonesLib/WorkArea.cpp

@@ -109,8 +109,9 @@ namespace
     WindowPool windowPool;
 }
 
-WorkArea::WorkArea(HINSTANCE hinstance, const FancyZonesDataTypes::WorkAreaId& uniqueId) :
-    m_uniqueId(uniqueId)
+WorkArea::WorkArea(HINSTANCE hinstance, const FancyZonesDataTypes::WorkAreaId& uniqueId, const FancyZonesUtils::Rect& workAreaRect) :
+    m_uniqueId(uniqueId),
+    m_workAreaRect(workAreaRect)
 {
     WNDCLASSEXW wcex{};
     wcex.cbSize = sizeof(WNDCLASSEX);
@@ -129,8 +130,7 @@ WorkArea::~WorkArea()
 HRESULT WorkArea::MoveSizeEnter(HWND window) noexcept
 {
     m_windowMoveSize = window;
-    m_highlightZone = {};
-    m_initialHighlightZone = {};
+    m_highlightedZones.Reset();
     ShowZonesOverlay();
     Trace::WorkArea::MoveOrResizeStarted(m_layout.get(), m_layoutWindows.get());
     return S_OK;
@@ -144,42 +144,23 @@ HRESULT WorkArea::MoveSizeUpdate(POINT const& ptScreen, bool dragEnabled, bool s
     }
 
     bool redraw = false;
-    POINT ptClient = ptScreen;
-    MapWindowPoints(nullptr, m_window, &ptClient, 1);
 
     if (dragEnabled)
     {
-        auto highlightZone = ZonesFromPoint(ptClient);
+        POINT ptClient = ptScreen;
+        MapWindowPoints(nullptr, m_window, &ptClient, 1);
 
-        if (selectManyZones)
-        {
-            if (m_initialHighlightZone.empty())
-            {
-                // first time
-                m_initialHighlightZone = highlightZone;
-            }
-            else
-            {
-                highlightZone = m_layout->GetCombinedZoneRange(m_initialHighlightZone, highlightZone);
-            }
-        }
-        else
-        {
-            m_initialHighlightZone = {};
-        }
-
-        redraw = (highlightZone != m_highlightZone);
-        m_highlightZone = std::move(highlightZone);
+        redraw = m_highlightedZones.Update(m_layout.get(), ptClient, selectManyZones);
     }
-    else if (m_highlightZone.size())
+    else if (!m_highlightedZones.Empty())
     {
-        m_highlightZone = {};
+        m_highlightedZones.Reset();
         redraw = true;
     }
 
     if (redraw && m_zonesOverlay)
     {
-        m_zonesOverlay->DrawActiveZoneSet(m_layout->Zones(), m_highlightZone, Colors::GetZoneColors(), FancyZonesSettings::settings().showZoneNumber);
+        m_zonesOverlay->DrawActiveZoneSet(m_layout->Zones(), m_highlightedZones.Zones(), Colors::GetZoneColors(), FancyZonesSettings::settings().showZoneNumber);
     }
 
     return S_OK;
@@ -192,7 +173,8 @@ HRESULT WorkArea::MoveSizeEnd(HWND window) noexcept
         return E_INVALIDARG;
     }
 
-    MoveWindowIntoZoneByIndexSet(window, m_highlightZone);
+    MoveWindowIntoZoneByIndexSet(window, m_highlightedZones.Zones());
+    m_highlightedZones.Reset();
 
     Trace::WorkArea::MoveOrResizeEnd(m_layout.get(), m_layoutWindows.get());
 
@@ -505,7 +487,7 @@ void WorkArea::ShowZonesOverlay() noexcept
     if (m_window && m_layout)
     {
         SetAsTopmostWindow();
-        m_zonesOverlay->DrawActiveZoneSet(m_layout->Zones(), m_highlightZone, Colors::GetZoneColors(), FancyZonesSettings::settings().showZoneNumber);
+        m_zonesOverlay->DrawActiveZoneSet(m_layout->Zones(), m_highlightedZones.Zones(), Colors::GetZoneColors(), FancyZonesSettings::settings().showZoneNumber);
         m_zonesOverlay->Show();
     }
 }
@@ -515,9 +497,7 @@ void WorkArea::HideZonesOverlay() noexcept
     if (m_window)
     {
         m_zonesOverlay->Hide();
-        m_keyLast = 0;
         m_windowMoveSize = nullptr;
-        m_highlightZone = {};
     }
 }
 
@@ -532,8 +512,7 @@ void WorkArea::UpdateActiveZoneSet() noexcept
     CalculateZoneSet();
     if (m_window && m_layout)
     {
-        m_highlightZone.clear();
-        m_zonesOverlay->DrawActiveZoneSet(m_layout->Zones(), m_highlightZone, Colors::GetZoneColors(), FancyZonesSettings::settings().showZoneNumber);
+        m_zonesOverlay->DrawActiveZoneSet(m_layout->Zones(), {}, Colors::GetZoneColors(), FancyZonesSettings::settings().showZoneNumber);
     }
 }
 
@@ -547,10 +526,10 @@ void WorkArea::CycleWindows(HWND window, bool reverse) noexcept
 
 void WorkArea::ClearSelectedZones() noexcept
 {
-    if (m_highlightZone.size() && m_layout)
+    if (!m_highlightedZones.Empty() && m_layout)
     {
-        m_highlightZone.clear();
-        m_zonesOverlay->DrawActiveZoneSet(m_layout->Zones(), m_highlightZone, Colors::GetZoneColors(), FancyZonesSettings::settings().showZoneNumber);
+        m_highlightedZones.Reset();
+        m_zonesOverlay->DrawActiveZoneSet(m_layout->Zones(), {}, Colors::GetZoneColors(), FancyZonesSettings::settings().showZoneNumber);
     }
 }
 
@@ -609,7 +588,7 @@ void WorkArea::CalculateZoneSet() noexcept
     }
 
     m_layout = std::make_unique<Layout>(appliedLayout.value());
-    m_layout->Init(m_workAreaRect, m_monitor);
+    m_layout->Init(m_workAreaRect, m_uniqueId.monitorId.monitor);
 
     if (!m_layoutWindows)
     {
@@ -639,16 +618,6 @@ LRESULT WorkArea::WndProc(UINT message, WPARAM wparam, LPARAM lparam) noexcept
     return 0;
 }
 
-ZoneIndexSet WorkArea::ZonesFromPoint(POINT pt) noexcept
-{
-    if (m_layout)
-    {
-        return m_layout->ZonesFromPoint(pt);
-    }
-
-    return {};
-}
-
 void WorkArea::SetAsTopmostWindow() noexcept
 {
     if (!m_window)
@@ -667,11 +636,6 @@ void WorkArea::SetAsTopmostWindow() noexcept
     SetWindowPos(m_window, windowInsertAfter, 0, 0, 0, 0, flags);
 }
 
-void WorkArea::LogInitializationError()
-{
-    Logger::error(L"Unable to get monitor info, {}", get_last_error_or_default(GetLastError()));
-}
-
 #pragma endregion
 
 LRESULT CALLBACK WorkArea::s_WndProc(HWND window, UINT message, WPARAM wparam, LPARAM lparam) noexcept

src/modules/fancyzones/FancyZonesLib/WorkArea.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <FancyZonesLib/FancyZonesDataTypes.h>
+#include <FancyZonesLib/HighlightedZones.h>
 #include <FancyZonesLib/Layout.h>
 #include <FancyZonesLib/LayoutAssignedWindows.h>
 #include <FancyZonesLib/util.h>
@@ -10,7 +11,7 @@ class ZonesOverlay;
 class WorkArea
 {
 public:
-    WorkArea(HINSTANCE hinstance, const FancyZonesDataTypes::WorkAreaId& uniqueId);
+    WorkArea(HINSTANCE hinstance, const FancyZonesDataTypes::WorkAreaId& uniqueId, const FancyZonesUtils::Rect& workAreaRect);
     ~WorkArea();
 
 public:
@@ -26,34 +27,6 @@ public:
         return true;
     }
 
-    inline bool InitWorkAreaRect(HMONITOR monitor)
-    {
-        m_monitor = monitor;
-
-#if defined(UNIT_TESTS)
-        m_workAreaRect = FancyZonesUtils::Rect({ 0, 0, 1920, 1080 });
-#else
-
-        if (monitor)
-        {
-            MONITORINFO mi{};
-            mi.cbSize = sizeof(mi);
-            if (!GetMonitorInfoW(monitor, &mi))
-            {
-                return false;
-            }
-
-            m_workAreaRect = FancyZonesUtils::Rect(mi.rcWork);
-        }
-        else
-        {
-            m_workAreaRect = FancyZonesUtils::GetAllMonitorsCombinedRect<&MONITORINFO::rcWork>();
-        }
-#endif
-
-        return true;
-    }
-
     FancyZonesDataTypes::WorkAreaId UniqueId() const noexcept { return { m_uniqueId }; }
     const std::unique_ptr<Layout>& GetLayout() const noexcept { return m_layout; }
     const std::unique_ptr<LayoutAssignedWindows>& GetLayoutWindows() const noexcept { return m_layoutWindows; }
@@ -79,8 +52,6 @@ public:
     void ClearSelectedZones() noexcept;
     
     void CycleWindows(HWND window, bool reverse) noexcept;
-    
-    void LogInitializationError();
 
 protected:
     static LRESULT CALLBACK s_WndProc(HWND window, UINT message, WPARAM wparam, LPARAM lparam) noexcept;
@@ -90,32 +61,22 @@ private:
     void InitLayout(const FancyZonesDataTypes::WorkAreaId& parentUniqueId) noexcept;
     void CalculateZoneSet() noexcept;
     LRESULT WndProc(UINT message, WPARAM wparam, LPARAM lparam) noexcept;
-    ZoneIndexSet ZonesFromPoint(POINT pt) noexcept;
     void SetAsTopmostWindow() noexcept;
 
-    HMONITOR m_monitor{};
-    FancyZonesUtils::Rect m_workAreaRect{};
+    const FancyZonesUtils::Rect m_workAreaRect{};
     const FancyZonesDataTypes::WorkAreaId m_uniqueId;
     HWND m_window{}; // Hidden tool window used to represent current monitor desktop work area.
-    HWND m_windowMoveSize{};
     std::unique_ptr<Layout> m_layout;
     std::unique_ptr<LayoutAssignedWindows> m_layoutWindows;
-    ZoneIndexSet m_initialHighlightZone;
-    ZoneIndexSet m_highlightZone;
-    WPARAM m_keyLast{};
-    size_t m_keyCycle{};
     std::unique_ptr<ZonesOverlay> m_zonesOverlay;
+    HighlightedZones m_highlightedZones;
+
+    HWND m_windowMoveSize{};
 };
 
-inline std::shared_ptr<WorkArea> MakeWorkArea(HINSTANCE hinstance, HMONITOR monitor, const FancyZonesDataTypes::WorkAreaId& uniqueId, const FancyZonesDataTypes::WorkAreaId& parentUniqueId) noexcept
+inline std::shared_ptr<WorkArea> MakeWorkArea(HINSTANCE hinstance, const FancyZonesDataTypes::WorkAreaId& uniqueId, const FancyZonesDataTypes::WorkAreaId& parentUniqueId, const FancyZonesUtils::Rect& workAreaRect)
 {
-    auto self = std::make_shared<WorkArea>(hinstance, uniqueId);
-    if (!self->InitWorkAreaRect(monitor))
-    {
-        self->LogInitializationError();
-        return nullptr;
-    }
-
+    auto self = std::make_shared<WorkArea>(hinstance, uniqueId, workAreaRect);
     if (!self->Init(hinstance, parentUniqueId))
     {
         return nullptr;