Skip to content

Add Dynamic Taskbar Theme Switcher mod - #5274

Open
VitorAsse wants to merge 4 commits into
ramensoftware:mainfrom
VitorAsse:add-dynamic-taskbar-theme-switcher
Open

Add Dynamic Taskbar Theme Switcher mod#5274
VitorAsse wants to merge 4 commits into
ramensoftware:mainfrom
VitorAsse:add-dynamic-taskbar-theme-switcher

Conversation

@VitorAsse

@VitorAsse VitorAsse commented Aug 29, 2026

Copy link
Copy Markdown

Summary

Adds Dynamic Taskbar Theme Switcher, a self-contained Windows 11 taskbar mod
that automatically switches between two bundled Taskbar Styler themes per
monitor when a normal window is open or a window is maximized/fullscreen.

The mod is derived primarily from Windows 11 Taskbar Styler and also adapts
concepts from Dynamic Taskbar Transparency and Taskbar Background Helper. The
source metadata and README identify the upstream projects, authors, theme
contributors, Minecraft Hotbar author, license, usage, differences, conflicts,
and limitations.

Local validation completed successfully with the repository's PR validator.
The mod also compiles successfully for x86-64 and ARM64 with Windhawk 1.7.3.

Changelog

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

  • Initial public release.
  • Adds per-monitor switching between desktop and window themes.
  • Supports maximized/fullscreen or any-normal-window triggers.
  • Adds delayed return to the desktop theme and Minecraft/native appearance
    controls.
  • Keeps a raised Minecraft taskbar below normal application windows.
  • Adds a neutral adaptive background to the native window theme.
  • Incorporates reviewer feedback: removes the copied stats timer, restores
    taskbar position and z-order only when the mod changed them, protects shared
    settings used by hooks, and fixes Native customization behavior and labels.
  • Replaces permanent 250 ms polling with debounced WinEvent-driven refreshes
    and a 60-second safety scan, while caching the per-monitor dynamic-window
    snapshot.
  • Limits taskbar hooks, XAML initialization, and system-wide event tracking to
    the Explorer process that actually owns a taskbar.
  • Improves raised-taskbar restoration and avoids nested SetWindowPos calls in
    hook paths.
  • Adds an option to preserve the original Native icon and indicator appearance.
  • Documents process-wide XAML resource leakage, positioning interactions, and
    offline limitations of image-based themes.

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): OpenAI Codex
    • 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

Copy link
Copy Markdown

Thanks for the pull request! This repository uses a two-stage review: an AI review that you run yourself, followed by a human review.

To get started, comment /ai-review. Once you're happy with the result, comment /ready-for-reviewer to hand it over to a human reviewer.

See the pull request review process for the full details.

@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 Aug 29, 2026
@VitorAsse

Copy link
Copy Markdown
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 Aug 29, 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 biggest question here is structural: this is a hard fork of an actively maintained mod, and that shapes most of what follows.

1. This is a fork of Windows 11 Taskbar Styler, not a new mod. Of the 21,243 lines, roughly 20,400 are a verbatim copy of windows-11-taskbar-styler.wh.cpp — the whole XAML diagnostics/styling engine plus all ~60 bundled community themes. Only about 1,050 lines are new (the per-monitor trigger thread, the RuleThemeScope split, the Minecraft sizing/raise logic). The README is upfront about this, which is appreciated, but it doesn't change the practical problem:

  • The catalog gets a second copy of the styling engine and of every theme. Any engine fix or theme update upstream has to be hand-merged here forever, and users will hit bugs that were already fixed in the original.
  • Taskbar Styler is actively maintained by m417z (development happens at m417z/my-windhawk-mods), so this isn't an abandoned-mod situation where a rewrite is the only option.
  • The dynamic per-monitor part also overlaps taskbar-background-helper (which already does per-monitor maximized-window detection and is explicitly designed to run alongside Taskbar Styler) and dynamic-taskbar-transparency.

The genuinely new idea — "switch between two of the bundled themes per monitor depending on window state" — is a feature request for Taskbar Styler, not a reason to duplicate it. Please open an issue (or a PR against m417z/my-windhawk-mods) proposing a desktopTheme/windowTheme pair plus the trigger settings as an option on the existing mod. The Minecraft-specific sizing/raise controls are a separate discussion — they're very theme-specific and would more naturally live as theme style constants.

Everything below applies to the code as submitted, in case the fork route continues in some form.

2. Leftover upstream telemetry. StartStatsTimer() (L20178, called at L21125) was copied over wholesale. It arms a 24-hour threadpool timer that Wh_GetUrlContents https://github.com/ramensoftware/windows-11-taskbar-styling-guide/releases/download/stats-v6/<theme>.txt — the upstream project's own usage-stats endpoint. It happens to be inert because it reads Wh_GetStringSetting(L"theme"), and this mod no longer declares a theme setting, so it returns early on an empty string. Either way it shouldn't be here: a third-party mod shouldn't report usage to another project's endpoint. Delete StartStatsTimer/StopStatsTimer, g_statsTimer, and their call sites.

3. The mod repositions and re-tops every taskbar unconditionally, even when its own feature is off.

PositionTaskbarWindowProc (L20706) only uses ShouldRaiseTaskbarFromBottom to decide the offset, not whether to move the window at all:

int distanceInPixels = 0;
bool raiseFromBottom = !context->forceBottom && ShouldRaiseTaskbarFromBottom(hWnd);
if (raiseFromBottom) { ... }
int targetTop = monitorInfo.rcMonitor.bottom - taskbarHeight - distanceInPixels;
if (std::abs(taskbarRect.top - targetTop) > 1) { setWindowPos(...); }

So with distanceInPixels == 0 it still snaps the taskbar to rcMonitor.bottom - taskbarHeight. UpdateTaskbarPositions() runs on load (Wh_ModAfterInit), on every settings change, on unload, and on every theme switch — meaning a user who picked a non-Minecraft desktop theme, or set raiseFromBottom to 0, still gets their taskbar force-moved. The concrete breakage is auto-hide: a hidden Shell_TrayWnd sits with only a couple of pixels on screen, targetTop differs by nearly the full taskbar height, and the mod drags it into view.

EnsureTaskbarWindowZOrder (L20534) has the mirror-image problem — when not raising, it re-asserts HWND_TOPMOST on any taskbar that isn't topmost, and UpdateTaskbarZOrders() runs it on every 250 ms poll tick, so it will fight anything else that manages taskbar z-order indefinitely.

Both should be no-ops unless the mod actually raised that window: track the taskbars you moved (and their original z-order state) and only touch those, both while running and when restoring.

4. 250 ms polling doing very expensive work, plus the same work inside global SetWindowPos hooks. DynamicThemeThreadProc (L20857) runs four times a second, forever, inside explorer.exe, and each tick does:

  • GetConnectedMonitors()EnumDisplayMonitors
  • FindDynamicThemeMonitors()EnumWindows over every top-level window on the desktop, calling IsDynamicWindowCandidate on each, which does a DwmGetWindowAttribute(DWMWA_CLOAKED) (a cross-process call into DWM) and OpenProcess + QueryFullProcessImageNameW per window
  • UpdateTaskbarZOrders() — another full EnumWindows

And AdjustTaskbarWindowZOrder, reached from the process-wide SetWindowPos/DeferWindowPos hooks, calls FindLowestDynamicWindowForTaskbar → yet another full z-order walk with the same per-window DWM/OpenProcess calls, plus IsWindowAboveInZOrder walking it again.

This should be event-driven. taskbar-background-helper solves exactly this problem with SetWinEventHook — see its handler around L950, which tracks maximized windows per monitor from EVENT_OBJECT_CREATE/DESTROY/SHOW/HIDE/LOCATIONCHANGE/CLOAKED/UNCLOAKED instead of enumerating. At minimum, cache the per-HWND process name and the cloaked/class checks rather than recomputing them for every window on every tick.

5. Data race on g_desktopThemeName (and g_settings) against the always-live position hooks. ShouldRaiseTaskbarFromBottom (L20482) reads the std::wstring g_desktopThemeName and g_settings.minecraft.raiseFromBottom:

return !g_forceTaskbarBottom.load() && !IsMonitorUsingWindowTheme(monitor) &&
       g_desktopThemeName == L"Minecraft_Hotbar" &&
       g_settings.minecraft.raiseFromBottom > 0;

It's called from SetWindowPos_Hook, DeferWindowPos_Hook, MoveWindow_Hook and the WM_WINDOWPOSCHANGING subclass — i.e. from arbitrary Explorer UI threads — and those hooks stay installed for the whole of Wh_ModSettingsChanged, where LoadSettings() reassigns g_desktopThemeName from Windhawk's thread. Concurrent read/write of a std::wstring is UB: the reader can compare against a buffer that was just freed. Stopping the poll thread first doesn't help, because the hooks aren't the poll thread.

Precompute the decision in LoadSettings into atomics, e.g.:

std::atomic<bool> g_raiseMinecraftTaskbar;   // desktopTheme == Minecraft && raiseFromBottom > 0
std::atomic<int>  g_raiseFromBottomDip;

and read only those from the hook paths.

6. The window theme's themeResourceVariables are never applied. ProcessAllStylesFromSettings (L19582):

ProcessThemeStyles(g_desktopThemeName.c_str(), RuleThemeScope::desktop, false, true);
ProcessThemeStyles(g_windowThemeName.c_str(),  RuleThemeScope::window,  true,  false);

processResourceVariables is true only for the desktop theme. Three bundled themes declare themeResourceVariablesLayerMicaUI, Pills, Blob — so selecting any of them as Window theme renders it incorrectly. And in the other direction, resource variables are merged into a resource dictionary and aren't filtered by RuleThemeScope, so a desktop theme's variables also stay applied while the window theme is active. Resource variables need the same desktop/window switching treatment as the style rules, or these themes should at least be excluded from the window-theme dropdown.

7. nativeIconSize and nativeIndicator silently do nothing when Native is the desktop theme. In ProcessThemeStyles (L19553):

} else if (themeScope == RuleThemeScope::window && wcscmp(themeName, L"Native") == 0) {
    AddTaskIconSizeRules(g_settings.nativeIconSize, themeScope);
    AddIndicatorCustomizationRules(g_settings.nativeIndicator, themeScope);
}

Native is offered in the Desktop theme dropdown too, and a user who picks it there will change these settings and see nothing happen. Drop the themeScope condition (the Minecraft settings above it are correctly applied in both scopes).

8. The restart-Explorer dialog still identifies as the upstream mod. kRestartExplorerPromptTitle (L10578) is L"Windows 11 Taskbar Styler - Windhawk", so a user of this mod gets a prompt attributing the failure to a mod they may not even have installed.

9. No screenshot in the README. This is about as visual as a mod gets — ~60 themes, a custom Minecraft hotbar layout, a raised floating taskbar. A GIF showing the switch happening as a window is maximized would communicate the mod far better than the prose does. Images must be hosted on i.imgur.com or raw.githubusercontent.com.

Optional improvements

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

  • clickThroughTaskbar is declared as a number. The settings block has - clickThroughTaskbar: 1, and Windhawk infers the type from the default, so this renders as a numeric input instead of a checkbox. Upstream declares - clickThroughTaskbar: false; use true/false here too (the Wh_GetIntSetting read stays valid for a boolean).

  • Dead settings-reading machinery. The custom-rules settings were removed from the block but the code that reads them is still there and now always no-ops: Wh_GetStringSetting(L"styleConstants[%d]", i) (L18589), L"controlStyles[%d].target" (L18970), L"themeResourceVariables[%d]" (L19228). That also makes the whole processUserStyles parameter of ProcessThemeStyles dead. Either drop those code paths or re-declare the settings.

  • minecraft.indicator.iconGap is read but not declared. loadIndicator reads <prefix>.iconGap for both prefixes, but the settings block only declares iconGap under nativeIndicator. The Minecraft one therefore always resolves to 0. Add the setting for symmetry, or skip the read for the Minecraft prefix.

  • g_windowThemeName initializer disagrees with the settings default. It's L"FrostyGlass" at L10571 while both the settings block and LoadSettings' fallback use Native. Harmless (it's overwritten before use) but confusing.

  • Mixed string-setting styles in LoadSettings. The loadString lambda uses the RAII string_setting_unique_ptr, but desktopTheme, windowTheme, dynamicTrigger and xamlDiagnosticsHandling are read with raw Wh_GetStringSetting + manual Wh_FreeStringSetting right below it. Use the lambda (or WindhawkUtils::StringSetting) throughout.

Functionality notes

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

  • Rewriting WINDOWPOS on WM_WINDOWPOSCHANGED is fragile. ReportRaisedTaskbarPosition mutates the WINDOWPOS the subclass proc received before calling DefSubclassProc, so every later subclass in the chain and Explorer's own handler see a position that isn't what the system reported. The comment explains why, and the equivalent on WM_WINDOWPOSCHANGING is legitimate (that's what the message is for), but on WM_WINDOWPOSCHANGED you're lying to a notification. If Explorer ever starts trusting that struct for something else, this breaks quietly. Worth double-checking whether handling only WM_WINDOWPOSCHANGING plus the SetWindowPos/DeferWindowPos hooks is enough.

  • The raise feature depends on an unrelated setting for its subclass. EnsureClickThroughSubclass — which installs the WM_WINDOWPOSCHANGING handler that keeps the raised taskbar in place — is reached from HandleClickThroughElement, and that is gated on g_settings.clickThroughTaskbar in ApplyCustomizations (L18164). With click-through disabled the subclass isn't installed until the first dynamic theme switch happens to run RefreshDynamicThemesForCurrentThread, so the raised taskbar can be less stable in that window. Installing the subclass from the raise path directly would decouple them.

  • raiseFromBottom being Minecraft-only is arbitrary. ShouldRaiseTaskbarFromBottom hard-codes g_desktopThemeName == L"Minecraft_Hotbar". Several of the bundled dock-style themes (Dock Like, Luminosity Dock, the OS26 Mac Dock variants, One UI 8.5 Dock) would benefit from the same offset. The README documents the restriction as intentional, but there doesn't seem to be a technical reason for it.

  • Window-candidate size threshold is in physical pixels. IsDynamicWindowCandidate requires >= 120 x 80 device pixels, so on a 200% display the effective logical threshold is half what it is at 100%. Scaling it by the window's DPI would make the trigger behave consistently across mixed-DPI setups.

  • Defaults are quite assertive. Out of the box the mod picks Minecraft Hotbar, raises the taskbar 90 DIP, makes Shell_TrayWnd non-topmost and pushes it below application windows. That's a large, surprising change for someone who just enables the mod to see what it does. Consider defaulting raiseFromBottom to 0 and letting users opt in.


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 Aug 29, 2026
@VitorAsse

Copy link
Copy Markdown
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 Aug 29, 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 dynamic per-monitor switching idea is genuinely useful and the new code is carefully written and well commented. The blocker is structural: the submission is a full copy of another mod's engine, and a few of the new mechanisms fight Explorer harder than they need to.

1. The mod is a fork of Windows 11 Taskbar Styler, not a new mod

Of the 21,254 lines, ~20,400 are a verbatim copy of windows-11-taskbar-styler — the XAML diagnostics tap, the styling/matching engine, the image loader, the restart-Explorer prompt, and the entire bundled theme catalog. The genuinely new part is roughly 850 lines.

This is the pattern the maintainer most consistently pushes back on: a duplicated catalog entry means every future Taskbar Styler fix, every new Windows build compatibility change, and every theme update has to be re-merged into this copy by hand, and it will diverge within weeks. It also puts users in a bad spot — your own README has to open with "disable Windows 11 Taskbar Styler first", which is the symptom, not a workaround.

The functionality itself doesn't exist yet, so it's worth having. The right shape is one of:

  • Preferred: add the feature to Windows 11 Taskbar Styler as an option (a second theme + a trigger + the return delay), as a PR against this repo / to m417z. That mod already has per-element rule resolution, so a RuleThemeScope-style filter is a natural fit — much like what you already built here.
  • Alternatively, a small companion mod that only does the per-monitor state tracking and drives Taskbar Styler, the way taskbar-background-helper is explicitly designed to work alongside the styler rather than replace it.

Please don't spend effort on the items below until this is settled — most of them apply either way, but the packaging decision changes what code even stays.

2. Leftover download-stats timer pointing at the upstream project

StartStatsTimer (around line 20189) is copied over intact, including the endpoint:

static constexpr WCHAR kStatsBaseUrl[] =
    L"https://github.com/ramensoftware/"
    L"windows-11-taskbar-styling-guide/"
    L"releases/download/stats-v6/";

It's called from Wh_ModInit and would submit theme-usage counters to the styling guide's release assets — a different project's telemetry, from a mod that isn't that project. It happens to be inert today only because the callback bails on Wh_GetStringSetting(L"theme"), and this mod declares no theme setting, so the string is always empty. That's an accident, not a design. Please delete StartStatsTimer / StopStatsTimer and the g_statsTimer global entirely.

3. Data race on the theme-name globals and g_settings

g_desktopThemeName and g_windowThemeName are plain std::wstring globals (lines 10579-10580) written by LoadSettings() on the Windhawk settings thread:

g_desktopThemeName = *desktopTheme ? desktopTheme : L"Minecraft_Hotbar";

and read concurrently from Explorer threads while that write is in flight:

  • ShouldRaiseTaskbarFromBottom() does g_desktopThemeName == L"Minecraft_Hotbar" and runs from SetWindowPos_Hook / DeferWindowPos_Hook / MoveWindow_Hook and from the WM_WINDOWPOSCHANGING/WM_WINDOWPOSCHANGED subclass proc — i.e. on any Explorer UI thread, at any time.
  • ProcessAllStylesFromSettings() reads both via .c_str(), and can be reached from OnWindowCreated (the CreateWindowExW hook) on a UI thread.

Wh_ModSettingsChanged stops the polling thread first, but the function hooks stay installed for the whole settings change, so there is nothing serialising these. A concurrent std::wstring assignment/read is UB — with SSO it's a torn buffer, without it a freed pointer.

The cheap fix is to not read strings on the hot path at all: derive a std::atomic<bool> g_raiseTaskbarFromBottomEnabled in LoadSettings() (desktopTheme == L"Minecraft_Hotbar" && raiseFromBottom > 0) and have ShouldRaiseTaskbarFromBottom test that. For the styling path, either take g_windowThemeMonitorsMutex (or a dedicated one) around both the write and the reads, or snapshot the names into locals before handing them to the UI threads. The same reasoning applies to the std::wstring members of g_settings.minecraft.indicator / g_settings.nativeIndicator.

4. Taskbar z-order and position are enforced unconditionally, not only when the feature is on

UpdateTaskbarZOrders() runs on every 250 ms tick, and EnsureTaskbarWindowZOrder ends with:

if (!ShouldRaiseTaskbarFromBottom(hWnd)) {
    if (!isTopmost) {
        setWindowPos(hWnd, HWND_TOPMOST, 0, 0, 0, 0, flags);
    }
    return;
}

So whenever the taskbar is not being raised — which includes every monitor showing the window theme, and every user who picked a non-Minecraft desktop theme or set raiseFromBottom to 0 — the mod re-asserts HWND_TOPMOST four times a second. Explorer deliberately drops the taskbar's topmost status (SetWindowPos(..., HWND_BOTTOM, ...) clears WS_EX_TOPMOST) when a full-screen window takes over the monitor, so this will pull the taskbar back over full-screen games and videos within 250 ms.

UpdateTaskbarPositions(true) has the mirror problem: on enable, on every settings change and on unload it walks all taskbars and moves each one to rcMonitor.bottom - taskbarHeight, regardless of whether the raise feature was ever used. For anyone running a mod that repositions the taskbar, that silently yanks it back to the bottom.

Both should be no-ops unless the raise-from-bottom feature is actually active — and on teardown, restore only what the mod actually changed (track which windows you moved / un-topmosted, and touch only those).

5. 250 ms polling loop with an expensive per-window probe

DynamicThemeThreadProc wakes every 250 ms and, on each tick, runs EnumWindows over every top-level window on the desktop; for each candidate IsDynamicWindowCandidate calls DwmGetWindowAttribute(DWMWA_CLOAKED) plus OpenProcess + QueryFullProcessImageNameW, then it runs EnumWindows a second time for UpdateTaskbarZOrders(). That's dozens of process handles opened and closed several times a second, forever, inside Explorer — a measurable idle-CPU and battery cost for a feature that changes state a handful of times an hour.

The events you need are all available: taskbar-background-helper solves the exact same "is there a maximized/fullscreen window on this monitor" question event-driven, with SetWinEventHook for EVENT_OBJECT_CREATE..HIDE, EVENT_OBJECT_LOCATIONCHANGE and EVENT_OBJECT_CLOAKED/UNCLOAKED on a dedicated message-loop thread, maintaining a per-monitor window set incrementally. That also removes the 2-tick debounce and the return-delay timer's dependence on the poll interval.

6. Full desktop window walk inside the SetWindowPos hook

AdjustTaskbarWindowZOrder calls FindLowestDynamicWindowForTaskbar, which walks the whole GetTopWindow/GW_HWNDNEXT chain and runs the full IsDynamicWindowCandidate probe (DWM query + OpenProcess + QueryFullProcessImageNameW) on every window — and this happens synchronously inside SetWindowPos_Hook / DeferWindowPos_Hook, on Explorer's UI thread, every time a taskbar window is repositioned. Opening process handles from inside a SetWindowPos hook is a lot of work in a very hot path.

If you keep the "taskbar below app windows" behaviour, cache the result: the same per-monitor window set the WinEvent hook maintains (item 5) already tells you the bottom-most relevant window, so the hook can read a precomputed HWND instead of re-deriving it.

7. The restart-Explorer prompt still identifies itself as the other mod

constexpr WCHAR kRestartExplorerPromptTitle[] =
    L"Windows 11 Taskbar Styler - Windhawk";

Users of this mod get a modal dialog titled with a different mod's name, offering to kill explorer.exe. Use this mod's name (or build it from WH_MOD_ID).

8. README: add a visual, and drop the "self-contained" claim

This is a purely visual mod and the README has no screenshot or GIF. A short GIF of the taskbar switching between the two themes as a window is maximized would communicate the whole mod in two seconds; a pair of before/after screenshots also works. Only i.imgur.com and raw.githubusercontent.com are allowed image hosts.

Separately, "The mod is self-contained" isn't accurate — many of the bundled themes (Windows 7, Windows XP, Sun Valley, One UI 8.5, Plasma, …) reference PNG assets by https://raw.githubusercontent.com/... URLs and fetch them at runtime, exactly as the upstream styler does. Worth rewording to "doesn't require the other mods to be installed".

Optional improvements

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

  • clickThroughTaskbar: 1 is a number setting, so Windhawk renders it as a numeric input rather than a checkbox. Use clickThroughTaskbar: true (the code reads it with Wh_GetIntSetting, which returns 0/1 for booleans, so no code change is needed).

  • LoadSettings mixes styles: the loadString lambda uses the file's own string_setting_unique_ptr, but desktopTheme, windowTheme, dynamicTrigger and xamlDiagnosticsHandling use raw Wh_GetStringSetting + manual Wh_FreeStringSetting. Use the RAII wrapper (or WindhawkUtils::StringSetting) consistently.

  • loadIndicator reads <prefix>.iconGap for both indicator groups, but iconGap is only declared under nativeIndicatorminecraft.indicator.iconGap always resolves to 0. Either declare it for the Minecraft group too, or skip the read when the prefix is minecraft.indicator.

  • Dead settings-driven code inherited from the styler: ProcessSingleTargetStylesFromSettings reads controlStyles[%d].target, LoadStyleConstants reads styleConstants[%d], and ProcessResourceVariablesFromSettings reads themeResourceVariables[%d] — none of those settings exist in this mod's settings block, so all three loops break on the first iteration. Drop the settings-backed paths (keep the theme-supplied constants) or re-declare the settings if you actually want custom rules.

  • g_windowThemeName is initialised to L"FrostyGlass" (line 10580) while the settings default and the LoadSettings fallback are both Native. Harmless since LoadSettings always runs first, but the disagreement is confusing.

  • IsTaskbarTopLevelWindow only matches on class name, and the EnumWindows callers don't filter by GetCurrentProcessId(). With more than one explorer.exe shell instance, each copy of the mod will SetWindowPos the other's taskbars. A GetWindowThreadProcessId check would scope it.

  • The 120 x 80 minimum window size in IsDynamicWindowCandidate is raw pixels; on a 200% display that's a much smaller logical window than on 100%. Scale it by the window's DPI, or drop the check (the WS_EX_TOOLWINDOW / class / process filters already do most of the work).

Functionality notes

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

  • The windowThemeStableChecks[monitor] >= 2 debounce means entering the window theme takes 250-500 ms after the window is actually maximized, on top of whatever the poll happens to catch. Switching to the event-driven approach in item 5 removes the need for it.

  • AdjustTaskbarWindowPos sets windowPos->flags |= SWP_NOSENDCHANGING on the taskbar's own moves. That suppresses WM_WINDOWPOSCHANGING for anything else subclassing Shell_TrayWnd — other taskbar mods that adjust size/position from that message will silently stop working while the raise feature is on. Worth mentioning under "Compatibility and limitations" if it stays.

  • The raise-from-bottom controls are hard-wired to Minecraft_Hotbar as the desktop theme (g_desktopThemeName == L"Minecraft_Hotbar"). It's documented, but the mechanism is theme-independent — exposing it as a plain "float the taskbar N px above the bottom" option would be more useful and would remove the string comparison from the hot path at the same time.

  • IsIgnoredDynamicWindowProcess hard-codes a list of shell host executables (startmenuexperiencehost.exe, searchhost.exe, …). These get renamed and replaced between Windows builds, so the trigger will start misfiring on newer builds. Filtering on window characteristics (cloaked, WS_EX_NOACTIVATE, no taskbar presence) ages better than a name list.

  • MoveWindow_Hook builds a WINDOWPOS and then sets SWP_NOSENDCHANGING on windowPos.flags, but MoveWindow takes no flags — that assignment is discarded. Harmless, just noise.

  • A 6-second default for desktopReturnDelaySeconds is on the long side for a first impression; 1-2 seconds still kills the flicker while feeling responsive.


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 Aug 29, 2026
@VitorAsse

Copy link
Copy Markdown
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 Aug 29, 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 switching idea is a reasonable one and the new code is thoughtfully commented, but the submission has one structural problem that dominates everything else, plus a set of correctness and cost issues in the new logic.

1. This is a hard fork of Windows 11 Taskbar Styler rather than a new mod. Diffing the submission against mods/windows-11-taskbar-styler.wh.cpp gives ~1,780 added and ~790 removed lines out of 21,403 — roughly 95% of the file is a verbatim copy of the styling engine and all 56 bundled themes. The README even instructs users to disable Taskbar Styler first, so it is a replacement, not a complement.

That matters practically, not just on principle: the taskbar's XAML tree changes with almost every Windows feature update, and the styler's themes and selectors are actively maintained against those changes. A forked copy stops receiving them, so it will silently drift and break, and users who report a broken theme won't know which of the two mods to file against. The project's consistent preference is to extend an existing mod rather than merge a near-duplicate.

Concretely: the dynamic per-monitor switching would be much better as an option inside Windows 11 Taskbar Styler itself (development happens at https://github.com/m417z/my-windhawk-mods), where it would work with every theme, stay in sync with the engine, and not duplicate 20k lines. Note also that the detection half already exists in mods/taskbar-background-helper.wh.cpp (per-monitor maximized/fullscreen tracking driving a per-monitor taskbar change), so what's genuinely new here is a fairly small amount of glue.

2. The window theme silently loses its theme resource variables. ProcessAllStylesFromSettings (line 19643) passes processResourceVariables = true only for the desktop theme:

ProcessThemeStyles(g_desktopThemeName.c_str(), RuleThemeScope::desktop, false, true);
ProcessThemeStyles(g_windowThemeName.c_str(),  RuleThemeScope::window,  true,  false);

37 of the 56 bundled themes define themeResourceVariables (TranslucentTaskbar, Matter, BottomDensy, Windows7, Aeris, Plasma, WindowGlass*, Surface, Luminosity*, LayerMicaUI, Fluid, TintedGlass, UltraWideFriendly, LiquidGlass*, Borderless, Command Center, FluentGlass, OS26*, FrostyGlass, FrostedAcrylic, Pills, Blob, Minecraft Hotbar, One UI 8.5*, …). Pick any of them as the window theme and its resource variables are never merged, so it renders wrong with no error. The default (Native) happens to have none, which is probably why this wasn't noticed.

There's no clean fix while keeping the current design — resource variables are merged into Application.Current().Resources(), which is process-wide, so they can't be scoped per monitor at all. That also means the README's limitation note is much narrower than reality: it isn't just LayerMicaUI/Pills/Blob, it's every theme in that list. At minimum the two theme dropdowns should be restricted (or the limitation documented for all affected themes); ideally the switching lives in the styler where this can be solved once.

3. Every accepted WinEvent triggers a full system-wide window rescan. DynamicThemeThreadProc (line 20924) registers five out-of-context hooks with idProcess = 0, idThread = 0 — including EVENT_OBJECT_LOCATIONCHANGE — and every event that passes the class filter just does SetEvent(g_dynamicThemeRefreshEvent), which wakes the loop into a complete rescan: GetConnectedMonitors(), FindDynamicThemeMonitors() (an EnumWindows that calls DwmGetWindowAttribute + OpenProcess + QueryFullProcessImageNameW per visible top-level window), then UpdateTaskbarZOrders() (another EnumWindows, plus a full desktop z-order walk in IsWindowAboveInZOrder per taskbar).

EVENT_OBJECT_LOCATIONCHANGE fires continuously while any window anywhere on the system is dragged, resized or animated, and events arriving during a scan re-arm the event immediately, so this degenerates into a back-to-back scan loop with dozens of OpenProcess calls per iteration.

The hook set itself is fine — taskbar-background-helper uses the same one. The difference is that it handles each event incrementally for the single hWnd in the event and keeps per-monitor state, instead of rescanning the world: see WinEventProc with MonitorState::UpdateWindowState. Please do the same, or at least debounce (re-arm a ~100–200 ms timer instead of scanning on the event) and drop LOCATIONCHANGE in favour of EVENT_SYSTEM_MOVESIZEEND.

4. The scan thread and its system-wide hooks start in every explorer.exe process. Wh_ModAfterInit (line 21292) computes an initialize flag from whether a taskbar/XAML host was actually found, then calls StartDynamicThemeThread() unconditionally outside that check:

    if (initialize) {
        InitializeSettingsAndTap();
    }

    StartDynamicThemeThread();
}

With "Launch folder windows in a separate process" enabled, or transiently during an Explorer restart, several explorer.exe instances each spin up the thread, register five system-wide WinEvent hooks and run the scan loop — multiplying the cost in item 3 while doing nothing useful (IsTaskbarTopLevelWindow filters to the current process, so those instances can never act). Gate the thread on the taskbar actually being present, and start it from the same place the taskbar is discovered.

5. SetWindowPos / DeferWindowPos / MoveWindow are hooked process-wide and do a GetClassName before the enable check. The three hooks are installed unconditionally in Wh_ModInit (line 21238), and both AdjustTaskbarWindowPos (line 20620) and AdjustTaskbarWindowZOrder (line 20599) start with IsTaskbarTopLevelWindow(hWnd) — so every window-positioning call anywhere in Explorer pays two GetClassName + GetWindowThreadProcessId kernel round-trips, even though the feature only applies to Minecraft Hotbar as the desktop theme. ShouldRaiseTaskbarFromBottom (line 20526) has the same shape: it calls MonitorFromWindow and takes g_windowThemeMonitorsMutex inside IsMonitorUsingWindowTheme before checking g_raiseTaskbarFromBottomEnabled.

Test the cheap atomics first in all of these, and preferably don't install the hooks at all when the raise feature is off (the settings are known in Wh_ModInit; re-installing on a settings change is what Wh_ModSettingsChanged is for).

6. The taskbar z-order/position takeover isn't reliably reversible. While raised, the mod strips WS_EX_TOPMOST from Shell_TrayWnd/Shell_SecondaryTrayWnd, moves the window, and re-asserts both from three places at once: the background thread, the SetWindowPos hook (which issues a nested SetWindowPos_Original while the outer call is still in flight), and WM_WINDOWPOSCHANGED. Two problems:

  • RestoreRaisedTaskbar (line 20480) returns early without erasing its entry whenever GetMonitorInfoW/GetWindowRect fails, taskbarHeight <= 0, or the SetWindowPos fails. On unload that restore is attempted exactly once (StopDynamicThemeThreadUpdateTaskbarPositions(true)), so any of those transient failures leaves the taskbar raised and non-topmost until Explorer is restarted — the mod's effects must disappear when it's disabled. Verify the restore landed and retry, and re-apply WS_EX_TOPMOST even if the move fails.
  • ReportRaisedTaskbarPosition (line 20678) rewrites the OS-supplied WINDOWPOS of WM_WINDOWPOSCHANGED before passing it down the subclass chain. Every other subclass on Shell_TrayWnd — several Windhawk taskbar mods subclass it — and Explorer's own window proc then see a fabricated position. It's understandable as a way to stop Explorer pushing the taskbar back down, but please note it in the README's conflicts list at minimum, since it changes what unrelated mods observe.

7. "Native Windows taskbar" isn't native. Selecting Native unconditionally applies AddTaskIconSizeRules + AddIndicatorCustomizationRules with the settings-block defaults #009DD6 / #FF9D00, so the "native" theme gets blue/orange running indicators instead of the system accent colour, and there's no way to opt out — the setting group has no enable toggle and no "system default" value. Since Native is also the default window theme, the out-of-the-box experience is a recoloured taskbar that the UI describes as native. Add an enable toggle (or treat an empty colour string as "leave the system value alone") and default to not customizing.

8. No screenshot or GIF in the README. This is an entirely visual mod whose whole point is what the taskbar looks like in each state, and the README currently has no images at all (the upstream styler README shows every theme). Please add at least a screenshot of the two states, ideally a short GIF of the switch. Only i.imgur.com and raw.githubusercontent.com are allowed image hosts.

Optional improvements

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

  • Dead settings reads and an unreachable parser. The code still reads styleConstants[%d] (line 18649), controlStyles[%d].target / controlStyles[%d].styles[%d] (lines 19030/19046) and themeResourceVariables[%d] (line 19288), but none of those exist in the settings block any more, so the loops break on the first iteration and ProcessSingleTargetStylesFromSettings and its rule parser are unreachable. The processUserStyles parameter is only ever true for the window theme, which makes the asymmetry look intentional. Either expose the advanced-styling settings or delete the paths.

  • verticalOffset and iconGap are the same setting. AddIndicatorCustomizationRules (line 19431) uses indicator.verticalOffset + indicator.iconGap as a single TranslateTransform Y, so "Vertical position" and "Extra spacing from icons" do exactly the same thing (only their allowed ranges differ), for both the Minecraft and native indicator groups. Merge them into one setting, or make iconGap actually affect spacing (e.g. the icon Margin).

  • Indicator width/height can't be set to 0. In LoadSettings, indicator.inactiveWidth = value ? std::clamp(value, 1, 100) : defaultInactiveWidth; (lines 21135, 21140, 21144) treats a stored 0 as "unset" and substitutes a hardcoded default. The whole fallback is unnecessary: the settings block already declares the defaults, and Wh_GetIntSetting returns them. Just clamp the value.

  • #ifndef DWMWA_CLOAKED / #define DWMWA_CLOAKED 14 (lines 20238-20239)DWMWA_CLOAKED is an enumerator of DWMWINDOWATTRIBUTE in dwmapi.h, not a macro, so the guard always fires and the macro shadows the enum. dwmapi.h is already included; just use the enum.

  • DynamicWindowProcessName puts a WCHAR path[MAX_PATH * 4] (8 KB) buffer on the stack inside an EnumWindows callback. MAX_PATH plus a QueryFullProcessImageNameW retry, or a std::wstring sized once, would be tidier.

Functionality notes

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

  • A switch on one monitor visibly rebuilds the others. All taskbars (Shell_TrayWnd and every Shell_SecondaryTrayWnd) run on the same Explorer UI thread, and RefreshDynamicThemesForCurrentThread re-applies ReapplyCustomizationsForSubtree over every XAML root on the thread. So maximizing a window on monitor 1 tears down and re-applies the full customization tree for monitor 2's taskbar as well. Given the engine's design there's no obviously cheaper alternative, but expect flicker on multi-monitor setups, especially with the 6 s return delay causing a second full pass.

  • The "is this an app window" heuristic is fairly coarse. IsDynamicWindowCandidate (line 20313) combines a hardcoded class blocklist, a hardcoded process blocklist, and a 120×80 DIP minimum size. Legitimately small windows (calculator-style utilities, PiP players, small tool dialogs) won't trigger the window theme, and each new shell surface will need a blocklist entry. The closer approximation to what the shell itself considers an app window is the alt-tab test — owner window, WS_EX_TOOLWINDOW/WS_EX_APPWINDOW, WS_EX_NOACTIVATE, the shell window and the DesktopWindow prop — as in IsWindowActiveCandidate.

  • The default desktop theme needs the network to render. Minecraft Hotbar's background is an ImageBrush pointing at raw.githubusercontent.com (line 9537), inherited from the styler. It's existing behaviour, but worth knowing that the shipped default won't look right offline or on a first run before the image is fetched.

  • The 5 s safety rescan runs forever. kSafetyRefreshMilliseconds (line 20952) keeps the loop doing a full EnumWindows + UpdateTaskbarZOrders pass every 5 seconds even when nothing has happened and no delay timer is pending. If the WinEvent coverage is right, this shouldn't be needed at all; if it is needed as a net, a much longer interval (30–60 s) would cost far less.


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 Aug 29, 2026
@VitorAsse

Copy link
Copy Markdown
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 Aug 30, 2026
@windhawk-reviewer

Copy link
Copy Markdown

This pull request has already had 3 AI reviews in the last 24 hours, which is the limit, so no review was posted this time.

Comment /ai-review again after 2026-08-30 21:22 UTC (in 20 hours) to get another one.

@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 Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-author The author's turn: request an AI review, or respond to one that was posted.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant