fix: Menu-bar widget guards, bypass setNeedsDisplayInRect#3216
Open
Incipiens wants to merge 4 commits into
Open
fix: Menu-bar widget guards, bypass setNeedsDisplayInRect#3216Incipiens wants to merge 4 commits into
Incipiens wants to merge 4 commits into
Conversation
Several widget redraw and resize paths could fire while a widget was not yet attached to its hosting NSStatusItem button, or while the button itself had no associated NSWindow (windowNumber == -1). When that happened, calls into setFrameSize/display/NSStatusItem.length would still reach AppKit, which forwarded them to WindowServer and caused log spamming to occur. The same paths were also unconditionally re-adding the widget view to its host on every status-item setup, causing needless view-hierarchy churn on toggle. This change introduces a small hasUsableHost helper on WidgetWrapper that requires both a non-nil superview and a window with a valid windowNumber, and uses it to early-return from: - WidgetWrapper.setWidth (before setFrameSize/widthHandler) - WidgetWrapper.redraw (before display) For the per-widget status item, SWidget.widthHandler and MenuBar.recalculateWidth now require the menu-bar button to have a window before assigning NSStatusItem.length, which is what actually triggers the WindowServer round-trip and log spam. SWidget.setMenuBarItem and MenuBarView.addWidget are also made idempotent: they only call addSubview when the view isn't already parented to the intended host, removing repeated reparent work on toggle and one-view changes. Two unrelated fixes are included in the same change because they surfaced while reproducing the menu-bar issue on macOS Tahoe: - Modules/Net/preview.swift: the network preview chart was being initialised without an explicit frame, leaving it sized 0x0 until the first layout pass. Provide an explicit (0, 0, 0, 140) frame so the preview renders correctly on first display. - Modules/Net/readers.swift: guard the CWPHYMode.mode11be case in the CustomStringConvertible extension behind `#if compiler(<6.2)`. The symbol is not exposed by the CoreWLAN headers shipped with the Tahoe SDK (Xcode 26/Swift 6.2), so unconditionally referencing it breaks the build on that toolchain. Older toolchains (Sonoma, Sequoia) continue to render Wi-Fi 7 networks as "802.11be".
…layInRect spam The hasUsableHost guards added in the previous commit got us about halfway. On macOS 26 (Tahoe) I discovered later on that WindowServer was still logging _CGXPackagesSetWindowConstraints: Invalid window at roughly 3.9 hits/sec whenever widgets were visible, and lldb pointed at why: every spam line originated from a widget calling needsDisplay = true/display() inside its status-item button, not from anything the hasUsableHost guards covered. The actual chain is that NSView.setNeedsDisplayInRect: now posts a CoreFoundation notification on every view invalidation. NSStatusBarWindow observes that notification on Tahoe and responds by calling NSStatusItem._windowNeedsReplicantUpdate:, which pushes a window-geometry constraint to SkyLight via _CGXPackagesSetWindowConstraints. If the status item's backing window is in any transitional state at that moment (which is much more frequent on Tahoe due to menu bar overflow, the notch, and Liquid Glass animation), SkyLight logs Invalid window and drops the request. Every widget redraw round-trips through WindowServer regardless of whether NSStatusItem.length is ever touched, and swapping display() for needsDisplay = true does nothing because both end up at setNeedsDisplayInRect: internally. This change replaces the AppKit invalidation path with a layer-contents pipeline. Each WidgetWrapper now sets wantsLayer = true at init and gains a renderToLayer() method that renders the widget's existing draw(_:) output off-screen into a CGContext-backed CGImage and assigns it directly to layer.contents inside a CATransaction with implicit animations disabled. The notification chain never fires because setNeedsDisplayInRect: is never called. Call sites that previously triggered redraws are swapped accordingly: - Every needsDisplay = true and display() call in Mini, Memory, Speed, Battery, Dot, LineChart, BarChart, NetworkChart, Stack, and Text widgets now calls renderToLayer() instead. - Charts.displayIfVisible and the CombinedView paths swap the same way. - WidgetWrapper.redraw() is now main-thread-aware and dispatches renderToLayer(). - The widget re-renders on viewDidMoveToWindow and viewDidChangeEffectiveAppearance so its contents are correct after host changes or appearance shifts. The hasUsableHost guards and the previous commit's idempotent addSubview behaviour are retained. They're independent of this change and still useful for skipping work when the status item isn't hosted yet. A new attachToMenuBar(retries:) helper on SWidget and configureMenuBarButton(retries:) on MenuBar replace the inline status-bar setup so the bind-to-button work waits for a valid backing window instead of firing immediately on creation. Measured on macOS 26.5 over a 10h run with all modules active: pre-fix WindowServer was logging the spam at ~3.9 hits/sec sustained whenever Stats was visible. Post-fix: 0.025 hits/sec during active use, 0.006 hits/sec idle, with two small launch-time bursts of ~5 hits each before the renderToLayer pipeline takes over. That's a ~150x reduction during active use and ~650x idle. Stats itself stays flat at 0% CPU and ~64 MB RSS over the run.
…uards # Conflicts: # Kit/Widgets/LineChart.swift # Kit/Widgets/Memory.swift # Kit/Widgets/Mini.swift # Kit/Widgets/NetworkChart.swift
Four fixes to the layer-contents pipeline: - Override viewDidChangeBackingProperties on WidgetWrapper to call renderToLayer(). Without this the cached layer.contents image stays at its previous scale when the widget moves between displays of different DPI (like disconnecting a Retina external), leaving widgets blurry or oversharp until the next setValue tick refreshes them. - Set layerContentsRedrawPolicy = .never. Without this the default .duringViewResize policy lets AppKit autonomously call draw(_:) on view geometry changes (which happen often when text widgets re-measure on each value update), going through setNeedsDisplayInRect: and re-triggering the notification chain renderToLayer is meant to bypass. Setting .never tells AppKit the application owns layer contents entirely. - Switch contentsGravity from .resize to .center. The transient frame between a resize and the next renderToLayer call is normally a single frame, but .resize stretches the cached image to fill the new bounds, which looks obviously wrong for text widgets during that frame. .center shows the old image at natural size (possibly clipped) which reads correctly. - Switch the CGContext pixel format from premultipliedLast (RGBA) to premultipliedFirst + byteOrder32Little (BGRA). BGRA is the native pixel layout for CoreGraphics and CoreAnimation on Apple platforms, so handing the CGImage to layer.contents no longer requires a swizzle pass before GPU upload. The perf difference is in the low microseconds for a status bar widget redrawing at ~1 Hz so it's not measurable in practice, but it matches the native pixel layout used by IOSurface and CoreAnimation.
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
1 participant
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.
Firstly, sorry for the earlier PR that was opened. I had thought it addressed the issue, but it actually hadn't. At least not fully. Using lldb, I was able to verify the actual cause.
Breaking on
-[NSStatusItem _windowNeedsReplicantUpdate:]produced a backtrace that hits[NSView setNeedsDisplayInRect:]+284in AppKit, right before the Stats widget code. This was consistent on every Invalid window log entry.NSStatusBarButton) is invalidated bysetNeedsDisplay = trueorsetNeedsDisplayInRect:.-[NSView setNeedsDisplayInRect:], on Tahoe, posts a CoreFoundation notification at offset +284.NSStatusBarWindowis registered as an observer of that notification (selector_viewHierarchyDidUpdate).-[NSStatusItem _windowNeedsReplicantUpdate:], which pushes a window-geometry constraint to SkyLight via_CGXPackagesSetWindowConstraintsto update the menu bar's replicant copy of the status item content.windowNumber <= 0, mid-recycle, behind notch, in overflow under Control Center, etc), SkyLight logsInvalid windowand drops the request.The trigger is, therefore, any view invalidation inside a status item button. On pre-Tahoe macOS, either the notification was not posted from
setNeedsDisplayInRect:orNSStatusBarWindowdid not observe it; on Tahoe it does both, so every widget redraw round-trips through SkyLight.This PR, as a result, moves widget rendering to layer-backed views with manual
layer.contents. It renders to aCGImageon the main thread and assignsview.layer.contents = cgImagedirectly. This avoidssetNeedsDisplayInRect:and the notification chain entirely. This reuses each widget's existing draw(_:) by handing it aCGContext, so the per-widget drawing code didn't have to change, only the call sites that previously invalidated views. I spent a few hours reading into this yesterday, and it seems as if this is the only mitigation that removes the spam even when items remain visible.Transitional states for the status item's backing window are much more frequent on Tahoe than older macOS due to menu bar overflow, the notch, and Liquid Glass animation, so the SkyLight call almost always lands while the window is in one. This fix never calls setNeedsDisplayInRect: in the first place, so the notification never fires and the SkyLight call never happens regardless of what state the window is in.
I measured the impact of this fix on macOS 26.5 over a 10h run: pre-fix ~3.9 hits/sec, post-fix 0.025/sec active and 0.006/sec idle. All APIs used in this fix are available on macOS 11 and newer.