Repository navigation
wall: fill the screen with the image; cap each tile at 0.5 Mpx - #1331
Conversation
The live image was a ~600x330 box under a title, a countdown card and an
empty scoreboard. It is now the whole page.
Layout (app/_wall, wall.css, core/view.ts):
- The stage is the viewport; the image is contain-fit and centred.
imageArea no longer reserves a caption band.
- A row of controls over the top (wordmark, title, LIVE, viewers, pause
icon, "Direct the scene", own GPU when lending is on, isolation, menu)
and a strip over the bottom (restart button, last race in one line,
"auto in m:ss" when a restart is scheduled, and a spec line built from
live numbers: image size in device pixels, GPU count and name, median
fps). Both sit on gradient scrims. The controls fade after 3 s with no
pointer, key or touch; reduced motion shows and hides without a fade.
- Each half is tagged on the image ("FCVM · restored from a snapshot",
"EC2 · launched from an AMI"); the captions under the image are gone.
- The race's bars show only while a race runs and for 8 s after it.
- The sentence, the counters and the recent actions move to the drawer's
About section (menu icon). The activity feed on the page becomes one
toast, newest only, for 4 s.
- "Get your own GPU" is hidden when the fleet lends none; the restart
button is hidden when it would offer zero GPUs.
- "Click a square" is a one-time hint the first time the pointer is over
the image.
- Seams and the hover outline show within 1.5 s of pointer movement, on
a long press, or with keyboard focus, since the pointer now rests over
the image.
- Phone portrait: image full width at the top, strip under it, the
button in a sticky dock. Landscape uses the desktop layout.
Quality (core/layout.ts, core/bandwidth.ts, frames.worker.ts):
- streamHeightFor takes the tile's aspect and caps the height so the
tile is at most MAX_TILE_PX = 0.5e6 pixels, snapped down to a stream
height. Mosaic, opened VM (native size) and own GPU placement use it.
The renderer README's grid-budget table puts a T4 terrain tile at 0.4
to 0.5 M pixel-samples per frame at 30 fps.
- Bit rates follow fleet/PROTOCOL.md's quality table: lo 0.25 / std 0.8
/ hi 4 bits per pixel, floors 0.15 / 0.6 / 2 Mbit/s, caps 20 / 20 /
160 Mbit/s. fitBandwidth moves every stream but the opened one
together: all to q=lo, then all down one height.
- The frame canvas draws with imageSmoothingQuality "high".
The title, the lines and the gaps between buttons pass the pointer to
the image; only the controls take it, so the top and bottom tiles can
still be hovered and clicked.
The e2e viewport goes from 1440x900 to 1024x640 locally and from
1280x800 to 960x600 in CI. The image now fills the viewport, and the
mock draws every pixel in JavaScript and encodes each stream with its
own ffmpeg; at 1440x900 (a 1440x810 image) the suite's later tests fell
behind on it (the aggregated test synced 2 of 3 regions in 30 s in three
full runs, against 2 to 4 s alone).
Tests: unit tests for the pixel cap, the bitrate table and the
all-together fit, each failing on the old layout.ts/bandwidth.ts; view
tests for the stage, side tags, spec line, last-race line, seam hold and
toast; e2e tests updated to the new layout plus a full-bleed test (fit,
tags, divider on halfLineX, spec line, hint once, idle fade) and a phone
landscape test. The q=lo resize test now derives its budget from the
page's own layout and bit-rate functions and expects all 15 tiles at lo.
|
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 29 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 |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05d01e0377
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
ejc3
left a comment
There was a problem hiding this comment.
RED-VERIFIED: both findings in this review are fixed in #1332 (42a5359) with tests observed failing first: e2e "every control stays on screen from just above the phone breakpoint up" and unit "shortens a half's tag to its name when that half is too narrow for the whole tag". Answered in each thread.
The live render was a ~600x330 box in the bottom half of the page, under a title, a "Next restart" card and an empty scoreboard. This makes the image the page and stops a large screen from asking a renderer for more pixels than its GPU can sample.
Contract. fleet/web only. The image is contain-fit to the whole viewport; the controls sit over it; each tile's stream is capped at 0.5 M pixels; the page's bit-rate model matches fleet/PROTOCOL.md. The race itself is unchanged here (its bars show over the image while it runs); the PR stacked on this one moves it onto the picture.
Layout (
app/_wall/*,app/wall.css,core/view.ts)imageAreano longer reserves a caption band. A 1257x932 browser window showed the image at about 600x335; it now shows it at 1257x707.fcvm.dev, "Every GPU, live", a LIVE badge once the image is whole, then viewers, a pause icon, "Direct the scene" (was "Play"), "Get your own GPU" (only when the fleet lends GPUs), an isolation icon and a menu icon.Last race: fcvm back in 3.2 s · EC2 back in 27 s · 8.2× faster),auto in 4:12when a restart is scheduled, and a spec line from live numbers:1440×810 · 15 × Tesla T4 · 15 fps · path traced(image in device pixels, live tiles, the most common GPU name, median fps).FCVM · restored from a snapshot,EC2 · launched from an AMI; the name alone on an image narrower than 640 px). The captions under the image are gone.Quality (
core/layout.ts,core/bandwidth.ts,frames.worker.ts)streamHeightFortakes the tile's aspect and caps the height atsqrt(MAX_TILE_PX / aspect), snapped down to a stream height, withMAX_TILE_PX = 0.5e6. The mosaic, an opened VM's native size and the own-GPU view use it. A renderer draws at the largest height any subscriber asks, and the grid-budget table in fleet/renderer/README.md puts a T4 terrain tile at 0.4 to 0.5 M pixel-samples per frame at 30 fps; above that one viewer would drop every viewer's picture under one sample per pixel. Example: 16:9 quarter tiles on a 1920x1080 window at 2x asked for 540 (960x540 = 518,400 px), now 432.fitBandwidthmoves every stream but the opened one together: all to q=lo, then all down one height. It used to drop the smallest tiles to lo first, which left lo tiles beside std ones.imageSmoothingQuality = "high"(it was the default, low).Evidence
pnpm lint && pnpm typecheck && pnpm test: 217 unit tests pass.core/layout.tsandcore/bandwidth.tswith only the new constants added so the tests compile:never asks one tile for more than MAX_TILE_PX ...(540 asked, 518,400 px),prices streams as fleet/PROTOCOL.md's quality table ...andmoves every tile together ...all fail; with the change all pass.pnpm build && pnpm test:e2e: 55 passed in 9.1 min (Chromium, the WebKit subset, the screenshot run). New: "the render is the page" (contain-fit to the viewport, tags on the image either side of the divider, divider centred onhalfLineX, spec line, hint shown once per browser, controls fading and coming back, seams gone at rest) and "phone, landscape: full-bleed like the desktop". Updated to the new layout: the restart race (toast, then the last-race line), the schedule (auto in), counters and recent actions in the drawer, the pause icon's label, the opened VM's native height within the cap, the phone layout.Test viewport. The e2e default viewport goes from 1440x900 to 1024x640, and CI's from 1280x800 to 960x600. The image now fills the viewport, and the mock draws every pixel in JavaScript and encodes each stream with its own ffmpeg. At 1440x900 (a 1440x810 image) the suite's later tests fell behind on it: the aggregated test synced 2 of 3 regions within 30 s in three full runs, against 2 to 4 s when run alone or after five tests. 1024x640 asks for a 1024x576 image, half the pixels.
Downstream impact. The staging page's layout and the heights it asks renderers for. A viewer whose window covers more than 0.5 M pixels per tile gets a smaller stream scaled up instead of a larger one.