Skip to content

Add Windows 11 Native Four Column Snap Layout mod - #5436

Open
Hoffelhas wants to merge 11 commits into
ramensoftware:mainfrom
Hoffelhas:main
Open

Add Windows 11 Native Four Column Snap Layout mod#5436
Hoffelhas wants to merge 11 commits into
ramensoftware:mainfrom
Hoffelhas:main

Conversation

@Hoffelhas

@Hoffelhas Hoffelhas commented Sep 10, 2026

Copy link
Copy Markdown

Windows 11 Native Four Column Snap Layout

Add an extra native Windows 11 Snap Layout with four equal vertical columns. This is especially useful on ultrawide monitors, where four equal vertical zones make better use of the available horizontal screen space.

Windows 11 four-column Snap Layout

What it does

  • Adds the four-column layout as an additional option without replacing the
    existing Windows layouts.
  • Appears in Win+Z, maximize-button hover, and the Snap Bar shown
    when dragging a window to the top of the screen.
  • Integrates with Windows' native Snap Layout model, so Snap Assist and Snap
    Groups continue to work normally.

Developed and tested on Windows 11 build 26200.9445 on x86-64.

Mod authorship

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

Adds an extra native Windows 11 Snap Layout with four equal vertical columns
@windhawk-reviewer windhawk-reviewer Bot added the waiting-for-author The author's turn: request an AI review, or respond to one that was posted. label Sep 10, 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.

@Hoffelhas

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 Sep 10, 2026
Rename SnapLayout hook array to satisfy PR validation
@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.


Nice, self-contained approach — cloning a native layout through the DLL's own vector helper (instead of memcpy-ing a struct with a std::wstring and a std::vector inside) is the right call, and the structural pre-checks before patching are good defensive practice. The hook prototypes also look right for the MSVC x64 ABI (this, then the hidden return buffer, then the args), which matches how other mods declare struct-returning member hooks, e.g. taskbar-vertical.wh.cpp#L2877. A few things to fix:

1. Hard-coded C:\WINDOWS\... path — and force-loading the DLL at all.

