Skip to content

Update the mod to the version 1.1.0 - #5029

Merged
m417z merged 6 commits into
ramensoftware:mainfrom
babamohammed2022:patch-21
Aug 11, 2026
Merged

Update the mod to the version 1.1.0#5029
m417z merged 6 commits into
ramensoftware:mainfrom
babamohammed2022:patch-21

Conversation

@babamohammed2022

@babamohammed2022 babamohammed2022 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

The mod has been updated version to 1.1.0 with a minor change

Changelog

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

  • Fixed a compatibility problem

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):
    • 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.

Updated version to 1.1.0 and added support for Windows 11 23H2 and 24H2. Enhanced keyboard shortcuts for auto-advancing selection.
@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 6, 2026
@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.

@babamohammed2022

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 Aug 6, 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.


Three things need attention: the new explorer.exe target doesn't do what the PR description implies, the auto-cycle setting was neutered instead of re-defaulted, and the new Tab-hold code is unreachable.

1. @include explorer.exe added alongside windhawk.exe — this doesn't run the mod in the shell, and creates two competing launchers

This is a tool mod using the wiki launcher boilerplate verbatim, and there is no branch anywhere on the current executable name. So in the real explorer.exe, Wh_ModInit falls through to g_isToolModProcessLauncher = true (line 3709) and Wh_ModAfterInit spawns explorer.exe -tool-mod "aero-flip3d-recreation". No mod logic runs inside the shellWhTool_ModInit is never called there. The only thing the new include changes is which image hosts the dedicated tool process. Consequences:

  • Non-deterministic host. windhawk.exe and explorer.exe are now both launchers; both spawn a child, and both children race for the windhawk-tool-mod_aero-flip3d-recreation mutex — the loser calls ExitProcess(1). So whichever wins is a race. If the explorer.exe image genuinely is what makes Win+Tab work, the mod will silently regress to the broken behavior whenever the windhawk.exe child wins.
  • Extra process spawn per Explorer launch. Every explorer.exe process becomes a launcher — "launch folder windows in a separate process", explorer.exe <path> invocations from other apps, Explorer restarts — and each spawns a throwaway child that immediately exits on the mutex.
  • A hollowed explorer.exe in the process list. The surviving tool process shows up as a second "Windows Explorer" with no window; users may kill it or "restart" the wrong entry. It is also a valid injection target for every other explorer.exe-targeted mod the user has enabled (all the taskbar mods, etc.), which will now initialize inside a shell-less process.
  • If Windhawk ever misses injecting into that child (mod disabled/updated mid-spawn), it runs as a real explorer.exe with -tool-mod "…" on its command line and pops open a stray Explorer window.

The wiki is explicit that the target is changed from windhawk.exe to explorer.exe when a specific process is required, not added to it — see Mods as tools. So please pick one include, and add a code comment stating what capability the explorer.exe image provides, because RegisterHotKey doesn't discriminate by image name and the hollow clone is not the shell process. My suspicion is that the real effect is startup timingexplorer.exe starts at logon, so the tool process comes up earlier and wins the Win+Tab claim before Task View takes it — which is the same race the kWinTabClaimAttempts 20 → 50 bump addresses. If that's what's happening, the include change may not be needed at all; worth testing the attempt bump alone on the 23H2/24H2 machines.

If you do need code running inside the real explorer.exe, do it the way simple-window-switcher does: keep both includes, branch on the exe name, and do the explorer-specific work there instead of letting explorer become a second launcher.

2. autoCycle is now a dead setting, and Win-held auto-cycle is forced on for everyone

Wh_GetIntSetting(L"autoCycle") is still read into g_settings.autoCycle (line 758) but the value is now only logged — nothing reads it. Dead settings should be removed; nothing needs to be kept "for settings-file compatibility", Windhawk ignores stale keys in the settings file.

But the bigger problem is the behavior change behind it. Line 2907 now arms the 250 ms auto-cycle timer for every Win+Tab activation:

const bool wantsAutoCycle = triggerModifier == TriggerModifier::Win;

The old default was deliberately off, with the reason spelled out in the description you removed: "Off by default so Tab lets you step to an exact window." Since releasing Win confirms the selection, an unconditional 250 ms advance while Win is held makes it impossible to land on a specific window — the user now has to release Win at the right instant. That's a regression for anyone using the mod the old way.

Simplest fix that solves both: keep the setting wired up and just flip its default.

- autoCycle: true
  $name: Auto-cycle while Win is held
  $description: Automatically advance the selection every 250 ms while Win is held, in addition to Tab/Shift+Tab.
const bool wantsAutoCycle = g_settings.autoCycle && triggerModifier == TriggerModifier::Win;

3. The new Tab-hold auto-advance is unreachable, and duplicates behavior that already worked in 1.0.0

LowLevelKeyboardProc swallows every VK_TAB event while a session is active — line 1501 returns 1 unconditionally for Tab, both down and up. So the overlay window never receives WM_KEYDOWN/WM_KEYUP for Tab, and the new SetTimer(kTabHoldCycleTimerId) / KillTimer at lines 3017–3037 only runs in the fallback where SetWindowsHookExW(WH_KEYBOARD_LL, …) failed to install. The WM_TIMER branch at line 3084 is dead along with it.

Holding Tab also already auto-advanced before this PR: hardware key auto-repeat keeps firing the LL hook, which posts WM_FLIP3D_NAVIGATE throttled to 180 ms (lines 1490–1502) — faster than the new 350 ms timer, so even if the new path were reachable it would be a slowdown. The README line "Hold Tab down to keep advancing automatically without repeated presses" describes 1.0.0 behavior.

Please drop the timer (constants, the WM_KEYDOWN/WM_KEYUP handlers, the WM_TIMER branch and the KillTimer in CleanupFlipResourcesOnOverlayThread) — or, if the intent was a different hold cadence, change the 180 ms throttle in the LL hook instead, which is the path that actually runs.

Optional improvements

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

  • Stale comments after the attempt bump: line 776 still says "It retries exactly 20 times" and line 3453 says "bounded 20-attempt sequence", but kWinTabClaimAttempts is now 50.
  • The README "Settings" section (line 47) still documents auto-cycle as "automatically advance … Off by default", which contradicts the new $description in the settings block. Whichever way item 2 is resolved, the two should agree.
  • README line 26 still says "This is the first version" — the mod is at 1.1.0 now.
  • If the Tab-hold timer stays for the no-LL-hook fallback, the comment on line 773 ("NOTE: 4 is kWinTabClaimTimerId — must not collide") is misleading: the claim timer lives on the controller window and this one on the overlay window, so timer IDs can't collide between them anyway.

Functionality notes

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

  • The Win+Tab claim is inherently a race: RegisterHotKey can only succeed if nobody currently owns the combo, so the retry loop only wins if Task View happens to release it during those 5 seconds. Widening the window from 2 s to 5 s makes it more likely, not reliable. Since the LL keyboard hook already handles Win+Tab deterministically whenever the hotkey isn't owned (line 1455), it may be worth considering whether the RegisterHotKey path earns its keep at all — dropping it would remove BeginWinTabClaim/ContinueWinTabClaim, the claim timer, the g_winTabHotkeyOwned coordination flag and the whole class of "which path fired first" bugs described in the comment at lines 793–799.
  • If item 2 is resolved by keeping auto-cycle on, note that a Tab press (LL hook, 180 ms throttle) and an auto-cycle tick (250 ms timer) are independent, so a quick Tab press landing right next to a tick advances the deck twice. Sharing one debounce timestamp between the two paths would make the step count predictable.


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 6, 2026
Updated mod description and settings for Aero Flip 3D. Changed auto-cycle setting to be enabled by default and clarified keyboard shortcuts.
Added a variable to capture the return value of WhTool_ModInit in the initialization process.
@babamohammed2022

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 Aug 6, 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 dominant change here is moving the mod out of its dedicated tool process and into explorer.exe. That's the wrong direction, and most of the findings below follow from it.

1. Moving from windhawk.exe (tool mod) into explorer.exe should be reverted

This mod is a textbook tool mod, and it was correctly merged as one. It installs no function hooks at all — everything it does works from any process: SetWindowsHookExW(WH_KEYBOARD_LL / WH_MOUSE_LL), RegisterHotKey, EnumWindows, DwmRegisterThumbnail, GDI. The wiki page names exactly the two problems this PR reintroduces: multiple explorer.exe processes, and a mistake or instability in the mod affects the entire Windows shell. The fact that the PR had to hand-roll IsMainExplorerProcess() to dedupe across Explorer instances is itself the signal the wiki calls out — the tool-mod launcher's named mutex exists precisely for that.

The blast radius is not theoretical for this particular mod. Hosted in the shell it adds to explorer.exe: two extra threads, a full-screen topmost window, full-screen GDI bitmaps + PrintWindow/BitBlt desktop captures, up to 128 DWM thumbnail registrations with per-frame ALPC updates, a process-wide timeBeginPeriod(1), and a low-level keyboard hook that — while the switcher is open — returns 1 for essentially every key (the unconditional return 1; at the end of the if (active) block). In the tool process, a wedged UI thread meant a dead switcher and a killable process; in explorer.exe the same wedge means system-wide keyboard input is swallowed until the 60 s failsafe, and recovery means restarting the shell.

