Repository navigation
feat(studio): let an embedder draw the preview edge to edge with fillBox - #4408
Merged
Merged
Conversation
NLEPreview always drew its picture 8px in from each side on a dark band, so an embedder that sizes the box to the film saw a smaller picture in a black frame. fillBox drops that inset from both the layout and the stage-size math. Default look is unchanged.
jrusso1020
approved these changes
Sep 24, 2026
jrusso1020
left a comment
Collaborator
There was a problem hiding this comment.
Approve at 8809db94.
- Default sizing is unchanged: old
viewport - 16 - 2·gutter, newviewport - 2·(8 + gutter), identical for every gutter.resolvePreviewStageSizeandPREVIEW_STAGE_INSET_PXhave no other callers inpackages/, so halving the constant's meaning (per side instead of both sides) touches nothing else. p-2removal is inert: all three viewport children (the stage container, the zoom HUD, the reset button) areabsolute, and absolute offsets resolve against the padding box, so the padding never moved them.- One source for the inset: layout (
style={{ inset: insetPx }}) and sizing now share it. A small bonus: the oldinset-2was0.5rem, so in a host app whose root font size is not 16px the layout and the 16px JS subtraction already disagreed. Now they can't. - The ResizeObserver effect re-subscribes on
insetPx, so togglingfillBoxre-measures.
Tests: NLEPreview.test.ts passes, 11/11. Two mutations, each failing only the new test:
- ignoring
fillBox(insetPxalways 8) - decoupling the layout inset from the sizing inset (
style={{ inset: 8 }}withfillBoxon)
— Rames
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
NLEPreviewgets an optionalfillBoxprop. When it is on, the picture is drawn edge to edge in the preview's box instead of 8px in from each side on a dark band. Without the prop nothing changes.Why
An app that embeds
NLEPreviewand sizes its box to the film's shape saw the film smaller than the box, framed by a black band. In a 640x360 box with a 16:9 film, the picture was 611.5x344. There was no way to turn the band off: the inset lived in two places (the inner container'sinset-2and a 16px subtraction inresolvePreviewStageSize), and neither was reachable from props.How
PREVIEW_STAGE_INSET_PX = 8).fillBoxsets it to 0.insetand the stage-size math, so the layout and the sizing cannot drift apart.resolvePreviewStageSizetakes the inset as a parameter that defaults to the constant, so existing callers keep their numbers.p-2is removed: every child of the viewport is absolutely positioned, so the padding never affected layout.Test plan
NLEPreview.test.ts: in a 640x360 box, the default stays 611.5556x344 with an 8px inset, andfillBoxgives a 640x360 stage with no inset. It passes on this branch (11 passed, exit 0). It fails whenfillBoxis ignored (exit 1, only this test red).tsc --noEmiton packages/studio,oxlintandoxfmt --checkon the changed files: clean.NLEPreviewin a 640x360 box (pink outline) with a 16:9 registry example, same URL both times. Measured in the browser: main draws 611.5x344 inside the box; this branch withfillBoxdraws 640x360. With the prop off, this branch's screenshot is pixel-identical to main.Before
main: the picture sits 8px in from the box on a black band.
After
this branch with
fillBox: the picture fills the box.Review
Independent adversarial review of this head: blocking none, should fix none. Notes: the new test covers a same-shape landscape box without the ruler gutter; the gutter and letterbox paths reuse the same arithmetic (checked by hand,
2 * (inset + gutter)equals the old single subtraction when the inset is 8). Every consumer of the stage geometry (pan and zoom clamp, ruler, deselect outside the frame, drop coordinates) reads live rects, so none assumes the inset.