Skip to content

Commit 0e045fa

Browse files
authored
perf(capture): route window capture through ScreenCaptureKit to cut SkyLight leaks (#613)
* perf(capture): route window capture through ScreenCaptureKit to cut SkyLight leaks The `leaks` tool against a fresh Thaw process on macOS 26.4.1 attributed 250 `NSMutableDictionary` instances (~42 KB of ~44 KB total leaked bytes) to a single allocation site: `CGRectCreateDictionaryRepresentation` inside the private `SLSWindowListCreateImageFromArrayProxying`, called by every `SLWindowListCreateImageFromArray` invocation. The dictionary is allocated with retain count 1 and never released. The path was being exercised by every Thaw window-capture call site: `MenuBarItemImageCache.individualCapture`, `compositeCapture`, and `refreshImages`, plus the three color-sampler call sites in `MenuBarManager.updateAverageColorInfo`, `IceBarColorManager.updateWindowImage`, and `MenuBarSearchModel.updateAverageColorInfo`. The leak lives inside Apple's SkyLight framework, so there is no way to release the dictionary from Swift; the only levers we have are to stop calling SkyLight or to call it less. This change introduces SCK-backed async equivalents `Bridging.captureWindowsImageSCK` (using `SCScreenshotManager.captureImage(contentFilter:configuration:)` with an `SCContentFilter(display:including:)` filter) and corresponding `ScreenCapture.captureWindowsAsync` / `captureWindowAsync` wrappers, then migrates every capture call site whose target windows fit within display bounds. The menu-bar item cache (`compositeCapture`, `individualCapture`) becomes async and routes through SCK; the three color samplers keep their public synchronous signatures but wrap the capture in a `Task` so the SCK call runs off the main actor and the resulting state mutation hops back to the owning `@MainActor` class via the captured `self`. The `refreshImages` path cannot move to SCK: it captures items in the hidden and always-hidden sections, which Thaw positions at large negative x past the display's left edge. Both SCK filter shapes reject those windows on macOS 26: `SCContentFilter(display:including:)` returns error -3812 (sourceRect outside display bounds) and `SCContentFilter(desktopIndependentWindow:)` returns error -3811 (stream start failure). To shrink that residual floor, `refreshImages` gains an opt-in `viaSCK` parameter and `runLiveRefreshLoop` is restructured: tick-global guards (`lastMoveOperationOccurred`, `isResettingLayout`) hoist out of the per-section loop, the `.visible` section refreshes individually via the leak-free SCK path, and `.hidden` plus `.alwaysHidden` items batch into a single SkyLight call per tick. A full all-sections refresh now leaks one dictionary per tick instead of three. On a fresh `MallocStackLogging=1` launch with the menu bar idle, hidden + always-hidden sections expanded, and `Settings → Menu Bar Layout` opened, `leaks` reports 122 leaks / 10.8 KB total: 53 `NSMutableDictionary` instances (all from the batched SkyLight `refreshImages` call) and 19 framework CGRegion leaks from AppKit's `_regionForOpaqueDescendants` display-cycle path. Versus the original baseline of 507 leaks / 44 KB that is a 79% reduction in dictionary leak count and a 76% reduction in leaked bytes. The remaining 53 instances are the irreducible Apple-framework floor until either `SLSWindowListCreateImageFromArrayProxying` stops dropping the dictionary or SCK adds support for capturing windows positioned outside any `SCDisplay.frame`. Apple Feedback drafts for the SkyLight leak, the AppKit CGRegion leak, and a separate `AppIntents` `NSProgress` retain cycle observed when materializing `SetFocusFilterIntent` metadata are tracked separately and will be filed via Feedback Assistant. * refactor: code suggestions Tightens the SCK migration's edges in response to follow-up review. Five small fixes across four files; no leak-rate change, all behavioural correctness. `Bridging.captureWindowsImageSCK` now requires an exact match between the requested `windowIDs` and the `SCWindow`s resolved from `SCShareableContent`. The previous `!scWindows.isEmpty` guard let a partial subset through, which is unsafe for cache composites (the post-capture crop math assumes every requested window's bounds is covered) and for the color samplers (a missing `menuBarWindow` would silently produce a wallpaper-only strip whose `averageColor` no longer represents the menu bar). On mismatch we now log the requested vs matched counts plus the missing IDs and return `nil` so callers fall back cleanly to SkyLight or skip the tick. `IceBarColorManager` adds a generation-token pattern to `updateWindowImage`. The function becomes `async`, bumps `windowImageGeneration` before suspending, and writes `windowImage` after the SCK await only if the generation still matches. A new `clearWindowImage()` helper bumps + nils together so `stopPeriodicRefresh` and the notification sink can invalidate any capture in flight from before the clear; the previous fire-and-forget shape let a late completion undo `self.windowImage = nil`. Four sinks and `updateAllProperties` are now wrapped in `Task { await updateWindowImage(...); updateColorInfo(...) }` so the post-refresh color read sees the freshly captured image instead of the previous cycle's leftover, which closes the visible ~5 s color lag users would see after a theme change or first IceBar open while the previous async capture was still in flight. `updateAllProperties` keeps its synchronous signature so `IceBar.show()` doesn't ripple async upstream. `MenuBarManager.updateAverageColorInfo` is split into a sync fire-and-forget wrapper and an awaitable `updateAverageColorInfoAsync()` that uses `withTaskGroup` for concurrent per-screen captures and collects results back on the enclosing `@MainActor` so the `averageColors` / `averageColorInfo` writes are complete before `await` returns. `captureAdaptiveColorWithRetry` is rewritten as a single async retry loop that awaits each capture before reading `averageColors`; the previous fire-and-forget call meant the post-call read saw stale state and burned all ten retries even when the first capture would have succeeded. The wake-poll Timer sink wraps its body in a `Task` that awaits the async variant before reading `after = averageColors`, so the stabilization detector (`wakePollDidChange`, `wakePollStableCount`, `wakePollPrevColors`) sees real frame-to-frame changes instead of always-stale snapshots that previously prevented it from ever marking the wake as settled within the 10 s window. `MenuBarSearchModel.updateAverageColorInfo` gets the same generation-token treatment as `IceBarColorManager`. A new `clearAverageColorInfo()` helper bumps the token and nils `averageColorInfo`; the panel-visibility-off sink and the screen-params sink now route through it, and the capture Task captures the generation before suspending and skips its write if the token has advanced. Closes the previously-possible races where a late capture undid an intentional clear or where out-of-order completions from rapid screen switches let a stale color overwrite a fresher one. The public function stays sync because no callers in this file read `averageColorInfo` immediately after. * refactor: code suggestions Two follow-up tightenings in `Bridging.captureWindowsImageSCK`. No leak-rate change, both behavioural correctness for edge cases the existing callers happen not to hit today. Display selection now requires a single `SCDisplay` whose `frame` fully contains BOTH the effective capture bounds AND the union of all selected windows. The previous chain (`intersects(effectiveBounds)` → `intersects(unionBounds)` → `displays.first`) could pick a display that only partially overlapped the windows, and `SCContentFilter(display:including:)` silently clips at `display.frame`, so anything outside the chosen display was lost from the capture without an error. Tightening to `contains` returns a clean `nil` for cross-display spans and frame-reporting quirks so callers fall back (cache paths excluded-item retry) or skip the tick (`refreshImages viaSCK: true`) instead of feeding clipped pixels to downstream crops and `averageColor`. The diagLog warning now reports both bounds so the rejection is debuggable. `configuration.ignoreShadowsDisplay` no longer treats `options.isEmpty` as if `.boundsIgnoreFraming` were set. The legacy `SLWindowListCreateImageFromArray` API treats an empty option set as the default capture (framing and shadows preserved); only the explicit `.boundsIgnoreFraming` flag suppresses them. The previous `|| options.isEmpty` clause would have given an unexpected frameless capture to any future caller that relied on the function's `options: CGWindowImageOption = []` default. All current callers pass either `.boundsIgnoreFraming` (cache paths) or `.nominalResolution` (color samplers), so observable behaviour is unchanged today.
1 parent e5d3556 commit 0e045fa

6 files changed

Lines changed: 420 additions & 128 deletions

File tree

Shared/Bridging/Bridging.swift

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
// Licensed under the GNU GPLv3
88

99
import Cocoa
10+
@preconcurrency import ScreenCaptureKit
1011

1112
// MARK: - Bridging
1213

@@ -560,3 +561,122 @@ extension Bridging {
560561
return image
561562
}
562563
}
564+
565+
// MARK: - ScreenCaptureKit Window Capture
566+
567+
extension Bridging {
568+
/// Captures a composite image of an array of windows using ScreenCaptureKit.
569+
///
570+
/// Async, leak-free replacement for captureWindowsImage. Use this for any
571+
/// window set whose union bounds fit within a display. For menu-bar items
572+
/// in hidden / always-hidden sections (positioned at large negative x),
573+
/// stay on captureWindowsImage: SCK's display+including filter returns
574+
/// error -3812 for sourceRects outside display bounds, and the
575+
/// desktopIndependentWindow filter returns -3811 for those windows too.
576+
///
577+
/// - Parameters:
578+
/// - windowIDs: The identifiers of the windows to capture.
579+
/// - screenBounds: The bounds to capture, specified in screen coordinates.
580+
/// Pass nil (or CGRect.null) to capture the minimum rectangle that
581+
/// encloses the selected windows.
582+
/// - options: Capture options. boundsIgnoreFraming maps to
583+
/// ignoreShadowsDisplay; nominalResolution forces 1x scale.
584+
/// - Returns: The captured image, or nil if capture failed.
585+
static func captureWindowsImageSCK(
586+
windowIDs: [CGWindowID],
587+
screenBounds: CGRect? = nil,
588+
options: CGWindowImageOption = []
589+
) async -> CGImage? {
590+
guard !windowIDs.isEmpty else {
591+
diagLog.warning("captureWindowsImageSCK: empty windowIDs")
592+
return nil
593+
}
594+
595+
let content: SCShareableContent
596+
do {
597+
content = try await SCShareableContent.excludingDesktopWindows(
598+
false,
599+
onScreenWindowsOnly: false
600+
)
601+
} catch {
602+
diagLog.error("captureWindowsImageSCK: SCShareableContent failed: \(error)")
603+
return nil
604+
}
605+
606+
// Preserve caller's z-order so the composite renders correctly.
607+
let scWindows = windowIDs.compactMap { id in
608+
content.windows.first { $0.windowID == id }
609+
}
610+
611+
// Require an exact match. Partial captures are unsafe: cache composites
612+
// rely on the result covering every requested window's bounds for the
613+
// post-capture crop math, and color samplers rely on every requested
614+
// window being included for the averaged color to mean anything.
615+
// Fail fast so callers fall back cleanly to SkyLight or skip the tick.
616+
guard scWindows.count == windowIDs.count else {
617+
let matched = Set(scWindows.map(\.windowID))
618+
let missing = windowIDs.filter { !matched.contains($0) }
619+
diagLog.warning("captureWindowsImageSCK: SCK resolved \(scWindows.count)/\(windowIDs.count) requested windows; missing IDs: \(missing)")
620+
return nil
621+
}
622+
623+
// Union of selected window frames; used both as default bounds and
624+
// to find the host display.
625+
let unionBounds = scWindows.reduce(CGRect.null) { $0.union($1.frame) }
626+
let effectiveBounds: CGRect = {
627+
if let screenBounds, !screenBounds.isNull {
628+
return screenBounds
629+
}
630+
return unionBounds
631+
}()
632+
633+
// Require a single display that fully contains BOTH the effective
634+
// capture bounds and the union of all selected windows. Using
635+
// intersects here would let a display that only partially overlaps
636+
// through, producing a silently clipped capture; SCContentFilter
637+
// (display:including:) clips at display.frame, so anything outside is
638+
// lost without an error. A clean nil lets callers fall back or skip.
639+
guard let display = content.displays.first(where: {
640+
$0.frame.contains(effectiveBounds) && $0.frame.contains(unionBounds)
641+
}) else {
642+
diagLog.warning("captureWindowsImageSCK: no display fully contains effectiveBounds=\(effectiveBounds) and unionBounds=\(unionBounds)")
643+
return nil
644+
}
645+
646+
let filter = SCContentFilter(display: display, including: scWindows)
647+
648+
let configuration = SCStreamConfiguration()
649+
configuration.showsCursor = false
650+
// boundsIgnoreFraming on the legacy API means "skip the window frame".
651+
// For a display+including filter the equivalent is ignoreShadowsDisplay;
652+
// no per-window shadow toggle exists on this filter shape. Empty
653+
// options matches the legacy SkyLight default of keeping framing, so
654+
// honor only the explicit flag here.
655+
configuration.ignoreShadowsDisplay = options.contains(.boundsIgnoreFraming)
656+
657+
let scale: CGFloat = options.contains(.nominalResolution)
658+
? 1.0
659+
: CGFloat(filter.pointPixelScale)
660+
661+
configuration.sourceRect = CGRect(
662+
x: effectiveBounds.origin.x - display.frame.origin.x,
663+
y: effectiveBounds.origin.y - display.frame.origin.y,
664+
width: effectiveBounds.width,
665+
height: effectiveBounds.height
666+
)
667+
configuration.width = Int((effectiveBounds.width * scale).rounded())
668+
configuration.height = Int((effectiveBounds.height * scale).rounded())
669+
670+
do {
671+
let image = try await SCScreenshotManager.captureImage(
672+
contentFilter: filter,
673+
configuration: configuration
674+
)
675+
diagLog.debug("captureWindowsImageSCK: captured \(windowIDs.count) windows → \(image.width)×\(image.height)px")
676+
return image
677+
} catch {
678+
diagLog.error("captureWindowsImageSCK: SCScreenshotManager.captureImage failed: \(error)")
679+
return nil
680+
}
681+
}
682+
}

Thaw/MenuBar/IceBar/IceBarColorManager.swift

Lines changed: 70 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ final class IceBarColorManager: ObservableObject {
1717

1818
private var windowImage: CGImage?
1919

20+
/// Monotonically incremented by updateWindowImage and clearWindowImage.
21+
/// A capture in flight stamps the value it observed; on completion it only
22+
/// writes windowImage if the value still matches, so a late completion
23+
/// can't undo a freshly cleared image or overwrite a newer capture.
24+
private var windowImageGeneration: Int = 0
25+
2026
private var cancellables = Set<AnyCancellable>()
2127

2228
/// Cancellable for the periodic refresh timer, active only while the Thaw Bar is visible.
@@ -42,7 +48,10 @@ final class IceBarColorManager: ObservableObject {
4248
else {
4349
return
4450
}
45-
updateWindowImage(for: screen)
51+
Task { [weak self] in
52+
guard let self else { return }
53+
await self.updateWindowImage(for: screen)
54+
}
4655
}
4756
.store(in: &c)
4857

@@ -82,7 +91,8 @@ final class IceBarColorManager: ObservableObject {
8291
return
8392
}
8493
// Clear window image on display changes to prevent memory growth
85-
self.windowImage = nil
94+
// and invalidate any in-flight capture from before the change.
95+
self.clearWindowImage()
8696
guard
8797
let iceBarPanel,
8898
iceBarPanel.isVisible,
@@ -91,9 +101,13 @@ final class IceBarColorManager: ObservableObject {
91101
else {
92102
return
93103
}
94-
updateWindowImage(for: screen)
95-
withAnimation {
96-
self.updateColorInfo(with: iceBarPanel.frame, screen: screen)
104+
let frame = iceBarPanel.frame
105+
Task { [weak self] in
106+
guard let self else { return }
107+
await self.updateWindowImage(for: screen)
108+
withAnimation {
109+
self.updateColorInfo(with: frame, screen: screen)
110+
}
97111
}
98112
}
99113
.store(in: &c)
@@ -106,10 +120,17 @@ final class IceBarColorManager: ObservableObject {
106120
.sink { [weak self, weak iceBarPanel] isVisible in
107121
guard let self else { return }
108122
if isVisible {
109-
// Refresh windowImage immediately so the first color update isn't stale.
123+
// Refresh windowImage immediately so the first color
124+
// update isn't stale. Awaiting inside a Task so
125+
// updateColorInfo reads the fresh capture, not the
126+
// previous cycle's leftover.
110127
if let iceBarPanel, let screen = iceBarPanel.screen, screen == .main {
111-
self.updateWindowImage(for: screen)
112-
self.updateColorInfo(with: iceBarPanel.frame, screen: screen)
128+
let frame = iceBarPanel.frame
129+
Task { [weak self] in
130+
guard let self else { return }
131+
await self.updateWindowImage(for: screen)
132+
self.updateColorInfo(with: frame, screen: screen)
133+
}
113134
}
114135
self.startPeriodicRefresh(for: iceBarPanel)
115136
} else {
@@ -137,9 +158,13 @@ final class IceBarColorManager: ObservableObject {
137158
else {
138159
return
139160
}
140-
updateWindowImage(for: screen)
141-
withAnimation {
142-
self.updateColorInfo(with: iceBarPanel.frame, screen: screen)
161+
let frame = iceBarPanel.frame
162+
Task { [weak self] in
163+
guard let self else { return }
164+
await self.updateWindowImage(for: screen)
165+
withAnimation {
166+
self.updateColorInfo(with: frame, screen: screen)
167+
}
143168
}
144169
}
145170
}
@@ -148,11 +173,19 @@ final class IceBarColorManager: ObservableObject {
148173
private func stopPeriodicRefresh() {
149174
periodicRefreshCancellable?.cancel()
150175
periodicRefreshCancellable = nil
151-
// Clear the window image to free memory when IceBar is hidden
176+
// Clear the window image to free memory when IceBar is hidden.
177+
clearWindowImage()
178+
}
179+
180+
/// Clears windowImage and invalidates any in-flight capture. Use whenever
181+
/// callers want a synchronous nil state that an outstanding async capture
182+
/// must not be allowed to overwrite.
183+
private func clearWindowImage() {
184+
windowImageGeneration += 1
152185
windowImage = nil
153186
}
154187

155-
private func updateWindowImage(for screen: NSScreen) {
188+
private func updateWindowImage(for screen: NSScreen) async {
156189
let windows = WindowInfo.createWindows(option: .onScreen)
157190
let displayID = screen.displayID
158191

@@ -163,14 +196,22 @@ final class IceBarColorManager: ObservableObject {
163196
return
164197
}
165198

166-
guard let image = ScreenCapture.captureWindows(
167-
with: [menuBarWindow.windowID, wallpaperWindow.windowID],
168-
screenBounds: withMutableCopy(of: wallpaperWindow.bounds) { $0.size.height = 1 },
169-
option: .nominalResolution
170-
) else {
171-
return
172-
}
199+
let windowIDs = [menuBarWindow.windowID, wallpaperWindow.windowID]
200+
let bounds = withMutableCopy(of: wallpaperWindow.bounds) { $0.size.height = 1 }
201+
202+
// Stamp our generation before suspending. If the counter advances while
203+
// we await (a clearWindowImage, a stopPeriodicRefresh, or a newer
204+
// updateWindowImage call), our completion is stale and must skip the
205+
// write so we don't undo intentional clears or clobber a fresher image.
206+
windowImageGeneration += 1
207+
let generation = windowImageGeneration
173208

209+
let image = await ScreenCapture.captureWindowsAsync(
210+
with: windowIDs,
211+
screenBounds: bounds,
212+
option: .nominalResolution
213+
)
214+
guard generation == windowImageGeneration, let image else { return }
174215
windowImage = image
175216
}
176217

@@ -201,7 +242,14 @@ final class IceBarColorManager: ObservableObject {
201242
}
202243

203244
func updateAllProperties(with frame: CGRect, screen: NSScreen) {
204-
updateWindowImage(for: screen)
205-
updateColorInfo(with: frame, screen: screen)
245+
// Keep the public signature synchronous so IceBar.show doesn't ripple
246+
// async upstream. Wraps the async refresh+color combo in a Task so
247+
// updateColorInfo reads the fresh capture instead of the previous
248+
// cycle's leftover.
249+
Task { [weak self] in
250+
guard let self else { return }
251+
await self.updateWindowImage(for: screen)
252+
self.updateColorInfo(with: frame, screen: screen)
253+
}
206254
}
207255
}

0 commit comments

Comments
 (0)