Repository navigation
fix(studio): show each timeline row its own layer instead of a flat bar - #4917
Conversation
A row got a frame only if it spanned under 92% of the film and its id did not look like a backdrop; every other row drew a plain grey bar. Every element row now gets its frame, and the capture fades the row element's siblings first, so the frame shows that layer and not the whole film. Studio's dev server and the CLI share one capture helper; Studio's own copy is gone.
Edit accuracy: accurate 1216 (base branch 1216), smooth 1055 of thoseThe gate passes. Quarantined, measured but not gated (1)
|
7c14c23 to
82b1724
Compare
… captured picture
…ayers as pictures
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 241d0955. I read the full source diff, plus thumbnailPages.withPage, the row builders in timelineDOM.ts, the clip-content order in useRenderClipContent, and the text and style commit paths. Approving.
What I checked:
- Single-layer capture (
screenshotClip.ts).- It fades each sibling on the path up to
<html>. The element, its subtree and its ancestors keep their own paint. - The restore list stores each sibling's exact inline value and priority, and runs in reverse. So a sibling recorded twice still ends on its original value.
- The Vite path opens and closes a page per capture, so it doesn't need a clear.
- On the CLI path,
withPageruns one call per page at a time. A second capture can't see the first one's fades. If the clear infinallythrows, the page is dropped.
- It fades each sibling on the path up to
- Text rows.
- Composition hosts and audio rows are decided before
el.text, so a host never turns into a text row. - Video and img rows aren't text-bearing tags, so they never take the text branch.
- The curated style list includes
background-image,background-color, the font properties andcolor, so the background-image check actually reads a value. timelineElementsChangedcompares text by value, so a rebuilt row with the same words doesn't re-render.
- Composition hosts and audio rows are decided before
- Realm checks. Core's
isHtmlElement/isMediaElementtest node type, namespace and tag. A foreign-realm<video>keeps its source length, and a foreign-realm row keeps its selector. - Cache key. It's a SHA-1 of the selector plus the index, under
v5, so#a\.band#a_bno longer share an entry.
Tests I ran (vitest at this head, clean install):
- Studio:
useRenderClipContent,refreshTimelineRowText,TextClipContent,timelinePlayerSyncandtimelineElementHelperspass, 49 of 49. - studio-server:
screenshotClipandthumbnailpass, 29 of 29. - CLI
studioServer.test.ts: 35 of 36 pass. The one failure is "Studio project lint endpoint > surfaces findings that require the complete project graph", which returned 500. It's in the lint route, which this PR doesn't touch, so I read it as my local setup. - The isolation test is meaningful. With the
clearElementScreenshotIsolationcall removed from the CLI path, "undoes a row's isolation after its screenshot" fails.
Nits (not blocking):
-
Only the layer's own paint is checked, not its children's. Any child that is a text tag with no children counts as text, including one that only draws something. Two examples I ran through
readTimelineText:<div><div style="background:red;height:4px"></div><div>Title</div></div>becomes a text row "Title", so the coloured bar is dropped.- A child
<span>with abackground-imagealso becomes a text row.
Treating a child with its own background image as "keep the picture" would cover the second case.
-
Text with a transparent fill becomes an empty strip.
color: transparentwith-webkit-text-stroke, the outline-text look, returnscolor: "transparent". The row then draws invisible words on the light strip. Falling back to the picture when the computed colour's alpha is 0 would avoid that. -
A style-panel edit leaves a text row's look stale. Style commits skip the preview reload (
domStyleCommit.ts:102), just as text commits do. Only text commits callrefreshTimelineRowText, so changing a text layer's colour, weight or font keeps the old look in its row until the next rebuild. Calling it from the style commit's resync would match. -
A layer with no box at the seek time gets a full-frame capture of only its branch.
getElementScreenshotClipfades the siblings before the< 4pxcheck, so returningundefinedstill leaves them faded during the full-page screenshot. That's a good picture for a wrapper whose children are absolutely positioned. For a layer that isn't shown at that time, though, it comes out blank where main showed the whole frame. Measuring before fading would keep the old fallback if that's preferred. -
The row shows
textContentas written, sotext-transform: uppercasereads as lowercase. This is cosmetic.
Verdict: APPROVE
Reasoning: Each row now shows its own layer: text layers are drawn as text, and other layers are captured alone with every sibling's opacity restored exactly. The CLI clear is pinned by a test that fails without it. What's left are edge cases where a row's look drifts from the layer, not correctness problems.
— Rames Jusso
What
Layer rows in the timeline now show their own layer instead of a flat bar. A text layer's row draws its own words as text, in its own weight and colour and its font as Studio has it; a visual layer's row shows a picture of that layer alone. Closes desktop's text-clip report (text clips such as Subtitle and Cta tiled their rendered text as repeated thumbnails). On a project where most layers span the whole film, every one of those rows used to be a plain pale bar.
Why it happened
Three things stacked:
useRenderClipContentonly gave a layer row a picture when it was shorter than 92% of the film and its id did not contain backdrop, background, overlay, scrim or mask. A full-length layer failed the first test and fell back to the flat bar.data-composition-id).timelineElementHelpers.tschecked "is this an HTML element" withinstanceofagainst the preview window. The preview's nodes carry the editor window's prototypes (documented in coreruntime/domRealm.ts), so the check was false for those rows on every load I ran; core's notes say it happens on some loads. The same check also stopped video and audio rows from reading their source length.Fix
isTextBearingTag/isEditableTextLeafindomEditingDom.ts) gets its words and computed font family, weight and colour when the row is built (readTimelineText, called fromapplyMediaMetadataFromElement, which both row builders go through). A layer that paints a background image or gradient is not a text layer and keeps its picture. A text row renders its words on one clipped line (TextClipContent), with no capture, on the layer's own background colour when it has an opaque one, otherwise on a theme strip chosen for contrast (light behind dark text, dark behind light text). The timeline's change check compares text by value, and a canvas text edit, which skips the preview reload, re-reads the edited layer's row (refreshTimelineRowText).isEditableTextLeafmoves fromdomEditingLayers.tstodomEditingDom.tsbesideisTextBearingTag, so the timeline can use it without an import cycle.getElementScreenshotClipin studio-server) now serves both the Vite and the CLI capture paths; the copy insidevite.browser.tsis deleted. Each faded sibling gets an inlineopacity: 0 !important; the page keeps its own list of what it faded and the exact previous inline opacity, so no marker is added to the markup and each sibling's inline opacity is put back exactly. The CLI, which reuses one loaded page for later captures, restores them after every screenshot (clearElementScreenshotIsolation); without that, the next capture on the page came back blank.#a\.band#a_bno longer share one cached picture, and long selectors are no longer cut at 80 characters.@hyperframes/core/runtime/dom-realm, and the timeline helpers use them.Tests
useRenderClipContent.test.ts: a full-length row with an id gets a picture.useRenderClipContent.test.ts: a row with text rendersTextClipContent. Fails without the branch.timelineElementHelpers.test.ts:readTimelineTextreads anh1with aspanas one line with its font, weight and colour, and returns nothing for a container holding a non-text child, an empty div, or a non-text tag. The container case fails without the text-only rule.screenshotClip.test.ts: every sibling on the element's ancestor path is faded out, and the element, its subtree and its ancestors are left alone. A sibling with its ownopacity: 1 !importantis still hidden and gets that exact value back on clear, as does one whose opacity isvar(--alpha, 0|1). After a clear, isolating a different element hides only its own siblings.thumbnail.test.ts:#a\.band#a_bare captured separately; the old key served the second from the first's cache.studioServer.test.ts: a selector capture clears its isolation after the screenshot. It fails without the clear. The describe block now shuts its server down after each test, because the thumbnail browser lease is module-wide and leaked one test's fake browser into the next.timelineElementHelpers.test.ts: a row whose node comes from another realm keeps its#idselector, and a foreign<video>still yields its source length. Both fail on the old check.Cost
Measured cold, on a 60 s fixture film with 24 rows, scrolling the whole timeline:
About one capture per row, so the first open of a long film takes longer to fill in. A second open hits the thumbnail cache. (That run predates text rows; those cards are now drawn as text and need no capture.)
Rows on screen first, cold, no scrolling, six rows in view, two runs each:
On the gradient film every capture this PR makes goes to a visible row (6 rows plus the composition card), where main spends 3 of its 6 on rows off screen. The last of the six lands later only because Studio's capture path renders one page at a time, about 5.5 s each; batching captures is the follow-up.
The CLI path was checked separately on the built CLI (
hyperframes previewfrom source always serves through Vite): Title, then Waves, then the whole frame requested at once on one page. Before the clear, Waves and the whole frame came back blank; with it, all three are right.Limitation
Text rows show the words present when the timeline is built or the text is edited in Studio; words a script swaps in during playback are not tracked. A font that only the composition loads falls back to Studio's fonts in the row.
A layer is captured without its siblings, so a layer whose look depends on what is behind it (
mix-blend-mode,backdrop-filter) is shown against its parent's background rather than the sibling it blends with.Before
A text-heavy fixture: Backdrop and Shape are visual layers; Title, Subtitle and Cta are text. On main the full-length rows are flat bars, Subtitle is a blurred, tiled picture of its words, and Cta tiles a picture of the whole frame.
After
Backdrop and Shape show their own pictures; Title, Subtitle and Cta show their own words as text, in their own colour and font (the font as Studio has it), Cta on its own red.