feat(studio): add resolution selector to render export bar#664
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
vanceingalls
left a comment
There was a problem hiding this comment.
Verdict: Comment — one blocker (silent-drop bug in the Studio dev-server adapter), otherwise solid.
The route + types + CLI-server adapter are wired end-to-end. Tests on the route are good. But the Studio's own in-process adapter — the one that actually runs when a user opens vite dev and clicks Export — drops outputResolution on the floor. That's the most common Studio code path and exactly the smoke test marked unchecked in this PR's own test plan. It should be caught and fixed before merge.
Cross-references for context: this is part of the 4K stack — #660 (presets), #661 (init --resolution), #662 (frame cache budget), #663 (render --resolution), this one (#664 — Studio UI), and #665 (preset dedup, on top).
Blockers
[blocker] packages/studio/vite.config.ts:243-247 — outputResolution is silently dropped when rendering from the Studio dev server.
const job = createRenderJob({
fps: opts.fps as 24 | 30 | 60,
quality: opts.quality as "draft" | "standard" | "high",
format: opts.format,
});The Studio Vite adapter (the one that runs in bun run --cwd packages/studio dev, which the docs added in this PR explicitly tell users to use) does not forward opts.outputResolution into createRenderJob. Picking 4K in the new dropdown sends resolution: "landscape-4k" over the wire, the route validates it, the adapter receives it on opts, and then this call site discards it. The user gets a 1080p MP4 with no error.
Why TS didn't catch it: the local lazy-loaded type at packages/studio/vite.config.ts:75-87 redeclares createRenderJob with only {fps, quality, format} — it doesn't mirror the real RenderConfig, so adding outputResolution to RenderConfig (or to StudioApiAdapter.startRender) doesn't ripple here. That same type duplication is what made this slip past bun typecheck.
The fix is one line at the createRenderJob({...}) call (and adding outputResolution?: CanvasResolution to the local type). The CLI adapter at packages/cli/src/server/studioServer.ts:219 already does the right thing — this is just the parallel adapter.
This is exactly the failure mode the test plan flagged with [ ] In-app manual smoke test (Studio dev server + 1080p project + click Export at 4K) — not run; follow-up before merge. Running that smoke would have surfaced this immediately. Strong recommend running it as part of the merge gate.
Important
[important] No studio component test covers the new dropdown.
The PR description claims bun run --cwd packages/studio test — 271/271 pass, but there's no RenderQueue.test.tsx and no assertion that:
- the resolution
<select>renders all 5 options (auto+ 4 presets), - the
onChangeflows the value intostartRender's 4th arg, - picking
autocauses the request body to omitresolutionrather than send"auto"(line 81 ofuseRenderQueue.tsis the load-bearing branch and it's untested).
The route tests are excellent. The studio glue is a thin wrapper, but the auto-omission contract is exactly the kind of thing that quietly breaks on a refactor. A short RTL/jsdom test on FormatExportButton + a unit test on useRenderQueue.startRender's body construction would lock both in.
[important] Type drift — ResolutionPreset in useRenderQueue.ts:14 shadows CanvasResolution from @hyperframes/core.
packages/core/src/core.types.ts:22 already exports CanvasResolution = "landscape" | "portrait" | "landscape-4k" | "portrait-4k", and packages/core/src/index.ts re-exports it. The new alias in useRenderQueue.ts is identical — and the inline literal in packages/core/src/studio-api/types.ts:73 is a third copy of the same union, plus the VALID_RESOLUTIONS set in render.ts:62 is a fourth.
Bumping this from "nit" to "important" because #665 in the same stack is literally the dedup PR — ideally this PR uses import type { CanvasResolution } from "@hyperframes/core" so #665 doesn't have to come back to it. Otherwise #665's diff is going to grow to fix the very thing this PR introduced.
Nits
[nit] RenderQueue.tsx — RESOLUTION_OPTIONS hardcodes preset literals instead of deriving from CANVAS_DIMENSIONS.
Same theme as the type drift point. The label/title strings ("1080p", "3840×2160 — supersamples...") are user-facing copy that legitimately doesn't belong in core.types, so a small RESOLUTION_OPTIONS table here is fine. But the value field should be CanvasResolution | "auto" rather than four ad-hoc string literals — that way adding a new preset to core.types (e.g. an 8K row) at least gets a TS error here instead of silently missing from the UI.
[nit] Resolution selection doesn't persist.
useState("auto") resets on tab switch / project switch / refresh. Acceptable for v1; worth a TODO + follow-up issue. localStorage-keyed-by-projectId is the obvious next step.
[nit] Layout coupling — "resolution select must always be leftmost" is now load-bearing for rounded-l to land correctly.
RenderQueue.tsx:142 hardcodes rounded-l on the resolution select. The format select used to do ${showQuality ? "" : "rounded-l"} and now drops it (line 163). The math works today because resolution is unconditional, but if anyone later hides it (e.g. behind a feature flag), the leftmost element loses rounded-l. A comment would be enough — or compute the leftmost at render time.
[nit] No aria-label on the resolution <select> — only title.
title is a tooltip, not an accessible name; screen readers will announce "combobox" with no context. Same gap exists on the pre-existing format/quality selects, so this is parity, not a regression — call it out and fix all three together when convenient.
Praise
- Route tests are sharp. The four cases (forwards, omits, drops invalid, round-trips all four presets) cover the contract precisely.
it("drops an invalid resolution string ... not a 400")with the inline comment about the producer being source-of-truth is exactly the kind of decision-recording-via-test that ages well. auto-as-omission, not as enum value (useRenderQueue.ts:73-81— the comment explaining "Sending the string 'auto' would fail the route's validation set" is good defensive code + good documentation. Saves the next person an hour.- Producer is the validation source-of-truth and the route deliberately accepts garbage and lets the producer fail with a clear message. Right call for layered systems where the producer is shared by CLI and Studio — keeps the contract in one place.
- Docs are accurate and constraints-honest. The "render-time only — composition files are not modified" line in
4k-rendering.mdxis the right framing and the constraints note ("aspect must match, integer scale only, not yet combined with HDR") sets correct user expectations.
CI
The two failing regression-shards (styles-a/b) checks are a Docker Hub pull flake — Set up Docker Buildx step failed with context deadline exceeded pulling moby/buildkit:buildx-stable-1. Not related to the diff (parent #663 is fully green on the same shards). A re-run should clear them. The :x: from Slack is almost certainly this flake plus the missing dev-server smoke — neither indicates a real test/typecheck failure in this PR's logic.
— Vai
miguel-heygen
left a comment
There was a problem hiding this comment.
Requesting changes because the Studio selector is not wired through every Studio render adapter.
The shared route now accepts resolution and forwards outputResolution to the StudioApiAdapter, and the embedded CLI Studio server passes that through to createRenderJob. But the Vite/package Studio adapter in packages/studio/vite.config.ts still creates the producer job with only:
createRenderJob({
fps,
quality,
format,
})So in bun run studio / package dev Studio, the UI selector appears and the POST body can include the selected resolution, but the actual producer job ignores it and renders at the authored size.
I verified the UI with agent-browser: the Renders panel shows the selector and 4K can be selected. The missing piece is adapter forwarding in the Vite Studio path. Please wire opts.outputResolution there too and add adapter-level coverage so the two Studio server implementations cannot drift again.
CI note: the red regression aggregate on this head came from Docker BuildKit/Docker Hub timeouts while booting buildx in styles-a/styles-b, before HyperFrames regression assertions ran. I am not treating that as caused by this PR.
I rechecked the live head before posting: 987144168e022997af93c250309e35b593be6f34.
00da353 to
41dc94a
Compare
9871441 to
246b1d2
Compare

What
Adds a resolution dropdown to Hyperframes Studio's export bar so users can render a composition at a non-native resolution (e.g. 1080p → 4K) without leaving the editor and without modifying the project files.
The dropdown sits next to the existing format and quality selectors. The default value
Autopreserves today's behavior — render at the composition's authored size. Picking4K(or any preset) plumbsoutputResolutionto the producer'sRenderConfig, which supersamples via ChromedeviceScaleFactor(the same machinery as PR #663'shyperframes render --resolutionflag).Stacked on #663.
Why
PRs #660–#663 made 4K a one-liner from the CLI. This PR closes the gap for users who live in Studio. The dropdown matches the CLI flag's semantics 1:1 — same presets, same constraint enforcement on the producer side, same supersample-via-DPR mechanism. Nothing about the project files changes when a user picks a resolution; it's purely a render-time override.
This intentionally does not ship a "permanent project resolution" feature (which would mutate
data-width/data-height/<meta viewport>/#stageCSS in the project files, likeinit --resolution). That UX is a separate concern — it would change the studio editor's coordinate system and needs its own design pass. Render-time override is the single change most users actually want from Studio: "give me a 4K MP4 of this thing."How
Studio (
packages/studio/src/components/renders/):RenderQueue.tsx—FormatExportButtongains a third<select>. Five options:Auto(default, no override) plus the four canonical presets. Each option carries a tooltip — the4Kentry warns "Slower, larger files." Layout matches the existing format/quality pattern (rounded-l on the leftmost select).useRenderQueue.ts—startRendergains an optionalresolutionparam. When set to a preset, it's added to the JSON body. When set to"auto", the field is omitted entirely so it doesn't trip the route's enum validation.App.tsx— call site forwards the new arg.Core API route (
packages/core/src/studio-api/routes/render.ts):/projects/:id/renderbody now readsresolutionand validates it against the same four-value enum used by the producer + CLI.outputResolutiononadapter.startRender(opts).Adapter type (
packages/core/src/studio-api/types.ts):StudioApiAdapter.startRender(opts)opts gainoutputResolution?: "landscape" | "portrait" | "landscape-4k" | "portrait-4k".resolveDeviceScaleFactorfor the validation contract.CLI adapter (
packages/cli/src/server/studioServer.ts):startRendernow passesoutputResolution: opts.outputResolutionthrough tocreateRenderJob. One-line change since the underlyingRenderConfigalready accepts the field (PR feat(cli): add --resolution flag to hyperframes render for one-line 4k #663).Docs (
docs/guides/4k-rendering.mdx):Test plan
packages/core/src/studio-api/routes/render.test.ts):landscape-4k) to the adapteroutputResolutionwhen the request doesn't specify onebun run --cwd packages/core test— 683/683 passbun run --cwd packages/cli test— 283/283 passbun run --cwd packages/studio test— 271/271 passdocs/guides/4k-rendering.mdxStudio section is now accurate.