LoadLibraryExW(
    L"C:\\WINDOWS\\SystemApps\\MicrosoftWindows.Client.Core_cw5n1h2txyewy\\SnapLayout.dll",
    ...

The Windows directory is not always C:\WINDOWS — on machines where Windows is installed to another drive/folder this always fails, and the mod dies with "Failed to load SnapLayout.dll".

More importantly, force-loading a XAML component DLL into every explorer.exe — even for users who never open a snap flyout — is heavier than needed, and it forces the FreeLibrary dance in Wh_ModUninit (unloading a WinRT/XAML component DLL mid-session is something best avoided). The convention for a DLL that may load late is to hook LoadLibraryExW in kernelbase.dll and apply that DLL's symbol hooks when it loads naturally, plus a Wh_ModAfterInit re-check for the already-loaded case. adaptive-microphone-icon-visibility.wh.cpp#L939-L985 is a clean example of exactly this shape (note it resolves LoadLibraryExW from kernelbase.dll — hooking the kernel32 import misses loads that go straight to kernelbase):

HMODULE kernelBase = GetModuleHandleW(L"kernelbase.dll");
auto pLoadLibraryExW =
    (decltype(&LoadLibraryExW))GetProcAddress(kernelBase, "LoadLibraryExW");
WindhawkUtils::SetFunctionHook(pLoadLibraryExW, LoadLibraryExW_Hook,
                               &LoadLibraryExW_Original);

That also lets you drop g_loadedSnapLayoutModule and the FreeLibrary in Wh_ModUninit entirely. If you'd rather keep force-loading, at least build the path from GetWindowsDirectory — see taskbar-disappearing-icon-fix-win11.wh.cpp#L91-L113.

2. Hard-coded layout indices + count == 6 make the mod silently do nothing on many setups.

if (count != kNativeLayoutCount || sourceIndex >= count) { return false; }
...
CloneAndAppendFourColumn(returnBuffer, 3)   // snap bar
CloneAndAppendFourColumn(returnBuffer, 4)   // flyout

Windows does not always offer the same six layouts in the same order — the set varies with the display's effective width and orientation (portrait/narrow displays get a reduced/different set), and it changes between builds. On any of those configurations count != 6 or index 3/4 isn't the 2×2 four-zone layout, so the mod quietly does nothing at all, with no log line explaining why.

You already have the predicate you need. Search for the source layout instead of hard-coding its position:

bool CloneAndAppendFourColumn(RawVector* vec) {
    if (!vec || !EmplaceLayout_Original) {
        return false;
    }

    const SIZE_T count = GetVectorCount(vec, kSnapLayoutSize);
    for (SIZE_T i = 0; i < count; i++) {
        BYTE* source =
            reinterpret_cast<BYTE*>(vec->first) + i * kSnapLayoutSize;
        if (!IsExpectedSourceLayout(source)) {
            continue;
        }
        void* newLayout = EmplaceLayout_Original(vec, source);
        return newLayout && PatchToFourColumns(newLayout);
    }

    Wh_Log(L"No 2x2 four-zone source layout among %zu layouts", count);
    return false;
}

This makes the mod work across the full range of layout sets and builds, and it removes the snap-bar-vs-flyout index branching in Layouts_Hook (g_inSnapBarLoad is then only needed for the once-per-load dedup).

3. EmplaceLayout_Hook installs a detour that isn't needed.

// This pass-through hook exists so Windhawk resolves the private vector helper
// and exposes a callable original/trampoline pointer.
void* __cdecl EmplaceLayout_Hook(void* vectorThis, const void* sourceLayout) {
    return EmplaceLayout_Original(vectorThis, sourceLayout);
}

SYMBOL_HOOK already supports resolve-without-hooking — pass nullptr as the hook function and only originalFunction is filled in. From the doc comment on the constructor in windhawk_utils.h: "hookFunction — The hook function to set, or nullptr to only retrieve the symbol's address". As written, every internal push_back on a std::vector<SnapLayout> inside SnapLayout.dll is detoured through your no-op, for no benefit. Fix:

{
    {LR"(private: struct SnapLayout & __cdecl std::vector<struct SnapLayout,class std::allocator<struct SnapLayout> >::_Emplace_one_at_back<struct SnapLayout const &>(struct SnapLayout const &))"},
    &EmplaceLayout_Original,
    nullptr,  // Resolve only, no hook.
},

and delete EmplaceLayout_Hook. custom-corner-radius.wh.cpp#L472-L496 uses this form for several capture-only symbols.

4. The picker-height adjustment is latched and pinned to one magic number.

if (hr == 0 && value && g_flyoutCustomAdded && *value == kNativePickerHeight) {
    *value += kExtraPickerHeight;
}

Two problems:

  • g_flyoutCustomAdded is set to true and never cleared. If a later Layouts() call fails to append (different layout set, a build change, mod about to be disabled), the picker still gets +80 and the flyout shows an 80px empty strip. Set it from the result of each call instead of only on success:
    g_flyoutCustomAdded = CloneAndAppendFourColumn(returnBuffer);
  • The *value == 244 equality means the fix-up applies on exactly one configuration. Anywhere the native height differs (a different layout count per item 2, a build that retunes the flyout), the extra row is simply clipped and the new layout is invisible — the worst failure mode, since the rest of the mod appears to work. Deriving the delta is more robust, e.g. scale by row count (rows = ceil(n/2), *value = *value * (rows + 1) / rows) computed from the layout count you actually appended to, or at minimum keep a sanity range rather than an exact match.

5. README has no screenshot.

This is a purely visual mod — please add a screenshot or GIF of the Win+Z flyout with the four-column layout to the README. Only i.imgur.com and raw.githubusercontent.com are allowed as image hosts.

Optional improvements

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

  • @license is missing. The file comment says "Source code is published under the MIT License", but there's no // @license MIT metadata line, so the license won't show on the mod page. Add it to match.
  • All four symbol hooks are mandatory. _Emplace_one_at_back<struct SnapLayout const &> in particular is an MSVC STL implementation detail whose mangled name has changed across STL versions — if it ever changes, the mod fails to load entirely rather than degrading. The first SYMBOL_HOOK field is a list of alternatives precisely for this; consider adding the older spellings you can find (e.g. the _Emplace_reallocate / emplace_back forms) as fallbacks.
  • Silent bail-outs. Every structural check returns false with no Wh_Log. For a mod built on undocumented internals, a log line on each bail-out ("count was %zu, expected 6", "layout at index %zu is %ux%u with %zu zones") turns an unreproducible bug report into a one-line diagnosis. Wh_Log compiles down to a cheap check and is off by default, so there's no cost.
  • Partial-failure leftovers. In CloneAndAppendFourColumn, if EmplaceLayout_Original succeeds but the post-emplace count check or PatchToFourColumns fails, the appended (unpatched) clone stays in the vector — the user sees a duplicate 2×2 quadrant layout. It's practically unreachable given the pre-checks, but the post-emplace count != kCustomLayoutCount check in particular can only ever fire in that lose-lose way, so it may be worth dropping it or popping the clone back off.
  • Formatting. The repo has a .clang-format (Chromium, 4-space indent); running it would compress the one-argument-per-line style considerably and match the rest of mods/.

Functionality notes

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

  • The clone inherits the source layout's identity fields. PatchToFourColumns rewrites the grid (+0x20/+0x24) and the four zones, but leaves +0x00+0x1F (which looks like a std::wstring, going by the MSVC x64 layout) and +0x40+0x4F untouched. Worth checking what those hold: if +0x00 is a display/automation name, the new item will be announced by screen readers as the quadrant layout, and if the tail bytes are a layout id used for persistence or Snap Groups matching, two layouts now share it. Not necessarily a problem, but worth a look since you already have the offsets mapped.
  • Configurable columns. A setting for the column count (3/4) or custom ratios would make this considerably more broadly useful and would head off near-duplicate submissions for other column splits later on. Purely a suggestion.
  • The Snap Bar dedup is empirical. g_inSnapBarLoad / g_snapBarCustomAdded guard against SnapModel::Layouts() being called more than once per SnapBarViewModel::LoadLayouts. That's fine as an observation, but since Layouts() returns a fresh vector each time, it also means the fallback if the internal call pattern changes is a duplicate entry in the bar rather than a crash — good. Also note g_inSnapBarLoad is thread-local, so if any part of the snap bar load ever moves to another thread, that path silently takes the flyout branch.


Next steps:

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

See the review process for details.

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

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 Sep 10, 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.


Overall this is careful work for a mod poking at undocumented structures: the source layout is validated before use, the clone is created with SnapLayout.dll's own _Emplace_one_at_back (rather than a byte copy, which would double-own the inner std::wstring/std::vector and would also cross the allocator boundary), the native layouts themselves are never modified, and late loading of SnapLayout.dll is handled through kernelbase!LoadLibraryExW — the recommended form. I also didn't find an existing mod that overlaps (two-sided-snapping touches snapping but through a completely different mechanism), so a standalone mod is the right call here. One thing to address:

Picker height: the fixed +80 and the [160, 400] sanity window are tied to one build at one display scale. get_PickerHeight returns an int, and the neighbouring SnapBarViewModel::LoadLayouts(double scale, ...) you hook takes an explicit scale factor — which suggests these sizes may be scale-dependent rather than DIPs. If they are, then on a 125%/150% display the real extra row is 100/120 px, so +80 leaves the added layout clipped, and a 6-layout picker (shown on larger displays) would land above kMaxReasonablePickerHeight and get no adjustment at all — the layout is added but silently cut off. Even at 100%, the +80 assumes the extra item always starts a new row, which only holds while Windows ships an even number of layouts.

Please verify the picker at 125% and 150% scaling and with the 6-layout picker. A version that sidesteps both the unit question and the layout count is to scale proportionally instead of adding a constant — record the layout count before/after in Layouts_Hook (thread-local, same as g_flyoutCustomAdded), derive the row counts, and scale:

// In Layouts_Hook, before/after the CloneAndAppendFourColumn call:
g_flyoutLayoutsBefore = count;   // layouts returned by Windows
g_flyoutLayoutsAfter  = count + 1;

// In PickerHeight_Hook:
const int rowsBefore = (g_flyoutLayoutsBefore + kItemsPerRow - 1) / kItemsPerRow;
const int rowsAfter  = (g_flyoutLayoutsAfter  + kItemsPerRow - 1) / kItemsPerRow;
if (hr == 0 && value && rowsBefore > 0 && rowsAfter > rowsBefore) {
    *value = MulDiv(*value, rowsAfter, rowsBefore);
}

(That slightly over-adds the outer padding, but it's scale-independent and degrades gracefully.) Related: the adjustment also assumes Layouts() and get_PickerHeight run on the same thread and in that order, since g_flyoutCustomAdded is thread_local — worth a comment noting the assumption, because if it ever stops holding the picker just silently isn't resized.

Optional improvements

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

  • Use result instead of returnBuffer in Layouts_Hook. The callee returns the hidden return-buffer pointer in the return register, so result is the same pointer without depending on the assumption about which argument slot the buffer arrives in. if (!result) return result; and then operate on result.

  • Restore the snap-bar flags with an RAII guard. SnapBarLoadLayouts_Hook restores g_inSnapBarLoad/g_snapBarCustomAdded after the original returns, so a C++/WinRT exception propagating out of LoadLayouts would leave g_inSnapBarLoad stuck true on that thread and quietly disable the mod for the rest of the session. A small scope guard fixes it — see the ScopedFlag helper in taskbar-multirow.wh.cpp#L84.

  • CloneAndAppendFourColumn leaves the clone in the vector if PatchToFourColumns fails. You note it should be unreachable, and it very likely is, but if it ever isn't the user gets two identical quadrant entries in the picker plus (since the function returns false) no height adjustment. Worth at least saying so in the comment, since there's no easy way to pop the element back off.

  • Some defensive code is unreachable. ReadU32 can only fail on a null argument and every caller passes a derived non-null pointer; GetVectorCount's elementSize == 0 check is always false (both call sites pass a non-zero constant); and GetZoneVector's lastOut output is never read by either caller. Dropping them would make the real validation (IsNativeQuadrantLayout) stand out more.

  • Wh_ModUninit is empty and can be removed — all lifecycle callbacks are optional.

  • @architecture x86-64 note: that value is the right choice for a shell mod, but be aware it isn't x64-only — on ARM64 devices explorer.exe is a predefined shell process, so the mod loads there natively too. That's fine (if the offsets don't match, your validation makes it a no-op rather than a crash), but the README's "Currently tested only on x86-64 Windows" may read to users as "won't load on ARM64", which isn't the case.

Functionality notes

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

  • The clone inherits the source layout's identity. Everything outside the grid fields (name/accessibility text, and any layout id/kind stored past +0x40) is copied verbatim from the 2x2 quadrant layout. Worth checking what Narrator announces for the new entry, and whether Snap Groups distinguishes the two layouts when restoring a group — if the id is what identifies a layout, the four-column group could be restored as quadrants.

  • The layout is added to every layout set, regardless of display. On a narrow or portrait display, four 25% columns are barely usable, and Windows deliberately trims which layouts it offers by display size. The 2x2-source requirement only partly covers that. Consider gating on the monitor's working width — or better, offer a four-row variant for portrait monitors: the clone has exactly four zones either way, so it's the same code with the column/row writes swapped (WriteU32(layout, 0x20, 1); WriteU32(layout, 0x24, 4); and WriteU32(zone, 0x24, i)). A setting choosing columns / rows / both would also make it less likely that someone submits a near-duplicate "four row snap layout" mod later. Note that a general "N columns" setting isn't as cheap, since the clone is fixed at four zones and N != 4 would require adding or removing zones.

  • The snap-bar de-duplication assumes Layouts() is called synchronously on the load thread. g_inSnapBarLoad is thread_local, so if a future build moves part of LoadLayouts onto a worker thread, those calls take the flyout path and the Snap Bar would get one extra entry per call. Nothing to do now — just something to keep in mind if the Snap Bar ever shows duplicates.

  • Residual risk from the hardcoded offsets. The validation is nicely fail-closed, but GetZoneVector only checks that the three zone-vector pointers are non-null and ordered before IsNativeQuadrantLayout dereferences them, so on a build where sizeof(SnapLayout) changed but the total byte count still divides by 0x50, misaligned garbage that happens to look like a 4-element vector would be dereferenced and could fault in Explorer. The odds are low (it also needs two dwords equal to 2 at the right spots) and there's no clean alternative for an undocumented struct — but if you want to bound it further, a Windows build-range check around the whole feature would do it.

  • First Win+Z may pause while symbols resolve. SnapLayout.dll most likely loads lazily on the first snap interaction, so the HookSymbols call runs inside your LoadLibraryExW hook and blocks that thread while Windhawk resolves/downloads symbols. It's cached afterwards and it's the standard pattern, so this is just an FYI.


Next steps:

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

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Sep 10, 2026
AI review improvements - Make picker height scaling-aware and refine review fixes
@Hoffelhas

Copy link
Copy Markdown
Author

/ready-for-reviewer

@windhawk-reviewer

Copy link
Copy Markdown

@Hoffelhas /ready-for-reviewer can't be applied here: the most recent AI review covers a46bff7, but the current head of this pull request is faa3b6e. Comment /ai-review to get a review of the current code.

@Hoffelhas

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 Sep 10, 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.


Nice, self-contained mod — no existing mod in the catalog touches SnapLayout.dll, the hook set is collected into a single HookSymbols call per module, the array name matches the module it's resolved against, the late-load path correctly hooks kernelbase!LoadLibraryExW (and re-checks in Wh_ModAfterInit), and reusing the DLL's own _Emplace_one_at_back for the deep copy is the right call. Two things to fix:

1. Symbol resolution is retried on every LoadLibraryExW call after a failure

HookSnapLayoutDll releases the claim when resolution fails:

if (!WindhawkUtils::HookSymbols(module, snapLayoutDllHooks,
                                ARRAYSIZE(snapLayoutDllHooks))) {
    Wh_Log(L"Failed to resolve one or more SnapLayout.dll symbols");

    g_snapLayoutHookClaimed = false;   // <-- allows unlimited retries
    return false;
}

and LoadLibraryExW_Hook calls HandleSnapLayoutDllIfLoaded(true) on every successful load while the claim is free. So on any build where these private symbols don't resolve — a future Windows update renaming SnapModel::Layouts, the STL internal _Emplace_one_at_back instantiation disappearing, symbols unavailable — every subsequent LoadLibraryExW in explorer.exe re-runs a full symbol resolution of SnapLayout.dll. Explorer loads DLLs continuously (shell extensions, thumbnail/property handlers, COM servers), and HookSymbols must not be called repeatedly for the same module: each extra call invalidates the cached result and forces a slow re-resolution, potentially including a symbol download. In practice this turns a "mod doesn't work on this build" into "Explorer is unusably slow". A retry can also re-register hooks for the symbols that did resolve on the previous attempt.

Fix: treat a failed resolution as permanent — simply don't release the claim (or track it in a separate g_snapLayoutHookFailed flag).

    if (!WindhawkUtils::HookSymbols(module, snapLayoutDllHooks,
                                    ARRAYSIZE(snapLayoutDllHooks))) {
        Wh_Log(L"Failed to resolve one or more SnapLayout.dll symbols");
        return false;  // keep the claim: never retry for this module
    }

For reference, taskbar-icon-size.wh.cpp#L3097 claims the flag with exchange(true) and never resets it, and settings-to-control-panel.wh.cpp#L2026 keeps an explicit "already tried and failed" flag with a comment spelling out this exact reason.

2. Consider making the layout configurable instead of hardcoding "four columns"

Everything that makes this mod four-column-specific is two constants in PatchToFourColumns (WriteU32(layout, 0x20, 4) / the i < 4 loop). Exposing that as a setting — e.g. columns: 4 with $name: Number of columns — costs a ==WindhawkModSettings== block, one Wh_GetIntSetting in Wh_ModSettingsChanged, and a clamp, and it turns a single-purpose mod into a general "add an N-column Snap Layout" mod. Without it, the natural follow-up is separate near-duplicate submissions for three and five columns, which the maintainer consistently pushes back on. A setting also gives users a way to turn the extra layout off per-monitor-setup concerns (see the functionality notes below) without uninstalling.

Note that the zone count is bounded by kMaxReasonableZoneCount on read, but the clone only has 4 SnapZone objects to reuse, so an N != 4 setting would need N zones — for N < 4 you'd need to shrink the zone vector, and for N > 4 you'd need to grow it. If that turns out to require another private STL symbol, a 2–4 column range (reusing 4 zones is only possible for N = 4) may not be practical, in which case just say so and keep it as is.

Optional improvements

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

  • Dead forward declaration. bool HookSnapLayoutDll(HMODULE module, bool applyHookOperations); at the top of the file is never needed — the definition precedes both call sites (HandleSnapLayoutDllIfLoaded and Wh_ModInit). It's also filed under the kernelbase!LoadLibraryExW section header, which is a bit confusing.

  • Cheap sanity checks on the assumed struct size. kSnapLayoutSize (0x50) is the one constant that, if wrong on some build, makes the mod hand a bogus SnapLayout const& to the native deep copy and then write into memory it doesn't own. Two nearly free guards:

    • In GetVectorCount, also require the capacity to be a whole number of elements ((end - first) % elementSize == 0), not just the size.
    • After the emplace, verify the element landed where the assumed size says it should before patching:
      void* newLayout = EmplaceLayout_Original(vec, source);
      if (!newLayout ||
          newLayout != reinterpret_cast<BYTE*>(vec->last) - kSnapLayoutSize) {
          Wh_Log(L"Unexpected layout element size; aborting");
          return false;
      }
  • hr == 0SUCCEEDED(hr) in PickerHeight_Hook, for the usual HRESULT idiom.

  • Most mods start their lifecycle callbacks with Wh_Log(L">") so the log shows the init/uninit sequence; Wh_ModInit / Wh_ModAfterInit here only log on the failure paths.

Functionality notes

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

  • Only the first Layouts() result per Snap Bar load gets the extra layout. During SnapBarViewModel::LoadLayouts, g_snapBarCustomAdded suppresses the append on the second and later calls, so within one load operation different consumers see different layout sets. That's fine if the extra calls feed something unrelated (or if the results are accumulated into one collection, which is presumably why you added the guard), but it's a real hazard if the Snap Bar correlates two Layouts() results by index — e.g. builds the visuals from one and the drop targets/zone rects from the other. Then clicking the custom entry would apply a different layout's zones. Worth confirming what each call is used for; if they're independent, adding to all of them is the more robust behavior.

  • Picker-height scaling relies on thread-local breadcrumbs that are never reset. g_flyoutCustomAdded / g_flyoutLayoutsBefore / g_flyoutLayoutsAfter are written by every non-Snap-Bar Layouts() call on the thread, not just the picker's, and are never cleared. So get_PickerHeight scales by whatever the most recent Layouts() call on that thread left behind — including a call from an unrelated consumer, or a stale one from a previous picker session if XAML re-measures without re-querying layouts. It'll usually be the same ratio and therefore harmless, but it is a guess rather than a fact about the picker's own item count. I don't see a clean way to read the view model's actual item count, so this may just be the cost of the approach — flagging it so it's a conscious trade-off.

  • MulDiv(*value, rowsAfter, rowsBefore) scales the picker's chrome too. If the native height is padding + rows * rowHeight, multiplying the whole thing by 3/2 also multiplies the padding by 3/2, so the flyout ends up somewhat taller than needed rather than exactly one row taller. Adding *value / rowsBefore has the same problem. If you can determine a per-row height (or the padding) the result would be tighter; otherwise a slightly generous height is the safer failure mode.

  • The clone inherits everything about the source layout except the grid. PatchToFourColumns rewrites +0x20/+0x24 and the four zones' origin/span fields; every other field — including whatever lives at +0x00..+0x1F (looks like a std::wstring, so probably the layout's name/accessibility label) and the 16 bytes at +0x40..+0x4F — is a verbatim copy of the native quadrant layout. If any of those is a layout identity used by Snap Assist or for Snap Group persistence, the new layout is indistinguishable from the quadrant one; if +0x00 is the display/accessibility name, Narrator and tooltips will describe the four-column layout as the quadrant layout. Worth checking what those fields are.

  • The layout is added unconditionally, regardless of monitor geometry. Four columns on a 1080p or portrait display gives ~270px zones, and the extra entry (plus the taller picker) shows up there too. Windows varies its own layout set by monitor, but SnapModel::Layouts() doesn't obviously give you the target monitor, so there may be no clean way to gate this — an on/off setting (see item 2 above) would at least let users opt out.

  • Fragility of the approach, for the record. Hardcoded struct offsets plus a private STL implementation symbol (_Emplace_one_at_back is an MSVC STL internal that changes name across STL versions and can be inlined away) means this can stop resolving on any Windows/toolchain update. The README documents this, IsNativeQuadrantLayout is a genuinely good structural check before anything is written, and there's no better alternative for a deep copy of a type with a nested std::wstring and std::vector — just noting that the failure mode should stay "mod quietly does nothing" (which is what fixing item 1 ensures), never "Explorer misbehaves".


Next steps:

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

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Sep 10, 2026
AI review - Prevent repeated SnapLayout symbol resolution after failure
@Hoffelhas

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 Sep 10, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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

Comment /ai-review again after 2026-09-11 09:11 UTC (in 20 hours) to get another one.

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

Copy link
Copy Markdown
Author

Hi @m417z, I want to go for ready-for-reviewer, but I need to do ai-review first. Would it please be possible to do that now instead of waiting?

@m417z m417z added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Sep 10, 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 retry fix from the previous round is correct: the claim is now permanent, so a build where the private symbols don't resolve can no longer turn into repeated HookSymbols calls against SnapLayout.dll. The rest of the mod continues to read well — one HookSymbols call per module with an array name that matches the module it's resolved against, the late-load path via kernelbase!LoadLibraryExW plus the Wh_ModAfterInit re-check (which also covers "SnapLayout.dll arrived as a dependency", since it re-checks the module list rather than the filename argument), the deep copy delegated to the DLL's own _Emplace_one_at_back, structural validation before anything is written, no persistent system changes, and nothing to tear down in Wh_ModUninit. Only one item from the previous round is still open:

The "four columns" vs. "N columns" decision should be made before merge, because @id is permanent.

Everything that makes this mod four-column-specific is two constants in PatchToFourColumnsWriteU32(layout, 0x20, 4) and the for (unsigned int i = 0; i < 4; i++) loop — plus the zoneCount != 4 requirement in the source search. The previous review asked about exposing that as a setting; the code is unchanged and there's no note in the pull request explaining why, so it's worth settling now rather than after merge:

  • @id and the mod name are the mod's permanent identity. windows-11-native-four-column-snap-layout can't be renamed later, so if the column count becomes configurable after merge you're stuck with either a misleading id or a second mod.
  • Without a setting, the natural follow-up is separate three-column and five-column submissions from other users, which the maintainer consistently pushes back on (he prefers extending an existing mod over near-duplicates).
  • A column-count setting also gives users a knob for the monitor-geometry issue in the functionality notes below (four columns on a 1080p or portrait display) without uninstalling.

A generalization that avoids the zone-allocation problem the previous review raised: instead of looking specifically for a 2×2 quadrant layout, search for a native layout that already has exactly N zones and rewrite it to N columns × 1 row. Depending on the build and monitor, Windows ships layouts with 2 and 3 zones alongside the 4-zone quadrant one, so N = 2..4 may need no new zone allocation at all — IsNativeQuadrantLayout becomes "is this a layout with exactly N single-cell zones", and PatchToFourColumns becomes WriteU32(layout, 0x20, N) plus an i < N loop. Only N > 4 would additionally need std::vector<struct SnapZone,...>::_Emplace_one_at_back<struct SnapZone const &> to grow the zone vector, which may or may not be present in the DLL.

If it turns out that no native source layout with the right zone count exists on the builds you can test, or that the zone-vector growth symbol isn't available, that's a fine answer — please just say so in a pull request comment so the human reviewer sees the reasoning instead of an unaddressed finding.

Optional improvements

Minor polish — none of this affects users, so it's your call. The first four were in the previous review and are repeated here so everything is in one place.

  • Dead forward declaration. bool HookSnapLayoutDll(HMODULE module, bool applyHookOperations); (line 118) is never needed — the definition at line 507 precedes both call sites (lines 586 and 621). It's also filed under the kernelbase!LoadLibraryExW section header, which is a bit misleading.

  • hr == 0SUCCEEDED(hr) in PickerHeight_Hook, for the usual HRESULT idiom.

  • Two cheap guards on the assumed struct sizes. kSnapLayoutSize (0x50) is the assumption with the widest blast radius: the source scan in CloneAndAppendFourColumn walks candidates at that stride and IsNativeQuadrantLayoutGetZoneVector dereferences three pointers read at +0x28/+0x30/+0x38 of each candidate. Today the only thing standing between a changed element size and a wild read is (last - first) % kSnapLayoutSize == 0 in GetVectorCount; a future size that happens to be a multiple or divisor of 0x50 would pass it, land the scan mid-struct, and the pointer sanity checks (non-null, ordered, size divisible by 0x38, count ≤ 16) can be satisfied by coincidence. The desired failure mode on any such change is "mod quietly does nothing", so two nearly free extra constraints are worthwhile:

    • In GetVectorCount, also require the capacity to be a whole number of elements ((end - first) % elementSize == 0), not just the size.
    • After the emplace, verify the new element landed where the assumed size says it should, before patching:
      void* newLayout = EmplaceLayout_Original(vec, source);
      if (!newLayout ||
          newLayout != reinterpret_cast<BYTE*>(vec->last) - kSnapLayoutSize) {
          Wh_Log(L"Unexpected layout element size; aborting");
          return false;
      }
  • Most mods start their lifecycle callbacks and hooks with Wh_Log(L">") so the log shows the sequence; here Wh_ModInit / Wh_ModAfterInit / the hooks only log on failure paths, which makes "did the hook fire at all?" harder to answer from a user's log.

  • GetZoneVector duplicates GetVectorCount. It reads the three pointers at +0x28/+0x30/+0x38 and then redoes the ordering/divisibility math. It could memcpy them into a RawVector and call GetVectorCount(&v, kSnapZoneSize), keeping the vector-shape validation in one place.

Functionality notes

Non-critical observations about the feature behavior itself. These are unchanged from the previous review — repeated in condensed form so the human reviewer has the full picture; several are things only you can confirm on a real machine.

  • Only the first Layouts() result per Snap Bar load gets the extra layout. g_snapBarCustomAdded suppresses the append on the second and later calls within one SnapBarViewModel::LoadLayouts, so different consumers inside one load see different layout sets. Fine if the later calls feed something unrelated, but a hazard if the Snap Bar correlates two Layouts() results by index (visuals from one, drop-target rects from the other) — then clicking the custom entry would apply a different layout's zones. Worth confirming that dropping a window onto the four-column entry in the Snap Bar really applies four columns.

  • Picker-height scaling relies on thread-local breadcrumbs that are never reset. g_flyoutCustomAdded / g_flyoutLayoutsBefore / g_flyoutLayoutsAfter (lines 84-86) are written by every non-Snap-Bar Layouts() call on the thread and never cleared, so get_PickerHeight scales by whatever the most recent such call left behind. Usually the same ratio and harmless; but a consumer with a different layout set (Windows varies its set by monitor) can leave a smaller ratio behind, which under-scales and clips the added row. Conversely, if get_PickerHeight ever runs on a thread that didn't make the Layouts() call, the breadcrumbs are zero and no adjustment happens at all. A precise version would mirror what you already do for the Snap Bar: if SnapLayoutPickerViewModel has its own load/initialize function you can hook, scope the count recording to that call the way ScopedSnapBarState scopes the Snap Bar, instead of treating "not in a Snap Bar load" as "must be the picker".

  • MulDiv(*value, rowsAfter, rowsBefore) scales the picker's chrome too. If the native height is padding + rows * rowHeight, multiplying the whole value by 3/2 also multiplies the padding, so the flyout ends up somewhat taller than needed rather than exactly one row taller. Over-shooting is the safer failure mode, so this is fine as-is unless you can determine a per-row height.

  • The clone inherits everything about the source layout except the grid. PatchToFourColumns rewrites +0x20/+0x24 and the four zones' origin/span fields; every other field is a verbatim copy of the quadrant layout — including whatever lives at +0x00..+0x1F (32 bytes, so most likely a std::wstring: the layout's display or accessibility name) and the 16 bytes at +0x40..+0x4F. Two concrete checks: (1) what does the tooltip / Narrator announce for the new entry — "four columns" or the quadrant layout's name? (2) snap four windows into the columns, minimize the snap group from the taskbar and restore it — if +0x40 holds a layout identity used for snap-group persistence, the restore may come back as quadrants.

  • The layout is added unconditionally, regardless of monitor geometry. Four columns on a 1080p or portrait display gives ~270px zones, and the extra entry plus the taller picker shows up there too. SnapModel::Layouts() doesn't obviously expose the target monitor, so there may be no clean way to gate it — a setting (see the item above) would at least let users opt out.

  • Fragility, for the record. Hardcoded struct offsets plus a private MSVC STL implementation symbol (_Emplace_one_at_back can be renamed across STL versions or inlined away) means this can stop resolving on any Windows or toolchain update. The README documents it, IsNativeQuadrantLayout is a genuinely good structural check before anything is written, and there's no better option for deep-copying a type with a nested std::wstring and std::vector — just noting that the failure mode needs to stay "mod quietly does nothing", which the permanent-claim fix now ensures.


Next steps:

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

See the review process for details.

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

Hoffelhas commented Sep 10, 2026

Copy link
Copy Markdown
Author

/ready-for-reviewer

Regarding the suggestion to generalize the mod to N columns: I’d prefer to keep this submission intentionally focused on the missing four-column layout.

Windows already provides native two- and three-column layouts, while supporting more than four columns or adding other formats would require additional reverse-engineering. Generalizing the implementation now would expand the scope and complexity of the mod without a demonstrated user need.

If there is meaningful demand for arbitrary/custom column counts in the future, I think that would be better handled as a separate generalized mod.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-reviewer Ready for a human reviewer, and in the queue for one. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-reviewer Ready for a human reviewer, and in the queue for one.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants