fix(react-ecs): stop dividing UI layout by devicePixelRatio - #1515
Closed
leanmendoza wants to merge 1 commit into
Closed
fix(react-ecs): stop dividing UI layout by devicePixelRatio#1515leanmendoza wants to merge 1 commit into
leanmendoza wants to merge 1 commit into
Conversation
`uiScaleFactor` is now exactly the contain-fit of the design resolution inside the canvas, and `scaleOnDim` resolves 'Nvw'/'Nvh' and string `fontSize` to N% of the canvas dimension, as in CSS. devicePixelRatio is a density hint for picking a 1x/2x/3x asset, and each renderer computes it differently, so dividing by it made UI size inversely proportional to whichever value the scene happened to get. The field stays on PBUiCanvasInformation and `ScaleContext.ratio` stays in the public API. No renderer change is required. BREAKING CHANGE: scenes that set a virtual size and were calibrated against the current behaviour will render devicePixelRatio times larger. Scenes that pass no virtual size keep uiScaleFactor === 1 and are unaffected.
Deploying js-sdk-toolchain with
|
| Latest commit: |
2995d67
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1bc39cdd.js-sdk-toolchain.pages.dev |
| Branch Preview URL: | https://fix-remove-device-pixel-rati.js-sdk-toolchain.pages.dev |
Contributor
Test this pull request
|
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
1 participant
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
Removes
devicePixelRatiofrom the two places where it participates in UI layout:UiScaleSystem—uiScaleFactoris now exactly the contain-fit of the design resolution inside the canvas.scaleOnDim—'Nvw'/'Nvh'and stringfontSizenow resolve to N % of the canvas dimension, as they do in CSS. Note this path applies even when no virtual size is set.The field stays on
PBUiCanvasInformationandScaleContext.ratiostays in the public API. Nothing about what renderers report changes, and no renderer change is required.Why
devicePixelRatioanswers "how many physical pixels does one canvas unit occupy?" — a density hint, for picking a 1x / 2x / 3x asset. Each renderer answers it differently:All three are defensible readings of a density hint. None of them is a layout unit — and dividing the contain-fit by it is what turns whichever one you get into one.
Measured with one scene and one build, an element declared
width: 1600against a1600x720design:The Unity row shows the divisor is not a density at all: the window was 2244 px wide on a 3024 px display, and
2244 / 3024is the reported1.3476. Scene UI ended up covering 74 % of its design — the same fraction of the monitor the window happened to occupy. Drag the window and the number moves.The Godot row is the other end: because that client publishes a canvas normalized to 1600 wide, the contain-fit against a
1600x720design is exactly1.0000, so the entire 32 % shortfall is the divisor.This also matches CSS and React Native, where the ratio is exposed but never enters layout (
50vwis half the viewport, never divided bywindow.devicePixelRatio), and the protobuf, which already states: "the width of the canvas, in virtual pixels; this value does not change when the pixel ratio changes."Impact
Every renderer is internally consistent — each reports the canvas in the same unit in which it interprets
UiTransformpx — so this fixes all of them at once with no client change.Breaking for content calibrated against current behaviour, which will render
devicePixelRatiotimes larger. Two things scope it:uiScaleFactor === 1and are unaffected.1600x720default on the Godot mobile client, the contain-fit is1.0000, so removing the divisor leavesuiScaleFactorat 1 — the "scenes not using any virtual screen at all" bucket stops being affected there.Tests
virtual-scale-array.spec.tsx— the two tests that pinned the old behaviour now assert the new one, plus a regression guard that sweepsdevicePixelRatioover[0, 1, 2, 3.5]and requires an identical layout.0is kept in the sweep because it used to be a divide-by-zero guard, and there must no longer be a division to guard.label.spec.tsx—scaleFontSizeexpectations updated ('10vw'on an 800-wide context is now 10 % of 800), plus a case asserting the result does not depend onctx.ratio.npx jest test/react-ecs— 120 passed. No change to the API report.