[ColorPicker] Ensure that the hotkey always works (#10542)

* Fix multiple instances running

* Make ColorPicker invokable from the runner

* Use a local mutex

* Enable running ColorPicker as standalone as well as a in PT

* Add some logging

* Remove newline

This will happen sooner or later and is not needed in the PR

* Update mutex name

* Update names

* Add logging

PowerToys.sln

@@ -221,6 +221,9 @@ EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SvgThumbnailProvider", "src\modules\previewpane\SvgThumbnailProvider\SvgThumbnailProvider.csproj", "{8FFE09DA-FA4F-4EE1-B3A2-AD5497FBD1AD}"
 EndProject
 Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ColorPicker", "src\modules\colorPicker\ColorPicker\ColorPicker.vcxproj", "{655C9AF2-18D3-4DA6-80E4-85504A7722BA}"
+	ProjectSection(ProjectDependencies) = postProject
+		{D9B8FC84-322A-4F9F-BBB9-20915C47DDFD} = {D9B8FC84-322A-4F9F-BBB9-20915C47DDFD}
+	EndProjectSection
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ColorPickerUI", "src\modules\colorPicker\ColorPickerUI\ColorPickerUI.csproj", "{BA58206B-1493-4C75-BFEA-A85768A1E156}"
 EndProject

src/modules/colorPicker/ColorPicker/ColorPicker.vcxproj

@@ -54,6 +54,9 @@
     <ClCompile Include="trace.cpp" />
   </ItemGroup>
   <ItemGroup>
+    <ProjectReference Include="..\..\..\common\logger\logger.vcxproj">
+      <Project>{d9b8fc84-322a-4f9f-bbb9-20915c47ddfd}</Project>
+    </ProjectReference>
     <ProjectReference Include="..\..\..\common\SettingsAPI\SetttingsAPI.vcxproj">
       <Project>{6955446d-23f7-4023-9bb3-8657f904af99}</Project>
     </ProjectReference>
@@ -69,6 +72,7 @@
     <None Include="Resources.resx" />
   </ItemGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
+  <Import Project="..\..\..\..\deps\spdlog.props" />
   <ImportGroup Label="ExtensionTargets">
     <Import Project="..\..\..\..\packages\Microsoft.Windows.CppWinRT.2.0.200729.8\build\native\Microsoft.Windows.CppWinRT.targets" Condition="Exists('..\..\..\..\packages\Microsoft.Windows.CppWinRT.2.0.200729.8\build\native\Microsoft.Windows.CppWinRT.targets')" />
   </ImportGroup>

src/modules/colorPicker/ColorPicker/dllmain.cpp

@@ -4,6 +4,7 @@
 #include <interface/powertoy_module_interface.h>
 #include "trace.h"
 #include "Generated Files/resource.h"
+#include <common/logger/logger.h>
 #include <common/SettingsAPI/settings_objects.h>
 #include <common/utils/os-detect.h>
 #include <common/utils/resources.h>
@@ -29,6 +30,17 @@ BOOL APIENTRY DllMain(HMODULE hModule,
     return TRUE;
 }
 
+namespace
+{
+    const wchar_t JSON_KEY_PROPERTIES[] = L"properties";
+    const wchar_t JSON_KEY_WIN[] = L"win";
+    const wchar_t JSON_KEY_ALT[] = L"alt";
+    const wchar_t JSON_KEY_CTRL[] = L"ctrl";
+    const wchar_t JSON_KEY_SHIFT[] = L"shift";
+    const wchar_t JSON_KEY_CODE[] = L"code";
+    const wchar_t JSON_KEY_ACTIVATION_SHORTCUT[] = L"ActivationShortcut";
+}
+
 struct ModuleSettings
 {
 } g_settings;
@@ -50,12 +62,101 @@ private:
 
     HANDLE send_telemetry_event;
 
+    Hotkey m_hotkey;
+
+    // Handle to event used to invoke ColorPicker
+    HANDLE m_hInvokeEvent;
+
+    void parse_hotkey(PowerToysSettings::PowerToyValues& settings)
+    {
+        auto settingsObject = settings.get_raw_json();
+        if (settingsObject.GetView().Size())
+        {
+            try
+            {
+                auto jsonHotkeyObject = settingsObject.GetNamedObject(JSON_KEY_PROPERTIES).GetNamedObject(JSON_KEY_ACTIVATION_SHORTCUT);
+                m_hotkey.win = jsonHotkeyObject.GetNamedBoolean(JSON_KEY_WIN);
+                m_hotkey.alt = jsonHotkeyObject.GetNamedBoolean(JSON_KEY_ALT);
+                m_hotkey.shift = jsonHotkeyObject.GetNamedBoolean(JSON_KEY_SHIFT);
+                m_hotkey.ctrl = jsonHotkeyObject.GetNamedBoolean(JSON_KEY_CTRL);
+                m_hotkey.key = static_cast<unsigned char>(jsonHotkeyObject.GetNamedNumber(JSON_KEY_CODE));
+            }
+            catch (...)
+            {
+                Logger::error("Failed to initialize ColorPicker start shortcut");
+            }
+        }
+        else
+        {
+            Logger::info("ColorPicker settings are empty");
+        }
+
+        if (!m_hotkey.key)
+        {
+            Logger::info("ColorPicker is going to use default shortcut");
+            m_hotkey.win = true;
+            m_hotkey.alt = false;
+            m_hotkey.shift = true;
+            m_hotkey.ctrl = false;
+            m_hotkey.key = 'C';
+        }
+    }
+
+    bool is_process_running()
+    {
+        return WaitForSingleObject(m_hProcess, 0) == WAIT_TIMEOUT;
+    }
+
+    void launch_process()
+    {
+        Logger::trace(L"Launching ColorPicker process");
+        unsigned long powertoys_pid = GetCurrentProcessId();
+
+        std::wstring executable_args = L"";
+        executable_args.append(std::to_wstring(powertoys_pid));
+
+        SHELLEXECUTEINFOW sei{ sizeof(sei) };
+        sei.fMask = { SEE_MASK_NOCLOSEPROCESS | SEE_MASK_FLAG_NO_UI };
+        sei.lpFile = L"modules\\ColorPicker\\ColorPickerUI.exe";
+        sei.nShow = SW_SHOWNORMAL;
+        sei.lpParameters = executable_args.data();
+        if (!ShellExecuteExW(&sei))
+        {
+            DWORD error = GetLastError();
+            std::wstring message = L"ColorPicker failed to start with error = ";
+            message += std::to_wstring(error);
+            Logger::error(message);
+        }
+
+        m_hProcess = sei.hProcess;
+    }
+
+    // Load the settings file.
+    void init_settings()
+    {
+        try
+        {
+            // Load and parse the settings file for this PowerToy.
+            PowerToysSettings::PowerToyValues settings =
+                PowerToysSettings::PowerToyValues::load_from_settings_file(get_key());
+
+            parse_hotkey(settings);
+        }
+        catch (std::exception ex)
+        {
+            Logger::warn(L"An exception occurred while loading the settings file");
+            // Error while loading from the settings file. Let default values stay as they are.
+        }
+    }
+
 public:
     ColorPicker()
     {
         app_name = GET_RESOURCE_STRING(IDS_COLORPICKER_NAME);
         app_key = ColorPickerConstants::ModuleKey;
         send_telemetry_event = CreateDefaultEvent(CommonSharedConstants::COLOR_PICKER_SEND_SETTINGS_TELEMETRY_EVENT);
+        m_hInvokeEvent = CreateDefaultEvent(CommonSharedConstants::SHOW_COLOR_PICKER_SHARED_EVENT);
+        init_settings();
     }
 
     ~ColorPicker()
@@ -109,6 +210,7 @@ public:
             PowerToysSettings::PowerToyValues values =
                 PowerToysSettings::PowerToyValues::from_json_string(config, get_key());
 
+            parse_hotkey(values);
             // If you don't need to do any custom processing of the settings, proceed
             // to persists the values calling:
             values.save_to_settings_file();
@@ -124,23 +226,11 @@ public:
     virtual void enable()
     {
         ResetEvent(send_telemetry_event);
+        ResetEvent(m_hInvokeEvent);
         // use only with new settings?
         if (UseNewSettings())
         {
-            unsigned long powertoys_pid = GetCurrentProcessId();
+            launch_process();
-
-            std::wstring executable_args = L"";
-            executable_args.append(std::to_wstring(powertoys_pid));
-
-            SHELLEXECUTEINFOW sei{ sizeof(sei) };
-            sei.fMask = { SEE_MASK_NOCLOSEPROCESS | SEE_MASK_FLAG_NO_UI };
-            sei.lpFile = L"modules\\ColorPicker\\ColorPickerUI.exe";
-            sei.nShow = SW_SHOWNORMAL;
-            sei.lpParameters = executable_args.data();
-            ShellExecuteExW(&sei);
-
-            m_hProcess = sei.hProcess;
-
             m_enabled = true;
         }
     };
@@ -150,12 +240,47 @@ public:
         if (m_enabled)
         {
             ResetEvent(send_telemetry_event);
+            ResetEvent(m_hInvokeEvent);
             TerminateProcess(m_hProcess, 1);
         }
 
         m_enabled = false;
     }
 
