-
Notifications
You must be signed in to change notification settings - Fork 182
Viewport resolution scale parameter 2 #759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Viewport resolution scale parameter 2 #759
Conversation
🦋 Changeset detectedLatest commit: a568f6a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
We ran this through some QA and I have left a few comments about what we would like changed before we can merge this.
In addition to those suggestions below we would also like:
- The new setting to be exposed to the UI by modifying something like this (untested, please verify):
if (isSectionEnabled(settingsConfig, SettingsSections.UI)) {
/* Setup all view/ui related settings under this section */
const viewSettingsSection = this.buildSectionWithHeading(settingsElem, SettingsSections.UI);
if (isSettingEnabled(settingsConfig, Flags.MatchViewportResolution))
this.addSettingFlag(viewSettingsSection, this.flagsUi.get(Flags.MatchViewportResolution));
// Your new setting here
if (isSettingEnabled(settingsConfig, NumericParameters.ViewportResolutionScale))
this.addSettingNumeric(viewSettingsSection, NumericParameters.ViewportResolutionScale);
// ... Other view settings
}- A changeset added with accompanying long form change log entry, see: #759 (comment) for how to make a changset - When you make the changeset, it will be a minor change bump to both the library and UI library packages.
| 'Viewport Resolution Scale', | ||
| 'Scale factor for viewport resolution when MatchViewportResolution is enabled. 1.0 = 100%, 0.5 = 50%, 2.0 = 200%.', | ||
| 0.1 /*min*/, | ||
| 10.0 /*max*/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did some QA on this and we think 3x is a sane maximum. Values above that will commonly cause the encoder to fail as this will exceed the maximum encoding size on our H.264 settings - e.g. 4096x4096.
| 10.0 /*max*/, | |
| 3.0 /*max*/, |
Frontend/Docs/Settings Panel.md
Outdated
| ### UI | ||
| | **Setting** | **Description** | | ||
| | --- | --- | | ||
| | **Match viewport resolution** | Resizes the Unreal Engine application resolution to match the browser's video element size.| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| | **Match viewport resolution** | Resizes the Unreal Engine application resolution to match the browser's video element size.| | |
| | **Match Viewport Resolution** | Resizes the Unreal Engine application resolution to match the browser's video element size. (Note: We recommend using `-windowed` on the UE side to allow scaling beyond monitor size.)| |
Frontend/Docs/Settings Panel.md
Outdated
| | **Setting** | **Description** | | ||
| | --- | --- | | ||
| | **Match viewport resolution** | Resizes the Unreal Engine application resolution to match the browser's video element size.| | ||
| | **Viewport Resolution Scale** | Scale factor for viewport resolution when Match Viewport Resolution is enabled. Range: 0.1-10.0, Default: 1.0. Values above 1.0 (e.g., 1.5, 2.0) can improve visual quality on small screens by requesting higher resolution streams. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| | **Viewport Resolution Scale** | Scale factor for viewport resolution when Match Viewport Resolution is enabled. Range: 0.1-10.0, Default: 1.0. Values above 1.0 (e.g., 1.5, 2.0) can improve visual quality on small screens by requesting higher resolution streams. | | |
| | **Viewport Resolution Scale** | Scale factor for viewport resolution when Match Viewport Resolution is enabled. Range: 0.1-3.0, Default: 1.0 (no scaling). Values above 1.0 (e.g., 1.5, 2.0) can improve visual quality on small screens by requesting higher resolution streams. | |
|
Hi @lukehb, |
Relevant components:
Problem statement:
When connecting from an iPhone or small screen device with MatchViewportRes set to true, the rendered resolution appears quite low, causing the visuals to look slightly pixelated.
More info here: #721
Solution
Added a new numeric parameter ViewportResolutionScale that multiplies the resolution dimensions sent to the onMatchViewportResolutionCallback when MatchViewportResolution is enabled. The scale factor is applied to both width and height in VideoPlayer.updateVideoStreamSize(), allowing users to request higher resolution streams (e.g., 1.5x or 2.0x) to improve visual quality on small screens.
Documentation
The new ViewportResolutionScale parameter is documented in Frontend/Docs/Settings Panel.md under the UI section.
Test Plan and Compatibility
Existing unit tests: Config.test.ts automatically covers the new parameter via the "should populate initial values for all settings" test (line 42-49), which validates all numeric parameters are registered