Repository navigation
Conversation
ryanbr
left a comment
There was a problem hiding this comment.
The background CoreMotion stop is the real win here. 60 Hz accelerometer+gyro fusion running behind the
lock screen — because onDisappear never fires when backgrounded and bluetooth-central keeps the app
alive — is a battery cost with no picture attached, and it would never show up in an idle-screen
measurement. Good catch, and the honest note that the sensor saving is NOT in the CPU table (the
simulator has no device motion) is the right way to report it.
Verifying the census test in both directions — removing the RecordingStatusLight gate fails it, and
aliasing poseStill back to bare reduceMotion fails the call-site check — is what makes it a guard
rather than decoration.
One correction needed, in the description rather than the code.
Key
noop.quietMotion, mirroring KotlinNoopPrefs.quietMotion
There is no such Kotlin pref. NoopPrefs (MainActivity.kt) has noop.onboarded,
noop.backgroundConnection, the terms keys — no quietMotion — and Android's gate is still
rememberPoseStill() = rememberReduceMotion() || rememberPowerSaveMode(), two signals with no in-app
toggle.
So this ships a user-facing setting on Apple only. That is fine as a decision — Apple was behind Android
on this gate and is now ahead on one sub-feature — but the line reads as "parity already satisfied",
which is the one thing that stops anyone from noticing. Please correct it and open the Android
follow-up, since the user who asked for this presumably wants it on whichever platform he is on.
The key name is right for when it lands: noop.quietMotion matches the noop. prefix the Kotlin keys
already use, so the two will match without renaming.
Not a whitelist concern, for the record: .noopbak carries profile/units/effort only, so a UI motion
preference correctly stays out of it.
Also needs a rebase — main has moved several times today.
…#909)
inside `Strand/Liquid` and so could only ever reach the liquid layer. Two things it
did not reach:
**1. The tilt sensor kept running.** `LiquidMotion` starts `CMMotionManager` device
motion at **60 Hz** — accelerometer + gyro sensor fusion — to drive the decorative
slosh, the "the bubbles shift when you tilt the phone" effect. #909 poses the Canvas
still, and because `acquire()` sits on the animated branch the sensor did stop when a
gauge posed. But `onDisappear` is NOT called when the app is backgrounded, and NOOP
declares `bluetooth-central` + `location` background modes, so the app keeps RUNNING
behind the lock screen while a strap is connected. The 60 Hz fusion kept being
delivered all day, for a picture nobody could see. `LiquidMotion` now stops at the
app boundary and re-arms on foreground, and `startIfWanted()` consults the gate
itself rather than trusting every call site to route around it.
**2. Everything outside the liquid layer.** A census (`QuietMotionCoverageTests`,
the Apple twin of #911's `PoseStillCoverageTest`) found five more never-settling
loops, none of which #909 covered:
- `TimeOfDayBackground` atmosphere drift — the Android twin has consulted battery
saver since #911; this side read Reduce Motion alone.
- `LiveSessionView`'s guardian breath (`repeatForever`).
- `ConnectionDot` (StatePill) and `PulseDot` (Components) halos — on screen for
long stretches: a connected strap in Settings, a backfill on every scaffold.
- `RecordingStatusLight`'s sync ring, which had **no motion gate at all** — it
pulsed under system Reduce Motion too, which was already a bug, and it ran
precisely while the strap was offloading history. (Exactly the bug #911 found in
the Android ConnectionDot.) The steady accent dot still says "syncing"; only the
loop stops.
- The onboarding glows and the radar sweep.
The gate moves out of `Strand/Liquid` and into `StrandDesign` as `NoopMotionState`,
so the design system's own surfaces can reach it — the Android twin has been in
`com.noop.ui.NoopMotion` since #911, read through `rememberPoseStill()`. Same shape:
one process-wide monitor, one observer, never per-view (the primitives alone have
dozens of call sites, and registering an observer as gauges scroll in and out would
be churn introduced by the change that exists to remove per-frame work).
**Three signals, OR-ed:** system Reduce Motion, Low Power Mode, and a new in-app
"Reduce motion in NOOP" toggle (Settings → Appearance, default OFF). The third is
the point of this change for the user who asked for it: he wants the app quiet
without putting the whole phone in battery saver. Key `noop.quietMotion`, mirroring
Kotlin `NoopPrefs.quietMotion`; live via `UserDefaults.didChangeNotification`, so
flipping it stops the sensor and poses the surfaces without leaving the screen.
iPhone 17 Pro simulator seeded with a real 746 MB store, Today idle, no touch input.
Three 15 s CPU windows after the first-run analysis had fully plateaued (that takes
~340 s at ~110% of a core on a freshly seeded DB — measuring before it settles is
what makes an idle number meaningless). Debug build.
| Today idle | CPU (% of one core) | RSS | load1 |
|---|---|---|---|
| animating (nothing asks for quiet) | 18.9 / 19.8 / 18.9 | 192 MB | 3.8 |
| **"Reduce motion in NOOP" ON** | **0.0 / 0.0 / 0.0** | 97-122 MB | 4.6 |
| system Reduce Motion ON (the #909 path) | 0.0 / 0.0 / 0.0 | 157-182 MB | 4.2 |
| animating, re-checked | 16.8 / 16.1 / 15.2 | 99-115 MB | 9.2-13.4 |
The last row is the not-in-quiet behaviour check: still animating, unchanged. It
reads lower than the first only because the machine was under 2-3x the load — per
process CPU-seconds are depressed by contention, so the two animating rows bracket
the real figure rather than either being it.
Two honest limits on those numbers:
- I could not toggle real Low Power Mode in the simulator. The Low Power row is
the Reduce Motion route, which is the same `staticGauge` / `LiquidSkyStatic` /
`paused:` branch low-power users take. What is inferred, not exercised, is only
that iOS reports the flag — which `IOSDiagnostics` already relies on.
- **The sensor saving is not in that table.** `isDeviceMotionAvailable` is false in
the simulator, so the 60 Hz CoreMotion feed never ran in any of these runs. The
interval it requests (`deviceMotionUpdateInterval = 1/60`) and the missing
background stop are read off the code, not measured. On device it is additional
to the figures above, not included in them.
Scroll was not re-measured: the earlier investigation could not reproduce the user's
jank in the simulator at all (zero `body` rebuilds during driven scrolls, 60 fps),
and the simulator is 60 Hz where his iPhone 16 Pro is 120 Hz with an 8.33 ms budget.
Any claim that this fixes scrolling would be simulator-only and unsupported.
- One-shot animations (`CountUpText`, `staggeredAppear`, `LiquidPressStyle`).
They settle and stop, so they are not the cost battery saver asks an app to
avoid — same line #911 drew.
- `TimelineView(.periodic…)` clocks. A 1 s elapsed-time label is not per-frame
drawing.
- The classic Today top-bar "Swipe / Tap" hint cycler. Two runloop wakeups per
11.5 s, and it carries a discoverability affordance rather than decoration —
removing it would cost information, not motion.
- `BreathingView` / `WatchBreatheView` run a 20 Hz `Timer.publish` even when not
breathing. Real, but bounded by that screen being on-screen, and fixing it means
restructuring the publisher rather than adding a gate. Left for its own change.
`QuietMotionCoverageTests` passes here, and **fails against the ungated tree** —
checked in both directions: removing the gate from `RecordingStatusLight` fails the
loop census by file and line, and aliasing `poseStill` back to bare `reduceMotion` in
`ConnectionDot` fails the breathe-call-site census (the call-site check requires the
file to reach `NoopMotionState`, not merely to contain the token). It fails rather
than skips when the repo root is unlocatable, and it pins that the gate still reads
all three signals and stays live — losing one half is invisible, since the screen
looks right in whichever mode still works.
`swift build` clean for StrandDesign; `xcodebuild` clean for NOOPiOS (simulator) and
the macOS Strand target; `Tools/i18n_audit.py --ci` green with the new Settings copy
translated into all eight catalog locales.
…ion (#941) Three comments claimed the new `noop.quietMotion` key mirrored an existing Kotlin pref `NoopPrefs.quietMotion`. It does not exist — `grep -r quietMotion android/` returns nothing, and Android's gate is still the two-signal `rememberPoseStill() = rememberReduceMotion() || rememberPowerSaveMode()` (`NoopMotion.kt:149`). The wording was worse than a stale reference. `NoopPrefs` DOES exist, so a reader checking the claim finds the object and stops; and the repo's parity contract means "mirrors Kotlin X" reads as "parity already satisfied", which is exactly what stops anyone noticing that it is not. The name was not even spelled the way that file spells things — `NoopPrefs` stores `KEY_*` constants, so the eventual Kotlin symbol is `KEY_QUIET_MOTION`, not `quietMotion`. All three now say the third signal is Apple-only, name the two signals Android actually reads, and point at #941 for the follow-up. The key STRING stays pinned in `QuietMotionCoverageTests` with the reason made explicit: it is the cross-platform contract because `.noopbak` carries the setting by key rather than by symbol name, so Android must adopt it verbatim and an edit here would silently break a round-trip that by then has users. No behaviour change. StrandDesign: 36 tests, 0 failures.
d33e714 to
f6dfee6
Compare
|
You are right, and the framing is the part I want to acknowledge, not just the fact. // android/app/src/main/java/com/noop/ui/NoopMotion.kt:149
fun rememberPoseStill(): Boolean = rememberReduceMotion() || rememberPowerSaveMode()Corrected, follow-up opened as #941, and rebased onto It was worse than I realised when I wrote it, in two ways. First, the claim was in three source comments, not only the PR description — Second, your "the one thing that stops anyone from noticing" is precisely the mechanism. The tell I should have caught myself: the name is not even spelled the way that file spells things. What the key string still is, and why it stays pinned. What I did not change, because it is true: the gate moving into #941 carries the exact work: the Verified after the rebase: |
|
Housekeeping only — this is approved, clean and green, and the one commit pushed after your approval (
The reason for it: the old wording claimed parity with a Kotlin symbol that does not exist ( |
ryanbr
left a comment
There was a problem hiding this comment.
Re-checked against current main. The substance holds up; two things stand between it and merging.
I verified the load-bearing claims rather than taking them, and they're all true: LiquidCore does set deviceMotionUpdateInterval = 1.0/60.0 and start device motion; project.yml does declare bluetooth-central and location, so the app really does keep running behind the lock screen with a strap connected; Android's NoopMotion.rememberPoseStill() really is the two-signal rememberReduceMotion() || rememberPowerSaveMode(); and RecordingStatusLight on main really does carry nothing but an accessibilityLabel — no gate of any kind, so it pulsed under system Reduce Motion too. Your own correction is right as well: quietMotion appears in zero files under android/.
QuietMotionCoverageTests is the strongest part of this. I went looking for the failure mode these census tests usually have — scanning nothing and passing — and it's closed on both ends: repoRoot() throws rather than skips when project.yml isn't found, and the census asserts it scanned more than 100 files. Stripping // comments so a marker described in prose isn't censused is the right call too. Putting it in StrandDesign also means it runs in CI, which is worth more than usual here: app-build.yml is disabled, so Strand/Liquid/, TodayView, SettingsView and OnboardingWizard are otherwise compiled by nothing at all. This is the only automated reach into those files.
Blocking: it needs a rebase. CONFLICTING DIRTY against current main, but it's narrow — Strand/Resources/Localizable.xcstrings is the only conflict, from parallel string additions. SettingsView.swift and TodayView.swift auto-merge cleanly. i18n_audit.py --ci main passes on the branch as it stands.
Needs a decision: the toggle is iOS-only. "Reduce motion in NOOP" gives Apple users a third signal Android users have no way to reach — Android is still the two-signal gate. The parity rule wants the twin moved in the same PR or a reason it isn't, and the description documents the gap without resolving it. Either add NoopPrefs.quietMotion + the Settings row + a third term in rememberPoseStill() here, or say it's a follow-up and I'll open the issue. I'd lean follow-up — this is already +548 across 14 files and the Android half is self-contained.
One nit, non-blocking: observing UserDefaults.didChangeNotification app-wide to catch one key means syncToPolicy() runs on every defaults write anywhere in the app. It's harmless — stop() and startIfWanted() both guard on started, and quietNow is three cheap reads — but filtering on the key (or KVO on it) would sit better in a change whose whole point is not doing work nobody asked for.
|
The conflict is cleared — vishk23/noop#14 merges current main into It was mechanical: both sides only added catalogue keys, nothing edited on both (base 3283, you +2, main +40). So the resolution is main's catalogue in main's order plus your two keys, which already carried all eight locales. 105 lines added, 1 removed; every other file auto-merged. Also worth flagging, since it changes what "approved" covered here: app-build was disabled from 21 July until today, so this branch's earlier checks never compiled Nothing of yours changed; your 15 files still contribute +552/-63. The only open question from the review is unchanged and not a blocker: the "Reduce motion in NOOP" toggle is Apple-only while Android's gate stays two-signal. Happy to take that as a follow-up issue unless you would rather move the twin here. |
refs/pull/<n>/merge is frozen at the last time the PR merged CLEANLY, so for a conflicting or stale PR it still resolves - to a merge against an OLD main. Probing #931 through it would have built main as of 30 July, 30+ commits behind current, and reported SUCCESS. A false green is worse than the throwaway branch this replaces, which at least merged real main. Checks out refs/pull/<n>/head and merges origin/main in the runner instead, so the result is always against main as it is right now, and a conflicting PR fails at the merge step - before any xcodebuild - instead of compiling something nobody asked for.
…ilt sensor (#909) (#931) LiquidMotion ran CMMotionManager device motion at 60 Hz to drive the decorative slosh. onDisappear does not fire when the app is backgrounded, and NOOP declares bluetooth-central + location, so with a strap connected that sensor fusion kept being delivered all day behind the lock screen for a picture nobody could see. It now stops at the app boundary and re-arms on foreground, and startIfWanted() consults the gate itself rather than trusting call sites. A census (QuietMotionCoverageTests, the Apple twin of #911's PoseStillCoverageTest) found five more never-settling loops #909 never reached: the day-cycle atmosphere drift, the guardian breath, the ConnectionDot and PulseDot halos, the onboarding glows and radar sweep, and RecordingStatusLight's sync ring - which had no motion gate at all, so it pulsed under system Reduce Motion too, while the strap was offloading history. The gate moves into StrandDesign as NoopMotionState so the design system's own surfaces can reach it, reading three OR-ed signals: system Reduce Motion, Low Power Mode, and a new in-app "Reduce motion in NOOP" toggle (default OFF). The census test lives in the StrandDesign package deliberately: swift-packages runs it by default, and it fails loudly rather than skipping if it cannot find the tree or scans too few files - so it cannot pass by censusing nothing. Known asymmetry: the in-app toggle is Apple-only; Android's gate stays the two-signal rememberPoseStill(). Follow-up rather than a blocker.
|
Merged as 838f582, authored to you. The only thing left was the Verified before landing: The census test is the part of this I'd point other contributors at. It fails loudly if it can't find the tree or scans too few files, so it can't pass by censusing nothing — which is the failure mode these usually have. One thing deliberately left: the in-app "Reduce motion in NOOP" toggle is Apple-only, Android's gate stays the two-signal |
…ilt sensor (ryanbr#909) (ryanbr#931) LiquidMotion ran CMMotionManager device motion at 60 Hz to drive the decorative slosh. onDisappear does not fire when the app is backgrounded, and NOOP declares bluetooth-central + location, so with a strap connected that sensor fusion kept being delivered all day behind the lock screen for a picture nobody could see. It now stops at the app boundary and re-arms on foreground, and startIfWanted() consults the gate itself rather than trusting call sites. A census (QuietMotionCoverageTests, the Apple twin of ryanbr#911's PoseStillCoverageTest) found five more never-settling loops ryanbr#909 never reached: the day-cycle atmosphere drift, the guardian breath, the ConnectionDot and PulseDot halos, the onboarding glows and radar sweep, and RecordingStatusLight's sync ring - which had no motion gate at all, so it pulsed under system Reduce Motion too, while the strap was offloading history. The gate moves into StrandDesign as NoopMotionState so the design system's own surfaces can reach it, reading three OR-ed signals: system Reduce Motion, Low Power Mode, and a new in-app "Reduce motion in NOOP" toggle (default OFF). The census test lives in the StrandDesign package deliberately: swift-packages runs it by default, and it fails loudly rather than skipping if it cannot find the tree or scans too few files - so it cannot pass by censusing nothing. Known asymmetry: the in-app toggle is Apple-only; Android's gate stays the two-signal rememberPoseStill(). Follow-up rather than a blocker.
perf(motion): make Low Power Mode mean "stop everything decorative" (#909)
#909 taught the live gauges to pose still in Low Power Mode, but it read the flag
inside
Strand/Liquidand so could only ever reach the liquid layer. Two things itdid not reach:
1. The tilt sensor kept running.
LiquidMotionstartsCMMotionManagerdevicemotion at 60 Hz — accelerometer + gyro sensor fusion — to drive the decorative
slosh, the "the bubbles shift when you tilt the phone" effect. #909 poses the Canvas
still, and because
acquire()sits on the animated branch the sensor did stop when agauge posed. But
onDisappearis NOT called when the app is backgrounded, and NOOPdeclares
bluetooth-central+locationbackground modes, so the app keeps RUNNINGbehind the lock screen while a strap is connected. The 60 Hz fusion kept being
delivered all day, for a picture nobody could see.
LiquidMotionnow stops at theapp boundary and re-arms on foreground, and
startIfWanted()consults the gateitself rather than trusting every call site to route around it.
2. Everything outside the liquid layer. A census (
QuietMotionCoverageTests,the Apple twin of #911's
PoseStillCoverageTest) found five more never-settlingloops, none of which #909 covered:
TimeOfDayBackgroundatmosphere drift — the Android twin has consulted batterysaver since perf(ui): pose the animating surfaces still in battery saver (#909) #911; this side read Reduce Motion alone.
LiveSessionView's guardian breath (repeatForever).ConnectionDot(StatePill) andPulseDot(Components) halos — on screen forlong stretches: a connected strap in Settings, a backfill on every scaffold.
RecordingStatusLight's sync ring, which had no motion gate at all — itpulsed under system Reduce Motion too, which was already a bug, and it ran
precisely while the strap was offloading history. (Exactly the bug perf(ui): pose the animating surfaces still in battery saver (#909) #911 found in
the Android ConnectionDot.) The steady accent dot still says "syncing"; only the
loop stops.
The gate moves out of
Strand/Liquidand intoStrandDesignasNoopMotionState,so the design system's own surfaces can reach it — the Android twin has been in
com.noop.ui.NoopMotionsince #911, read throughrememberPoseStill(). Same shape:one process-wide monitor, one observer, never per-view (the primitives alone have
dozens of call sites, and registering an observer as gauges scroll in and out would
be churn introduced by the change that exists to remove per-frame work).
Three signals, OR-ed: system Reduce Motion, Low Power Mode, and a new in-app
"Reduce motion in NOOP" toggle (Settings → Appearance, default OFF). The third is
the point of this change for the user who asked for it: he wants the app quiet
without putting the whole phone in battery saver. Key
noop.quietMotion; live viaUserDefaults.didChangeNotification, so flipping it stops the sensor and poses thesurfaces without leaving the screen.
Measured
iPhone 17 Pro simulator seeded with a real 746 MB store, Today idle, no touch input.
Three 15 s CPU windows after the first-run analysis had fully plateaued (that takes
~340 s at ~110% of a core on a freshly seeded DB — measuring before it settles is
what makes an idle number meaningless). Debug build.
The last row is the not-in-quiet behaviour check: still animating, unchanged. It
reads lower than the first only because the machine was under 2-3x the load — per
process CPU-seconds are depressed by contention, so the two animating rows bracket
the real figure rather than either being it.
Two honest limits on those numbers:
the Reduce Motion route, which is the same
staticGauge/LiquidSkyStatic/paused:branch low-power users take. What is inferred, not exercised, is onlythat iOS reports the flag — which
IOSDiagnosticsalready relies on.isDeviceMotionAvailableis false inthe simulator, so the 60 Hz CoreMotion feed never ran in any of these runs. The
interval it requests (
deviceMotionUpdateInterval = 1/60) and the missingbackground stop are read off the code, not measured. On device it is additional
to the figures above, not included in them.
Scroll was not re-measured: the earlier investigation could not reproduce the user's
jank in the simulator at all (zero
bodyrebuilds during driven scrolls, 60 fps),and the simulator is 60 Hz where his iPhone 16 Pro is 120 Hz with an 8.33 ms budget.
Any claim that this fixes scrolling would be simulator-only and unsupported.
Not gated, deliberately
CountUpText,staggeredAppear,LiquidPressStyle).They settle and stop, so they are not the cost battery saver asks an app to
avoid — same line perf(ui): pose the animating surfaces still in battery saver (#909) #911 drew.
TimelineView(.periodic…)clocks. A 1 s elapsed-time label is not per-framedrawing.
11.5 s, and it carries a discoverability affordance rather than decoration —
removing it would cost information, not motion.
BreathingView/WatchBreatheViewrun a 20 HzTimer.publisheven when notbreathing. Real, but bounded by that screen being on-screen, and fixing it means
restructuring the publisher rather than adding a gate. Left for its own change.
Verified
QuietMotionCoverageTestspasses here, and fails against the ungated tree —checked in both directions: removing the gate from
RecordingStatusLightfails theloop census by file and line, and aliasing
poseStillback to barereduceMotioninConnectionDotfails the breathe-call-site census (the call-site check requires thefile to reach
NoopMotionState, not merely to contain the token). It fails ratherthan skips when the repo root is unlocatable, and it pins that the gate still reads
all three signals and stays live — losing one half is invisible, since the screen
looks right in whichever mode still works.
swift buildclean for StrandDesign;xcodebuildclean for NOOPiOS (simulator) andthe macOS Strand target;
Tools/i18n_audit.py --cigreen with the new Settings copytranslated into all eight catalog locales.