+    virtual bool on_hotkey(size_t hotkeyId) override
+    {
+        if (m_enabled)
+        {
+            Logger::trace(L"ColorPicker hotkey pressed");
+            if (!is_process_running())
+            {
+                launch_process();
+            }
+
+            SetEvent(m_hInvokeEvent);
+            return true;
+        }
+
+        return false;      
+    }
+
+    virtual size_t get_hotkeys(Hotkey* hotkeys, size_t buffer_size) override
+    {
+        if (m_hotkey.key)
+        {
+            if (hotkeys && buffer_size >= 1)
+            {
+                hotkeys[0] = m_hotkey;
+            }
+
+            return 1;
+        }
+        else
+        {
+            return 0;
+        }
+    }
+
     virtual bool is_enabled() override
     {
         return m_enabled;

src/modules/colorPicker/ColorPickerUI/App.xaml.cs

@@ -18,7 +18,7 @@ namespace ColorPickerUI
     {
         private Mutex _instanceMutex;
         private static string[] _args;
-        private int _powerToysPid;
+        private int _powerToysRunnerPid;
         private bool disposedValue;
         private ThemeManager _themeManager;
 
@@ -27,23 +27,27 @@ namespace ColorPickerUI
             _args = e?.Args;
 
             // allow only one instance of color picker
-            _instanceMutex = new Mutex(true, @"Global\ColorPicker", out bool createdNew);
+            _instanceMutex = new Mutex(true, @"Local\PowerToys_ColorPicker_InstanceMutex", out bool createdNew);
             if (!createdNew)
             {
                 _instanceMutex = null;
-                Application.Current.Shutdown();
+                Environment.Exit(0);
                 return;
             }
 
             if (_args?.Length > 0)
             {
-                _ = int.TryParse(_args[0], out _powerToysPid);
+                _ = int.TryParse(_args[0], out _powerToysRunnerPid);
-            }
 
-            RunnerHelper.WaitForPowerToysRunner(_powerToysPid, () =>
+                RunnerHelper.WaitForPowerToysRunner(_powerToysRunnerPid, () =>
+                {
+                    Environment.Exit(0);
+                });
+            }
+            else
             {
-                Environment.Exit(0);
+                _powerToysRunnerPid = -1;
-            });
+            }
 
             _themeManager = new ThemeManager(this);
             base.OnStartup(e);
