Skip to content

Update TopBar For Windows - #5446

Merged
m417z merged 35 commits into
ramensoftware:mainfrom
wasixgamer:main
Sep 11, 2026
Merged

Update TopBar For Windows#5446
m417z merged 35 commits into
ramensoftware:mainfrom
wasixgamer:main

Conversation

@wasixgamer

@wasixgamer wasixgamer commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Changelog

If this pull request updates an existing mod, describe the changes below:

  • Add Resource Monitor to show CPU, GPU, and RAM Usage on the topbar.
  • Fixed Rough Corners for the flyouts, and context menus.(Note: uses DWM to round corners which is Windows 11 exclusive)
  • Added option for moving or rearranging the TrayPanel(right panel) items.
  • Made Battery Icon look better
  • Fixed Win+D(Show Desktop) hides the TopBar.
  • Fixed Bluetooth panel not finding new devices.
  • Updated themes: NoIslands, and GreenBar.

Mod authorship

If this pull request introduces a new mod, please complete the section below.

This mod was created by:

    • The submitter, without AI assistance
    • The submitter, with AI assistance
    • Claude
    • ChatGPT
    • Gemini
    • Another AI (please specify): DeepSeek(for updating)
    • Other (please specify):

Please select the options that best apply. Your selection does not affect the acceptance criteria, but it helps reviewers understand the context of the code and provide relevant feedback.

@windhawk-reviewer windhawk-reviewer Bot added the waiting-for-author The author's turn: request an AI review, or respond to one that was posted. label Sep 11, 2026
@windhawk-reviewer

Copy link
Copy Markdown

New commits were pushed, so this pull request left the human review queue and is back to waiting-for-author.

Comment /ai-review to get an AI review of the updated code, then /ready-for-reviewer to hand it over to a human reviewer again. See the pull request review process for details.

@windhawk-reviewer windhawk-reviewer Bot removed the waiting-for-reviewer Ready for a human reviewer, and in the queue for one. label Sep 11, 2026
@wasixgamer

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Sep 11, 2026
@windhawk-reviewer

Copy link
Copy Markdown

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.


The new Resource Monitor is the main source of findings — its sampling path is shared mutable state reached from several concurrent one-second workers, and some of it runs on the XAML UI thread.

1. The resource samplers race on shared PDH state (crash potential).

Two independent one-second timers each spawn their own worker that samples the same PDH handles:

  • g_resourceTimerUpdateResourceButton() (line 6436) → RunInBackgroundresource::GetUsage()
  • g_resourceFlyoutTimerUpdateResourceFlyoutContent() (line 6675) → RunInBackgroundresource::GetUsage() + resource::GetDetailedInfo()

Neither timer waits for the previous worker, and g_infoGpuCombo.SelectionChanged (line 6783) and the tab buttons call UpdateResourceFlyoutContent() directly, so overlapping workers are easy to hit. They then collide on:

  • PdhCollectQueryData(g_cpuQuery) / PdhCollectQueryData(g_gpuQuery) from two threads at once (GetCpu() line 420, GetGpu() line 457). PDH query/counter handles are not thread-safe.
  • Unsynchronised lazy init of function-local statics. In GetCpuClockMHz() (line 493) two threads can both observe query == nullptr, both PdhOpenQuery, and one overwrites counter so it no longer belongs to the surviving query.
  • GetVramUsed() (line 896) is the worst case — when gpuIndex changes it does PdhCloseQuery(vramQuery) and rebuilds the counter list while another thread may be inside PdhCollectQueryData(vramQuery) on the handle that was just closed. Switching GPUs in the combo box while the flyout timer is ticking is exactly that sequence.

Simplest fix: put the whole sampling path behind one mutex (the same one resource::Initialize() already uses). Cleaner fix: keep a single long-lived sampling thread and have both timers ask it for the latest snapshot, instead of spawning a thread per tick.

2. Blocking WMI / COM / WLAN calls on the XAML UI thread.

