Skip to content

Commit 2995d67

Browse files
committed
fix(react-ecs): stop dividing UI layout by devicePixelRatio
`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.
1 parent 0f012e0 commit 2995d67

4 files changed

Lines changed: 69 additions & 44 deletions

File tree

packages/@dcl/react-ecs/src/components/utils.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ export function getScaleAndUnit(scaleUnit: ScaleUnit): [number, ScaleUnits] {
5353
/**
5454
* @internal
5555
*/
56-
export function scaleOnDim(scale: number, dim: number, pxRatio: number) {
57-
return (dim / 100) * (scale / pxRatio)
56+
export function scaleOnDim(scale: number, dim: number) {
57+
return (dim / 100) * scale
5858
}
5959

6060
/**
@@ -192,10 +192,13 @@ export function calcOnViewport(value: ScaleUnit, ctx: ScaleContext | undefined =
192192
const [scale, unit] = getScaleAndUnit(value)
193193
if (!ctx) return scale
194194

195-
const { height, width, ratio } = ctx
195+
// `ctx.ratio` is intentionally not read: '1vw' is 1% of the canvas width by
196+
// definition, exactly as in CSS. It stays on ScaleContext as an informational
197+
// density hint for callers that need to pick an asset resolution.
198+
const { height, width } = ctx
196199

197-
if (unit === 'vh') return scaleOnDim(scale, height, ratio)
200+
if (unit === 'vh') return scaleOnDim(scale, height)
198201

199202
// by default, we scale by 'vw' (width)
200-
return scaleOnDim(scale, width, ratio)
203+
return scaleOnDim(scale, width)
201204
}

packages/@dcl/react-ecs/src/system.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,20 @@ export function createReactBasedUiSystem(engine: IEngine, pointerSystem: Pointer
142142

143143
if (!canvasInfo) return
144144

145-
const { width, height, devicePixelRatio } = canvasInfo
145+
const { width, height } = canvasInfo
146146
const { virtualWidth, virtualHeight } = activeVirtualSize
147147
if (!virtualWidth || !virtualHeight) return
148148

149-
// Normalize by devicePixelRatio so virtual px map to logical px (matching the
150-
// vw/vh path); without it the scale was inflated on high-dpr mobile screens.
151-
const ratio = devicePixelRatio || 1
152-
const nextScale = Math.min(width / virtualWidth, height / virtualHeight) / ratio
149+
// The scale factor is the contain-fit of the design resolution inside the canvas,
150+
// and nothing else.
151+
//
152+
// devicePixelRatio is deliberately absent. It is a density hint — "how many physical
153+
// pixels per canvas unit", for picking a 1x/2x/3x asset — not a layout unit, the same
154+
// role it has in CSS and React Native, where it is exposed but never enters layout.
155+
// Dividing by it made UI size inversely proportional to a quantity the scene author
156+
// does not control and that measures something different on every renderer: panel
157+
// density on mobile, OS display scaling on web, display/window on native desktop.
158+
const nextScale = Math.min(width / virtualWidth, height / virtualHeight)
153159
if (Number.isFinite(nextScale) && nextScale !== getUiScaleFactor()) {
154160
// Track ownership when updating to avoid cross-system conflicts.
155161
setUiScaleFactor(nextScale, uiScaleFactorOwner)

test/react-ecs/label.spec.tsx

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,23 +113,34 @@ describe('UiText React Ecs', () => {
113113
})
114114

115115
it('should scale font size using viewport width by default', () => {
116-
expect(scaleFontSize(16, undefined, scaleCtx)).toBeCloseTo(17.56)
116+
// 16 + 0.39% of 800
117+
expect(scaleFontSize(16, undefined, scaleCtx)).toBeCloseTo(19.12)
117118
})
118119

119120
it('should scale font size using viewport height when scale unit is "vh"', () => {
120-
expect(scaleFontSize(16, '10vh', scaleCtx)).toBeCloseTo(46)
121+
// 16 + 10% of 600
122+
expect(scaleFontSize(16, '10vh', scaleCtx)).toBeCloseTo(76)
121123
})
122124

123125
it('should scale font size correctly when scale unit is "vw"', () => {
124-
expect(scaleFontSize(16, '10vw', scaleCtx)).toBeCloseTo(56)
126+
// 16 + 10% of 800
127+
expect(scaleFontSize(16, '10vw', scaleCtx)).toBeCloseTo(96)
125128
})
126129

127130
it('should handle scaling with a numeric value', () => {
128-
expect(scaleFontSize(16, 10, scaleCtx)).toBeCloseTo(56)
131+
expect(scaleFontSize(16, 10, scaleCtx)).toBeCloseTo(96)
129132
})
130133

131134
it('should handle scaling with a numeric value and unit "vw"', () => {
132-
expect(scaleFontSize(16, '10.5vw', scaleCtx)).toBeCloseTo(58)
135+
expect(scaleFontSize(16, '10.5vw', scaleCtx)).toBeCloseTo(100)
136+
})
137+
138+
it('should not depend on the context ratio', () => {
139+
// 'Nvw' is N% of the canvas width by definition, exactly as in CSS. The density
140+
// hint on the context must not change what a viewport unit resolves to.
141+
for (const ratio of [0.5, 1, 2, 3]) {
142+
expect(scaleFontSize(16, '10vw', { ...scaleCtx, ratio })).toBeCloseTo(96)
143+
}
133144
})
134145
})
135146

test/react-ecs/virtual-scale-array.spec.tsx

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -311,11 +311,12 @@ describe('Virtual scale factor with mixed % and pixel values', () => {
311311
uiRenderer.destroy()
312312
})
313313

314-
it('should normalize the scale factor by devicePixelRatio', async () => {
314+
it('should not let devicePixelRatio affect the scale factor', async () => {
315315
const { engine, uiRenderer } = setupEngine()
316316
const UiTransform = components.UiTransform(engine)
317317
const UiCanvasInformation = components.UiCanvasInformation(engine)
318-
// Physical-pixel canvas on a high-DPR (mobile) screen: 3840x2160 @ dpr 2.
318+
// High-density screen: 3840x2160 @ dpr 2. The design resolution fits the canvas
319+
// exactly twice over, so that — and only that — is the scale factor.
319320
UiCanvasInformation.create(engine.RootEntity, {
320321
devicePixelRatio: 2,
321322
width: 3840,
@@ -328,41 +329,45 @@ describe('Virtual scale factor with mixed % and pixel values', () => {
328329

329330
await engine.update(1)
330331

331-
// scale = Math.min(3840/1920, 2160/1080) / 2 = 2 / 2 = 1
332-
expect(getUiScaleFactor()).toBe(1)
333-
334-
const panelEntity = (entityIndex + 1) as Entity
335-
expect(UiTransform.get(panelEntity).width).toBe(300) // 300 * 1
336-
expect(UiTransform.get(panelEntity).height).toBe(300) // 300 * 1
337-
338-
uiRenderer.destroy()
339-
})
340-
341-
it('should treat a missing/zero devicePixelRatio as 1', async () => {
342-
const { engine, uiRenderer } = setupEngine()
343-
const UiTransform = components.UiTransform(engine)
344-
const UiCanvasInformation = components.UiCanvasInformation(engine)
345-
UiCanvasInformation.create(engine.RootEntity, {
346-
devicePixelRatio: 0,
347-
width: 3840,
348-
height: 2160,
349-
interactableArea: { left: 0, right: 0, top: 0, bottom: 0 }
350-
})
351-
const entityIndex = engine.addEntity() as number
352-
353-
uiRenderer.setUiRenderer(() => <Panel1 />, { virtualWidth: 1920, virtualHeight: 1080 })
354-
355-
await engine.update(1)
356-
357-
// dpr 0 must not divide by zero — falls back to 1, so scale = 2
332+
// scale = Math.min(3840/1920, 2160/1080) = 2, undivided.
358333
expect(getUiScaleFactor()).toBe(2)
359334

360335
const panelEntity = (entityIndex + 1) as Entity
361336
expect(UiTransform.get(panelEntity).width).toBe(600) // 300 * 2
337+
expect(UiTransform.get(panelEntity).height).toBe(600) // 300 * 2
362338

363339
uiRenderer.destroy()
364340
})
365341

342+
it('should produce the same scale factor whatever devicePixelRatio reports', async () => {
343+
// The regression guard: two canvases identical in every way except the density
344+
// hint must lay out identically. dpr 0 is included because it used to be a
345+
// divide-by-zero guard, and there must no longer be a division to guard.
346+
for (const devicePixelRatio of [0, 1, 2, 3.5]) {
347+
const { engine, uiRenderer } = setupEngine()
348+
const UiTransform = components.UiTransform(engine)
349+
const UiCanvasInformation = components.UiCanvasInformation(engine)
350+
UiCanvasInformation.create(engine.RootEntity, {
351+
devicePixelRatio,
352+
width: 3840,
353+
height: 2160,
354+
interactableArea: { left: 0, right: 0, top: 0, bottom: 0 }
355+
})
356+
const entityIndex = engine.addEntity() as number
357+
358+
uiRenderer.setUiRenderer(() => <Panel1 />, { virtualWidth: 1920, virtualHeight: 1080 })
359+
360+
await engine.update(1)
361+
362+
expect(getUiScaleFactor()).toBe(2)
363+
364+
const panelEntity = (entityIndex + 1) as Entity
365+
expect(UiTransform.get(panelEntity).width).toBe(600) // 300 * 2
366+
367+
uiRenderer.destroy()
368+
}
369+
})
370+
366371
it('should use scale factor 1 when UiCanvasInformation is not yet available', async () => {
367372
const { engine, uiRenderer } = setupEngine()
368373
const UiTransform = components.UiTransform(engine)

0 commit comments

Comments
 (0)