[PowerDisplay] Run the tray Exit through Shutdown so teardown is not skipped - #49580
Merged
Conversation
…skipped The tray context menu's Exit item was wired straight to Environment.Exit(0), which ends the process without running any of the teardown Shutdown() already performs: TrayIconService.Destroy() (Shell_NotifyIcon NIM_DELETE, icon and menu handles, window procedure restore) and MainWindow.Dispose(), which is what cancels the CLI named-pipe server and disposes the hotkey service, the message hook and the view model. Point the Exit action at Shutdown() instead. The tray menu command is dispatched from the subclassed main-window procedure, so it already runs on the UI thread that owns those objects, and Shutdown() still ends with Environment.Exit(0). The named-pipe terminate message has always gone through Shutdown(), so this only makes the tray menu agree with it. Two other exit paths still call Environment.Exit(0) directly and are left alone here: the runner Terminate event, where adding teardown work to the module-disable path needs its own validation against the runner's shutdown timeout, and the WaitForPowerToysRunner watchdog, whose callback runs on a background thread and would need to be marshalled first.
Shawn Yuan (shuaiyuanxx)
approved these changes
Jul 30, 2026
leileizhang (lei9444)
approved these changes
Jul 30, 2026
moooyo
enabled auto-merge (squash)
July 30, 2026 09:09
moooyo
added a commit
that referenced
this pull request
Jul 31, 2026
## Summary of the Pull Request Scrolling the mouse wheel over the Power Display tray icon adjusts brightness, without opening the flyout. - New **Tray icon mouse wheel** setting: `Off` / `Primary display` / `All displays`, defaulting to **`Off`**. It is scoped to the tray icon — the flyout sliders accept wheel input regardless, as they always have. The existing **Mouse wheel increment** setting supplies the per-notch step. - **Off by default.** The gesture consumes a wheel notch that would otherwise reach the window under the pointer, and acting on it installs a system-wide `WH_MOUSE_LL` hook. Neither is something an existing installation should acquire silently on upgrade. With the setting `Off` no hook is ever installed and no notch is ever consumed, so this PR changes no existing behaviour until the user opts in: 1958 insertions, 2 deletions, and both deletions are refactors of lines this feature reuses. - **No feedback UI.** Brightness is self-evidencing — you scroll and the screen changes — so the display itself is the feedback. The notification icon is untouched: same tooltip, same text, same legacy notification-icon protocol. ## PR Checklist - [x] Closes: #49410 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [x] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx No new binaries or projects — everything lands in existing assemblies. Communication is unchecked because #49410 is still Needs-Triage. ## Detailed Description of the Pull Request / Additional comments ### Why a low-level hook The Shell does not forward `WM_MOUSEWHEEL` to a notification icon's callback window under any `NOTIFYICON_VERSION`, and a click-through overlay placed over the icon cannot receive wheel input either. `TrayIconMouseWheelListener` therefore installs a `WH_MOUSE_LL` hook — but only transiently, and only when it will act on the result: - Nothing is installed at all while the setting is `Off`, which is the default. - Installed in `EnsureHook()` when the UI thread confirms the pointer is inside the rectangle from `Shell_NotifyIconGetRect` **and** `CanAdjustBrightnessFromTrayWheel` says some monitor can accept a brightness write. - Removed in `DisarmCore()` as soon as either condition stops holding, the pointer leaves the rectangle, or the mode changes. - A notch is consumed (the hook proc returns non-zero) only while armed and only for points inside the armed rectangle, so a wheel event Power Display will not act on still reaches the window under the cursor. The hook runs on a dedicated background thread with its own message loop; the proc itself only enqueues a sample and posts a drain request. Deltas are marshalled to the UI thread in batches, and `WheelDeltaAccumulator` folds high-resolution deltas (precision wheels, touchpads) into whole notches. Each sample carries the hover generation it was captured under, so samples from a hover the UI thread has already retired are discarded rather than applied late. ### Hover detection The Shell sends `WM_MOUSEMOVE` to the icon's callback window while the pointer is over it. `TrayIconService.HandleTrayMouseMove` resolves the rectangle with `Shell_NotifyIconGetRect` and caches it for a second, because that message repeats for every pixel of travel. `TrayIconService` gains nothing else: no protocol change, no new hover UI, no polling. The rest of the file — and `MainWindow.xaml` — is untouched. ### Linked brightness While linked brightness is on, a notch has to move the whole group, so it goes through `MainViewModel.LinkedBrightness` rather than the individual monitor setters. The new master value is taken from the planner's value for the monitor the wheel named, **not** from the current master. The master is positional only — `SeedInitialLinkedBrightness` takes it from the lowest-numbered linked monitor and never writes hardware, and every monitor-list rebuild re-seeds it — so it can sit arbitrarily far from the monitor the wheel is aimed at. Stepping it relative to itself would apply a wrong-sized or wrong-signed change, and a master already clamped at 0/100 would swallow the notch while writing nothing at all. The setting description calls out that linked brightness widens the scope, so `Primary display` is not literally a single display while it is on. ### What is deliberately not here An earlier revision of this PR showed the target and percentage in a custom overlay as you scrolled. Doing that meant the standard Shell tooltip would not do (it cannot be shown on demand), which meant an own window, which meant suppressing the Shell tooltip so the two did not collide, which meant `NOTIFYICON_VERSION_4`, which changed the callback packing and made the app responsible for all hover text — including for keyboard and touch users, who never reach a cursor-anchored overlay and would have been left with no visible tooltip at all. That chain was about half the diff, for a readout that adds little on top of watching the screen change. It is gone. If a readout is wanted later it can be argued on its own merits, separately from this feature. The same revision also gated the flyout sliders on this setting. That bundled two unrelated things behind one switch — turning off tray scrolling would also have stopped the contrast and volume sliders responding to the wheel — so the setting is now scoped to the tray icon and named accordingly. An earlier revision also routed the tray **Exit** action through `Shutdown()`. That fixes a pre-existing teardown leak which has nothing to do with this feature, so it now lives in #49580 and is out of scope here. This branch does not depend on it: the hook thread is a background thread and the process is ending either way. ## Validation Steps Performed - Unit tests: `PowerDisplay.Lib.UnitTests` 215 passed, `Settings.UI.UnitTests` 165 passed. - Builds: `PowerDisplay` and Settings UI, x64 Debug, no warnings. - Automated coverage is in `PowerDisplay.Lib.UnitTests`: target selection per mode, wheel accumulation including negative deltas, partial notches and direction reversal, half-open rectangle containment, and settings serialization and round-trip for the new mode, including that a settings file predating the feature loads as `Off`. `Settings.UI.UnitTests` covers the view-model index mapping and pins the enum values to the ComboBox item order. - The Win32 glue in `TrayIconService` and `TrayIconMouseWheelListener` is not unit tested. Manual passes performed: scrolling over the icon in both modes, the icon parked in the notification overflow, high-resolution wheel input, brightness boundaries, live monitor refresh while hovering, tray icon hidden and re-enabled, Explorer restart, the context menu and left-click, `Off` stopping tray scrolling while the flyout sliders keep working, and confirming a notch that Power Display will not act on still reaches the window under the cursor. Not verified, needing hardware this branch has not been run on: - Multiple taskbars, where the tray icon is on a secondary display and `Primary display` mode adjusts a monitor the user may not be looking at. - Mixed-DPI setups, for the `Shell_NotifyIconGetRect` rectangle and the hook's physical-pixel hit test. --------- Co-authored-by: Yu Leng <yuleng@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Copilot-Session: 5d7f36fe-d175-4aa9-a3c7-b370d952d1d3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
PowerDisplay's tray context menu Exit ended the process with
Environment.Exit(0), skipping the teardown thatApp.Shutdown()alreadyperforms. Point it at
Shutdown()instead — a one-line change.What Exit was skipping:
TrayIconService.Destroy()—Shell_NotifyIcon(NIM_DELETE), the icon andpopup-menu handles, and restoring the subclassed window procedure. Without the
NIM_DELETE, the notification area can keep showing a stale PowerDisplay iconuntil the Shell next validates it, which in practice is when the pointer passes
over it.
MainWindow.Dispose()— which cancels the CLI named-pipe server'sCancellationTokenSourceand disposes the hotkey service, the message hook andMainViewModel(monitor manager, display-change watcher, per-monitor viewmodels).
Environment.Exitdoes not run finalizers, so none of that happened by anotherroute.
The named-pipe terminate message (
PowerDisplayTerminateAppMessage) has alwaysgone through
Shutdown(), so this only makes the tray menu agree with a paththat is already shipping. The tray menu command is dispatched from the
subclassed main-window procedure, so it already runs on the UI thread that owns
these objects, and
Shutdown()still ends withEnvironment.Exit(0)— theprocess exits unconditionally either way.
PR Checklist
it can be reviewed on its own.
the work hasn't been agreed, this work might be rejected
process-exit wiring inside
App.OnLaunched, which has no test harness;validated manually.
changed strings.
teardown fix.
Detailed Description of the Pull Request / Additional comments
Deliberately not in scope
Two other paths still call
Environment.Exit(0)directly, and both arepre-existing and unchanged here:
Constants.TerminatePowerDisplayEvent()) —the module-disable and PowerToys-exit path. Its callback is already marshalled
to the UI thread by
NativeEventWaiter, so it could be routed the same way,but adding teardown work to the runner's shutdown path should be validated
against the runner's shutdown timeout on its own rather than riding along with
a tray-menu fix.
RunnerHelper.WaitForPowerToysRunnerwatchdog, whose callback runs on abackground thread and would need marshalling to the UI thread first.
Happy to follow up on either if reviewers would rather see them fixed together.
Validation Steps Performed
disappears immediately rather than lingering until hover.
once, not twice.
powerdisplayCLI still works after a launch/tray-Exit/launch cycle,confirming the named pipe was released rather than left to process teardown.
quitting PowerToys both still exit the process.