Skip to content

feat(core): add 4k canvas resolution presets#660

Open
jrusso1020 wants to merge 1 commit intomainfrom
05-07-feat_core_add_4k_canvas_resolution_presets
Open

feat(core): add 4k canvas resolution presets#660
jrusso1020 wants to merge 1 commit intomainfrom
05-07-feat_core_add_4k_canvas_resolution_presets

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented May 7, 2026

What

Adds landscape-4k (3840×2160) and portrait-4k (2160×3840) presets to CanvasResolution and CANVAS_DIMENSIONS in @hyperframes/core. Foundation for end-to-end 4K rendering support.

Why

There is no codified way today to mark a composition as 4K. The string union CanvasResolution = "landscape" | "portrait" is the only enum used by templates, generators, and stage-zoom math; without 4K members, scaffolds and helpers always emit 1080p dimensions even when the underlying engine + encoder pipeline can already handle larger viewports (Chrome setViewport, ffmpeg libx264/libvpx-vp9/prores_ks all scale fine).

This is PR 1 of a 3-PR stack making 4K a first-class option:

  1. PR feat(core): add 4k canvas resolution presets #660 (this) — core types/constants + parser detection.
  2. PR feat(cli): add --resolution flag to hyperframes init for 4k scaffolding #661hyperframes init --resolution 4k flag to scaffold 4K projects.
  3. PR fix(engine): byte-budget the frame data uri cache to bound memory at 4k #662 — byte-budget the frame data-URI cache so 4K renders don't OOM.

How

  • CanvasResolution extended additively (no breaking change).
  • CANVAS_DIMENSIONS gains landscape-4k / portrait-4k entries.
  • parseResolutionFromHtml accepts data-resolution="landscape-4k|portrait-4k" and infers 4K from data-composition-width/data-composition-height (long side ≥ 2560 → UHD variant).
  • parseResolutionFromCss reuses the same dimension-aware classifier so inline #stage { width/height } styles are detected too.
  • Helper extracted: resolveResolutionFromDimensions(w, h).
  • cli/info.ts cleanup: replaces a nested parsed.resolution === "portrait" ? 1080 : 1920 ternary with a CANVAS_DIMENSIONS[parsed.resolution] lookup so it stays correct for 4K.

Stage-CSS generators (templates/base.ts, generators/hyperframes.ts) already index CANVAS_DIMENSIONS[resolution], so they pick up the new presets automatically.

Test plan

Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve. Clean foundation. Type extension is additive, parser handles both attribute-driven and dimension-inferred paths, cli/info.ts cleanup is a real bug fix (the old ternary would have returned 1080p dims for a 4K composition). Nice that resolveResolutionFromDimensions was extracted instead of being copy-pasted.

Important

  • importantpackages/core/src/parsers/htmlParser.ts:142 — The UHD threshold is long side >= 2560. That misclassifies 1440p (QHD, 2560×1440) as landscape-4k. 1440p is a real, common authoring resolution that isn't 4K — its dimensions are closer to 1080p than to UHD. Either move the threshold up (e.g. >= 3000 or test a stricter rule like width >= 3840 || height >= 3840), or carve landscape-1440p out as its own preset later. Worth fixing here because the misclassification is silent: a user with a 2560×1440 composition will see their preset stamped as landscape-4k and not know why downstream behavior changed. Add a regression test for the 1440 boundary.

  • importantpackages/core/src/parsers/htmlParser.ts:138-148resolveResolutionFromDimensions only emits landscape / portrait when width === height (square). The function falls through isLandscape = width > height → false → returns the portrait branch, so a 1080×1080 square ends up as portrait. That may be the existing behavior (the old w > h ? "landscape" : "portrait" had the same bias), so not a regression — but worth a one-liner comment that square is treated as portrait by convention, and a test pinning that.

Nits

  • nitpackages/core/src/core.types.ts:22 — Naming. landscape-4k vs landscape reads asymmetrically (landscape implies HD, but only by convention). If the team is open to it, landscape-1080p / landscape-2160p would be self-documenting and let landscape become an alias. Not a blocker, but the vocabulary will outlive this PR — worth raising before more presets land.

  • nitpackages/core/src/core.types.ts:24-26 — No cinema-4K (4096×2160) or 8K. The current set is fine for v1; just flagging that 4k in the alias map (added in #661) commits us to "4K means UHD" — if cinema 4K ever gets a preset, the alias becomes ambiguous.

  • nitpackages/core/src/parsers/htmlParser.ts:122-128 — The four-way string equality on data-resolution is fine for 4 values, but it'll get unwieldy if more presets land. (VALID_CANVAS_RESOLUTIONS as readonly string[]).includes(resolutionAttr ?? "") would scale better. (Bonus: if you take the cleanup from #665 and pull VALID_CANVAS_RESOLUTIONS into core, this becomes free.)

Praise

Type extension is genuinely additive (no breaking change), the info.ts cleanup is the kind of debt-paydown that prevents silent 4K dimension bugs later, and the parser tests cover both attribute and dimension-inference paths. Good foundation for the rest of the stack.

— Vai

Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for the dimension fallback.

resolveResolutionFromDimensions() currently treats any long side >= 2560 as a 4K preset, so a 2560x1440 QHD canvas is reported as landscape-4k even though the preset added here is 3840x2160. That mislabels valid non-4K/custom canvases as one of the new presets.

Concrete repro from this head: parsing an HTML root with data-composition-width="2560" data-composition-height="1440" returns landscape-4k.

Suggested fix: make the fallback stricter, ideally exact known-preset matching for 1920x1080, 1080x1920, 3840x2160, and 2160x3840, or at least use a true UHD boundary. Please add a boundary test that pins 2560x1440 as not landscape-4k and 3840x2160 as landscape-4k.

I rechecked the live head before posting: 555c51fcf60f3cbe5221b4ec171f394e5ff2140d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants