Skip to content

Add Window Closing Animations Mod - #5187

Open
TheSerphh wants to merge 2 commits into
ramensoftware:mainfrom
TheSerphh:main
Open

Add Window Closing Animations Mod#5187
TheSerphh wants to merge 2 commits into
ramensoftware:mainfrom
TheSerphh:main

Conversation

@TheSerphh

Copy link
Copy Markdown

Key features:

  • Add 7 custom animations (Fire, Shatter, CRT, Glitch, Fold, Iris, Pixelate).
  • Hook DefWindowProcW, ShowWindow, SetWindowPos, DestroyWindow for universal coverage.
  • Add ExitProcess hook safeguard to prevent browser cut-offs.
  • Implement dynamic UWP/sandbox exclusion to fix Snipping Tool/Photos crashes.
  • Optimize rendering engine with high-precision time-delta (QPC) math.

Changelog

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

  • N/A - Initial release of a new mod.

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.

Key features:
- Add 7 custom animations (Fire, Shatter, CRT, Glitch, Fold, Iris, Pixelate).
- Hook DefWindowProcW, ShowWindow, SetWindowPos, DestroyWindow for universal coverage.
- Add ExitProcess hook safeguard to prevent browser cut-offs.
- Implement dynamic UWP/sandbox exclusion to fix Snipping Tool/Photos crashes.
- Optimize rendering engine with high-precision time-delta (QPC) math.
@windhawk-reviewer

Copy link
Copy Markdown

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

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

See the pull request review process for the full details.

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

Copy link
Copy Markdown
Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 23, 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 animation effects themselves are nice, but the integration with Windhawk and with the host process has several blocking problems — a detached thread that outlives the mod, a window class that is never unregistered, and synchronous 800 ms message-pump waits inside ShowWindow/SetWindowPos/DestroyWindow. There's also a large overlap with an existing mod that needs to be resolved first.

1. Substantial overlap with the existing "Windows Animations" mod. mods/windows-animations.wh.cpp already ships closing animations with @include *, hooking the same four functions (DefWindowProcW, ShowWindow, SetWindowPos, DestroyWindow — see L6237-L6243), and its close effect list — Square Shatter, Cyber Glitch, Retro TV Off, Pixel Melt, Perlin Dissolve, Thanos Snap — covers four of your seven almost one-for-one (Shatter, Glitch, CRT, Pixelate). Only Fire, Fold and Iris are genuinely new. The maintainer's strong preference is to extend an existing mod rather than merge a near-duplicate: two @include * mods that both hook DefWindowProcW and both hide/re-close windows will also actively fight each other if a user enables both. Please either contribute the three new effects to that mod (as an option / PR to its author), or explain in the PR description what this mod does that the existing one structurally cannot.

2. std::thread(...).detach() — the mod can be unloaded while animation threads are still running. TriggerAnimIfReady detaches every animation thread, and Wh_ModUninit doesn't wait for any of them. Windhawk FreeLibrarys the mod as soon as Wh_ModUninit returns, so any thread still inside ExecuteAnimation — including one just sleeping in the Sleep(1) loop — has its code and return address unmapped underneath it, crashing the host. This is guaranteed on any disable/update while a window is closing (and closing a window is exactly when a user would toggle the mod off). Fire-and-forget threads with no join point aren't acceptable; keep the thread handles and wait for them:

std::mutex g_threadsMutex;
std::vector<HANDLE> g_animThreads;
std::atomic<bool> g_unloading{false};
// ...
void Wh_ModUninit() {
    g_unloading = true;               // animation loop checks this and bails early
    std::vector<HANDLE> threads;
    { std::lock_guard g(g_threadsMutex); threads.swap(g_animThreads); }
    for (HANDLE h : threads) { WaitForSingleObject(h, INFINITE); CloseHandle(h); }
    // ... only now GdiplusShutdown / UnregisterClass
}

See mods/windows-animations.wh.cpp#L6149-L6164 for the signal-then-WaitForSingleObject(..., INFINITE)-then-CloseHandle shape. (Note the mutex must not be held while waiting — copy the handles out, release, then wait.)

3. The WindhawkAnimOverlay window class is never unregistered, and is registered under the wrong HINSTANCE. ExecuteAnimation calls RegisterClassW on every animation and ignores the result, and nothing ever calls UnregisterClass. A class is not removed when the mod DLL unloads, so after the first unload the registration survives with lpfnWndProc pointing into the unmapped mod image. On the next load RegisterClassW fails with ERROR_CLASS_ALREADY_EXISTS, the code carries on, and CreateWindowExW happily creates a window against the stale class → messages dispatched to a dangling AnimWndProc → crash or worse. Two fixes needed:

4. The ShowWindow / SetWindowPos / DestroyWindow hooks block the caller for up to animDuration + 1000 ms while running a nested message pump. TriggerAnimIfReady(..., waitForFinish = true) sits in MsgWaitForMultipleObjects + PeekMessage/DispatchMessage inside the hooked call. Two separate problems:

  • Hang: any app that hides or destroys a top-level window freezes for ~0.8 s, in every process on the system. During app shutdown that's per window.
  • Reentrancy: dispatching arbitrary messages from inside DestroyWindow re-enters the app while a window is mid-teardown — the app can process another WM_CLOSE, run timers, re-enter its own close path, or destroy the same window again. This is a real crash source, not a theoretical one.

The animation must be asynchronous: let the original call proceed immediately and drive the overlay purely from the worker thread, the way mods/genie-minimize-animation.wh.cpp#L315 does (start the thread, return, never block the UI thread).

5. WM_PROCEED_WITH_CLOSE is a WM_USER-range message posted to windows the mod doesn't own. WM_USER + 0x4242 (0x4642) is class-defined — its meaning belongs to whatever class the window is. Posting it into arbitrary third-party windows can trigger unrelated app logic, and conversely an app that already uses that value and forwards it to DefWindowProcW will have its window destroyed out from under it. Worse, the mod only handles the message in DefWindowProcW_Hook — so for any window whose proc doesn't route unhandled messages to DefWindowProcW (dialogs going through DefDlgProcW, ANSI apps using DefWindowProcA, frameworks with their own default handling), the window is hidden by TriggerAnimIfReady and then never destroyed: the app looks closed but the window and its thread stay alive forever.

The robust pattern is to re-post the original message with a bypass prop instead of inventing one — see mods/windows-animations.wh.cpp#L5916-L5923 and its FinishClose at L4448-L4461. If you do need a private message, get it from RegisterWindowMessageW (returns a process-unique value in 0xC0000xFFFF that cannot collide with a class's own WM_USER messages).

6. Hiding a window is not closing it. ShowWindow_Hook fires the destruction animation on any SW_HIDE, and SetWindowPos_Hook on any SWP_HIDEWINDOW. Apps hide top-level windows constantly without closing them — minimize-to-tray, splash screens, wizard/page swaps, windows temporarily hidden during a layout change. Every one of those gets a "the window is being destroyed" burn/shatter animation plus the 0.8 s stall. On top of that, SetPropW(hWnd, L"WindhawkAnimDone", ...) is never removed, so once a window has been hidden it is permanently blacklisted — when it is actually closed later, no animation plays. The existing mod gates this behind an explicit opt-in setting ("Animate windows hidden to the tray") for exactly this reason; at minimum do the same, and RemovePropW when the window is re-shown.

7. GDI leak and use-after-free in ExecuteAnimation's teardown. The cleanup at the end of the function is in the wrong order:

DestroyWindow(hOverlay);
DeleteObject(hOverlayBmp);   // still selected into hdcMem -> fails, bitmap leaks
DeleteDC(hdcMem);            // graphics still wraps this HDC
ReleaseDC(NULL, hdcScreen);
DeleteObject(hCapturedBmp);  // `original` still wraps this HBITMAP
// ... `graphics` and `original` destruct here, after their backing objects are gone
  • DeleteObject on a bitmap that is still selected into a DC fails, so a full-window-sized 32bpp DIB section leaks on every single animation, in every process. Restore the original bitmap first: SelectObject(hdcMem, hOldBmp); DeleteDC(hdcMem); DeleteObject(hOverlayBmp); — see mods/genie-minimize-animation.wh.cpp#L312-L313.
  • Graphics graphics(hdcMem) and Bitmap original(hCapturedBmp, NULL) are function-scope objects that destruct after DeleteDC/DeleteObject. The GDI+ docs are explicit that you must not delete the source GDI bitmap until the Bitmap object is gone. Wrap them in an inner scope (or std::optional + reset()) so they're destroyed before their backing objects.

