fix: use custom viewport dimensions in streaming frame metadata and image resolution#1033
Conversation
CDP's Page.screencastFrame metadata returns physical device dimensions instead of the emulated viewport, causing frame messages to report incorrect deviceWidth/deviceHeight when a custom viewport is set. Use the viewport dimensions captured at screencast start instead of the CDP metadata values, since the screencast image is already captured at the configured viewport size. Closes vercel-labs#1031
|
@hyunjinee is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
screencast dimensions
Emulation.setDeviceMetricsOverride only changes the CSS viewport, but
screencast captures the actual browser content area. This caused frame
images to have incorrect dimensions (e.g., 1000x451 instead of
1000x1000)
when a custom viewport was set.
- Call Browser.setContentsSize after setDeviceMetricsOverride so the
content area matches the emulated viewport
- Restart active screencast when viewport dimensions change so
maxWidth/maxHeight parameters are updated
- Skip redundant screencast restarts when dimensions are unchanged
- Extend E2E test to verify actual JPEG image dimensions, not just
metadata
Kept no-op viewport guard from feature branch, adopted main's session-switch comment and notify_one() addition in set_viewport. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
ctate
left a comment
There was a problem hiding this comment.
Thanks for this fix, @hyunjinee!
One note: Browser.setContentsSize doesn't appear in the official CDP docs — worth confirming it's stable across Chrome versions, or consider Browser.setWindowBounds as an alternative. Also, the silenced let _ = on that call means failures go unnoticed; a debug! log would help with future debugging.
Looks good overall!
…ontentsSize failures - Add viewport_size to LaunchOptions so --window-size matches the configured viewport from the start, reducing reliance on the experimental Browser.setContentsSize CDP call at runtime - Log Browser.setContentsSize failures instead of silently ignoring them with let _ =
|
@ctate Thanks for the review! Re:
Re: Fixed — replaced with Additional improvement Added |
The daemon's stderr pipe is closed after startup, so log crate subscribers cannot output during normal operation. eprintln! is visible during startup and in tests, matching the existing convention.
Summary
deviceWidth/deviceHeightin frame metadata, instead of CDP'sPage.screencastFramemetadata which returns the physical device resolution regardless of viewport emulationBrowser.setContentsSizewhen viewport changes, so screencast captures frames at the correct resolutionmaxWidth/maxHeightparameters are updated--window-sizeat Chrome launch time viaLaunchOptions.viewport_size, reducing reliance on the experimentalsetContentsSizeat runtimesetContentsSizefailures viaeprintln!instead of silently ignoring withlet _ =Context
Fixes #1031. PR #952 fixed
statusmessages andPage.startScreencastparameters to use the configured viewport, but two issues remained:Emulation.setDeviceMetricsOverrideonly changes the CSS viewport — screencast captures the actual browser content area, which was still at the default--window-size=1280,720Test plan
deviceWidth/deviceHeightand image resolution now match the custom viewporte2e_stream_frame_metadata_respects_custom_viewportverifies both metadata and actual JPEG image dimensions (800x600)cargo checkpasses cleanly