fix(studio): fit preview reset to composition dimensions#1085
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
Read the full NLEPreview.tsx, useTimelinePlayer.ts seek path, playbackSeek.ts, and composition-probe.ts. Both fixes are correct and well-tested; CI is fully green (Studio smoke + all regression shards). A few non-blocking cleanup notes.
Fix 1 — keep-playing seek resumes the adapter/RAF (the frozen-0:00 fix)
The extraction to playbackSeek.ts makes the decision testable, and the logic is sound across every case I traced:
keepPlaying && storeWasPlaying && forward && nextTime < duration→ activelyadapter.play()+startRAFLoop()+setIsPlaying(true). This is the real fix: previouslykeepPlayingonly preserved the store'sisPlayingflag and assumed the adapter+RAF were still live — which broke when the preview-focus path had already paused the adapter, leaving Pause-icon + frozen0:00. Now it re-syncs the adapter to the store state. The "restarts playback when the iframe adapter was paused" test pins exactly this.keepPlaying=false→shouldStopAfterSeektrue → stop. ✓keepPlayingfrom paused store → neither resume nor stop → stays paused (no spurious resume). ✓- reverse shuttle →
shouldStopAfterSeektrue → stop (reverse RAF can't survive a seek). ✓
hf#1076 adjacency (Home's flag): clean. The playhead clamp produces nextTime = max(0, min(duration, time)), and shouldResumeForwardPlaybackAfterSeek gates on nextTime < duration — so a seek clamped to the end won't spuriously resume. The clamp and the resume-check compose correctly; no conflict.
One micro-edge (not worth changing): keepPlaying + playing + nextTime === duration hits neither branch (no resume, no explicit stop). If the RAF was already live it self-terminates at the end; only matters in the already-paused-adapter case seeked exactly to end, which is degenerate.
Fix 2 — composition-dimension-driven fit
resolvePreviewStageSize is a correct contain-fit (try width-constrained, fall to height-constrained on overflow), now driven by the real composition aspect ratio with the legacy portrait prop as fallback. Verified Home's edges against the math: portrait (1080×1920) in a 496×386 space → height-constrained 217.125×386 ✓; landscape → width-constrained; composition dims correctly override the portrait prop. The probe fix (reading plain [data-width][data-height] roots, not only [data-composition-id]) is the right call for plain-HTML compositions.
Non-blocking observations
-
Duplicate composition-size readers.
readCompositionSizeFromDocument(player/composition-probe.ts) andreadPreviewCompositionSize(studio/NLEPreview.tsx) are near-identical (same[data-composition-id][data-width][data-height] ?? [data-width][data-height]fallback, same parse+validate). Studio already imports from player (import { Player }), so NLEPreview could call the exportedreadCompositionSizeFromDocument(iframe.contentDocument)inside its try/catch. Two copies will drift if the validation rules ever change. -
Stage size is read single-shot in
onLoad. Fine for authored-HTML composition roots (the hyperframes model — root is in the served markup at load). But the player'sCompositionProbealready discoverscompositionSizeand surfaces it viaonReady(and it polls until the adapter is ready, so it handles async roots). Threading the probe'scompositionSizeup toNLEPreviewinstead of re-reading ononLoadwould both dedupe (#1) and be robust to any composition that materializes its root after the load event. Design suggestion, not a bug for the current cases. -
fallow-ignore-next-line unused-class-memberonruntimeInjectedandresolveDirectTimelineAdapterFromWindowin composition-probe.ts — if these are genuinely unused now, prefer deleting over suppressing (suppressed-unused accumulates dead code); if they're public API consumed by tests, the ignore is fine. Quick confirm.
Verdict
Both fixes correct, root causes accurately diagnosed, regression coverage added at the right layers (probe / stage-size / keep-playing seek), and the browser-verification in the PR body matches the code paths. CI green. The three items above are cleanup, not blockers — James/Miguel to merge.
— Rames Jusso (hyperframes)
jrusso1020
left a comment
There was a problem hiding this comment.
Approving — both fixes are correct and the root causes accurately diagnosed. Fix 1 (keep-playing seek) actively re-syncs the iframe adapter + RAF loop to the store's play state, resolving the frozen-0:00/Pause-icon divergence, with the right regression test pinning the adapter-was-paused case; clean composition with the hf#1076 playhead clamp. Fix 2 (reset-to-fit) drives stage sizing from real composition dimensions via a correct contain-fit, verified for portrait/narrow/aspect-mismatch, with the probe now reading plain data-width/data-height roots. CI fully green (Studio smoke + all regression shards). The three notes in my prior review (duplicate composition-size reader, single-shot onLoad read vs the probe's async discovery, fallow unused-member suppressions) are non-blocking cleanup.
c3c0d11 to
af50679
Compare
af50679 to
dce9bd3
Compare
Problem
Issue #1080 covered two Studio playback/preview failures:
A/Eplayhead jumps should preserve the correct playback state while the timeline is already playing.After browser testing the PR locally, there was one more playback-state failure in the same user flow: pressing Play and then
Afrom the preview-focused path could leave the visible transport frozen at0:00while the control still showed Pause.What this fixes
portraitprop.data-width/data-heightroots, not only[data-composition-id]roots.Root cause
There were two separate root causes:
NLEPreviewcomputed its fit stage from aportraitprop that the main Studio preview path does not pass, while the player probe only treated[data-composition-id]elements as composition roots. Plain HTML compositions with validdata-width/data-heightcould therefore preview inside a default 16:9 stage.seek(..., { keepPlaying: true })preserved Studio's ZustandisPlayingstate but assumed the underlying iframe adapter and RAF loop were still running. In the preview-focus path, the adapter could already be paused, leaving Studio showing Pause while the runtime stayed frozen at0:00.Reset zoom was doing what it was told: resetting transform to 100%. The underlying stage dimensions were wrong. The frozen
0:00case was a real transport-state divergence, not just a misleading test project.Verification
Local checks
bun run --cwd packages/core test -- initbun run --cwd packages/player test -- composition-probe hyperframes-playerbun run --cwd packages/studio test -- useTimelinePlayer.seekbun run --cwd packages/studio test -- useTimelinePlayer.seek usePlaybackKeyboardbun run --cwd packages/studio test -- NLEPreview previewZoom usePlaybackKeyboard useTimelinePlayer.seekbun run --cwd packages/core typecheckbun run --cwd packages/player typecheckbun run --cwd packages/studio typecheckbunx oxlint packages/player/src/composition-probe.ts packages/player/src/composition-probe.test.ts packages/studio/src/components/nle/NLEPreview.tsx packages/studio/src/components/nle/NLEPreview.test.ts packages/studio/src/player/hooks/useTimelinePlayer.ts packages/studio/src/player/hooks/useTimelinePlayer.seek.test.ts packages/studio/src/player/lib/playbackSeek.tsbunx oxfmt --check packages/player/src/composition-probe.ts packages/player/src/composition-probe.test.ts packages/studio/src/components/nle/NLEPreview.tsx packages/studio/src/components/nle/NLEPreview.test.ts packages/studio/src/player/hooks/useTimelinePlayer.ts packages/studio/src/player/hooks/useTimelinePlayer.seek.test.ts packages/studio/src/player/lib/playbackSeek.tsbun run --cwd packages/core buildbun run --cwd packages/player buildbun run --cwd packages/studio buildgit diff --checkBrowser verification
Using
agent-browseragainst local Studio:1080x1920, but Studio/player preview iframe fit as a 16:9496x279stage in a narrow viewport.217.125x386, and inner iframe fit to217x385.78usingwidth: 1080px; height: 1920px; scale(0.200926).Afreeze inproduct-promo: parent transport showed Pause at0:00/0:20, while iframewindow.__player.isPlaying()was false and runtime time stayed at0.A, and observed the parent control stayed Pause while time advanced past0:00; CDP also showedwindow.__player.isPlaying() === trueand runtime time advancing.agent-browser.Local proof artifacts:
/tmp/hyperframes-1080-artifacts/verified-portrait-fit-final.png/tmp/hyperframes-1080-artifacts/verified-reset-after-fix.png/tmp/hyperframes-1080-artifacts/verified-playhead-a-after-fix.png/tmp/hyperframes-1080-artifacts/issue-1080-browser-proof.webm/tmp/hyperframes-pr-1085-test/screenshot-1779847819316.png/tmp/hyperframes-pr-1085-test/play-a-continues.webmNotes
Temporary Studio repro projects and generated
.thumbnailsdirectories were removed before committing. No generated runtime/player/studio build artifacts are committed.Closes #1080