The bar freezes for as long as these take. The Display/Wi-Fi/Bluetooth buttons already use the right pattern (RunInBackgroundRunOnUiThread), but several paths still call straight through:

  • ApplyResourceFlyoutContentUI() runs on the UI thread once per second and calls resource::GetCpuFullName() (line 6464) and resource::GetAllGpuNames() (line 6509) — both are CoCreateInstance(WbemLocator) + ConnectServer + ExecQuery. GetCpuFullName() caches only on success, so if the query returns nothing it re-runs the full WMI connect every second.
  • PopulateDisplayPanel() calls brightness::Available() / brightness::Get() (lines 5092-5094) — a root\WMI query. It's reached from RepopulateLater(PopulateDisplayPanel) after toggling dark mode (line 5120, 7700).
  • PopulateSoundPanel() runs synchronously from flyout.Opening and calls audio::GetMasterVolume() (5338), audio::EnumerateOutputDevices() (5364) and audio::EnumerateSessions() (5402).
  • PopulateWifiPanel() calls wifi::GetStatus() (line 5655), which opens a WLAN handle and enumerates interfaces.
  • BuildTopBarContent() — which runs on every settings change — calls audio::GetMasterVolume() (7715), wifi::GetStatus() (7722) and GetBatteryInfo()QueryBatteryHealth() (7788), i.e. another WMI query.

Move the data gathering into the existing RunInBackground helper and push only the resulting snapshot back through RunOnUiThread, the way the Display button's onOpening handler at line 7663 already does.

3. The window-event hook subscribes to a much wider range than it uses.

g_windowEventHook = SetWinEventHook(EVENT_OBJECT_CREATE, EVENT_OBJECT_NAMECHANGE, ...);  // line 8417

That range is 0x80000x800C, which also covers EVENT_OBJECT_REORDER, EVENT_OBJECT_FOCUS, EVENT_OBJECT_SELECTION*, EVENT_OBJECT_STATECHANGE and — the expensive one — EVENT_OBJECT_LOCATIONCHANGE, which fires for every window move/resize, caret movement and hover-tracking update in every process on the desktop. Each of those is marshalled cross-process into this process only for WindowEventProc (line 8180) to drop it. Register only the events actually handled, e.g. one hook for EVENT_OBJECT_CREATEEVENT_OBJECT_HIDE and a second for EVENT_OBJECT_NAMECHANGE.

4. Per-second work that should be change-driven.

g_clockTimer ticks every second (line 8566) and does:

  • UpdateBatteryButton()BuildBatteryIcon()Markup::XamlReader::Load — a full XAML parse, every second, to redraw a battery percentage that changes a few times an hour. Cache the last (percentage, charging) pair and rebuild only when it changes.
  • UpdateWallpaperIfChanged()SystemParametersInfo(SPI_GETDESKWALLPAPER) + GetFileAttributesExW — a file stat every second, even though WM_SETTINGCHANGE already handles wallpaper changes (line 8304).

Separately, g_resourceTimer calls RunInBackground every second, and RunInBackground (line 4591) creates a brand-new thread with CoInitializeEx/CoUninitialize per job — a thread per second for the lifetime of the process. It also keeps ticking when the resource button is collapsed because all three show*Usage settings are off. Reuse one worker thread with a job queue, and stop the timer when there's nothing to show.

5. winrt::com_ptr::put() is reused across loop iterations.