The justification in the header comment ("the low-level input hook and the shell's Task View/Win+Tab registration are initialized in the same shell lifetime and startup order") is a hypothesis, not a diagnosis. Low-level keyboard hooks are process-agnostic — several merged tool mods install one from the windhawk.exe tool process and swallow Win-modified combos successfully, e.g. remap-copilot-key.wh.cpp (swallows Win+Shift+F23 and synthesizes Win key events), toggle-hidden-files.wh.cpp, caps-ime-switcher.wh.cpp. Topmost overlay windows + SetForegroundWindow from a tool process also work — see dynamic-island-for-windows.wh.cpp.

Please pin down what actually failed on those Windows 11 23H2/24H2 machines before changing the hosting model. The mod already logs every relevant failure — which one fired?

  • SetWindowsHookExW(keyboard) failed: %u — the hook never installed?
  • RegisterHotKey for Win+Tab never succeeded and the LL fallback also didn't fire?
  • The hook fired but ActivateFlip3D bailed (DWM composition disabled, not enough eligible windows, CreateOverlayWindow returned null, no thumbnails registered)?
  • The overlay appeared but never took the foreground?

Each of those has a targeted fix that keeps the mod out of the shell. If after that it genuinely turns out the feature cannot work from a tool process, that's worth raising with the maintainer explicitly rather than landing ~3,600 lines of GDI/DWM/animation code in explorer.exe.

Everything below applies if the Explorer hosting stays.

2. Wh_ModInit blocks Explorer's main thread on an unbounded wait

WhTool_ModInitImpl ends with:

WaitForSingleObject(g_hookReadyEvent.get(), INFINITE);
return g_hookInstallOk.load(std::memory_order_acquire);

For a process loaded before it starts executing, Wh_ModInit runs on the main thread (mod lifetime). In the tool process this was free — the process existed only for the mod. In explorer.exe it means every shell start now blocks the main thread until the worker thread has registered a window class, created a message-only window, tried RegisterHotKey, and spawned a second thread. And there's no timeout: if the worker thread never reaches SetEvent for any reason, explorer.exe never starts. Either drop the synchronous handshake (report init success optimistically and let the worker log its own failure), or use a bounded wait.

3. The worker threads outlive a failed Wh_ModInit, so the mod image is unmapped while its code is running

When CreateControllerWindow() fails, HookThreadProcImpl signals g_hookReadyEvent before returning, and only then does its ScopeGuard run — posting WM_QUIT to the input thread, joining it, UnregisterAllHotkeys, UnregisterFlipWindowClasses, logging. Meanwhile WhTool_ModInitImpl has already woken up, returned false, and Wh_ModInit returns FALSE. Windhawk then unloads the mod (and does not call Wh_ModUninit), so both the hook thread and the input thread are executing code from an image that is about to be unmapped — a crash in explorer.exe. The mod must be unloadable the moment Wh_ModInit/Wh_ModUninit returns; on the failure path, tear down before returning:

    WaitForSingleObject(g_hookReadyEvent.get(), INFINITE);
    if (!g_hookInstallOk.load(std::memory_order_acquire)) {
        WhTool_ModUninitImpl();  // join both threads before we're unloaded
        return false;
    }
    return true;

Related, same category: SafeDestroyOverlayWindow's fallback of ShowWindow(overlay, SW_HIDE) when DestroyWindow fails leaves a live window whose lpfnWndProc points into the mod image. If that ever happens on the unload path, the next message dispatched to it crashes the shell (and the following load then fails RegisterClassExW with ERROR_CLASS_ALREADY_EXISTS, silently killing the feature). Hiding isn't a safer fallback than failing — better to log and leave it, or retry the destroy on the owning thread.

4. IsMainExplorerProcess() is not a reliable single-instance guard

HWND taskbar = FindWindowW(L"Shell_TrayWnd", nullptr);
if (!taskbar) {
    // The initial shell can be injected before it creates the taskbar.
    return true;
}

Any explorer.exe that starts during a window in which no Shell_TrayWnd exists passes this check — logon, restart explorer, taskbar recreation. Two shell-hosted instances then race to install two low-level keyboard hooks, two controller windows, two overlays and duplicate global hotkeys, with no arbitration between them. The check is also evaluated exactly once in Wh_ModInit and never revisited. The tool-mod launcher's CreateMutex(L"windhawk-tool-mod_" WH_MOD_ID) handles this deterministically; if you stay in Explorer you need an equivalent process-wide mutex rather than a window-existence probe.

5. AttachThreadInput into arbitrary application threads, combined with an unbounded join on unload

ActivateSelectedWindow attaches the mod's UI thread input queue to the foreground app's thread and to the target window's thread. Attaching to a hung application serializes input queues and can wedge the attaching thread — and Wh_ModUninit joins that same thread with WaitForSingleObject(..., INFINITE), so a wedge there hangs the mod unload on the Windhawk engine thread inside the shell. The attach also looks unnecessary: at that point the overlay (owned by this very thread) is the foreground window, so SetForegroundWindow on the target succeeds without any attach. Consider activating the target before destroying the overlay and dropping the AttachThreadInput pair entirely.

Optional improvements

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

  • BOOL Wh_ModInit() ends with bool returnValue=WhTool_ModInit(); // This variable is used for the ModInit. The temporary and the comment add nothing (and the spacing is off); return WhTool_ModInit(); says the same thing.
  • WhTool_ModUninitImpl calls DisableHighResAnimationTimer() as its first statement, before the hook thread is joined — so it races with EnableHighResAnimationTimer() on that thread. In the losing interleaving timeBeginPeriod(1)/timeEndPeriod(1) end up unbalanced and explorer.exe keeps a 1 ms timer resolution after the mod is gone. Moving the call to after both joins removes the race entirely.
  • Stale terminology throughout: // Tool-process teardown joins the UI and input threads, // A tool mod must never attempt to continue after an access violation, // never take the tool-mod process down, // Dedicated Windhawk tool process lifecycle. If the mod stops being a tool mod, the WhTool_* indirection layer (Wh_ModInitWhTool_ModInitWhTool_ModInitImpl) is also just extra hops — the callbacks could be named Wh_Mod* directly.
  • #include <shellapi.h> is now unused — it was there for CommandLineToArgvW in the removed launcher boilerplate.
  • IsExplorerProcess() duplicates what @include explorer.exe already guarantees; IsMainExplorerProcess() (or its mutex replacement) is the only check that carries weight.
  • kWinTabClaimAttempts was raised 20 → 50, i.e. 50 RegisterHotKey(MOD_WIN | MOD_NOREPEAT, VK_TAB) attempts over ~5 s at every shell start. Win+Tab is owned by the shell for Task View, so this realistically never succeeds and the LL hook is what actually does the work — the logs will say which. If it never succeeds, the whole claim/retry timer is dead weight and could go.

Functionality notes

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

  • Auto-cycle now double-advances while Win+Tab is held. With autoCycle flipped to true by default, holding Win+Tab runs two independent advance sources at once: the 250 ms kAutoCycleTimerId timer, and the LL hook's VK_TAB hardware-repeat path (gated at 180 ms). Combined that's roughly a step every 100 ms, at an irregular cadence. Previously the setting was off by default, so the overlap was opt-in. Worth deciding which source owns "held" navigation — e.g. skip the auto-cycle tick when VK_TAB is physically down, or don't arm the timer until Tab is released.
  • The VK_TAB change in OverlayWndProc is a no-op. The diff splits case VK_RIGHT: case VK_DOWN: case VK_TAB: into two branches that both do NavigateSelection(1); return 0; — identical behavior before and after. Since the changelog lists "Enhanced keyboard shortcuts for auto-advancing selection" as a change, it looks like something was intended here that didn't land. The README line "Hold Tab down to keep advancing automatically without repeated presses" describes the pre-existing 180 ms repeat gate in LowLevelKeyboardProc, which this PR doesn't touch.
  • Only the Win+Tab trigger arms auto-cycle now (triggerModifier == TriggerModifier::Win), which is a sensible fix — Ctrl+Alt+F12 no longer starts a timer that immediately kills itself because no Win key is down.


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 6, 2026
Refactor comments and improve clarity on the mod's lifecycle and error handling. Adjusted the single-instance mutex logic and reduced the number of attempts for certain operations.
@babamohammed2022

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 Aug 6, 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 bulk of this PR is a single architectural change — moving ~3800 lines of GDI/DWM/threading code out of the dedicated tool process and into explorer.exe. That change is the main thing to resolve; the rest follows from it.

1. Moving from the tool-mod process into explorer.exe is the wrong direction, and the stated reasons don't hold up.

The mod installs zero function hooks — there is no Wh_SetFunctionHook, no WindhawkUtils::SetFunctionHook, no HookSymbols anywhere in the file. Everything it uses (SetWindowsHookExW(WH_KEYBOARD_LL/WH_MOUSE_LL), RegisterHotKey, EnumWindows, DwmRegisterThumbnail, GDI) is desktop/session-wide and works identically from any process on the desktop. That is the textbook definition of a tool mod, and it's what the mod already was. Hand-rolling a single-instance mutex (Local\Windhawk_AeroFlip3D_ExplorerSingleton) to dedupe across multiple explorer.exe instances is itself one of the listed signals that a mod belongs in a dedicated process.