@@ -83,5 +87,10 @@ namespace ColorPickerUI
             Dispose(disposing: true);
             GC.SuppressFinalize(this);
         }
+
+        public bool IsRunningDetachedFromPowerToys()
+        {
+            return _powerToysRunnerPid == -1;
+        }
     }
 }

src/modules/colorPicker/ColorPickerUI/Keyboard/KeyboardMonitor.cs

@@ -76,36 +76,40 @@ namespace ColorPicker.Keyboard
                 return;
             }
 
-            var name = Helper.GetKeyName((uint)virtualCode);
+            if ((System.Windows.Application.Current as ColorPickerUI.App).IsRunningDetachedFromPowerToys())
-
-            // If the last key pressed is a modifier key, then currentlyPressedKeys cannot possibly match with _activationKeys
-            // because _activationKeys contains exactly 1 non-modifier key. Hence, there's no need to check if `name` is a
-            // modifier key or to do any additional processing on it.
-            if (e.KeyboardState == GlobalKeyboardHook.KeyboardState.KeyDown || e.KeyboardState == GlobalKeyboardHook.KeyboardState.SysKeyDown)
             {
-                // Check pressed modifier keys.
+                var name = Helper.GetKeyName((uint)virtualCode);
-                AddModifierKeys(currentlyPressedKeys);
 
-                currentlyPressedKeys.Add(name);
+                // If the last key pressed is a modifier key, then currentlyPressedKeys cannot possibly match with _activationKeys
-            }
+                // because _activationKeys contains exactly 1 non-modifier key. Hence, there's no need to check if `name` is a
-
+                // modifier key or to do any additional processing on it.
-            currentlyPressedKeys.Sort();
+                if (e.KeyboardState == GlobalKeyboardHook.KeyboardState.KeyDown || e.KeyboardState == GlobalKeyboardHook.KeyboardState.SysKeyDown)
-
-            if (currentlyPressedKeys.Count == 0 && _previouslyPressedKeys.Count != 0)
-            {
-                // no keys pressed, we can enable activation shortcut again
-                _activationShortcutPressed = false;
-            }
-
-            _previouslyPressedKeys = currentlyPressedKeys;
-
-            if (ArraysAreSame(currentlyPressedKeys, _activationKeys))
-            {
-                // avoid triggering this action multiple times as this will be called nonstop while keys are pressed
-                if (!_activationShortcutPressed)
                 {
-                    _activationShortcutPressed = true;
+                    // Check pressed modifier keys.
-                    _appStateHandler.StartUserSession();
+                    AddModifierKeys(currentlyPressedKeys);
+
+                    currentlyPressedKeys.Add(name);
+                }
+
+                currentlyPressedKeys.Sort();
+
+                if (currentlyPressedKeys.Count == 0 && _previouslyPressedKeys.Count != 0)
+                {
+                    // no keys pressed, we can enable activation shortcut again
+                    _activationShortcutPressed = false;
+                }
+
+                _previouslyPressedKeys = currentlyPressedKeys;
+
+                if (ArraysAreSame(currentlyPressedKeys, _activationKeys))
+                {
+                    // avoid triggering this action multiple times as this will be called nonstop while keys are pressed
+                    if (!_activationShortcutPressed)
+                    {
+                        _activationShortcutPressed = true;
+
+                        _appStateHandler.StartUserSession();
+                    }
                 }
             }
         }