Same issue in TriggerAnimIfReady: hBmp is left selected when DeleteDC(hdcTarget) runs.

8. GDI+ startup/shutdown is racy and can be torn down under a live animation. g_gdiInitialized is a plain bool checked-then-set from TriggerAnimIfReady, which runs on any thread that closes a window — two windows closing on two UI threads (Explorer gives every CabinetWClass window its own thread) can both see false and call GdiplusStartup twice, leaking one token, or one thread can start drawing before the other has finished initializing. Separately, Wh_ModUninit calls GdiplusShutdown while detached animation threads are still holding Graphics/Bitmap objects → crash. Initialize once in Wh_ModInit and shut down only after all animation threads have been joined (item 2).

9. The ExitProcess hook stalls every process exit by up to animDuration + 200 ms. This is a system-wide hook in every injected process that makes shutdown slower for the sake of a cosmetic effect, and it doesn't actually solve the problem — it doesn't cover TerminateProcess, a WM_QUIT-driven exit that races the animation, or the mod being unloaded. Once animation threads are properly tracked and joined (item 2), this hook has no purpose; please drop it. (Blocking inside ExitProcess while a worker thread is still trying to UpdateLayeredWindow and PostMessage is also its own hang risk during shutdown.)

10. The overlay steals focus and flashes uninitialized content. CreateWindowExW doesn't set WS_EX_NOACTIVATE, and ShowWindow(hOverlay, SW_SHOW) activates the window — so every window close briefly yanks activation away from whatever window Windows was about to focus next. SW_SHOW is also called before the first UpdateLayeredWindow, so the layered window is displayed with an uninitialized surface for a frame. Fix both: add WS_EX_NOACTIVATE to the ex-style, and show the window only after the first UpdateLayeredWindow, with SW_SHOWNOACTIVATE — see mods/genie-minimize-animation.wh.cpp#L191-L196 and mods/classic-min-max-animations.wh.cpp#L970-L971.

11. Sleep(1) frame pacing burns a full CPU core per animation. The render loop redraws the whole overlay (GDI+ antialiased polygons, 100 rotated shard blits, etc.) as fast as it can and only yields 1 ms — with @include * and several windows closing at once that's several cores pegged, and the extra frames aren't visible anyway since the compositor only presents at refresh rate. Sync to the compose cycle with DwmFlush(), as mods/genie-minimize-animation.wh.cpp#L202 does (if (FAILED(DwmFlush())) Sleep(16); as a fallback).

12. The README has no screenshots or GIFs. This is a purely visual mod with seven distinct effects, and users choose the effect from a dropdown without any way to know what it looks like. Please add a short GIF per effect (only i.imgur.com and raw.githubusercontent.com are allowed image hosts). mods/windows-animations.wh.cpp is a good example of the expected presentation.

Optional improvements

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

  • Use WindhawkUtils::StringSetting instead of the raw get + Wh_FreeStringSetting pair in Wh_ModSettingsChanged. Also, Wh_GetStringSetting never returns NULL — it returns L"" on error or when unset — so effect ? effect : L"pixelate" is dead code (and would silently produce an empty effect name rather than the default anyway; compare against L"" if you want a fallback).

  • The class blocklist uses substring matching. wcsstr(className, L"IME") matches any class whose name merely contains IME (e.g. TIMEPickerWnd), and wcsstr(..., L"Progman") likewise. Use wcscmp / _wcsicmp for exact names, and keep wcsstr only where a prefix match is actually intended. GetClassNameW's return value is also unchecked — on failure className is uninitialized and read by wcsstr; zero the buffer or bail if it returns 0.

  • RemovePropW the WindhawkAnimDone prop. Window properties should be removed before the window is destroyed; right now they're never removed at all (see also item 6, where this has a functional consequence).

  • The exe-path sandbox detector could largely be metadata. applicationframehost.exe, shellexperiencehost.exe, startmenuexperiencehost.exe and screenclippinghost.exe are all better expressed as @exclude lines than as a runtime GetModuleFileNameW check. The \\windowsapps\\ substring is also broader than intended — it excludes every Store-distributed Win32 app (e.g. many normal desktop apps installed from the Store), not just UWP.

  • AnimWndProc calls the hooked DefWindowProcW. It works today only because IsRealTopLevelWindow rejects WS_EX_TOOLWINDOW, but it's fragile — call DefWindowProcW_Original (with a null check) instead.

  • animDuration has no upper clamp. A user entering 60000 gets a one-minute freeze on every close (and a one-minute stall in the ExitProcess hook). Clamp to something like 5000 ms.

  • std::thread construction can throw std::system_error under thread exhaustion; that exception would propagate out of the hook into arbitrary app code. Using CreateThread (which you need anyway for the join in item 2) sidesteps this.

  • Missing include: ::towlower comes from <cwctype>; you're relying on a transitive include from <string>/<algorithm>.

