From 09d1af9c46a83c5bbe408b958ce04849a89e43ba Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Thu, 6 Feb 2020 18:04:10 +0100 Subject: [PATCH] Common: remove hwnd_data_cache (#1223) The cache was introduced to improve performance by not querying the OS for the window process path every time we need to check if the window is interesting to FancyZones. Since then other changes were made to the the way we check the windows. Right now, the IsInterestingWindow function is called when: 1) WinKey + arrows are used 2) window is started to be dragged 3) window is created 1) and 2) are initiated by the user, happen only once per interaction so their performance impact can be dismissed. The 3) happens all the time but for the most part the check for WS_CHILD or GetAncestor(window, GA_ROOT) == window will filter those out. In the end, only top-level windows will be queried for their path. Removing the cache improves code readability and will make code maintenance easier. --- src/common/common.cpp | 59 +++++++++++++- src/common/common.h | 6 +- src/common/common.vcxproj | 2 - src/common/common.vcxproj.filters | 6 -- src/common/hwnd_data_cache.cpp | 129 ------------------------------ src/common/hwnd_data_cache.h | 55 ------------- 6 files changed, 60 insertions(+), 197 deletions(-) delete mode 100644 src/common/hwnd_data_cache.cpp delete mode 100644 src/common/hwnd_data_cache.h diff --git a/src/common/common.cpp b/src/common/common.cpp index 7be915b19c..68465fa0a4 100644 --- a/src/common/common.cpp +++ b/src/common/common.cpp @@ -1,6 +1,5 @@ #include "pch.h" #include "common.h" -#include "hwnd_data_cache.h" #include #pragma comment(lib, "dwmapi.lib") #include @@ -34,12 +33,66 @@ std::optional get_mouse_pos() { } } +// Test if a window is part of the shell or the task bar. +// We compare the HWND against HWND of the desktop and shell windows, +// we also filter out some window class names know to belong to +// the taskbar. +static bool is_system_window(HWND hwnd, const char* class_name) { + static auto system_classes = { "SysListView32", "WorkerW", "Shell_TrayWnd", "Shell_SecondaryTrayWnd", "Progman" }; + static auto system_hwnds = { GetDesktopWindow(), GetShellWindow() }; + for (auto system_hwnd : system_hwnds) { + if (hwnd == system_hwnd) { + return true; + } + } + for (const auto& system_class : system_classes) { + if (strcmp(system_class, class_name) == 0) { + return true; + } + } + return false; +} + WindowAndProcPath get_filtered_base_window_and_path(HWND window) { - return hwnd_cache.get_window_and_path(window); + WindowAndProcPath result; + auto root = GetAncestor(window, GA_ROOT); + if (!IsWindowVisible(root)) { + return result; + } + auto style = GetWindowLong(root, GWL_STYLE); + auto exStyle = GetWindowLong(root, GWL_EXSTYLE); + // WS_POPUP need to have a border or minimize/maximize buttons, + // otherwise the window is "not interesting" + if ((style & WS_POPUP) == WS_POPUP && + (style & WS_THICKFRAME) == 0 && + (style & WS_MINIMIZEBOX) == 0 && + (style & WS_MAXIMIZEBOX) == 0) { + return result; + } + if ((style & WS_CHILD) == WS_CHILD || + (style & WS_DISABLED) == WS_DISABLED || + (exStyle & WS_EX_TOOLWINDOW) == WS_EX_TOOLWINDOW || + (exStyle & WS_EX_NOACTIVATE) == WS_EX_NOACTIVATE) { + return result; + } + std::array class_name; + GetClassNameA(root, class_name.data(), static_cast(class_name.size())); + if (is_system_window(root, class_name.data())) { + return result; + } + auto process_path = get_process_path(root); + // Check for Cortana: + if (strcmp(class_name.data(), "Windows.UI.Core.CoreWindow") == 0 && + process_path.ends_with(L"SearchUI.exe")) { + return result; + } + result.hwnd = root; + result.process_path = std::move(process_path); + return result; } HWND get_filtered_active_window() { - return hwnd_cache.get_window(GetForegroundWindow()); + return get_filtered_base_window_and_path(GetForegroundWindow()).hwnd; } int width(const RECT& rect) { diff --git a/src/common/common.h b/src/common/common.h index 76eb5b18c1..a6d228361c 100644 --- a/src/common/common.h +++ b/src/common/common.h @@ -12,14 +12,16 @@ std::optional get_button_pos(HWND hwnd); std::optional get_window_pos(HWND hwnd); // Gets mouse postion. std::optional get_mouse_pos(); -// Gets active window, filtering out all "non standard" windows like the taskbar, etc. -HWND get_filtered_active_window(); + // Gets window ancestor (usualy the window we want to do stuff with), filtering out all "non standard" windows like the taskbar, etc. and provide the app process path struct WindowAndProcPath { HWND hwnd = nullptr; std::wstring process_path; }; WindowAndProcPath get_filtered_base_window_and_path(HWND window); +// Gets active window, filtering out all "non standard" windows like the taskbar, etc. +HWND get_filtered_active_window(); + // Calculate sizes int width(const RECT& rect); diff --git a/src/common/common.vcxproj b/src/common/common.vcxproj index 21cf2fa1bb..1be0c5e93f 100644 --- a/src/common/common.vcxproj +++ b/src/common/common.vcxproj @@ -102,7 +102,6 @@ - @@ -125,7 +124,6 @@ - diff --git a/src/common/common.vcxproj.filters b/src/common/common.vcxproj.filters index 87b34e6bff..1cf60d71d1 100644 --- a/src/common/common.vcxproj.filters +++ b/src/common/common.vcxproj.filters @@ -72,9 +72,6 @@ Header Files - - Header Files - Header Files @@ -132,9 +129,6 @@ Source Files - - Source Files - Source Files diff --git a/src/common/hwnd_data_cache.cpp b/src/common/hwnd_data_cache.cpp deleted file mode 100644 index 38f52aea25..0000000000 --- a/src/common/hwnd_data_cache.cpp +++ /dev/null @@ -1,129 +0,0 @@ -#include "pch.h" -#include "hwnd_data_cache.h" - -HWNDDataCache hwnd_cache; - -WindowAndProcPath HWNDDataCache::get_window_and_path(HWND hwnd) { - std::unique_lock lock(mutex); - auto ptr = get_internal(hwnd); - return ptr ? *ptr : WindowAndProcPath{}; -} - -HWND HWNDDataCache::get_window(HWND hwnd) { - std::unique_lock lock(mutex); - auto ptr = get_internal(hwnd); - return ptr ? ptr->hwnd : nullptr; -} - -WindowAndProcPath* HWNDDataCache::get_internal(HWND hwnd) { - auto root = GetAncestor(hwnd, GA_ROOT); - // Filter the fast and easy cases - if (!IsWindowVisible(root) || - is_invalid_hwnd(root) || - is_invalid_class(root) || - is_invalid_style(root)) { - return nullptr; - } - // Get the HWND process path from the cache - DWORD pid = GetWindowThreadProcessId(root, nullptr); - auto cache_ptr = get_from_cache(root, pid); - if (cache_ptr == nullptr) { - cache_ptr = put_in_cache(root, pid); - } - // If the app is a UWP app, check if it isnt banned - if (is_uwp_app(root) && is_invalid_uwp_app(cache_ptr->process_path)) { - // cache the HWND of the invalid app so we wont search for it again - invalid_hwnds.push_back(root); - return nullptr; - } - - return cache_ptr; -} - -WindowAndProcPath* HWNDDataCache::get_from_cache(HWND root, DWORD pid) { - auto next = next_timestamp(); - auto it = std::find_if(begin(cache), end(cache), [&](const auto& entry) { - return root == entry.data.hwnd && pid == entry.pid; - }); - if (it != end(cache)) { - it->atime = next; - return &(it->data); - } - else { - return nullptr; - } -} - -WindowAndProcPath* HWNDDataCache::put_in_cache(HWND root, DWORD pid) { - auto next = next_timestamp(); - auto it = std::min_element(begin(cache), end(cache), [](const auto& lhs, const auto& rhs) { - return lhs.atime < rhs.atime; - }); - it->atime = next; - it->pid = pid; - it->data.hwnd = root; - it->data.process_path = get_process_path(root); - return &(it->data); -} - -bool HWNDDataCache::is_invalid_hwnd(HWND hwnd) const { - return std::find(begin(invalid_hwnds), end(invalid_hwnds), hwnd) != end(invalid_hwnds); -} -bool HWNDDataCache::is_invalid_class(HWND hwnd) const { - std::array class_name; - GetClassNameA(hwnd, class_name.data(), static_cast(class_name.size())); - for (auto invalid : invalid_classes) { - if (strcmp(invalid, class_name.data()) == 0) - return true; - } - return false; -} -bool HWNDDataCache::is_invalid_style(HWND hwnd) const { - auto style = GetWindowLong(hwnd, GWL_STYLE); - // WS_POPUP need to have a border or minimize/maximize buttons, - // otherwise the window is "not interesting" - if ((style & WS_POPUP) == WS_POPUP && - (style & WS_THICKFRAME) == 0 && - (style & WS_MINIMIZEBOX) == 0 && - (style & WS_MAXIMIZEBOX) == 0) { - return true; - } - for (auto invalid : invalid_basic_styles) { - if ((invalid & style) != 0) { - return true; - } - } - style = GetWindowLong(hwnd, GWL_EXSTYLE); - for (auto invalid : invalid_ext_styles) { - if ((invalid & style) != 0) { - return true; - } - } - return false; -} -bool HWNDDataCache::is_uwp_app(HWND hwnd) const { - std::array class_name; - GetClassNameA(hwnd, class_name.data(), static_cast(class_name.size())); - return strcmp(class_name.data(), "Windows.UI.Core.CoreWindow") == 0; -} -bool HWNDDataCache::is_invalid_uwp_app(const std::wstring& process_path) const { - for (const auto& invalid : invalid_uwp_apps) { - // check if process_path ends in "invalid" - if (process_path.length() >= invalid.length() && - process_path.compare(process_path.length() - invalid.length(), invalid.length(), invalid) == 0) { - return true; - } - } - return false; -} - -unsigned HWNDDataCache::next_timestamp() { - auto next = ++current_timestamp; - if (next == 0) { - // Handle overflow by invalidating the cache - for (auto& entry : cache) { - entry.data.hwnd = nullptr; - } - } - return next; -} \ No newline at end of file diff --git a/src/common/hwnd_data_cache.h b/src/common/hwnd_data_cache.h deleted file mode 100644 index 738cda0add..0000000000 --- a/src/common/hwnd_data_cache.h +++ /dev/null @@ -1,55 +0,0 @@ -#pragma once - -#include -#include -#include - -#include "common.h" - -class HWNDDataCache { -public: - WindowAndProcPath get_window_and_path(HWND hwnd); - HWND get_window(HWND hwnd); -private: - // Return pointer to our internal cache - we cannot pass this to user - // since next call to get_* might invalidate that pointer - WindowAndProcPath* get_internal(HWND hwnd); - WindowAndProcPath* get_from_cache(HWND root, DWORD pid); - WindowAndProcPath* put_in_cache(HWND root, DWORD pid); - // Various validation routines - bool is_invalid_hwnd(HWND hwnd) const; - bool is_invalid_class(HWND hwnd) const; - bool is_invalid_style(HWND hwnd) const; - bool is_uwp_app(HWND hwnd) const; - bool is_invalid_uwp_app(const std::wstring& binary_path) const; - - // List of HWNDs that are not interesting - like desktop, cortana, etc - std::vector invalid_hwnds = { GetDesktopWindow(), GetShellWindow() }; - // List of invalid window basic styles - std::vector invalid_basic_styles = { WS_CHILD, WS_DISABLED }; - // List of invalid window extended styles - std::vector invalid_ext_styles = { WS_EX_TOOLWINDOW, WS_EX_NOACTIVATE }; - // List of invalid window classes - things like start menu, etc. - std::vector invalid_classes = { "SysListView32", "WorkerW", "Shell_TrayWnd", "Shell_SecondaryTrayWnd", "Progman" }; - // List of invalid persistent UWP app - like Cortana - std::vector invalid_uwp_apps = { L"SearchUI.exe" }; - - // Cache for HWND/PID pair to process path. A collision here, where a new process - // not in cache gets to reuse a cached PID and then reuses the same HWND handle - // seems unlikely. - std::mutex mutex; - // Handle timestamp wrap - unsigned next_timestamp(); - unsigned current_timestamp = 0; - struct Entry { - DWORD pid = 0; - // access time - when retiring element from cache we pick - // one with minimal atime value. We update this value - // every time we query the cache - unsigned atime = 0; - WindowAndProcPath data; - }; - std::vector cache{ 32 }; -}; - -extern HWNDDataCache hwnd_cache;