The ~90-line justification block at lines 95–178 asserts several things that aren't accurate:

  • "The tool process was a hollow explorer" (point 3, lines 138–148): it wasn't. The previous version had @include windhawk.exe, and the wiki launcher spawned windhawk.exe -tool-mod "aero-flip3d-recreation". explorer.exe was never launched as a host, and there was no IsMainExplorerProcess() / FindWindow(L"Shell_TrayWnd") process guard in the shipped 1.0.0 code — the whole "two instances, double hooks, fragile heuristic" story describes code that never existed in this mod.
  • "The DWM thumbnail parent must be in the shell" (point 2, lines 128–136): DwmRegisterThumbnail only requires the destination to be a top-level window owned by the calling process. simple-window-switcher — an Alt+Tab replacement with live DWM thumbnails, a full-screen switcher window, multi-monitor support and global hotkeys — registers all of its thumbnails from inside the windhawk.exe tool process.
  • "Win+Tab must be registered in the same process/lifetime as Explorer" (point 1): RegisterHotKey and WH_KEYBOARD_LL are per-desktop, not per-process. If the real problem is that Explorer's Task View claims Win+Tab before the mod can, the fix is to stop Explorer from claiming it, not to relocate the whole mod. simple-window-switcher does exactly that: the UI stays in the tool process, and a small explorer.exe-side component hooks RegisterHotKey to block the shell's own Alt+Tab registration (code), with @include windhawk.exe + @include explorer.exe and an exe-name branch in Wh_ModInit.

Please restore the tool-mod structure (@include windhawk.exe, WhTool_ModInit / WhTool_ModSettingsChanged / WhTool_ModUninit, and the launcher snippet copied verbatim from the wiki — a stray edit there makes review much harder). If the 23H2/24H2 Win+Tab problem is real and reproducible, add a separate, minimal explorer.exe component for just that piece. As it stands, a fault anywhere in this file — a GDI failure path, a DWM stall, a hook bug — takes down the whole shell, which is precisely what the tool-mod pattern exists to avoid.

Also please drop or rewrite the 95–178 comment block. Lines 122–126 additionally claim "Wh_ModInit below accepts only the primary taskbar-owning Explorer", which contradicts both the actual Wh_ModInit (line 3751 — it only takes a mutex) and the later comment at line 3739 that says the Explorer check was deliberately removed. Long, confidently-worded, factually wrong comments are worse than no comments.

2. The single-instance mutex picks an arbitrary explorer.exe and never recovers.

Wh_ModInit (line 3760) creates the named mutex and the first instance to get there wins; every other explorer.exe returns FALSE. Nothing re-arbitrates afterwards, so:

  • With "Launch folder windows in a separate process" enabled, a folder-window explorer.exe can win the race. When the user closes that window the process exits, the mutex is released, and no other Explorer instance ever retries — Win+Tab silently stops working until a settings change or a shell restart.
  • On an Explorer restart, the outgoing shell process may still hold the mutex while the new one initializes. The new (real) shell then loses and returns FALSE, and the old process dies moments later — nobody hosts the mod. This is the exact "restart explorer / taskbar recreation" scenario the comment at lines 3728–3734 claims to fix.

The tool-mod launcher doesn't have this problem: exactly one dedicated process is started and the framework manages it. This is another argument for going back to it rather than for hardening the mutex.

3. Wh_ModInit blocks Explorer's main thread, and Wh_ModUninit blocks indefinitely.

Per Mod lifetime, when a mod is loaded before the target process begins executing, Wh_ModInit runs on the target's main thread. ModInitImpl (line 3648) waits up to 5 s there for a worker thread to register a window class, create a message-only window, register a global hotkey and spawn a second thread — so every logon and every Explorer restart now gates the shell's startup on that handshake, and a stall costs a 5-second shell hang.

The unload path is worse. ModUninitImpl (lines 3672–3689) uses WaitForSingleObject(..., INFINITE) on both threads, and the UI thread's teardown (HookThreadProcImpl's ScopeGuard → FinishExitAfterAnimation) does cross-process work while that wait is outstanding: DwmUnregisterThumbnail per card (synchronous ALPC to dwm.exe) and BringWindowToTop / SetForegroundWindow on an arbitrary third-party application window. If any of those stall, the Windhawk engine thread and the shell stall with them.

Notably, the mod's own 1.0.0 comment said exactly this and it was deleted rather than acted on:

// A tool process may wait indefinitely: unlike explorer.exe this cannot stall the shell.

That reasoning was correct. In the shell host it no longer applies. Two things regardless of where the mod ends up hosted:

  • Don't activate a foreign window from the teardown path at all — FinishExitAfterAnimation is called from the thread's cleanup guard, where activating whatever happened to be selected is not wanted.
  • If the code stays in explorer.exe, the joins need a bound (and a defined behavior when the bound is hit), not INFINITE.

4. The mutex is leaked on the ModInitImpl failure path.

In Wh_ModInit, g_singleInstanceMutex is acquired and g_isExplorerProcess = true is set before ModInitImpl() runs (lines 3760–3774). When ModInitImpl fails it calls ModUninitImpl() and returns false, but nothing releases the mutex — and since Wh_ModInit returned FALSE, Wh_ModUninit is never called, so the release at line 3790 doesn't run either. Whether the handle is ever closed then depends entirely on the module actually being unloaded and static destructors running. Release it explicitly:

    if (!ModInitImpl()) {
        g_isExplorerProcess = false;
        g_singleInstanceMutex.reset();
        return FALSE;
    }
    return TRUE;

5. SafeDestroyOverlayWindow now deliberately leaves a live window with a WndProc inside the mod image.

The rewritten fallback (lines 2742–2759) logs and returns when DestroyWindow fails, and the comment argues this avoids a stale WndProc. It does the opposite: a surviving window keeps lpfnWndProc pointing into the mod image, UnregisterClassW then fails because a window of the class still exists, and once Wh_ModUninit returns and Windhawk unmaps the DLL, the next message dispatched to that window crashes Explorer. ShowWindow(SW_HIDE) was no better, so this isn't a regression as such — but neither branch is acceptable. The requirement is that no mod-owned window survives Wh_ModUninit. In practice DestroyWindow is called from the window's owning thread here so it shouldn't fail; if it somehow does, retry rather than accepting a dangling WndProc.

Optional improvements

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

  • GetCurrentModuleHandle() (line 945) falls back to GetModuleHandleW(nullptr). In the tool process that was harmless; in explorer.exe it means the window classes would be registered under Explorer's hInstance, and UnregisterClassW(..., GetCurrentModuleHandle()) would then never match — leaving a permanently registered class with a dangling WndProc. Better to fail the call than to fall back.
  • The FinishExitAfterAnimation retry (lines 2786–2795) is convoluted: SafeDestroyOverlayWindow(overlay) has already run, so !(overlay && IsWindow(overlay)) is effectively always true and the comment ("avoid re-activating unnecessarily if the first attempt succeeded") doesn't describe what the code does. A single bool activated flag set by the first attempt would be clearer.
  • The mutex name is hard-coded (line 3749). WH_MOD_ID is available as a wide string literal — L"Local\\Windhawk_" WH_MOD_ID keeps it in sync with the mod id.
  • Several comments describe the diff rather than the code, e.g. lines 3109–3113 ("Unified in a single case because the three labels previously had identical separate branches (diff no-op)") and line 857 ("reduced from 50"). These read as review notes and go stale immediately.
  • g_hookThreadId and g_inputThreadId are plain DWORDs written from the worker threads and read from ModUninitImpl on another thread. std::atomic<DWORD> would remove the data race for free.
  • ActivateSelectedWindow logs "did not bring hwnd to foreground (expected, ...)" — describing a failure as "expected" in the log makes the message useless for diagnosis. Either it's normal (drop the log) or it isn't (log it as a failure).

Functionality notes

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

  • The auto-cycle logic looks inverted, and it's now on by default. The new WM_TIMER guard (lines 3156–3174) advances the selection every 250 ms while Win is held and Tab is not held, and explicitly skips advancing while Tab is down. The original Vista/7 behavior is the other way round: holding Win+Tab cycles continuously, and releasing Tab while keeping Win held stops on the current window so you can pick it. As written, the normal browse gesture (hold Win, tap Tab a few times, then hold still) can never settle — the deck starts spinning at 4/s the moment you let go of Tab, and releasing Win confirms whatever happens to be in front at that instant. Combined with flipping the default from false to true, that makes precise selection impossible out of the box. Worth re-checking against the behavior you intended; if the timer is meant to mirror Vista, the condition should be tabDown, not !tabDown.
  • The overlay is sized to GetPrimaryMonitorRect() only, and the README documents this ("Aero Flip 3D appears on the main monitor"). Not a bug, just noting that a multi-monitor deck (or at least following the monitor the foreground window is on) would be a natural improvement.
  • kWinTabClaimAttempts was reduced from 50 to 20 (~2 s). Since the LL-hook fallback handles Win+Tab whenever the hotkey can't be claimed, it's worth confirming the retry loop earns its place at all — if the shell reliably owns the combo on Windows 11, the claim sequence is 2 s of startup work for nothing.


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 6, 2026
@babamohammed2022

Copy link
Copy Markdown
Contributor Author

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 bulk of this PR is a single architectural change — moving ~3800 lines of GDI/DWM/threading code out of the dedicated tool process and into explorer.exe. That change is the main thing to resolve; the rest follows from it.

1. Moving from the tool-mod process into explorer.exe is the wrong direction, and the stated reasons don't hold up.

The mod installs zero function hooks — there is no Wh_SetFunctionHook, no WindhawkUtils::SetFunctionHook, no HookSymbols anywhere in the file. Everything it uses (SetWindowsHookExW(WH_KEYBOARD_LL/WH_MOUSE_LL), RegisterHotKey, EnumWindows, DwmRegisterThumbnail, GDI) is desktop/session-wide and works identically from any process on the desktop. That is the textbook definition of a tool mod, and it's what the mod already was. Hand-rolling a single-instance mutex (Local\Windhawk_AeroFlip3D_ExplorerSingleton) to dedupe across multiple explorer.exe instances is itself one of the listed signals that a mod belongs in a dedicated process.