Functionality notes

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

  • Every animation plays the exact same "random" sequence. rand() is never seeded and its state is per-thread in the UCRT, so each freshly created animation thread starts from the default seed 1 — the shard velocities, ember spawn positions and glitch strip heights are byte-identical on every single close. Seed the thread's generator (or better, use a std::mt19937 seeded from QueryPerformanceCounter).

  • Non-premultiplied alpha with AC_SRC_ALPHA. UpdateLayeredWindow with ULW_ALPHA + AC_SRC_ALPHA expects premultiplied BGRA, but GDI+ writes straight (non-premultiplied) ARGB into a 32bpp DIB section. Fully-opaque pixels are unaffected (which is why the window content looks right), but everything semi-transparent — the flame polygons, embers, the char gradient — will render too bright and with halos. Either premultiply the DIB before the UpdateLayeredWindow call, or draw semi-transparent content into a premultiplied Bitmap (PixelFormat32bppPARGB).

  • PrintWindow returns a black or empty capture for many GPU-composited windows — Chromium/Electron apps, some UWP-hosted content, hardware-overlay video. There's no validation of the capture, so those apps get an 800 ms animation of a black rectangle, which looks worse than no animation. Consider checking the captured bitmap (or using the DWM thumbnail API as a fallback, which the existing mod does for exactly this case).

  • The shatter overlay padding is asymmetric in the wrong direction. padY = 150; offsetX = 50;offsetY is left at 0, so the 150 px of extra room is entirely below the window while the shard velocities are biased upward (s.vy = ... - 2.0f). Shards clip against the top edge of the overlay almost immediately. offsetY = 75 (or seeding vy downward) would fix it.

  • Shatter loses a strip of the window. cw = w / cols with integer division means up to 9 px on the right and bottom are never covered by any shard, so a thin sliver of the window vanishes instantly at animation start. Give the last row/column the remainder.

  • The glitch effect re-randomizes strip heights and offsets every frame, independent of the frame rate. At the current uncapped frame rate that reads as high-frequency strobing rather than a coherent digital tear; consider computing the strip layout once (or on a fixed ~60 ms cadence) and only animating the horizontal shift.

  • emberCount only affects one of seven effects, and pixelate's block size is hardcoded to pSize = 40 regardless of window size — a 3840-wide window gets 96 columns of blocks while a 400-wide one gets 10, so the effect's visual density varies a lot. Consider deriving the block count from the window size, and exposing it as a setting (and marking emberCount as fire-only more prominently, or nesting both under a per-effect group).

  • Owned windows and small windows never animate. GetWindow(hWnd, GW_OWNER) != NULL rejects most dialogs and tool windows, and the w < 150 || h < 150 cutoff rejects small utility windows. That's a reasonable safety default, but it means the majority of dialog closes are unanimated — worth mentioning in the README so users don't report it as a bug.

  • DPI awareness of the animation thread. The worker thread inherits the process default DPI context, which can differ from the context active on the UI thread that called GetWindowRect (an app can set a per-thread context). mods/classic-min-max-animations.wh.cpp#L962 calls SetThreadDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2) on its animation thread for this reason.


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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant