Repository navigation
perf: don't animate what isn't visible - #4
Conversation
fd38671 to
6e2728c
Compare
PR #4 stepped the budget back down by bumping a @State integer that nothing reads — it works only because SwiftUI invalidates on any state write, which is a detail, not a contract. Hold the interval itself in state instead: the task takes it up when motion starts and puts it back down when the window expires, and the code says what it means. Low Power Mode also only counted at the next re-render, since isLowPowerModeEnabled was read inline with nothing observing it. It now follows NSProcessInfoPowerStateDidChange. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Merged as 3590659, with one follow-up in 2c7d61a. Verified locally: builds clean, 83 tests green with the other three PRs merged alongside it, SwiftLint unchanged from The halo change is right, and the reasoning holds up under inspection. On the @State private var frameInterval: TimeInterval = MoriEyesView.restingIntervalThe task takes it up when motion starts and puts it back down when the window expires. Same behaviour, and the code now says what it means. One thing the review turned up that wasn't in scope: On the measurement. Withdrawing the CPU number rather than shipping one you couldn't stand behind was the right call, and it made the PR easier to trust, not harder. The argument stands on its own — a view drawing at The 30fps resting rate stays. Thanks. |
Two animation timelines run unconditionally while the notch is idle. Both are argued from the code below rather than from a benchmark — see the note on measurement at the end.
The halo
HaloViewdrivesTimelineView(.animation), so it re-evaluates at the display's native refresh rate (120Hz on ProMotion) for the entire lifetime of the app. But the ring is only visible for 0.6s afterpingAt:Outside that window it computes
elapsed = .infinity,alive = false, and draws atopacity(0). WithpingAt == nilit has never been visible at all, and still runs.This commit swaps the schedule for a finite
.explicitone derived frompingAt(0.6s at 60Hz). An explicit schedule stops firing once exhausted, and becausepingAtis aleton a value-type view, a new ping rebuilds the view and re-arms it.pingAt == nilnow installs no timeline at all. Same bloom, same decay curve, same colours.The eyes
MoriEyesViewhad its frame budget written in by hand:pausedis pinned tofalse, so the dot-matrix face redraws 60 times a second regardless of what it's showing. This one multiplies: the face renders once in the top bar and once per session row, so the cost scales with how many sessions are on screen — worst exactly when the app is most useful.Now: 1/30 at rest, 1/60 while an expression morph, celebration or wink is in flight, 1/15 under Low Power Mode.
.speaking,.alertand.listeningstay at 60fps — their oscillations (0.42s lid sine, 5.5Hz jitter, 0.9s ring pulse) don't resolve at 30. Breathing and gaze drift keep running throughout; nothing freezes.The transient window constants were hoisted out of the drawing code (
morphDuration0.34s,celebrationDuration1.1s,winkDuration0.36s) so the frame budget and the drawing can't drift apart.On measurement
I originally opened this quoting a CPU improvement. I withdrew it: my before and after weren't taken under comparable load, so the comparison wasn't valid. When I measured both builds back to back under the same conditions, the difference was inside the sampling noise — with the panel collapsed and one session, only a single face is animating, so halving its frame rate moves very little.
I'd rather give you no number than one I can't stand behind. If you want a figure, the condition where this should show up is the panel expanded with several sessions, where the per-row cost multiplies. I don't have a rigorous measurement of that.
The argument I am making is the one above: a view that is invisible shouldn't be redrawn 120 times a second, and a frame rate shouldn't be pinned when the API takes a parameter for it.
Two things I'd flag myself
MoriEyesViewgains a@Stateproperty that is mutated but never read, purely to invalidate the view when a transient window expires so the rate can step back down. It works and it's commented at the declaration and the mutation site, but it's the piece I'd most want a second opinion on.Scope
Two files,
HaloView.swiftandMoriEyesView.swift. No overlap with #1 or #3 — I dropped an occlusion-pausing commit that touchedNotchApp.swift,AppModel.swiftandNotchIslandView.swift, both because it collided with your other open PRs and because I couldn't verify it actually fires (the panel is.canJoinAllSpaces+.fullScreenAuxiliary, so it stays visible over fullscreen apps). Happy to bring it back separately if you want it.Testing
swift buildclean. I could not runswift test— only Command Line Tools here, so XCTest is unavailable, and that fails identically on unmodifiedmain. Verified by hand that a cold launch animates immediately and the face keeps moving. Please lean on your CI.🤖 Generated with Claude Code