Render the reduced-motion frame at raw engine time - #12
Open
andresain123 wants to merge 1 commit into
Open
Conversation
OrbSpec.reducedMotionT is already engine time — spec.paint.clock defines engine time as elapsedSeconds * presetSpeed * userSpeed, which is why the live TimelineView multiplies by effSpeed on the way in. The static branch multiplied it a second time, so every reduced-motion and paused frame rendered a different instant than the web build, which paints a bare frame(0.6). No preset has speed 1.0 — they run 1.625 to 6.63 — so all 18 (mode × size) pairs were affected; web@20 rendered t = 3.978 where the web renders 0.6. The frozen-time branch directly above already stated this rule and got it right. ReducedMotionParityTests renders both paths through the real SwiftUI Canvas and compares rasters, so the guard fails on painted output rather than on a recomputed number. Before the fix all 18 pairs differed by 4.3–32.5% of pixels; after, they are byte-identical. The branch previously had no coverage at all: the golden tests call orbFrame directly and the snapshot tests go through .orbFrozenTime, so neither ever rendered it.
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
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.
ThinkingOrbsKitrenders the reduced-motion / paused static frame at adifferent instant than the web build, for every state and size.
The two branches disagree
ports/ios/ThinkingOrbsKit/Sources/ThinkingOrbsKit/ThinkingOrb.swift:64-72Same rule, two answers. The comment on the second states the intent — "same
instant as the web" — that the code then breaks.
The web reference does not scale it (
src/ThinkingOrb.tsx:90-99):0.6is already engine time.spec.paint.clockdefines engine time aselapsedSeconds * presetSpeed * userSpeed, which is why the live loopmultiplies on the way in and the constant must not be multiplied again.
Measured
pausedreaches the identical line asreduceMotion, so the test needs nofaked accessibility environment. Both paths render through the real SwiftUI
CanvasviaImageRendererand the rasters are compared, so the guard failson painted output rather than on a recomputed number — headless, no simulator.
Before, all 18 (state × size) pairs differ:
No preset has speed 1.0 — they run 1.625 to 6.63 — so nothing is exempt.
connecting@20renders t = 3.978 where the web renders 0.6.After removing
* effSpeed, all 18 are byte-identical (0.00%, maxD 0), andthe existing suite is unchanged:
Also confirmed on a device:
ThinkingOrbsDemoon an iPhone 17 Pro simulatorwith Reduce Motion enabled in Settings, one simulator and one setting, only
ThinkingOrbsKitrebuilt between two captures. The frozen frame moves by5-25% of pixels per orb. The screen was verified genuinely static first — 0 of
3162132 pixels changed across 14 seconds.
Why the existing suite missed it
0.6is one of the four timestamps inspec/orbs-golden.json, picked so thisframe is verifiable — but
OrbGoldenTestsevaluatesorbFramedirectly andOrbSnapshotTestsgoes through.orbFrozenTime. Neither ever renders thereduceMotion || pausedbranch, so it had no coverage at all.ReducedMotionParityTestscloses that by comparing the two paths against eachother, so it stays honest if
reducedMotionTis ever retuned.Severity is parity, not breakage — t = 1.989 is still a valid pose. But it is
the pose reduced-motion users see, it disagrees with every other platform, and
pausedinherits it.