The ~90-line justification block at lines 95–178 asserts several things that aren't accurate:

  • "The tool process was a hollow explorer" (point 3, lines 138–148): it wasn't. The previous version had @include windhawk.exe, and the wiki launcher spawned windhawk.exe -tool-mod "aero-flip3d-recreation". explorer.exe was never launched as a host, and there was no IsMainExplorerProcess() / FindWindow(L"Shell_TrayWnd") process guard in the shipped 1.0.0 code — the whole "two instances, double hooks, fragile heuristic" story describes code that never existed in this mod.
  • "The DWM thumbnail parent must be in the shell" (point 2, lines 128–136): DwmRegisterThumbnail only requires the destination to be a top-level window owned by the calling process. simple-window-switcher — an Alt+Tab replacement with live DWM thumbnails, a full-screen switcher window, multi-monitor support and global hotkeys — registers all of its thumbnails from inside the windhawk.exe tool process.
  • "Win+Tab must be registered in the same process/lifetime as Explorer" (point 1): RegisterHotKey and WH_KEYBOARD_LL are per-desktop, not per-process. If the real problem is that Explorer's Task View claims Win+Tab before the mod can, the fix is to stop Explorer from claiming it, not to relocate the whole mod. simple-window-switcher does exactly that: the UI stays in the tool process, and a small explorer.exe-side component hooks RegisterHotKey to block the shell's own Alt+Tab registration (code), with @include windhawk.exe + @include explorer.exe and an exe-name branch in Wh_ModInit.

Please restore the tool-mod structure (@include windhawk.exe, WhTool_ModInit / WhTool_ModSettingsChanged / WhTool_ModUninit, and the launcher snippet copied verbatim from the wiki — a stray edit there makes review much harder). If the 23H2/24H2 Win+Tab problem is real and reproducible, add a separate, minimal explorer.exe component for just that piece. As it stands, a fault anywhere in this file — a GDI failure path, a DWM stall, a hook bug — takes down the whole shell, which is precisely what the tool-mod pattern exists to avoid.

Also please drop or rewrite the 95–178 comment block. Lines 122–126 additionally claim "Wh_ModInit below accepts only the primary taskbar-owning Explorer", which contradicts both the actual Wh_ModInit (line 3751 — it only takes a mutex) and the later comment at line 3739 that says the Explorer check was deliberately removed. Long, confidently-worded, factually wrong comments are worse than no comments.

2. The single-instance mutex picks an arbitrary explorer.exe and never recovers.

Wh_ModInit (line 3760) creates the named mutex and the first instance to get there wins; every other explorer.exe returns FALSE. Nothing re-arbitrates afterwards, so:

  • With "Launch folder windows in a separate process" enabled, a folder-window explorer.exe can win the race. When the user closes that window the process exits, the mutex is released, and no other Explorer instance ever retries — Win+Tab silently stops working until a settings change or a shell restart.
  • On an Explorer restart, the outgoing shell process may still hold the mutex while the new one initializes. The new (real) shell then loses and returns FALSE, and the old process dies moments later — nobody hosts the mod. This is the exact "restart explorer / taskbar recreation" scenario the comment at lines 3728–3734 claims to fix.

The tool-mod launcher doesn't have this problem: exactly one dedicated process is started and the framework manages it. This is another argument for going back to it rather than for hardening the mutex.

3. Wh_ModInit blocks Explorer's main thread, and Wh_ModUninit blocks indefinitely.

Per Mod lifetime, when a mod is loaded before the target process begins executing, Wh_ModInit runs on the target's main thread. ModInitImpl (line 3648) waits up to 5 s there for a worker thread to register a window class, create a message-only window, register a global hotkey and spawn a second thread — so every logon and every Explorer restart now gates the shell's startup on that handshake, and a stall costs a 5-second shell hang.

The unload path is worse. ModUninitImpl (lines 3672–3689) uses WaitForSingleObject(..., INFINITE) on both threads, and the UI thread's teardown (HookThreadProcImpl's ScopeGuard → FinishExitAfterAnimation) does cross-process work while that wait is outstanding: DwmUnregisterThumbnail per card (synchronous ALPC to dwm.exe) and BringWindowToTop / SetForegroundWindow on an arbitrary third-party application window. If any of those stall, the Windhawk engine thread and the shell stall with them.

Notably, the mod's own 1.0.0 comment said exactly this and it was deleted rather than acted on:

// A tool process may wait indefinitely: unlike explorer.exe this cannot stall the shell.

That reasoning was correct. In the shell host it no longer applies. Two things regardless of where the mod ends up hosted:

  • Don't activate a foreign window from the teardown path at all — FinishExitAfterAnimation is called from the thread's cleanup guard, where activating whatever happened to be selected is not wanted.
  • If the code stays in explorer.exe, the joins need a bound (and a defined behavior when the bound is hit), not INFINITE.

4. The mutex is leaked on the ModInitImpl failure path.

In Wh_ModInit, g_singleInstanceMutex is acquired and g_isExplorerProcess = true is set before ModInitImpl() runs (lines 3760–3774). When ModInitImpl fails it calls ModUninitImpl() and returns false, but nothing releases the mutex — and since Wh_ModInit returned FALSE, Wh_ModUninit is never called, so the release at line 3790 doesn't run either. Whether the handle is ever closed then depends entirely on the module actually being unloaded and static destructors running. Release it explicitly:

    if (!ModInitImpl()) {
        g_isExplorerProcess = false;
        g_singleInstanceMutex.reset();
        return FALSE;
    }
    return TRUE;

5. SafeDestroyOverlayWindow now deliberately leaves a live window with a WndProc inside the mod image.

The rewritten fallback (lines 2742–2759) logs and returns when DestroyWindow fails, and the comment argues this avoids a stale WndProc. It does the opposite: a surviving window keeps lpfnWndProc pointing into the mod image, UnregisterClassW then fails because a window of the class still exists, and once Wh_ModUninit returns and Windhawk unmaps the DLL, the next message dispatched to that window crashes Explorer. ShowWindow(SW_HIDE) was no better, so this isn't a regression as such — but neither branch is acceptable. The requirement is that no mod-owned window survives Wh_ModUninit. In practice DestroyWindow is called from the window's owning thread here so it shouldn't fail; if it somehow does, retry rather than accepting a dangling WndProc.

Optional improvements

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

  • GetCurrentModuleHandle() (line 945) falls back to GetModuleHandleW(nullptr). In the tool process that was harmless; in explorer.exe it means the window classes would be registered under Explorer's hInstance, and UnregisterClassW(..., GetCurrentModuleHandle()) would then never match — leaving a permanently registered class with a dangling WndProc. Better to fail the call than to fall back.
  • The FinishExitAfterAnimation retry (lines 2786–2795) is convoluted: SafeDestroyOverlayWindow(overlay) has already run, so !(overlay && IsWindow(overlay)) is effectively always true and the comment ("avoid re-activating unnecessarily if the first attempt succeeded") doesn't describe what the code does. A single bool activated flag set by the first attempt would be clearer.
  • The mutex name is hard-coded (line 3749). WH_MOD_ID is available as a wide string literal — L"Local\\Windhawk_" WH_MOD_ID keeps it in sync with the mod id.
  • Several comments describe the diff rather than the code, e.g. lines 3109–3113 ("Unified in a single case because the three labels previously had identical separate branches (diff no-op)") and line 857 ("reduced from 50"). These read as review notes and go stale immediately.
  • g_hookThreadId and g_inputThreadId are plain DWORDs written from the worker threads and read from ModUninitImpl on another thread. std::atomic<DWORD> would remove the data race for free.
  • ActivateSelectedWindow logs "did not bring hwnd to foreground (expected, ...)" — describing a failure as "expected" in the log makes the message useless for diagnosis. Either it's normal (drop the log) or it isn't (log it as a failure).

Functionality notes

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

  • The auto-cycle logic looks inverted, and it's now on by default. The new WM_TIMER guard (lines 3156–3174) advances the selection every 250 ms while Win is held and Tab is not held, and explicitly skips advancing while Tab is down. The original Vista/7 behavior is the other way round: holding Win+Tab cycles continuously, and releasing Tab while keeping Win held stops on the current window so you can pick it. As written, the normal browse gesture (hold Win, tap Tab a few times, then hold still) can never settle — the deck starts spinning at 4/s the moment you let go of Tab, and releasing Win confirms whatever happens to be in front at that instant. Combined with flipping the default from false to true, that makes precise selection impossible out of the box. Worth re-checking against the behavior you intended; if the timer is meant to mirror Vista, the condition should be tabDown, not !tabDown.
  • The overlay is sized to GetPrimaryMonitorRect() only, and the README documents this ("Aero Flip 3D appears on the main monitor"). Not a bug, just noting that a multi-monitor deck (or at least following the monitor the foreground window is on) would be a natural improvement.
  • kWinTabClaimAttempts was reduced from 50 to 20 (~2 s). Since the LL-hook fallback handles Win+Tab whenever the hotkey can't be claimed, it's worth confirming the retry loop earns its place at all — if the shell reliably owns the combo on Windows 11, the claim sequence is 2 s of startup work for nothing.

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.

While I do agree that some statements are sensationalistic, I've personally tried the mod on Windows 11 24H2 using @include windhawk.exe and it did not work and it works when replacing windhawk.exe with explorer.exe. I could try making the code more defensive in that case but proposing to make the mod non-functional seems like a suggestion to cause a regression in the code given that there were multiple reports related to this problem. The AI reviewer does not have a Windows testing environment therefore it is trying to undo an update that could make the mod more useful and functional. These are the logs:
13:17:58.095 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [759:LoadSettings]: LoadSettings: perspective=1 autoCycle=1
13:17:58.099 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [750:DetectPerformanceProfile]: DetectPerformanceProfile: ram=7810 MB cores=8 -> lowEnd=0 veryLowEnd=0 fps=62 deck=8
13:17:58.099 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [3376:HookThreadProcImpl]: HookThread: started, tid=8164
13:17:58.099 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [3408:HookThreadProcImpl]: HookThread: controller window created, hwnd=00000000000E0136
13:17:58.100 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [3328:TryRegisterHotkey]: TryRegisterHotkey: RegisterHotKey(Ctrl+Alt+F12) registered (id=15677)
13:17:58.100 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [3339:BeginWinTabClaim]: Win+Tab claim acquired on attempt 1 of 20
13:17:58.100 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [3435:HookThreadProcImpl]: HookThread: install OK, entering message loop
13:17:58.101 8552 explorer.exe [WH] [local@settings-to-control-panel] [1811:Wh_ModInit]: Initializing the Redirect Settings to Control Panel mod...
13:17:58.101 8552 explorer.exe [WH] [local@settings-to-control-panel] [1545:TryInstallPniduiHook]: [PNIDUI-HOOK] pnidui.dll not loaded yet
13:17:58.104 8552 explorer.exe [WH] [local@settings-to-control-panel] [1653:InstallImmersiveMenuHooks]: [PNIDUI-HOOK] Retry thread created
13:17:58.104 8552 explorer.exe [WH] [local@settings-to-control-panel] [1585:PniduiRetryThread]: [PNIDUI-HOOK] Retry thread started - waiting for pnidui.dll to load
13:17:58.319 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2780:ActivateFlip3DImpl]: ActivateFlip3D: 2 eligible top-level window(s) found
13:17:58.389 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2837:ActivateFlip3DImpl]: ActivateFlip3D: 2 card(s) prepared (simulated DWM; skipped: 0 no-rect, 0 thumbnail-failed)
13:17:58.389 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2500:RecomputeTargetsForCurrentSelection]: RecomputeTargetsForCurrentSelection: selectedIndex=0 desktopSelected=1 clientRect=1920x1080 dpi=120 cardCount=2
13:17:58.394 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2213:EnableHighResAnimationTimer]: EnableHighResAnimationTimer: timeBeginPeriod(1) OK
13:17:58.448 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2908:ActivateFlip3DImpl]: ActivateFlip3D: switcher opened with 2 card(s), persistent=0
13:17:58.541 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2709:DeactivateFlip3D]: DeactivateFlip3D: closing (activateSelected=1)
13:17:58.634 8552 explorer.exe [WH] [local@settings-to-control-panel] [1705:InstallSyscallFallback]: [SYSCALL-HOOK] NtUserTrackPopupMenuEx hooked successfully
13:17:58.635 8552 explorer.exe [WH] [local@settings-to-control-panel] [1293:InstallAAMHook]: [AAM-HOOK] CoCreateInstance(CLSID_ApplicationActivationManager) failed: 0x800401F0
13:17:58.707 8552 explorer.exe [WH] [local@settings-to-control-panel] [1358:InstallLegacyNameHook]: [MAP-LEGACY] Hook installed successfully
13:17:58.707 8552 explorer.exe [WH] [local@settings-to-control-panel] [1866:Wh_ModInit]: [TRAY-WATCHDOG] Watchdog thread created
13:17:58.707 8552 explorer.exe [WH] [local@settings-to-control-panel] [1758:TraySubclassWatchdogThread]: [TRAY-WATCHDOG] Watchdog thread started
13:17:58.786 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [3588:EntryPoint_Hook]: >
13:17:58.875 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2225:DisableHighResAnimationTimer]: DisableHighResAnimationTimer: timeEndPeriod(1)
13:17:59.653 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2780:ActivateFlip3DImpl]: ActivateFlip3D: 2 eligible top-level window(s) found
13:17:59.741 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2837:ActivateFlip3DImpl]: ActivateFlip3D: 2 card(s) prepared (simulated DWM; skipped: 0 no-rect, 0 thumbnail-failed)
13:17:59.748 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2500:RecomputeTargetsForCurrentSelection]: RecomputeTargetsForCurrentSelection: selectedIndex=0 desktopSelected=1 clientRect=1920x1080 dpi=120 cardCount=2
13:17:59.748 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2213:EnableHighResAnimationTimer]: EnableHighResAnimationTimer: timeBeginPeriod(1) OK
13:17:59.798 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2908:ActivateFlip3DImpl]: ActivateFlip3D: switcher opened with 2 card(s), persistent=0
13:18:00.027 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2500:RecomputeTargetsForCurrentSelection]: RecomputeTargetsForCurrentSelection: selectedIndex=1 desktopSelected=0 clientRect=1920x1080 dpi=120 cardCount=2
13:18:00.254 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2500:RecomputeTargetsForCurrentSelection]: RecomputeTargetsForCurrentSelection: selectedIndex=0 desktopSelected=0 clientRect=1920x1080 dpi=120 cardCount=2
13:18:00.448 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2500:RecomputeTargetsForCurrentSelection]: RecomputeTargetsForCurrentSelection: selectedIndex=1 desktopSelected=0 clientRect=1920x1080 dpi=120 cardCount=2
13:18:00.541 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2500:RecomputeTargetsForCurrentSelection]: RecomputeTargetsForCurrentSelection: selectedIndex=0 desktopSelected=0 clientRect=1920x1080 dpi=120 cardCount=2
13:18:00.651 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2500:RecomputeTargetsForCurrentSelection]: RecomputeTargetsForCurrentSelection: selectedIndex=1 desktopSelected=0 clientRect=1920x1080 dpi=120 cardCount=2
13:18:00.737 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2709:DeactivateFlip3D]: DeactivateFlip3D: closing (activateSelected=1)
13:18:01.082 8552 explorer.exe [WH] [local@aero-flip3d-recreation] [2225:DisableHighResAnimationTimer]: DisableHighResAnimationTimer: timeEndPeriod(1)
13:18:28.544 8552 explorer.exe [WH] [local@settings-to-control-panel] [1601:PniduiRetryThread]: [PNIDUI-HOOK] pnidui.dll not loaded after timeout, giving up

@babamohammed2022

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 Aug 6, 2026
@m417z

m417z commented Aug 6, 2026

Copy link
Copy Markdown
Member

cause a regression in the code

What's the regression?

We investigated some issue related to the Simple Window Switcher mod and thumbnail previews, and discovered that the API only works with a 64-bit process, while windhawk.exe is 32-bit. Is the regression related to thumbnail previews?

@babamohammed2022

babamohammed2022 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

cause a regression in the code

What's the regression?

We investigated some issue related to the Simple Window Switcher mod and thumbnail previews, and discovered that the API only works with a 64-bit process, while windhawk.exe is 32-bit. Is the regression related to thumbnail previews?

The regression is that the mod does not even start at all if using @include windhawk.exe but it does when using @include explorer.exe so I think that I'm forced to use this approach unless there is an analog functional and documented one that I'm missing

@m417z

m417z commented Aug 6, 2026

Copy link
Copy Markdown
Member

Could it be related to the Windhawk window running as administrator? WH_KEYBOARD_LL won't work in this case.

RegisterHotKey should work but perhaps it's already taken by explorer.exe. Perhaps that's why it works for you - when you target explorer.exe, you're able to call RegisterHotKey before explorer does.

You can test this with the Disable Windows Shortcuts mod - if you disable the shortcuts there, does the mod work with windhawk.exe as the target?

Simple Window Switcher bypasses this issue by targeting both explorer.exe (to prevent hotkey registration) and windhawk.exe (for the rest).

don't ask me how

Who should I ask? It's your mod, you should know how it works.

@babamohammed2022

babamohammed2022 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Could it be related to the Windhawk window running as administrator? WH_KEYBOARD_LL won't work in this case.

RegisterHotKey should work but perhaps it's already taken by explorer.exe. Perhaps that's why it works for you - when you target explorer.exe, you're able to call RegisterHotKey before explorer does.

You can test this with the Disable Windows Shortcuts mod - if you disable the shortcuts there, does the mod work with windhawk.exe as the target?

Simple Window Switcher bypasses this issue by targeting both explorer.exe (to prevent hotkey registration) and windhawk.exe (for the rest).

don't ask me how

Who should I ask? It's your mod, you should know how it works.

I will try the Disable Windows Shotcuts and let you know and related to the don't ask me how comment it was simply an affirmation related to the fact that changing the include made it work that's why. I will reach you out once I test it so that I can fix it properly by complying to the Windhawk approach but making it work correctly

@babamohammed2022

Copy link
Copy Markdown
Contributor Author

Could it be related to the Windhawk window running as administrator? WH_KEYBOARD_LL won't work in this case.

RegisterHotKey should work but perhaps it's already taken by explorer.exe. Perhaps that's why it works for you - when you target explorer.exe, you're able to call RegisterHotKey before explorer does.

You can test this with the Disable Windows Shortcuts mod - if you disable the shortcuts there, does the mod work with windhawk.exe as the target?

