Repository navigation
wall: wait for a late half instead of strobing it dimmed - #1349
Conversation
On the staging wall every fcvm tile flipped between in step and dimmed every few hundred ms (tilesSynced 16 -> 8 -> 16), measured on the owner's Mac. The fcvm half's frames reached the page about 0.13 s after the EC2 half's, and the page's decodes took about 0.25 s with spikes of 0.45 to 0.52 s. Two rules in chooseCommonT made that strobe: - With 8 tiles at each frame time the tie went to the latest, so T ran with the EC2 half and left the fcvm half behind; prevT then kept it there until a stall happened to let it back. - A tile within MAX_LAG_S was meant to hold the image, but the keeping-up tiles kept only 10 frames (0.33 s at 30 fps), so they had already dropped the frame it was showing. chooseCommonT now takes the latest frame time every keeping-up tile holds, so the image waits for the slowest. With none, it holds the picture while a keeping-up tile catches up to it, then falls back to the old rule. A tile in step on the previous draw keeps up until it is MAX_LAG_S + HOLD_EXTRA_S (0.55 s) behind, so a decode stall holds the image instead of flashing the tile. BUFFER_FRAMES goes from 10 to 18 so the keeping-up tiles still hold the frame such a tile shows. The no-torn-seam rule is unchanged: one T, everything else dimmed. Test: "a half that arrives late does not strobe" simulates the measured wall (16 tiles, in-order decodes, the worker's buffer and release, a draw every 1/60 s). Before: the fcvm tiles went dim 2 to 7 times each in 10 s. After: none, and the image stays within a second of the newest frame. pnpm test 225 passed, typecheck and lint clean, pnpm test:e2e 56 passed.
|
@codex review |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 23 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The wall waits for a late half instead of strobing it dimmed.
Why
The owner filmed the staging wall flashing. Sampling
window.__fleetevery 100 ms on the owner's Mac: all 8 fcvm tiles flipped between in step and dimmed together every few hundred ms (tilesSynced 16 -> 8 -> 16), each flip a 50% dim of half the image. The fcvm half's frames reached the page about 0.13 s after the EC2 half's (frame age on the page, and frame arrival measured at the network level from the Mac), and decodes took about 0.25 s with spikes of 0.45 to 0.52 s.chooseCommonTstrobed on that:prevTthen kept the fcvm half behind until a stall let it back.MAX_LAG_S(0.35 s) is meant to hold the image, but the keeping-up tiles kept onlyBUFFER_FRAMES= 10 frames (0.33 s), so they had already dropped the frame it showed.Change
MAX_LAG_S + HOLD_EXTRA_S(0.55 s) behind; the worker passes the previous draw's in-step tiles.BUFFER_FRAMES10 -> 18, enough at 30 fps for the keeping-up tiles to still hold what such a tile shows.The no-torn-seam rule is unchanged: one T, any tile not at T dimmed.
Evidence
a half that arrives late does not strobesimulates the measured wall: 16 tiles at 30 fps, the fcvm half 0.13 s late, in-order decodes of 0.24 to 0.26 s with 1 in 100 at 0.45 to 0.52 s, the worker's buffer cap and release, a draw every 1/60 s. On main the fcvm tiles went dim 2 to 7 times each in 10 s (observed failing); with this change none did, and T stays within a second of the newest frame.pnpm test: 225 passed.pnpm typecheck,pnpm lintclean.pnpm test:e2elocally: 56 passed (Chromium and WebKit).Contract and impact
Contract: a tile that is transiently late does not flash; the image waits for it up to 0.55 s. Page only (
fleet/web); ships with the web deploy. The fcvm half's remaining delivery lag is fixed separately (relay TCP_NODELAY PR, and the host AMI with #1343).