winrt::com_ptr<IDXGIAdapter1> adapter;
for (UINT i = 0; factory->EnumAdapters1(i, adapter.put()) != DXGI_ERROR_NOT_FOUND; ++i) {  // lines 848, 879

com_ptr::put() asserts that the pointer is null (WINRT_ASSERT) and returns &m_ptr. From the second iteration on, adapter still holds the previous adapter, so the assert trips in debug and the old IDXGIAdapter1 reference is silently overwritten and leaked in release. Declare adapter inside the loop body, or adapter = nullptr; at the top of each iteration. Same pattern in both GetVramTotal() and GetGpuLuidForIndex().

6. Bluetooth enumeration does a GATT battery read for every device.

bluetooth::Enumerate() calls GetBatteryPercent() for each device found (line 4261, and again for the WinRT fallback at 4304). GetBatteryPercent() (line 4187) does BluetoothLEDevice::FromBluetoothAddressAsync().get() + GetGattServicesAsync().get() + GetCharacteristicsAsync().get() + ReadValueAsync().get() — a full GATT connection attempt per device, which takes seconds for anything out of range. This happens on the "fast, no inquiry, instant" path too (bluetooth::Enumerate(false), line 7772), on every connect/disconnect refresh (6100) and on every radio toggle (5993), so the device list takes many seconds to appear and repeatedly wakes paired LE radios. Restrict the battery read to devices already reported connected, cache the value, and fill it in asynchronously after the list is on screen rather than blocking the enumeration.

Optional improvements

Cleanliness items — none of these affect users, so it's your call.

  • Dead code. Quite a lot accumulated across this rewrite:
    • CaptureScreenRect() (line 6171, ~95 lines of screen-scraping) and its only caller-less helper Bgra32ToBitmapImage() (6126)
    • void PopulateTrayPanel(); (line 1026) — forward-declared, never defined or called
    • resource::SumPdhCounter() (438), resource::GetProcessCount() (547), resource::Cleanup() (972 — never called, so the PDH queries are never closed), DetailedInfo::processCount/virtualMemoryTotal (never displayed)
    • ToHexString() (1688), MakeToggleTile() (4845), MakeTileRow() (4871)
    • struct TrayDragState / g_trayDragState (1080-1086) — left over from the drag-reorder approach that the right-click menu replaced
    • g_settings.showTrayButton (1058) — hard-set to false at 8845 and never read
    • g_backgroundJobs (4587) — incremented and decremented, never read; the comment above it says Wh_ModUninit drains it, but teardown actually waits on the thread handles
  • Unused includes and libraries. <uiautomation.h>, <psapi.h>, <commctrl.h>, <windowsx.h>, <thread> and <winrt/Windows.ApplicationModel.DataTransfer.h> have no corresponding usage, and -lpsapi -lcomctl32 in @compilerOptions are not needed. <tlhelp32.h> is only needed by the dead GetProcessCount().
  • [[clang::no_destroy]] wrapper form. The bare attribute is right for the nullable WinRT projected types (wuxc::Button, DispatcherTimer, …), but for the containers of strong XAML references it should be the std::optional wrapper, because .clear() releases the elements but keeps the heap buffer, and only running ~T() via optional::reset() fully frees it — g_tabButtons (1199), g_namedElements (1215), g_taskButtonsByHwnd (1220), g_detachedStyleRoots (1571). Conversely g_contextMenuTargetHwnd (1296, a plain HWND), g_stableWindowOrder (1218) and g_taskButtonLastTitle (1221) hold only plain values — their destructors are a heap free at worst, so the attribute is unnecessary there and is best removed so it doesn't get cargo-culted. Also, the UI-thread teardown block (8746-8798) releases only some of the annotated globals — g_soundFlyout/g_wifiFlyout/g_bluetoothFlyout/g_batteryFlyout, the matching buttons and panels, g_trayPanel, g_graphCanvas/g_graphLine/g_graphFill/g_statsGrid, g_statLabel0-3, g_statValue0-3, g_statCell0-3 and g_infoCpu*/g_infoRam*/g_infoGpu* are never set back to nullptr. It doesn't bite here because Wh_ModUninit ends in ExitProcess(0), but it's worth keeping the list consistent. Background: https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown (the #4-xaml-and-other-ui-thread-objects section covers this case).
  • WindhawkUtils::StringSetting instead of the hand-rolled copy helper. GetStringSettingCopy (1335) can be a one-liner, and the value ? value : L"" check is unnecessary — Wh_GetStringSetting never returns NULL, it returns L"":
    template <typename... Args>
    std::wstring GetStringSettingCopy(PCWSTR name, Args... args) {
        return WindhawkUtils::StringSetting::make(name, args...).get();
    }
  • RefreshBluetoothRadioState() on DPI change. Line 8293, inside the WM_DISPLAYCHANGE/WM_DPICHANGED case, with the trailing comment // initial radio state and an indentation level that doesn't match the block. Refreshing the Bluetooth radio because the DPI changed looks unintentional — is this a leftover?
  • @donateUrl instead of a README banner. The Patreon image at README line 23 could be // @donateUrl https://www.patreon.com/WasiXGamer/join — Windhawk surfaces that in the UI properly. A few mods already use it, e.g. mods/taskbar-multi-tray.wh.cpp#L10.
  • DWMWA_NCRENDERING_POLICY takes an enum, not a BOOL. Line 8079-8080 passes TRUE, which happens to equal DWMNCRP_DISABLED. Spelling it DWMNCRP_DISABLED makes the intent clear.
  • ApplyBlurToAllOpenPopups() matches very broadly. The class filter at 8106-8108 includes wcsstr(className, L"Window"), which catches nearly anything, and the function is called from MenuFlyoutSubItem::PointerEntered (7048), so a full EnumWindows sweep runs on every hover over a submenu item. Matching just the XAML popup class and hooking Opened would be enough.
  • Worker-thread bookkeeping in WhTool_ModUninit (9032-9048): the snapshot is taken under g_workerThreadsMutex, but g_workerThreads.clear() at 9048 is not, and a RunInBackground call that passed the g_shuttingDown check just before the flag was set can push a handle after the snapshot — that handle is then dropped without being waited on or closed. Harmless because ExitProcess(0) follows, but the clear() belongs inside the lock.
  • PairDevice's 30-second timeout doesn't actually time out. At 4356-4383, when future.wait_for(...) returns timeout the function returns without calling get(), so ~future for an std::async(std::launch::async, …) blocks until the pairing task finishes anyway. If you want a real bound, the pairing call needs its own cancellation rather than a future timeout.

Functionality notes

Non-critical observations about the feature behaviour itself.

  • The Bluetooth auto-refresh timer is never started. g_bluetoothAutoRefreshTimer is created in EnsureAutoRefreshTimers() (7480) with a 15 s interval and is only ever Stop()ped (7646, 8737) — and EnsureAutoRefreshTimers() itself is only reached from the Wi-Fi button's opening handler (7746). So the "more frequent" Bluetooth refresh doesn't run at all; the panel only refreshes via the header refresh button and the one-shot scan at 7777.
  • A connecting Wi-Fi network shows the Bluetooth glyph. Line 5782 uses icons::kBluetoothStroke as the trailing element for g_wifiConnectingSSID, with the leftover comment // use a generic loading dot? or just no trailing. A small ProgressRing (you already build one for the header at 5681) would read better. Also, g_wifiConnectingSSID is only set on the open-network path (5813) — for a secured network the flow goes through the password prompt and ConnectToWifi directly, so "Connecting…" never appears there.
  • monitorIndex numbering. The description says "1 = primary monitor. Otherwise the secondary monitor number", but GetBarMonitor() (7912) walks EnumDisplayMonitors, whose enumeration order isn't documented to put the primary first. Either say the index is "display enumeration order, not necessarily the Settings numbering", or pick the primary explicitly for index 1 and enumerate the rest.
  • The wallpaper layer assumes the primary monitor and a "Fill" wallpaper style. GetWallpaperBrush() (7197) stretches the whole wallpaper UniformToFill aligned top-left, which lines up with the desktop only when the bar is on the monitor that the wallpaper is scaled to and the desktop fit is Fill. On a secondary monitor, or with Fit/Center/Tile/Span, the strip won't match what's behind it.
  • IsTaskbarEligibleWindow() (2480) rejects every window that has an owner and every WS_EX_TOOLWINDOW. The shell's own rule also admits owned or tool windows that set WS_EX_APPWINDOW, so a few apps (some installers, Electron splash-then-main flows) won't get a task button.
  • g_restoreTimer (8600) polls once a second and force-restores/un-cloaks/re-topmosts the bar, on top of WM_SIZE, WM_SHOWWINDOW, WM_SYSCOMMAND and WM_WINDOWPOSCHANGING all refusing to let the window hide. It works, but it means nothing in the system can ever hide the bar, and the desktop-foreground branch deliberately bounces the window through HWND_BOTTOMHWND_TOPMOST (8670-8673), which can flicker. If the goal is just "Win+D must not hide the bar", the WM_WINDOWPOSCHANGING guard alone should cover it and the timer could go.
  • ApplyAllControlStyles() cost. Every rule target triggers a full recursive CollectMatchingElements walk of the root plus every registered detached root, and the function is called at the end of PopulateWifiPanel (5823) and PopulateBluetoothPanel (6117) — i.e. on every scan result. With the built-in rules alone that's ~15 tree walks per repopulate. Collecting the tree once and testing all rules against each element would scale better as the style list grows.
  • ApplyResourceUsageToButton() (6397) replaces g_resourceButton.Content(...) with a freshly built TextBlock every second, which allocates a new XAML element per tick and discards any style the user applied to the previous content. Keeping one TextBlock and calling .Text(...) on it avoids both.
  • GPU combo self-trigger. g_infoGpuCombo.SelectedIndex(g_selectedGpuIndex) at 6516 fires SelectionChanged (6783), which kicks off another UpdateResourceFlyoutContent() — one extra background sample each time the GPU tab is first shown. The g_populatingPanel guard used elsewhere would suppress it.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Sep 11, 2026
@wasixgamer

Copy link
Copy Markdown
Contributor Author

/ready-for-reviewer

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-reviewer Ready for a human reviewer, and in the queue for one. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Sep 11, 2026
@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-reviewer Ready for a human reviewer, and in the queue for one. labels Sep 11, 2026
@windhawk-reviewer

Copy link
Copy Markdown

New commits were pushed, so this pull request left the human review queue and is back to waiting-for-author.

Comment /ai-review to get an AI review of the updated code, then /ready-for-reviewer to hand it over to a human reviewer again. See the pull request review process for details.

@wasixgamer

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Sep 11, 2026
@windhawk-reviewer

Copy link
Copy Markdown

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.


The tool-mod boilerplate matches the wiki snippet verbatim, the settings block and LoadSettings are in sync, every LoadLibrary goes through LOAD_LIBRARY_SEARCH_SYSTEM32, and nothing contacts the network — good. The new resource monitor is where most of the problems are:

1. A new OS thread is created every second, forever. g_resourceTimer ticks once per second and calls UpdateResourceButton() (line 6448), which goes through RunInBackground() (line 4602) — i.e. _beginthreadex + CoInitializeEx + PDH collection + CoUninitialize + thread teardown, once per second for the lifetime of the process. While the resource flyout is open, g_resourceFlyoutTimer does the same thing again via UpdateResourceFlyoutContent() (line 6701), so it becomes two thread creations per second. That's a lot of churn for a sampler, and RunInBackground additionally walks and reaps the handle vector under a mutex on every call.

Use one long-lived sampler thread that waits on g_stopEvent with a 1 s timeout and publishes snapshots to the UI thread, instead of a thread per tick. taskbar-system-info.wh.cpp does exactly this — MetricsWorkerProc loops on WaitForSingleObject(g_metricsWorkerWakeEvent, interval) and publishes into a mutex-guarded snapshot, and it's joined on unload. Also worth skipping sampling entirely when showCpuUsage/showRamUsage/showGpuUsage are all off (right now the timer keeps sampling even though ApplyResourceUsageToButton collapses the button).

2. Blocking WMI / WLAN / COM calls still run on the XAML UI thread. Some paths were moved to workers in this PR (the Display, Wi-Fi and Bluetooth flyout-open handlers), but several equivalents weren't, and they freeze the whole bar for as long as the query takes — WMI in particular routinely takes hundreds of milliseconds to seconds on its first use in a process:

  • PopulateSoundPanel() (line 5326) runs audio::GetMasterVolume(), audio::GetMasterMute(), audio::EnumerateOutputDevices() (line 5375) and audio::EnumerateSessions() (line 5413) synchronously — this is the Sound flyout's Opening handler, so opening it stalls the bar.
  • ApplyResourceFlyoutContentUI() runs on the UI thread but calls resource::GetCpuFullName() (line 6476) and resource::GetAllGpuNames() (line 6525), both of which do CoCreateInstance + ConnectServer + ExecQuery. Worse, GetCpuFullName only caches on success (static std::wstring name stays empty on failure), so on a machine where the query fails it re-runs the whole WMI round-trip every second while the flyout is open. Both values are already available on the worker that computes DetailedInfo — fetch them there and pass them in.
  • GetBatteryInfo()QueryBatteryHealth() (line 6290) is WMI, and it's called from BuildTopBarContent() (line 7816), i.e. during initial bar construction, and again from the 1 s clock timer (line 8609).
  • The brightness wheel handler calls brightness::GetFast() (line 7328), which falls through to Get()WmiGet() on the first notch.
  • RefreshWifiButtonIcon() calls wifi::GetStatus() (WlanOpenHandle + enum + query) directly on the UI thread.
  • RepopulateLater(PopulateDisplayPanel) (lines 5131, 7728) routes back to the blocking PopulateDisplayPanel() (line 5088, brightness::Available()/brightness::Get()), bypassing the background path you added for the open handler.

3. SetAppsDarkMode() broadcasts a message from the UI thread. Line 3599:

SendMessageTimeout(HWND_BROADCAST, WM_SETTINGCHANGE, 0,
                   reinterpret_cast<LPARAM>(L"ImmersiveColorSet"), SMTO_ABORTIFHUNG, 200,
                   &result);

The 200 ms timeout is per window, and this runs inside the dark-mode tile's Click handler on the UI thread — with a few unresponsive top-level windows around, the bar visibly hangs. Move the whole SetAppsDarkMode call into RunInBackground and repopulate the panel from the completion.

4. Task-button icons are re-extracted from disk on every title change. RefreshTaskList calls UpdateTaskButtonStateBuildTaskButtonContent (line 2627) whenever a window's title changes, and that rebuilds the icon via GetWindowIconBitmapExtractCrispWindowIcon (line 2283), which does PrivateExtractIconsW against the executable on disk and can fall back to SendMessageTimeout(WM_GETICON, ..., 100) — all on the UI thread. Title changes are frequent (browser tab switches, media players, progress in a title bar), so with taskButtonContent set to iconAndText/iconOnly this is a recurring stutter. Cache the BitmapImage keyed by executable path (and physical size), and on a title-only change update just the TaskButtonText instead of rebuilding the whole content grid.

Optional improvements

Minor polish — none of this affects users, so it's your call.

Dead code. There's a fair amount of it, and some of it has comments that describe behavior the code doesn't have — worth a sweep, especially since the changelog mentions AI assistance:

  • CaptureScreenRect() (line 6182) and its only consumer-less helper Bgra32ToBitmapImage() (line 6137) — ~140 lines, never called.
  • resource::Cleanup() (line 977) is never called, so the PDH queries are never closed. The function-local static PDH handles in GetCpuClockMHz() (line 496) and GetVramUsed() (line 902) aren't closed by it either. Harmless here since the process exits on unload, but the function is misleading as-is.
  • resource::SumPdhCounter() (line 439), resource::GetProcessCount() (line 549, plus the <tlhelp32.h> include it's the only user of), ToHexString() (line 1695), MoveTrayItemTo() (line 7442), TrayDragState/g_trayDragState (lines 1085–1091) — the drag-reorder scaffolding is never wired up; only the right-click "Move left/right" menu is.
  • PopulateTrayPanel() forward-declared (line 1031) with no definition; icons::kBatteryChargingPath / kBatteryNotChargingPath (lines 1971, 1973) are unused and identical to each other; s_emptyGpuHistory (line 6580); g_lastWallpaperPath (line 1163, written never read); g_settings.showTrayButton (lines 1063, 8887, only ever assigned false); brightness::g_cachedWmiServices (line 3261, only ever assigned nullptr in teardown).
  • g_backgroundJobs (line 4598) is incremented and decremented but never read — and the comment above it says "Wh_ModUninit waits for the count to drain", which isn't what happens (it waits on the handle vector).
  • The comment above ActivateTaskWindow (line 2678) says "Single click activates immediately. No delay" but the Tapped handler now defers by GetDoubleClickTime() (line 2729).

Unused includes / link libraries. <uiautomation.h>, <windowsx.h>, <thread> (no std::thread anywhere), <dxgi1_3.h> (only IDXGIFactory1/IDXGIAdapter1 are used, <dxgi.h> covers those), winrt/Windows.ApplicationModel.DataTransfer.h, and <psapi.h>. In @compilerOptions, -lpsapi and -lcomctl32 don't appear to be needed (no psapi or comctl32 entry points are called).

[[clang::no_destroy]] hygiene. See Global objects and process shutdown for the background. Two directions here:

  • Unnecessary suppressions — g_contextMenuTargetHwnd (line 1302) is a plain HWND, g_stableWindowOrder (line 1224) is a std::vector<HWND> and g_taskButtonLastTitle (line 1227) is a std::map<HWND, std::wstring>. All three have heap-free-only (or trivial) destructors, which are safe at shutdown; the attribute is noise and invites cargo-culting. Drop it.
  • Containers that genuinely hold strong XAML refs — g_namedElements (line 1221), g_taskButtonsByHwnd (line 1226), g_tabButtons (line 1204), g_detachedStyleRoots (line 1578) — should use the [[clang::no_destroy]] std::optional<T> wrapper and be released with .reset(), not .clear(). .clear() does run the element destructors but keeps the container's heap buffer; optional::reset() is the form that fully releases.

Also, the teardown block at lines 8802–8840 releases only some of the no_destroy XAML globals — g_soundFlyout/g_wifiFlyout/g_bluetoothFlyout/g_batteryFlyout, the matching buttons and panels, g_graphCanvas, g_graphLine, g_graphFill, g_statsGrid, the g_stat* and g_info* blocks, and g_trayPanel are never nulled. Moot because Wh_ModUninit ends in ExitProcess(0), but the inconsistency makes the intent hard to read.

Teardown race on the cached endpoint volume. Line 8839 does audio::g_cachedEndpointVolume = nullptr; on the UI thread without taking audio::g_endpointMutex, while WhTool_ModUninit may still have worker threads running inside EndpointVolume() / SetMasterVolume() (it only waits 10 s and then proceeds regardless). Same shape for g_stopEvent, which is CloseHandled while a worker that outlived the 10 s wait may still call WaitForSingleObject on it. Unload-only, and the process exits immediately after, but taking the mutex for the assignment is a one-liner.

Stray call in WM_DISPLAYCHANGE. Line 8328 calls RefreshBluetoothRadioState() from the WM_DISPLAYCHANGE/WM_DPICHANGED handler (with the comment "initial radio state"), which spawns a worker and a WinRT Radio::RequestAccessAsync() round-trip every time the display configuration changes. Looks like a misplaced line.

Shared flags across threads. g_allowHide (line 1315) is written from the WhTool_ModUninit thread and read from the UI thread's window proc and timers as a plain bool; g_fullScreenAppActive is similar. std::atomic<bool> would be cleaner, and g_shuttingDown (line 4529, volatile LONG + InterlockedCompareExchange) could become one too.

Style resolution cost. ApplyRuleList (line 1758) looks a target up in g_namedElements and then also runs ResolveGeneralTarget(name), which does a full recursive VisualTreeHelper walk of the root plus every registered detached popup root — for every rule, on every ApplyAllControlStyles(). Since RefreshTaskList calls that whenever the task list changes (debounced to 200 ms by the WinEvent hook), that's a lot of repeated tree walking. Caching the parsed chains, or short-circuiting for targets that are known-unique registered names, would cut most of it.

ApplyBlurToAllOpenPopups() on hover. It's wired to MenuFlyoutSubItem::PointerEntered (line 7076), so every pointer-enter runs EnumWindows over all desktop top-level windows and calls DwmSetWindowAttribute on each match. Doing it from the flyout/menu Opened events only would be enough.

std::stoull in the Bluetooth WinRT fallback. Line 4306 can throw std::invalid_argument for an unexpected device ID; the catch (...) is around the whole enumeration loop, so one malformed ID aborts the entire fallback. std::from_chars or a per-item try/catch would be more robust.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

Single click is now delayed by GetDoubleClickTime(). CreateTaskButton's Tapped handler (line 2710) defers activation through g_taskClickTimer so a double-tap can cancel it. That's a correct way to disambiguate, but it means every task-button click takes ~500 ms (the default) to do anything, which reads as lag. The real taskbar activates on the first click and treats the second as a separate action. Worth considering whether the double-click-to-maximize gesture is worth that cost, or whether it should be opt-in. Also, GetDoubleClickTime() is read once when the timer is first created and never refreshed if the user changes it.

The bar is kept visible by a combination of guards plus a 1 s poll. WM_SIZE/SIZE_MINIMIZED re-shows, WM_SHOWWINDOW re-shows, WM_WINDOWPOSCHANGING strips SWP_HIDEWINDOW, and g_restoreTimer (line 8638) re-checks iconic/cloaked/visible/position/z-order every second. I understand why (the Win+D fix), and there isn't an obviously cleaner alternative for an ABE_TOP AppBar that has to survive Show Desktop — but with the three message-level guards in place the 1 s poll is largely redundant, and it means any legitimate attempt to hide the window (including by an accessibility or capture tool) is overridden. If you keep it, consider raising the interval.

monitorIndex semantics. The description says "1 = primary monitor. Otherwise the secondary monitor number", but GetBarMonitor() (line 7947) just takes the Nth monitor from EnumDisplayMonitors, whose ordering isn't documented to put the primary first. 0 also maps to the primary (via MonitorFromPoint), which the description doesn't mention. Matching on the MONITORINFOF_PRIMARY flag, or on the device name from EnumDisplayDevices, would be more predictable.

GPU usage is the busiest single engine. resource::GetGpu() (line 458) takes the maximum across all \GPU Engine(*)\Utilization Percentage instances, i.e. across all adapters and all engine types. On a hybrid-graphics laptop the number can jump between the iGPU and dGPU, which won't match the GPU selected in the flyout's dropdown. Filtering the engine instances by the selected adapter's LUID — the same way GetVramUsed() already does (line 900) — would make the button and the graph agree.

detailed.virtualMemoryTotal is computed in GetDetailedInfo but never displayed; only virtualMemoryUsed reaches the stats grid.

WhTool_ModSettingsChanged early-returns when g_uiDispatcherQueue is null (line 9005) with the comment "Settings will be loaded on next start" — but in a tool mod there is no next start unless the user disables and re-enables the mod. If XAML Islands failed to initialize, that branch means settings changes are silently ignored for the rest of the process's life.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Sep 11, 2026
@wasixgamer

Copy link
Copy Markdown
Contributor Author

/ready-for-reviewer

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-reviewer Ready for a human reviewer, and in the queue for one. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Sep 11, 2026
@m417z
m417z merged commit 50b1da2 into ramensoftware:main Sep 11, 2026
5 checks passed
@windhawk-reviewer windhawk-reviewer Bot removed the waiting-for-reviewer Ready for a human reviewer, and in the queue for one. label Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants