Skip to content

Commit d4936b6

Browse files
eordanoclaude
andauthored
chore: state the DevicePixelRatio contract as a local guarantee
The fixture's class summary and the carried-over sdk7-test-scenes plan both described DevicePixelRatio as the divisor scenes apply to resolve vh/vw and virtual-scale sizing. js-sdk-toolchain #1489 (on main 2026-08-12) took the ratio out of layout entirely: calcOnViewport now resolves vw/vh from width and height alone and leaves ScaleContext.ratio unread, and scaleFontSize resolves through that same call. The summary therefore asserted a consumer contract the consumer no longer honours - the failure mode CLAUDE.md names when it bans comments that narrate external behaviour, since that behaviour moved without this file changing. The summary now states only what the system under test guarantees: it reports the scene panel's own physical-pixels-per-UI-point density and republishes when that density or the viewport moves. The same stale premise would have made the skipped scene unfalsifiable: watching a vw/vh box resize proves nothing about the ratio on a current SDK. FIXNOTES now has the verification read the printed value, names Label (react-ecs exports no UiText), and points at src/index.ts, which mirrors only width and height into the scene's canvasInfo today. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018c638dR1vPysCMbYt2qQg5 Also includes (squashed): fix: publish the ratio the device-pixel-ratio dirty check compared WriteToCRDT() re-read the live panel through GetDevicePixelRatio() even though UpdateUICanvasInformationComponent() had computed and cached that same value one statement earlier, so the gate and the payload were two independent reads of a mutable panel property - the ratio that passed the dirty check was not necessarily the ratio scenes received. Initialize() now seeds lastDevicePixelRatio before its unconditional publish, which lets both write paths emit the cached field and makes the published ratio the one the gate is keyed on. It also removes a second panel read per republish. The fixture now covers the dirty check this change touches: no republish when neither the viewport nor the panel ratio moves, and a republish carrying the new ratio when the panel scale does, driven through the injectable UIDocument / PanelSettings seam. That republish case is also what bounds the unattached-panel window at startup - a panel that attaches after Initialize() is picked up by the next tick. The class comment lost its incident narration and its citation of a local investigation directory that exists in no repo, and the single captured message became a published-message list, which drops the null-forgiving read. The SDK-facing half - an sdk7-test-scenes scene plus two-monitor verification steps for the changed DevicePixelRatio semantics - belongs in that separate public repo; FIXNOTES.md carries the scene sketch and the manual steps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018c638dR1vPysCMbYt2qQg5
1 parent 8dc895d commit d4936b6

2 files changed

Lines changed: 55 additions & 47 deletions

File tree

Explorer/Assets/DCL/SDKComponents/SceneUI/Systems/UICanvasInformation/UICanvasInformationSystem.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public override void Initialize()
3030
base.Initialize();
3131

3232
interactableArea = new BorderRect { Bottom = 0, Left = Screen.width * 0.25f, Right = 0, Top = 0 };
33+
lastDevicePixelRatio = GetDevicePixelRatio();
3334

3435
WriteToCRDT();
3536
}
@@ -81,7 +82,7 @@ private void WriteToCRDT()
8182
component.InteractableArea = system.interactableArea;
8283
component.Width = Screen.width;
8384
component.Height = Screen.height;
84-
component.DevicePixelRatio = system.GetDevicePixelRatio();
85+
component.DevicePixelRatio = system.lastDevicePixelRatio;
8586
}, SpecialEntitiesID.SCENE_ROOT_ENTITY, this);
8687
}
8788
}
Lines changed: 53 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using Arch.Core;
22
using Arch.SystemGroups;
33
using CRDT;
4+
using CrdtEcsBridge.Components.Special;
45
using CrdtEcsBridge.ECSToCRDTWriter;
56
using DCL.ECSComponents;
67
using DCL.SDKComponents.SceneUI.Systems.UICanvasInformation;
@@ -10,52 +11,42 @@
1011
using NUnit.Framework;
1112
using SceneRunner.Scene;
1213
using System;
14+
using System.Collections.Generic;
1315
using UnityEngine;
1416
using UnityEngine.UIElements;
1517

1618
namespace DCL.SDKComponents.SceneUI.Tests
1719
{
18-
// Regression coverage for the dpr-vh-vw-positionunit bug: UICanvasInformationSystem used to
19-
// publish DevicePixelRatio = Screen.mainWindowDisplayInfo.width / Screen.width (the
20-
// monitor-native-width / window-client-width ratio). That quantity is not a device pixel
21-
// ratio for unity-explorer's scene UI - the scene panel (DCLScenePanelSettings) is
22-
// ConstantPixelSize @ scale 1, so 1 UI-Toolkit point == 1 framebuffer pixel regardless of
23-
// which monitor hosts the window or its native resolution. Scenes derive vh/vw (and, since
24-
// js-sdk-toolchain #1433, every virtual-screen-scaled px value and scaleFontSize) from this
25-
// field via `points = (dim / 100) * (scale / devicePixelRatio)`, so the bogus ratio silently
26-
// mis-sizes `vw`/`vh` elements and live-resizes them when the window crosses monitors.
27-
//
28-
// The fix plumbs the scene UIDocument into the system and reports the panel's own
29-
// `scaledPixelsPerPoint` (1.0f fallback while unattached) instead of the monitor/window
30-
// ratio - see report.md "## Patch" and potential-fix.patch in
31-
// bugreports-late-jul/dpr-vh-vw-positionunit/.
20+
/// <summary>
21+
/// Pins the contract of PBUiCanvasInformation.DevicePixelRatio: it reports the scene panel's own
22+
/// physical-pixels-per-UI-point density, republished whenever that density or the viewport moves.
23+
/// </summary>
3224
public class UICanvasInformationSystemShould : UnitySystemTestBase<UICanvasInformationSystem>
3325
{
34-
// Deliberately NOT 1.0 (the system's own unattached-panel fallback) and NOT a common OS
35-
// DPI preset (100/125/150/175/200/225/250/300/350%): this isolates the injected-UIDocument
36-
// seam from the OLD formula's monitorWidth/windowWidth ratio. Screen and
37-
// Screen.mainWindowDisplayInfo are static Unity APIs with no injectable abstraction - the
38-
// ROBUST patch variant is testable only because it additionally plumbs a UIDocument, which
39-
// IS injectable. The OLD formula is a ratio of two real screen-pixel widths in the test
40-
// runner's own window/monitor; landing on exactly 2.75 by coincidence is not realistic.
26+
/// <summary>Panel scale the system is expected to report verbatim; deliberately not the unattached-panel fallback of 1.</summary>
4127
private const float SCENE_PANEL_SCALE = 2.75f;
4228

29+
/// <summary>Panel scale applied mid-test to move the ratio and trip the dirty check.</summary>
30+
private const float RESCALED_SCENE_PANEL_SCALE = 1.5f;
31+
32+
/// <summary>Tolerance for comparing a reported ratio against the panel scale that produced it.</summary>
33+
private const float RATIO_TOLERANCE = 0.001f;
34+
35+
private readonly List<PBUiCanvasInformation> publishedComponents = new ();
36+
4337
private IECSToCRDTWriter ecsToCRDTWriter;
4438
private GameObject canvasGameObject;
4539
private PanelSettings panelSettings;
4640
private UIDocument canvas;
47-
private PBUiCanvasInformation? capturedComponent;
4841

4942
[SetUp]
5043
public void SetUp()
5144
{
45+
publishedComponents.Clear();
5246
ecsToCRDTWriter = Substitute.For<IECSToCRDTWriter>();
53-
capturedComponent = null;
5447

55-
// UICanvasInformationSystem.WriteToCRDT() rents the message from the writer and
56-
// populates it via a static delegate - capture that delegate and invoke it against a
57-
// fresh instance to observe what would have been written, exactly as the real
58-
// IECSToCRDTWriter implementation would.
48+
// The system never touches the message itself: it hands the writer a static delegate that
49+
// fills a rented instance, so the delegate has to be run to observe what would be written.
5950
ecsToCRDTWriter
6051
.When(x => x.PutMessage(
6152
Arg.Any<Action<PBUiCanvasInformation, UICanvasInformationSystem>>(),
@@ -67,11 +58,10 @@ public void SetUp()
6758
var data = callInfo.Arg<UICanvasInformationSystem>();
6859
var component = new PBUiCanvasInformation();
6960
prepareMessage(component, data);
70-
capturedComponent = component;
61+
publishedComponents.Add(component);
7162
});
7263

73-
// ConstantPixelSize mirrors the shipped DCLScenePanelSettings.asset (m_ScaleMode: 0)
74-
// but with a scale the OLD Screen-ratio formula cannot plausibly reproduce.
64+
// ConstantPixelSize mirrors the shipped DCLScenePanelSettings.asset (m_ScaleMode: 0).
7565
panelSettings = ScriptableObject.CreateInstance<PanelSettings>();
7666
panelSettings.scaleMode = PanelScaleMode.ConstantPixelSize;
7767
panelSettings.scale = SCENE_PANEL_SCALE;
@@ -80,14 +70,17 @@ public void SetUp()
8070
canvas = canvasGameObject.AddComponent<UIDocument>();
8171
canvas.panelSettings = panelSettings;
8272

73+
Assert.That(canvas.rootVisualElement?.panel, Is.Not.Null,
74+
"The scene UIDocument must be attached to a live panel, otherwise the system reports its unattached fallback instead of the panel ratio.");
75+
8376
var builder = new ArchSystemsWorldBuilder<World>(world);
8477

85-
// UICanvasInformationSystem is [UpdateInGroup(typeof(SyncedInitializationSystemGroup))] -
86-
// a CUSTOM group, so it must be injected into the builder before the system is, exactly as
87-
// production does in ECSWorldFactory.cs (InjectCustomGroup(new SyncedInitializationSystemGroup(...))).
88-
// Otherwise InjectToWorld throws Arch.SystemGroups.GroupNotFoundException in SetUp.
78+
// SyncedInitializationSystemGroup is a custom group: InjectToWorld throws GroupNotFoundException
79+
// unless it is injected first, as production does in ECSWorldFactory.
8980
builder.InjectCustomGroup(new SyncedInitializationSystemGroup(Substitute.For<ISceneStateProvider>()));
9081
system = UICanvasInformationSystem.InjectToWorld(ref builder, ecsToCRDTWriter, canvas);
82+
83+
world.Create(new SceneRootComponent());
9184
}
9285

9386
protected override void OnTearDown()
@@ -102,22 +95,36 @@ protected override void OnTearDown()
10295
[Test]
10396
public void ReportScenePanelPixelsPerPointAsDevicePixelRatio()
10497
{
105-
// Arrange-time invariant: the seam only exercises the fixed code path once the
106-
// UIDocument is actually attached to a live panel.
107-
Assert.That(canvas.rootVisualElement, Is.Not.Null);
108-
Assert.That(canvas.rootVisualElement.panel, Is.Not.Null);
98+
system.Initialize();
99+
100+
Assert.That(publishedComponents, Is.Not.Empty, "Initialize() must publish a PBUiCanvasInformation via PutMessage.");
101+
Assert.That(publishedComponents[0].DevicePixelRatio, Is.EqualTo(SCENE_PANEL_SCALE).Within(RATIO_TOLERANCE));
102+
}
109103

110-
// Act - Initialize() publishes unconditionally (bypasses the dirty-check).
104+
[Test]
105+
public void NotRepublishWhenViewportAndPixelsPerPointAreUnchanged()
106+
{
107+
system.Initialize();
108+
system.Update(0);
109+
110+
int publishedAfterFirstUpdate = publishedComponents.Count;
111+
system.Update(0);
112+
113+
Assert.That(publishedComponents.Count, Is.EqualTo(publishedAfterFirstUpdate), "A tick that changes neither the viewport nor the panel ratio must not republish.");
114+
}
115+
116+
[Test]
117+
public void RepublishWhenScenePanelPixelsPerPointChanges()
118+
{
111119
system.Initialize();
120+
system.Update(0);
112121

113-
// Assert
114-
Assert.That(capturedComponent, Is.Not.Null, "UICanvasInformationSystem.Initialize() must publish a PBUiCanvasInformation via PutMessage.");
122+
int publishedAfterFirstUpdate = publishedComponents.Count;
123+
panelSettings.scale = RESCALED_SCENE_PANEL_SCALE;
124+
system.Update(0);
115125

116-
// Pre-fix: DevicePixelRatio = Screen.mainWindowDisplayInfo.width / Screen.width (the
117-
// monitor/window ratio) - unrelated to SCENE_PANEL_SCALE, so this fails.
118-
// Post-fix: DevicePixelRatio = canvas.rootVisualElement.scaledPixelsPerPoint, i.e.
119-
// exactly the configured panel scale, so this passes.
120-
Assert.That(capturedComponent!.DevicePixelRatio, Is.EqualTo(SCENE_PANEL_SCALE).Within(0.001f));
126+
Assert.That(publishedComponents.Count, Is.EqualTo(publishedAfterFirstUpdate + 1));
127+
Assert.That(publishedComponents[^1].DevicePixelRatio, Is.EqualTo(RESCALED_SCENE_PANEL_SCALE).Within(RATIO_TOLERANCE));
121128
}
122129
}
123130
}

0 commit comments

Comments
 (0)