From 6d9cc301a747421cfd27254706e4e85ff2658591 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 19 Oct 2026 13:40:30 +0000 Subject: [PATCH] Turn the phone in one step, not through every shape A turn is not one shape change: the viewport passes through every shape between the one it left and the one it lands in, and each of them re-letterboxes the picture somewhere else. Painted, that run of hard jumps is the stutter. So for the length of a turn the slides are held at the size they already had, and the moment the viewport stops moving they take the size it ended at -- measured, not guessed. The picture moves exactly once, straight into the position and size it then keeps. The device's own `orientationchange` starts the hold, which is the earliest anything can know a turn is coming and the one signal that separates it from a window being dragged wider; failing that, the viewport swapping its long side for its short one does. A viewport that only changes height is the browser's chrome sliding away, and is never held. Underneath: snapping is off while the slides are held, so the container cannot re-anchor on the old height at the same time; a scroll arriving during the window is put back in the event rather than a frame later, so the stale position is never painted; and the per-frame realign only writes what actually differs instead of costing a layout sixty times a second. Measured over a recorded turn: the picture moves 12 times -> 2, once per turn; the worst of those was a 660px jump between two intermediate shapes and is now the one step into the final size; the reel is never off its step (was 660px out); and the clip never pauses. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MPZFnNdHbPGDTqQUNiE4ZN --- frontend/css/style.css | 21 +++ frontend/js/feed.js | 149 +++++++++++++++++++++- tests/smoke_rotate_smooth.py | 240 +++++++++++++++++++++++++++++++++++ 3 files changed, 403 insertions(+), 7 deletions(-) create mode 100644 tests/smoke_rotate_smooth.py diff --git a/frontend/css/style.css b/frontend/css/style.css index d334ba2..b46319e 100644 --- a/frontend/css/style.css +++ b/frontend/css/style.css @@ -2236,6 +2236,27 @@ body.feed-mode-open .mode-toggle-btn .icon-svg { height: 1px; } +/* A turn goes through every shape between the one it left and the one it + lands in, and each of those re-letterboxes the picture somewhere else -- + a run of jumps where there should be one. So for the length of the turn + the slides are held at the size it is heading for (feed.js sets the two + numbers): the picture moves once, straight into its final position and + size, and stays there while the viewport catches up to it. */ +.feed-view.is-turning .feed-slide { + width: var(--turn-w); + height: var(--turn-h); +} + +.feed-view.is-turning .feed-scroll { + /* The pinned slides are a different shape from the viewport until the turn + finishes, and none of that overflow is for scrolling through. Snapping + is off with it: the container would otherwise be re-anchoring on the old + height at the same time as the feed is pinning itself to the new one. */ + overflow: hidden; + scroll-snap-type: none; + scroll-behavior: auto; +} + /* Stands in for the slides above the rendered window so scroll position and snap points stay stable while slides are virtualized in and out. */ .feed-top-spacer { diff --git a/frontend/js/feed.js b/frontend/js/feed.js index 4cd0c31..fd0d063 100644 --- a/frontend/js/feed.js +++ b/frontend/js/feed.js @@ -60,6 +60,35 @@ App.feed = App.feed || {}; let settleStableFrames = 0; let settleHeight = -1; let settleDone = null; // run once the window closes, if anything asked + let lastMeasuredHeight = -1; + + // A turn is not just another resize. The viewport passes through every + // shape between the one it left and the one it lands in, and each one + // re-letterboxes the picture somewhere else: that run of hard jumps is + // what reads as a stutter. So for the length of the turn the slides are + // held at the size they already had, and the moment the viewport stops + // moving they take the size it ended at -- measured, not guessed, so the + // picture moves exactly once, straight into the place it then keeps. + // Only a real turn gets that: the browser's own chrome sliding away moves + // the viewport too, and holding for those would be worse than the jump it + // saves. + // + // `orientationchange` fires before the new metrics are readable, which is + // the earliest anything can know a turn is coming -- early enough to pin + // before the first new shape is painted. It is also the one signal that + // separates a turn from a window being dragged wider on a desktop. + const TURN_SIGNAL_MS = 1200; + // The hold comes off once the viewport has stopped moving -- sooner than + // the realign window closes, which goes on holding the reel in place for + // a while after that in case a last stale scroll arrives. Long enough + // that a viewport animating round in steps is not mistaken for one that + // has arrived, short enough not to be a wait. + const TURN_HOLD_FRAMES = 6; + let turnSignalAt = 0; + let turnPin = null; // the shape the slides are held at, mid-turn + let turnResolved = false; + let settleFromShape = null; // the shape the viewport had before this turn + let lastShape = null; // HUD auto-hide: the reels HUD fades out after this much inactivity and // reappears on any pointer movement / tap / scroll. Buttons keep their @@ -137,6 +166,10 @@ App.feed = App.feed || {}; const getTopSpacer = () => inRoot('feed-top-spacer'); const slideHeight = function() { + // Mid-turn the slides are the size the turn is heading for, not the + // size the viewport happens to be passing through, and every scroll + // position in the feed is a multiple of a slide. + if (turnPin) return turnPin.h; const scroller = getScroller(); const view = feedDocument().defaultView || window; return (scroller && scroller.clientHeight) || view.innerHeight || 1; @@ -1281,6 +1314,14 @@ App.feed = App.feed || {}; }; const onScroll = function() { + // A turn's re-anchor arrives as a scroll event carrying a position + // measured against the height from before it. Putting it back here, + // in the event, is what keeps that position from being painted -- + // waiting for the next frame means showing it first. + if (suppressScroll) { + realignToActive(); + return; + } if (scrollRaf) return; scrollRaf = requestAnimationFrame(() => { scrollRaf = null; @@ -1301,6 +1342,48 @@ App.feed = App.feed || {}; }); }; + const viewportShape = function() { + const scroller = getScroller(); + const view = feedDocument().defaultView || window; + return { + w: (scroller && scroller.clientWidth) || view.innerWidth || 1, + h: (scroller && scroller.clientHeight) || view.innerHeight || 1 + }; + }; + + // What a wrist did, as against what the browser's chrome did: the device + // said so, or the viewport swapped its long side for its short one. A + // viewport that only changes height -- chrome sliding away, a keyboard + // coming up -- is not a turn and is not worth a fade. + const isTurn = function(before, now) { + if (Date.now() - turnSignalAt < TURN_SIGNAL_MS) return true; + if (!before) return false; + return (before.w > before.h) !== (now.w > now.h); + }; + + // Holds the slides at `shape` -- the size they have as the turn starts -- + // until the viewport has finished moving. Where a turn arrives in one + // step, which is most of them, this lasts a few frames and changes + // nothing anybody sees. + const pinTurn = function(shape) { + turnPin = { w: shape.w, h: shape.h }; + const root = getRoot(); + if (!root) return; + root.style.setProperty('--turn-w', `${turnPin.w}px`); + root.style.setProperty('--turn-h', `${turnPin.h}px`); + root.classList.add('is-turning'); + }; + + const releaseTurn = function() { + if (!turnPin) return; + turnPin = null; + const root = getRoot(); + if (!root) return; + root.classList.remove('is-turning'); + root.style.removeProperty('--turn-w'); + root.style.removeProperty('--turn-h'); + }; + // Re-anchors the scroll position on the currently active video after the // viewport changes. The active slide is resolved by id (not by a possibly // stale scroll position) so an orientation change always keeps the same @@ -1318,12 +1401,23 @@ App.feed = App.feed || {}; state.feedActiveIndex = index; const h = slideHeight(); const start = windowBounds(index).start; + // Only write what actually differs. This runs on every frame of a + // turn, and a spacer height or a scrollTop written back unchanged + // still costs a layout and still sets the snap container off again -- + // the page fighting itself at the moment it is busiest. const spacer = getTopSpacer(); - if (spacer) spacer.style.height = `${start * h}px`; + const spacerHeight = `${start * h}px`; + if (spacer && spacer.style.height !== spacerHeight) spacer.style.height = spacerHeight; const scroller = getScroller(); - if (scroller) scroller.scrollTop = index * h; + const top = index * h; + if (scroller && Math.abs(scroller.scrollTop - top) > 0.5) scroller.scrollTop = top; const activeSlide = slidesByIndex.get(index); - if (activeSlide) measureFeedTitle(activeSlide); + // The title only needs re-measuring when the screen it has to fit in + // changed size, not sixty times while it doesn't. + if (activeSlide && h !== lastMeasuredHeight) { + lastMeasuredHeight = h; + measureFeedTitle(activeSlide); + } }; // Ends the realign window: one last realign, then real swipes count again. @@ -1336,8 +1430,10 @@ App.feed = App.feed || {}; clearTimeout(resizeSettleTimer); resizeSettleTimer = null; } + releaseTurn(); realignToActive(); suppressScroll = false; + lastShape = viewportShape(); const after = settleDone; settleDone = null; if (after) after(); @@ -1351,12 +1447,27 @@ App.feed = App.feed || {}; suppressScroll = false; return; } - const height = slideHeight(); - if (height === settleHeight) settleStableFrames += 1; + // The viewport's own height, not the pinned one: the pin is a fixed + // number and would look settled from the first frame. + const shape = viewportShape(); + if (shape.h === settleHeight) settleStableFrames += 1; else { - settleHeight = height; + settleHeight = shape.h; settleStableFrames = 0; } + // A turn arrives in several steps, and the first of them can be small + // enough to pass for the chrome sliding; it is the shape it ends up in + // that says what it was. + if (!turnPin && !turnResolved && isTurn(settleFromShape, shape)) { + pinTurn(settleFromShape); + } + // The viewport has stopped moving, so what it is now is better than + // what the pin guessed it would be. Letting go here, with the realign + // below in the same frame, is the one step the picture takes. + if (turnPin && settleStableFrames >= TURN_HOLD_FRAMES) { + releaseTurn(); + turnResolved = true; + } realignToActive(); if (settleStableFrames < SETTLE_STABLE_FRAMES && (Date.now() - settleStartedAt) < SETTLE_MAX_MS) { @@ -1368,14 +1479,36 @@ App.feed = App.feed || {}; resizeSettleRaf = requestAnimationFrame(finishSettle); }; + // The device announcing the turn: dim first, measure after. Nothing about + // the viewport has changed yet at this point, so the reel is already on + // its way out when the first new shape lands. + const onOrientation = function() { + turnSignalAt = Date.now(); + turnResolved = false; + if (state.feedOpen && state.feedActiveIndex >= 0 && !turnPin) { + pinTurn(viewportShape()); + } + onResize(); + }; + const onResize = function() { if (!state.feedOpen || state.feedActiveIndex < 0) return; // Suppress scroll handling while we realign so the container's re-snap // doesn't flip the active video, then re-enable it once layout settles. suppressScroll = true; + // The shape to compare against is the one from before the turn + // started, not the one from the previous step of it. + if (!resizeSettleTimer) { + settleFromShape = lastShape || viewportShape(); + turnResolved = false; + } settleStartedAt = Date.now(); settleStableFrames = 0; settleHeight = -1; + lastMeasuredHeight = -1; + if (!turnPin && !turnResolved && isTurn(settleFromShape, viewportShape())) { + pinTurn(settleFromShape); + } realignToActive(); if (resizeSettleRaf) cancelAnimationFrame(resizeSettleRaf); resizeSettleRaf = requestAnimationFrame(settleStep); @@ -1765,6 +1898,7 @@ App.feed = App.feed || {}; }); slidesByIndex.clear(); forgetPeeks(); + releaseTurn(); resumeTimes.clear(); state.feedActiveIndex = -1; state.feedActiveVideoId = null; @@ -1800,6 +1934,7 @@ App.feed = App.feed || {}; container.classList.add('open'); container.setAttribute('aria-hidden', 'false'); + lastShape = viewportShape(); document.body.classList.add('feed-mode-open'); document.body.style.overflow = 'hidden'; @@ -1810,7 +1945,7 @@ App.feed = App.feed || {}; // the visual viewport moves on its own as the browser's chrome // slides away, so both open the realign window too -- the watcher // below waits for the height to stop moving either way. - window.addEventListener('orientationchange', onResize); + window.addEventListener('orientationchange', onOrientation); if (window.visualViewport) { window.visualViewport.addEventListener('resize', onResize); } diff --git a/tests/smoke_rotate_smooth.py b/tests/smoke_rotate_smooth.py new file mode 100644 index 0000000..26dfbf8 --- /dev/null +++ b/tests/smoke_rotate_smooth.py @@ -0,0 +1,240 @@ +#!/usr/bin/env python3 +"""Turning the phone moves the picture once, not a dozen times. + +Run against a locally running backend: + + backend/main.py & + .venv/bin/python tests/smoke_rotate_smooth.py + +Its sibling smoke_rotate.py asks that a turn keep the same video. This one +asks what the turn looks like while it happens. + +A turn is not one shape change: the viewport passes through every shape +between the one it left and the one it lands in, and each one re-letterboxes +the picture somewhere else. Painted, that run of jumps is the stutter. So the +slides are held at the size the turn is heading for from its first frame: the +picture steps straight into the position and size it will keep, and stays +there -- no fade, no intermediate shapes, nothing dimmed. + +A viewport that only changes height is the browser's chrome sliding away, not +a wrist, and must not be pinned: every scroll on a phone does that. +""" +import json +import os +import re +import sys +from playwright.sync_api import sync_playwright + +CLIP = os.path.join(os.path.dirname(__file__), "fixtures", "clip.mp4") + +BASE = "http://127.0.0.1:5000/" +SERVER = "https://fixture.test" +CHANNEL = "clips" +PORTRAIT = {"width": 430, "height": 930} +LANDSCAPE = {"width": 930, "height": 430} +# The shapes a viewport goes through on its way round. +TURN = [{"width": 560, "height": 800}, {"width": 700, "height": 660}, + {"width": 830, "height": 520}, LANDSCAPE] +# The chrome sliding away: height only, and nothing announced it. +CHROME = {"width": 430, "height": 830} + +STATUS = {"online": True, "channels": [{ + "id": CHANNEL, "name": "Fixture Clips", + "description": "A channel that only exists for this test.", "options": []}]} + +SEED = """([server, channel]) => { + localStorage.setItem('config', JSON.stringify({ servers: [{ [server]: {} }] })); + localStorage.setItem('preferences', JSON.stringify({ [server]: { channelId: channel } })); + localStorage.removeItem('session'); +}""" + +# Samples every frame: how much of the reel is on screen, what shape the +# picture has, and whether it is still where it belongs. +WATCH = """() => { + window.__seen = []; + window.__watch = true; + const scroller = document.getElementById('feed-scroll'); + const tick = function() { + const active = document.querySelector('.feed-slide.is-active'); + const video = active ? active.querySelector('.feed-video') : null; + const box = video ? video.getBoundingClientRect() : null; + window.__seen.push({ + opacity: Number(getComputedStyle(scroller).opacity), + pinned: document.getElementById('feed-view').classList.contains('is-turning'), + shape: box ? [Math.round(box.width), Math.round(box.height)] : null, + top: active ? Math.round(active.getBoundingClientRect().top) : null, + paused: video ? video.paused : null + }); + if (window.__watch) requestAnimationFrame(tick); + }; + requestAnimationFrame(tick); +}""" + +STOP = "() => { window.__watch = false; return window.__seen; }" + +REST = """() => { + const scroller = document.getElementById('feed-scroll'); + const style = getComputedStyle(scroller); + return { + opacity: Number(style.opacity), + snap: style.scrollSnapType, + overflow: style.overflowY, + shape: (function() { + const box = document.querySelector('.feed-slide.is-active .feed-video'); + const rect = box ? box.getBoundingClientRect() : null; + return rect ? [Math.round(rect.width), Math.round(rect.height)] : null; + })(), + turning: document.getElementById('feed-view').classList.contains('is-turning'), + videoId: App.state.feedActiveVideoId, + index: App.state.feedActiveIndex, + scrollTop: Math.round(scroller.scrollTop), + expected: App.state.feedActiveIndex * scroller.clientHeight + }; +}""" + +GO = """(step) => { + const scroller = document.getElementById('feed-scroll'); + scroller.scrollTop = step * scroller.clientHeight; +}""" + +# The container's own re-anchor, on a position measured against the height +# from before the turn, arriving while the new one is already in place. +STALE = """(oldHeight) => { + const scroller = document.getElementById('feed-scroll'); + scroller.scrollTop = App.state.feedActiveIndex * oldHeight; +}""" + + +class Checks: + def __init__(self): + self.failed = 0 + + def ok(self, label, condition, detail=""): + mark = "PASS" if condition else "FAIL" + if not condition: + self.failed += 1 + print(f" [{mark}] {label}" + (f" -- {detail}" if detail and not condition else "")) + + +def items(): + return [{"id": f"vid-{i}", "title": f"clip {i}", + "url": f"https://fixture.test/v{i}.mp4", "thumb": "", "channel": CHANNEL, + "duration": 30, "isLive": False, "tags": []} for i in range(36)] + + +def shape_changes(seen): + """The frames where the picture changed shape, with how lit they were.""" + out = [] + for a, b in zip(seen, seen[1:]): + if a["shape"] and b["shape"] and a["shape"] != b["shape"]: + out.append({"from": a["shape"], "to": b["shape"], "opacity": b["opacity"]}) + return out + + +def do_turn(page, sizes, old_height, announced=True): + page.evaluate(WATCH) + page.wait_for_timeout(120) + if announced: + page.evaluate("() => window.dispatchEvent(new Event('orientationchange'))") + for i, size in enumerate(sizes): + page.set_viewport_size(size) + if i == 1: + page.evaluate(STALE, old_height) + page.wait_for_timeout(70) + page.wait_for_timeout(1400) + return page.evaluate(STOP) + + +def main(): + c = Checks() + + with sync_playwright() as p: + browser = p.chromium.launch(args=[ + "--no-sandbox", "--disable-dev-shm-usage", "--disable-gpu", + "--autoplay-policy=no-user-gesture-required", + "--renderer-process-limit=1", "--js-flags=--max-old-space-size=512", + ]) + page = browser.new_page(viewport=PORTRAIT) + crashes = [] + page.on("pageerror", lambda e: crashes.append(str(e))) + + page.route("**/api/status", lambda route: route.fulfill( + status=200, content_type="application/json", body=json.dumps(STATUS))) + page.route("**/api/videos", lambda route: route.fulfill( + status=200, content_type="application/json", + body=json.dumps({"items": items(), "pageInfo": {"hasNextPage": False}}))) + page.route(re.compile(r"\.mp4"), lambda route: route.fulfill( + status=200, content_type="video/mp4", body=open(CLIP, "rb").read())) + + page.goto(BASE, wait_until="domcontentloaded") + page.evaluate(SEED, [SERVER, CHANNEL]) + page.goto(BASE, wait_until="load") + page.wait_for_selector(".video-card", timeout=30000) + page.wait_for_timeout(500) + page.evaluate("() => App.feed.open()") + page.wait_for_selector(".feed-slide", timeout=20000) + page.wait_for_timeout(800) + for step in (1, 2, 3): + page.evaluate(GO, step) + page.wait_for_timeout(250) + page.wait_for_timeout(800) + before = page.evaluate(REST) + + print("\nturning it sideways, one shape at a time") + seen = do_turn(page, TURN, PORTRAIT["height"]) + changes = shape_changes(seen) + rest = page.evaluate(REST) + c.ok("the picture moved once, not through every shape on the way", + len(changes) == 1, str(changes)) + c.ok("and the one step it took was straight into the size it keeps", + bool(changes) and changes[0]["to"] == rest["shape"], + f"{changes} -> {rest['shape']}") + c.ok("nothing was dimmed to cover it", + all(s["opacity"] == 1 for s in seen), + str(min(s["opacity"] for s in seen))) + c.ok("the reel stayed pinned to its step the whole way", + all(s["top"] in (None, 0) for s in seen), + str(sorted({s["top"] for s in seen}))) + c.ok("the slides were held only for the turn, not after it", + 0 < len([s for s in seen if s["pinned"]]) < len(seen) * 0.75, + f"{len([s for s in seen if s['pinned']])} of {len(seen)} frames") + c.ok("the clip never paused for it", + not any(s["paused"] for s in seen), "") + + c.ok("and the feed is no longer marked as turning", not rest["turning"], str(rest)) + c.ok("snapping is back on", "y" in rest["snap"], str(rest["snap"])) + c.ok("and so is scrolling", rest["overflow"] == "scroll", str(rest["overflow"])) + c.ok("on the same video it was on", + rest["videoId"] == before["videoId"], + f"{before['videoId']} -> {rest['videoId']}") + c.ok("sitting on its snap point", + abs(rest["scrollTop"] - rest["expected"]) <= 1, str(rest)) + + print("\nand back upright") + seen = do_turn(page, list(reversed(TURN[:-1])) + [PORTRAIT], LANDSCAPE["height"]) + changes = shape_changes(seen) + back = page.evaluate(REST) + c.ok("one step on the way back too", len(changes) == 1, str(changes)) + c.ok("into the size it keeps", + bool(changes) and changes[0]["to"] == back["shape"], + f"{changes} -> {back['shape']}") + c.ok("still the same video", back["videoId"] == before["videoId"], str(back)) + + print("\nthe browser's chrome sliding away is not a turn") + seen = do_turn(page, [CHROME], PORTRAIT["height"], announced=False) + c.ok("the slides were never pinned for it", + not any(s["pinned"] for s in seen), "") + page.set_viewport_size(PORTRAIT) + page.wait_for_timeout(1200) + chrome = page.evaluate(REST) + c.ok("and it is still on the same video", + chrome["videoId"] == before["videoId"], str(chrome)) + + c.ok("nothing threw along the way", not crashes, str(crashes[:2])) + browser.close() + + print(f"\n{'OK' if not c.failed else 'FAILED'}: {c.failed} check(s) failed") + return 1 if c.failed else 0 + + +sys.exit(main())