Skip to content

Update Taskbar Virtual Desktop Switcher to v1.8#4844

Open
sb4ssman wants to merge 3 commits into
ramensoftware:mainfrom
sb4ssman:update-taskbar-vd-switcher-v1.8
Open

Update Taskbar Virtual Desktop Switcher to v1.8#4844
sb4ssman wants to merge 3 commits into
ramensoftware:mainfrom
sb4ssman:update-taskbar-vd-switcher-v1.8

Conversation

@sb4ssman

Copy link
Copy Markdown
Contributor

Updates taskbar-vd-switcher from v1.7 to v1.8 and resolves #4830.

The existing indicator mode is now fully customizable: users can choose independent active and inactive symbols or text (for example, 🟢 and 🔴) while preserving and as the defaults. Desktop indicators and the optional Task View button also have independent font-family settings.

Desktop entries are now native XAML ToggleButton controls. The active desktop owns the real Checked, CheckedPointerOver, and CheckedPressed states, allowing Windows 11 Taskbar Styler rules to target the active state directly instead of inferring it from a color.

Default tray placement

Four desktops with the Task View button

Tested

  • Live-tested in Explorer: custom active/inactive indicators, independent fonts, checked-state styling, desktop switching, settings reapply, and teardown behave as intended
  • Windhawk Clang syntax compilation passes
  • Exit-time-destructor audit passes
  • Settings YAML parsing and embedded/folder README parity pass
  • Current upstream PR validator passes without warnings

Changelog

  • Added customizable active and inactive indicator symbols/text to the existing indicator mode, with and retained as defaults
  • Added independent font-family settings for desktop indicators and the Task View button
  • Replaced desktop buttons with ToggleButton controls whose checked state follows the current virtual desktop; exposed native Checked, CheckedPointerOver, and CheckedPressed states for Taskbar Styler
  • Kept click feedback synchronized with the actual desktop while switching asynchronously
  • Hardened XAML lifecycle handling: button delegates, tooltips, and boxed content are cleared before grid removal; process-shutdown XAML owners use intentional lifetime retention; UI callbacks contain exceptions
  • Expanded the readme with a copy-ready 🟢/🔴 preset, the new settings, and the exact Taskbar Styler target

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.

@m417z

m417z commented Jul 20, 2026

Copy link
Copy Markdown
Member

A question about [[clang::no_destroy]]: did you add it, or did Claude add it? Using it is tricky, because using it too much may cause memory leaks, but not using it enough may cause hangs and crashes when the process terminates.

For example, using it with std::vector will leak memory because even if the vector is freed, the memory isn't. A solution that works is using [[clang::no_destroy]] std::optional<std::vector<ButtonEventState>>, but it makes the usage a bit less convenient.

I'm still exploring how to make it more convenient. Perhaps I should include WindhawkUtils::NoDestroy that's similar but with a nicer syntax.

Submission review

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

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


  • [[clang::no_destroy]] on g_settings is unnecessary and leaks its strings on every unload. ModSettings holds only std::wstring/int/bool members — its destructor just frees heap, which is safe to run at process shutdown (the OS reclaims it anyway), so it doesn't need the attribute. Suppressing the destructor instead means the ~30 std::wstring buffers are leaked on every normal unload (disable/reload), where ~ModSettings would otherwise have freed them cleanly. Keep the attribute on the WinRT-holding globals (g_buttonGrid, g_injectionParent, g_buttonEventStates, g_secondaryBars, etc. — those genuinely need it), and drop it from g_settings:
    ModSettings g_settings;   // no [[clang::no_destroy]] needed

sb4ssman added 2 commits July 21, 2026 09:30
ModSettings holds only std::wstring/int/bool; suppressing its destructor
leaked the string buffers on every normal unload. Addresses review feedback
on PR ramensoftware#4844.
Removes the standalone readme preset paragraph and surfaces the copyable
🟢/🔴 suggestion in the Active/Inactive indicator symbol settings
descriptions (and the readme settings table), so it's available at the
point of use in the settings panel.
@sb4ssman

Copy link
Copy Markdown
Contributor Author

Claude added it, not me — and it added it too broadly. These mods are written
with Claude assistance, and after an earlier review raised exit-time destructor
safety, it applied [[clang::no_destroy]] to every namespace-scope global with
a non-trivial destructor rather than only to the ones holding WinRT references.
g_settings was collateral from that sweep. Your explanation of the tradeoff is
what made the distinction concrete, so thank you for asking rather than just
flagging it.

The required fix is already in the current head (c5995bd3): g_settings is a
plain ModSettings g_settings; again, with a comment recording why it must
not be annotated so the sweep doesn't come back.

On the containers: g_autoRevokerList, g_buttonEventStates, and
g_secondaryBars here are still the bare form that keeps its heap buffer alive
after every unload. I've adopted the
[[clang::no_destroy]] std::optional<std::vector<...>> + reset() shape you
described in #4485 and #4841, and this mod should follow. I'd rather not push
that change here on top of a green PR without live-testing it first, so I'll
fold it into the next tested update rather than sneak it in now.

WindhawkUtils::NoDestroy would help. The failure mode I keep hitting is that
the correct choice depends on what the global owns — heap-only, a nullable
WinRT handle, or a container of WinRT references — and the attribute alone
doesn't express that. A wrapper whose reset/release semantics are part of its
type would make the right thing the easy thing.

@m417z

m417z commented Jul 23, 2026

Copy link
Copy Markdown
Member

WindhawkUtils::NoDestroy would help

I thought about it, but it wouldn't be different from [[clang::no_destroy]] or [[clang::no_destroy]] std::optional<...>. It's not difficult because of the syntax, it's difficult because it's truly tricky. Some destructors must be skipped on termination, but not all. Some objects must be freed on the correct thread, and for others it doesn't matter. Let me know if you have a better solution than the current one - using [[clang::no_destroy]] with std::optional.

@m417z

m417z commented Jul 23, 2026

Copy link
Copy Markdown
Member

I'd rather not push
that change here on top of a green PR without live-testing it first, so I'll
fold it into the next tested update rather than sneak it in now.

To clarify: are you going to update the PR, or are you saying that you prefer getting it merged as is?

@sb4ssman

Copy link
Copy Markdown
Contributor Author

I am updating it, we can hold off on the merge.

And I see the other comments. I am working them too.

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.

[Taskbar Virtual Desktop Switcher] Add custom indicator

2 participants