Simple Window Switcher bypasses this issue by targeting both explorer.exe (to prevent hotkey registration) and windhawk.exe (for the rest).

don't ask me how

Who should I ask? It's your mod, you should know how it works.

I've tried the Disable Windows Shortcuts mod and I've disabled WIN+TAB
image

However, this test confirms the problem because the first version of the mod (the one on the Windhawk store that includes windhawk.exe as target) does not work and there aren't even logs.
image

Therefore, I'd like to know if I need to use the approach of the mod mentioned above because in my case nothing is happening, I suppose that the mod fails to start

@m417z

m417z commented Aug 6, 2026

Copy link
Copy Markdown
Member

I'd like to understand why it doesn't work. I'll try the version from the store on my computer soon.

@babamohammed2022

Copy link
Copy Markdown
Contributor Author

I'd like to understand why it doesn't work. I'll try the version from the store on my computer soon.

OK, I understand. If possible, let me know the results once you test it

@m417z

m417z commented Aug 7, 2026

Copy link
Copy Markdown
Member

I tried both versions - before and after this PR. In both cases, it works for me except when the foreground program is running as administrator (for example, Windhawk or Task Manager).

@m417z m417z 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 Aug 7, 2026
@babamohammed2022

Copy link
Copy Markdown
Contributor Author

I tried both versions - before and after this PR. In both cases, it works for me except when the foreground program is running as administrator (for example, Windhawk or Task Manager).

Given this situation, I'd prefer to stall this PR (not close it) so that I can focus on fixing this problem more carefully since on my side it does not work to avoid releasing something that is not stable enough. Thanks for testing it

@m417z

m417z commented Aug 10, 2026

Copy link
Copy Markdown
Member

I noticed that you're specifying:

// @include         windhawk.exe
// @architecture    x86-64

In Windhawk v1.x, windhawk.exe is 32-bit, that's why the mod only works with Windhawk 2.0. Remove @architecture.

Removed architecture specification from metadata.
@babamohammed2022

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 Aug 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 bulk of this update is a revert of the tool-mod conversion that was required before 1.0.0 was merged, and the technical justification written into the file doesn't hold up. That's the main item; the rest are the lifecycle consequences plus a Tab regression.

1. Moving back to @include explorer.exe reverts the tool-mod conversion, and the stated reasons don't match how the code actually worked.

The mod still installs zero function hooks. SetWindowsHookEx(WH_KEYBOARD_LL/WH_MOUSE_LL), RegisterHotKey, EnumWindows, DwmRegisterThumbnail and the overlay window all work from any process on the desktop — which is exactly why the first review asked for the tool-mod shape, and it was merged that way.

The 90-line "WHY EXPLORER.EXE IS FUNDAMENTAL" block (:128-:201) argues against a process model that never existed in this mod:

  • "The tool process was a hollow explorer.exe" / "Wh_ModInit had to distinguish which explorer was the real one with FindWindow(Shell_TrayWnd)" — neither is true. 1.0.0 was @include windhawk.exe, and the launcher spawns a copy of windhawk.exe (GetModuleFileName(nullptr) + -tool-mod), never explorer.exe. There was no IsMainExplorerProcess() and no Shell_TrayWnd check anywhere in 1.0.0. Points 3 and 5 of the block rest entirely on this.
  • "Win+Tab must live in the same lifetime as explorer.exe"WH_KEYBOARD_LL hooks are per-desktop, not per-process; they run before hotkey dispatch regardless of which process installed them. And being inside explorer.exe does not help the RegisterHotKey(MOD_WIN, VK_TAB) claim either: hotkey registrations are global per-desktop, so the shell's existing registration blocks the combo from any thread in any process — including another thread of explorer.exe itself. g_winTabHotkeyOwned (:3456) will keep failing exactly as it did before.
  • "The DWM thumbnail parent must be in the shell"DwmRegisterThumbnail only requires the destination to be a top-level window owned by the calling process. Simple Window Switcher runs its whole switcher UI + DWM thumbnails in the dedicated tool process.

Meanwhile the move has real costs: a fault anywhere in ~3800 lines of GDI/DWM/animation code now restarts the shell; timeBeginPeriod(1) (:2297) now raises explorer.exe's timer resolution instead of a throwaway process's; and Wh_ModInit blocks Explorer's main thread on a handshake for up to 5 s at shell startup (item 3).

If Win+Tab genuinely behaves differently on 23H2/24H2, the cause worth chasing is the one already pointed at twice in the previous rounds — the shell owns the combo. The reference fix keeps the tool mod and adds a second, minimal injection target:

// @include         windhawk.exe
// @include         explorer.exe

…with the explorer.exe path doing nothing but hooking RegisterHotKey so the shell can't claim Win+Tab first, exactly as Simple Window Switcher does (its Wh_ModInit branch is at :4928). That gets you the ownership you want without hosting the switcher in the shell. Please restore the wiki launcher boilerplate verbatim and delete the rationale block, or explain concretely (with a repro) what fails in the tool process after the RegisterHotKey hook is in place.

2. The single-instance mutex picks an arbitrary explorer.exe, not the shell.

Wh_ModInit (:3759) is first-come-first-served: whichever explorer.exe creates Local\Windhawk_AeroFlip3D_ExplorerSingleton first hosts the switcher, and every other one returns FALSE. That is strictly weaker than the Shell_TrayWnd check the comment dismisses, because it has no notion of which Explorer won:

  • With "Launch folder windows in a separate process" enabled, or after restarting the shell while a folder-window process is alive, the mutex can be held by a transient process. When the user closes that folder window, Win+Tab silently stops working — and the real shell already returned FALSE, so nothing takes over until Explorer restarts.
  • On a mod reload (update / settings reload) the winner releases the mutex in Wh_ModUninit and re-acquires it in Wh_ModInit; a losing process reloading in that window can take it over.
  • CreateMutexW(nullptr, FALSE, …) never takes ownership and nothing ever waits on it, so it is only being used as a named-existence flag — a plain named event or an OpenMutex probe would express that more honestly.
  • On every ModInitImpl() failure path (:3625, :3634, :3652, :3657) Wh_ModInit returns FALSE with g_singleInstanceMutex still held and g_isExplorerProcess still true; since Wh_ModUninit isn't called after a failed init, the release depends entirely on the static destructor running at DLL unload. Release it explicitly before every return FALSE.

Going back to a tool mod removes all of this — the launcher's windhawk-tool-mod_ mutex already guarantees one instance, by construction.

3. Wh_ModInit can hang Explorer's startup — the 5 s bound buys nothing.

The comment at :3638 is explicit that the bounded handshake exists so a wedged worker thread can't block the shell's main thread. But the timeout path immediately calls ModUninitImpl() (:3651), which does WaitForSingleObject(g_hookThread.get(), INFINITE) (:3677) on that same thread:

DWORD wait = WaitForSingleObject(g_hookReadyEvent.get(), kHookInitTimeoutMs);
if (wait != WAIT_OBJECT_0) {
    ...
    ModUninitImpl();  // <- WaitForSingleObject(g_hookThread, INFINITE)
    return false;
}

The only reason the handshake times out is that the hook thread is stuck — and that's precisely the case where the join never returns. So the 5 s timeout converts "shell blocked forever with no log line" into "shell blocked forever with a log line". Joining is the right thing to do (the thread must not outlive the image), which is another reason this work belongs in a process where an indefinite wait is harmless.

4. Tab still doesn't advance the deck while Win is held — and the new tabDown guard now freezes it completely.

This is item 1 of the last review, unchanged (:1532):

if (vk == VK_TAB && keyDown && winDown && !altDown && !ctrlDown && !winTabOwnedElsewhere) {
    if (!active) { ... return 1; }
    return 1;   // <- swallowed before the `if (active)` Tab handler at :1568
}

g_winTabHotkeyOwned is false in the normal case (the shell owns Win+Tab), so every Tab — and Shift+Tab, which this condition doesn't even distinguish — pressed while Win is held returns here and never reaches the 180 ms-gated navigation at :1568.

The new auto-cycle guard then makes it worse (:3165):

bool tabDown = (GetAsyncKeyState(VK_TAB) & 0x8000) != 0;
if (!tabDown) {
    NavigateSelection(1);   // "let the LL hook drive" — but the LL hook can't, see above
}

The comment says the LL hook drives while Tab is down, but that path is unreachable. So holding Win+Tab down now advances nothing: the timer skips because Tab is down, and the hook swallows the repeat. That is exactly the behaviour the new README line advertises ("Hold Tab down to keep advancing automatically without repeated presses"), and it worked before this PR for anyone who had autoCycle on.

Gate the branch on the switcher being closed and let an active session fall through to the handler that already exists:

if (!active && vk == VK_TAB && keyDown && winDown && !altDown && !ctrlDown &&
    !winTabOwnedElsewhere) {
    PostToController(WM_FLIP3D_ACTIVATE,
                     static_cast<WPARAM>(TriggerModifier::Win),
                     static_cast<LPARAM>(shiftDown ? -1 : 1));
    return 1;
}

Once Tab actually reaches :1568, the tabDown guard does what its comment claims.

5. The system-wide key swallow is still armed before the blocking cross-process work (carried over).

ActivateFlip3DImpl sets g_hookSessionActive = true at :2906 and only then runs CaptureDesktopSnapshot() (:2910), whose first path is PrintWindow against Progman/WorkerW (:1313) — a synchronous WM_PRINT with no timeout — followed by a DwmRegisterThumbnail per eligible window (:2928). While that flag is set, LowLevelKeyboardProc returns 1 for essentially every key system-wide (:1624), and the only automatic recovery is the 60 s failsafe SetTimer, which lives on the very thread that would be stuck. This was already flagged last round; it matters more now that the "desktop" window being PrintWindowed lives on another thread of the same process the mod is hosted in. Set g_hookSessionActive only once the overlay is up (after ShowWindow, :2992), and give the hook its own deadline — stamp GetTickCount64() when the activate message is posted and stop swallowing if the UI thread hasn't confirmed within a couple of seconds.

Optional improvements

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

  • The comment blocks narrate the diff rather than the code. :96-:201 (two overlapping rationale blocks), plus "(diff no-op)" (:3111), "reduced from 50" (:856), "(autoCycle is true by default, so the overlap was opt-out before the fix)" (:3163), "Internal settings-changed helper (ex-shell, non-tool)" (:3709), "DisableHighResAnimationTimer AFTER the joins: calling it before created a race" (:3693). Comments that describe what changed relative to a previous revision go stale the moment the next revision lands — the PR description is the place for them. The shouty all-caps block in particular reads as generated argumentation rather than documentation.
  • The re-activation guard in FinishExitAfterAnimation can't do what its comment says (:2788-:2794): overlay was destroyed two lines earlier by SafeDestroyOverlayWindow(overlay), so !(overlay && IsWindow(overlay)) is true on essentially every path and the "only if the first attempt didn't happen" condition never guards anything. The GetForegroundWindow() != selectedHwnd test is doing all the work — drop the inner if or capture a bool activatedEarly before the destroy.
  • ActivateSelectedWindow's "gentle fallback" doesn't exist (:2709-:2715): the comment promises "we try a flash without ever touching AttachThreadInput", but the block only calls Wh_Log. Either add the FlashWindowEx or fix the comment. (Dropping AttachThreadInput is the right call.)
  • Unregister the window classes after the thread join, not inside the thread's scope guard. UnregisterFlipWindowClasses() runs at :3522 while the hook thread is still alive, so if a DestroyWindow failed (:3517, :2753) the class is still in use and UnregisterClassW fails — leaving a registration whose lpfnWndProc points into the unmapped image, and a RegisterClassExW that fails with ERROR_CLASS_ALREADY_EXISTS on the next load (the feature then silently never opens, since CreateOverlayWindow returns nullptr). Moving the call into ModUninitImpl after WaitForSingleObject(g_hookThread, INFINITE) makes it reliable: thread exit destroys any windows the thread still owned, so the unregister always succeeds.
  • g_persistentMode is still dead (carried over): the LL hook handles Ctrl+Alt+F12 itself and returns 1 (:1519-:1529), so WM_HOTKEY/kStickyHotkeyId (:3241) — the only place that sets g_persistentMode = true — can't run while the hook is installed. Either pass the persistent flag through WM_FLIP3D_ACTIVATE's lParam, or drop the flag and the RegisterHotKey(kStickyHotkeyId) registration.
  • The autoCycle $description no longer matches the code: wantsAutoCycle now also requires triggerModifier == TriggerModifier::Win (:2999), so the setting has no effect on a Ctrl+Alt+F12 session. That's the right behaviour, but the description should say so.

Functionality notes

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

  • The multi-monitor PrintWindow origin issue from the last round is unfixed (:1313): the bitmap is sized from GetPrimaryMonitorRect(), but PrintWindow renders Progman/WorkerW starting at the DC origin, i.e. from the virtual screen's top-left. With a monitor to the left of or above the primary one, the backdrop shows that monitor's content. Only the BitBlt fallback (:1324) offsets correctly. SetViewportOrgEx(memDc, windowRect.left - desktopRect.left, windowRect.top - desktopRect.top, nullptr) before the PrintWindow (restored to 0,0 before the GetPixel sampling) fixes it.
  • Cards still pop in and out at the deck-cap boundary on every navigation step (carried over): RebuildThumbnailZOrder (:2194) unregisters everything at depth >= g_perf.maxDeckCards, so the outgoing front card loses its thumbnails immediately instead of animating out, and the incoming card inflates from a degenerate rect near the deck anchor rather than sliding in from the back.
  • The README's keyboard table doesn't distinguish the two sessions. Tab/Shift+Tab and the auto-cycle only apply to the Win+Tab session; the Ctrl+Alt+F12 session has neither auto-cycle nor (per item 4's fix) the same Win-held semantics. Worth a line so the shortcut table matches what users get.
  • GdiBudgetAllowsBitmaps behaves differently now (:1185): the "< 8000 GDI objects" threshold could never realistically fire in a dedicated tool process, but explorer.exe legitimately carries thousands of GDI objects, so the desktop snapshot can now be skipped (silently degrading to the Aero gradient) on a busy shell. Another small argument for the tool process.
  • The overlay still covers only the primary monitor, as documented — spanning the virtual screen with a backdrop-only overlay on the secondaries would be closer to the original.


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 11, 2026
Updated mod to include windhawk.exe instead of explorer.exe, refined settings for auto-cycle, and improved documentation.
@babamohammed2022

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 Aug 11, 2026
@babamohammed2022 babamohammed2022 changed the title Bump version to 1.1.0 and enhance features Update the mod to the version 1.1.0 Aug 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 main thing here is that the pull request doesn't contain the changes it describes — the mod body is byte-identical to the merged 1.0.0. Items 2-5 are the ones you deferred at the end of the previous review; they're all still present in the code as it stands, so they're repeated here for the update.

1. The version bump carries no code change.

The full diff against main is three lines:

-// @version         1.0.0
+// @version         1.1.0
-// @architecture    x86-64
+

Everything after the metadata block is identical to the merged 1.0.0 — I diffed the head revision against main ignoring those two lines and the only difference is one added blank line at :11. But the changelog says "Enhanced Windows 11 compatibility", and the branch commits claim "support for Windows 11 23H2 and 24H2" and "Enhanced keyboard shortcuts for auto-advancing selection". None of that is in the diff.

A version bump publishes an update to every user who has the mod installed, so it should carry a real change. Either push the commits that were meant to be part of this, or drop the bump. (The @architecture removal on its own is fine — none of the merged tool mods declare it, and omitting it means x86 + x86-64, a superset of what was there — but it isn't a Windows 11 compatibility change.)

2. Tab still doesn't advance the deck while Win is held.

:1445:

if (vk == VK_TAB && keyDown && winDown && !altDown && !ctrlDown && !winTabOwnedElsewhere) {
    if (!active) {
        ...
        PostToController(WM_FLIP3D_ACTIVATE, ...);
        return 1;
    }
    return 1;   // <- the Tab that should advance the deck is dropped here
}

g_winTabHotkeyOwned is false in practice (the shell owns Win+Tab, so all 20 claim attempts fail), so this branch is taken for every Tab pressed while Win is held — including while the switcher is open — and it returns before the if (active) Tab handler at :1481 can post WM_FLIP3D_NAVIGATE. After opening with Win+Tab, holding Win and tapping Tab does nothing; only the arrow keys and the wheel navigate, and autoCycle is off by default. That is the core Flip 3D interaction, and it's what the README documents ("Navigate: Tab / Shift+Tab, arrow keys, or mouse wheel").

Gate the branch on the switcher being closed and let an active session fall through to the handler that already exists:

if (!active && vk == VK_TAB && keyDown && winDown && !altDown && !ctrlDown &&
    !winTabOwnedElsewhere) {
    PostToController(WM_FLIP3D_ACTIVATE,
                     static_cast<WPARAM>(TriggerModifier::Win),
                     static_cast<LPARAM>(shiftDown ? -1 : 1));
    return 1;
}

3. A Win+Tab that doesn't open the switcher pops the Start menu.

The hook swallows the Tab keydown at :1452 before the UI thread has decided anything. If ActivateFlip3DImpl then bails — composition disabled (:2771), fewer than two eligible windows (:2780), CreateOverlayWindow failure (:2789) — nothing is active, so the later Win keyup takes the pass-through path at :1540 with the suppression flags never armed, and Windows sees a bare Win press. The user gets neither Flip 3D nor Task View, plus a Start menu they didn't ask for.

Arm the suppression at the moment you swallow the Tab. PreparePendingModifierReleaseSuppression() can't be used as-is there, because g_triggerModifier is still None until the UI thread sets it at :2800 — set it explicitly:

g_suppressReleaseModifier.store(static_cast<int>(TriggerModifier::Win),
                                std::memory_order_release);
g_suppressNextModifierRelease.store(true, std::memory_order_release);
PostToController(WM_FLIP3D_ACTIVATE, ...);

The successful path already clears it again at :2802, so this only affects the failure case.

4. The system-wide key swallow is armed before the UI thread does blocking cross-process work.

ActivateFlip3DImpl sets g_hookSessionActive = true at :2801 and only then runs CaptureDesktopSnapshot() (:2805), whose first capture path is PrintWindow against Explorer's Progman/WorkerW (:1226) — a synchronous cross-process WM_PRINT with no timeout — followed by a DwmRegisterThumbnail per window and SetForegroundWindow/AttachThreadInput later on. While that flag is set, LowLevelKeyboardProc returns 1 for essentially every key (:1537), system-wide, and the only automatic recovery is the 60 s failsafe SetTimer, which is armed at :2891 — after the capture, on the very thread that would be stuck. If Explorer is hung, the keyboard is dead everywhere with no way out but disabling the mod with the mouse.

Two cheap mitigations, ideally both: set g_hookSessionActive only once the overlay is actually up (end of ActivateFlip3DImpl, after ShowWindow at :2887), and give the hook its own deadline — stamp GetTickCount64() when the activate message is posted, and have the hook clear the flag and stop swallowing if the UI thread hasn't confirmed the session within a few seconds.

5. On a multi-monitor layout the backdrop can capture the wrong monitor.

Sizing the bitmap from GetPrimaryMonitorRect() (:1167) is right, but the first capture path is PrintWindow(desktopHwnd, memDc, …) (:1226), and PrintWindow renders the window starting at the DC's origin — i.e. from the desktop window's own top-left, which is the virtual screen's top-left. Whenever a monitor sits to the left of / above the primary one, the virtual origin is negative and the bitmap ends up holding that monitor's region instead of the primary's. Only the BitBlt fallback (:1237) gets it right, because it offsets the source by desktopRect.left/top.

Shift the DC before printing (and restore it before the GetPixel sampling):

RECT windowRect = {};
GetWindowRect(desktopHwnd, &windowRect);
SetViewportOrgEx(memDc.get(), windowRect.left - desktopRect.left,
                 windowRect.top - desktopRect.top, nullptr);
BOOL ok = PrintWindow(desktopHwnd, memDc.get(), PW_RENDERFULLCONTENT);
SetViewportOrgEx(memDc.get(), 0, 0, nullptr);
Optional improvements

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

  • README text is now out of date with the version bump. "This is the first version, so some details may still be improved" (:25) reads oddly on 1.1.0, and "Requirements: Windows 10 version 1809 or later (64-bit)" (:58) no longer matches the metadata now that @architecture is gone (the mod is built for x86 as well).
  • Stray blank line at :11-:12: there are now two blank lines between ==/WindhawkMod== and ==WindhawkModReadme==.
  • WaitForSingleObject(g_hookReadyEvent.get(), INFINITE) (:3517) has no upper bound, and the catch (...) in HookThreadProc (:3456) returns without ever calling SetEvent. An exception before :3431 would hang the tool process forever during init. A bounded wait is what tiling-helper does (WaitForSingleObject(..., 5000) != WAIT_OBJECT_0 → fail init).
  • Ctrl+Alt+F12's WM_HOTKEY path is unreachable, so persistent mode is dead code. The hook computes manualShortcut and handles both directions itself, returning 1 (:1432-:1442), so RegisterHotKey(kStickyHotkeyId)'s WM_HOTKEY handler at :3119 can never run — and that is the only place g_persistentMode is ever set to true. The manual session therefore always opens with TriggerModifier::None and the 60 s failsafe armed, and the RegisterHotKey call buys nothing. Either pass the persistent flag through WM_FLIP3D_ACTIVATE (e.g. in lParam), or drop the registration plus the dead case kStickyHotkeyId and the g_persistentMode machinery.
  • Settings are applied on the wrong thread. WhTool_ModSettingsChanged (:3569) runs on a Windhawk thread and rewrites g_settings / g_perf while the UI thread reads them mid-frame (and reads g_isActive / g_hOverlayWnd unsynchronized). Posting a message to the controller window and calling LoadSettings() / DetectPerformanceProfile() there would keep all of it on one thread. Same shape: WhTool_ModUninitImpl calls DisableHighResAnimationTimer() (:3522) from the Windhawk thread before the UI thread is joined, touching the non-atomic g_highResTimerActive that the UI thread also toggles — the UI thread's own teardown already handles it.
  • g_hControllerWnd is read from the input thread in PostToController (:1072) while the UI thread writes it (:3405, :3399). Everything else shared across those two threads was made std::atomic; this one wasn't.
  • WM_NCDESTROY never reaches DefWindowProcW (:3062 in OverlayWndProc, and the WM_DESTROY case at :3145 in ControllerWndProc). DefWindowProc's WM_NCDESTROY handling is what frees the window's internally allocated data (window text, etc.), so returning 0 without it leaks a little per window. Clear the globals and then break so the tail DefWindowProcW runs.
  • Redundant thumbnail registration at open: ActivateFlip3DImpl registers a thumbnail per eligible window (:2823) only to call DwmQueryThumbnailSourceSize, and RebuildThumbnailZOrder (:2904) resets every one of them a moment later. That's an extra register/unregister ALPC round-trip per window on every activation, and for windows past the deck cap the registration is pure waste.
  • The try / catch (...) boundaries (9 of them) still can't catch what the comments say they catch: catch (...) doesn't catch SEH access violations on this toolchain, and nothing here throws except a std::vector allocation. Dropping them would remove a level of nesting from every callback.
  • GdiBudgetAllowsBitmaps / MemoryPressureAllowsSnapshot (:1098, :1105) trip only above 8000 GDI objects or below 100 MB free RAM. In a dedicated tool process that allocates a handful of GDI objects, neither can realistically fire.
  • Small ones:
    • wParam == kAnimationTimerId || wParam == g_animationTimerId (:3039) — SetTimer with an explicit ID returns that ID, so the two are always equal.
    • The local winTabOwnedElsewhere (:1415) holds "owned by us"; the name says the opposite, which is what makes item 2 easy to misread.
    • LoadSettings's comment "Performance profile is always auto-detected -- no manual override" (:755) contradicts the visibleWindowCount override at :737.
    • Stale comment at :2321 pointing at ComputeStackLayout, which no longer exists.
    • ActivateSelectedWindow (:2601): the overlay is the foreground window at that point, so plain SetForegroundWindow should work without the AttachThreadInput dance, which can block on a hung target thread.
    • The cross-thread SendMessageW branch at the top of DeactivateFlip3D (:2693) is unreachable — the overlay lives on the same thread as both window procs that call it.
    • The pasted tool-mod launcher matches the wiki snippet except that the leading comment block was trimmed; keeping it verbatim makes future diffs against the wiki clean.

Functionality notes

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

  • Navigation is debounced twice, which will bite once item 2 is fixed. The hook throttles Tab to one event per 180 ms (:1484) and NavigateSelection independently drops anything within kNavigationDebounceMs = 85 ms (:2517). That caps stepping at ~5.5 cards/s, so tapping Tab quickly to reach the 6th window silently drops presses. Real Flip 3D advanced once per keypress at whatever rate you pressed (plus auto-repeat when held) — the debounce is only needed for the wheel.
  • Cards pop in and out at the deck-cap boundary on every navigation step. NavigateSelection recomputes targets and then calls RebuildThumbnailZOrder (:2543-:2544), which skips (i.e. unregisters) every card at depth >= g_perf.maxDeckCards — so the outgoing front card, which has just been moved to depth count-1, loses its thumbnails immediately and vanishes instead of animating out. The card entering the deck has the opposite problem: ComputeSimulatedStackLayout parks off-deck cards at a degenerate rect on the deck anchor (:2416), so it inflates out of a point near the front of the deck rather than sliding in from the back. Keeping the deck registered for the duration of the transition, and parking off-deck cards at the deepest visible card's pose, would make the cascade read as continuous motion.
  • A quick Win+Tab tap still re-activates the window you were already on: ActivateFlip3DImpl opens with g_desktopSelected = true and ignores its initialDelta on the activation path (it is only used in the already-active branch at :2761). Real Flip 3D advanced one step on the first press.
  • Full re-registration per navigation is the cost centre, and there's no clean alternative — DWM z-order follows registration order, so a rotation genuinely invalidates the whole set. Just noting that with the cap at 8 it's still ~80 thumbnails re-registered per step plus ~80 DwmUpdateThumbnailProperties per frame; the strip-count scaling in StripCountForDepth is the right knob if it stutters.
  • Minimized windows will render as blank cards: IsWindowVisible is TRUE for them so they pass IsFlipEligibleWindow, but DWM has no live content to composite. GetRepresentativeWindowRect already special-cases IsIconic for geometry — the card itself may need a static fallback, or those windows could be excluded.
  • WM_SIZE / WM_DISPLAYCHANGE (:2963) recompute the target rects but never start a transition and never resize the overlay, so if the resolution changes while the switcher is open the cards keep the old geometry on an overlay that still covers the old rect.
  • Last strip overruns its source and destination by 1 px: rcSource.right = ((j + 1) * src.cx) / M + 1 and rcDestination.right = … + 1 (:2002, :2007) give src.cx + 1 / one pixel past the card edge for j == M - 1. DWM will most likely clamp, but the seam-overlap trick is only needed between strips, so clamping the last one to the exact edge removes the dependency.
  • Blank-capture detection treats "all 9 sampled pixels are black" as a failed capture (:1212), so a plain black wallpaper with no icons falls through all three capture paths to the Aero gradient. Comparing against a uniform colour rather than specifically black would be more robust.
  • No click-to-select: OverlayWndProc has no mouse-button handling and the mouse hook only consumes the wheel. Hit-testing the computed card rects on WM_LBUTTONDOWN (plus a hover highlight) is a large part of what made the original deck feel the way it did.
  • Only the primary monitor is covered, as documented — spanning the virtual screen (SM_XVIRTUALSCREEN / SM_CXVIRTUALSCREEN) with a backdrop-only overlay on the secondaries would be closer to the original.


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 11, 2026
@babamohammed2022

Copy link
Copy Markdown
Contributor Author

Given the situation, I think that these problems could be addressed in future as the main problem seems to be solved
/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 Aug 11, 2026
@m417z
m417z merged commit 8c4b180 into ramensoftware:main Aug 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 Aug 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