-
Notifications
You must be signed in to change notification settings - Fork 6k
Switch ui.window.devicePixelRatio from browser logical to physical. #17209
Changes from all commits
65fb5d3
bd58d35
2165d86
542be23
8705651
aab46b4
101083d
a328874
3af31ef
472615b
489ad43
7d31069
3ae5e40
16a0f0f
d8907b0
a2aaafb
19da8b6
2b99962
6ff40c2
8d087b4
44d045c
e456f3d
c34111b
47da9ce
81fd917
940e44d
1d864dc
2419fdb
d455d74
be2113d
6b227a7
5f21300
2eaae7e
50f6c64
158533c
de3b6a7
fcffbec
b710b87
68e85ca
01c5bf5
47a11e5
ea15a3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter/services.dart'; | ||
|
|
||
| void main() async { | ||
| const MethodChannel channel = | ||
| OptionalMethodChannel('flutter/web_test_e2e', JSONMethodCodec()); | ||
| await channel.invokeMethod<void>( | ||
| 'setDevicePixelRatio', | ||
| '1.5', | ||
| ); | ||
| runApp(MyApp()); | ||
| } | ||
|
|
||
| class MyApp extends StatefulWidget { | ||
| @override | ||
| MyAppState createState() => MyAppState(); | ||
| } | ||
|
|
||
| class MyAppState extends State<MyApp> { | ||
| @override | ||
| Widget build(BuildContext context) { | ||
| return MaterialApp( | ||
| key: const Key('mainapp'), | ||
| title: 'Integration Test App', | ||
| home: Image.asset('assets/images/sample_image1.png') | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,3 +16,7 @@ dev_dependencies: | |
| e2e: 0.2.4+4 | ||
| http: 0.12.0+2 | ||
| test: any | ||
|
|
||
| flutter: | ||
| assets: | ||
| - assets/images/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:html' as html; | ||
| import 'package:flutter/widgets.dart'; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:regular_integration_tests/image_loading_main.dart' as app; | ||
|
|
||
| import 'package:e2e/e2e.dart'; | ||
|
|
||
| void main() { | ||
| E2EWidgetsFlutterBinding.ensureInitialized() as E2EWidgetsFlutterBinding; | ||
|
|
||
| testWidgets('Image loads asset variant based on device pixel ratio', | ||
| (WidgetTester tester) async { | ||
| app.main(); | ||
| await tester.pumpAndSettle(); | ||
| final html.ImageElement imageElement = html.querySelector('img') as html.ImageElement; | ||
| expect(imageElement.naturalWidth, 1.5 * 100); | ||
| expect(imageElement.naturalHeight, 1.5 * 100); | ||
| expect(imageElement.width, 100); | ||
| expect(imageElement.height, 100); | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:io'; | ||
|
|
||
| import 'package:flutter_driver/flutter_driver.dart'; | ||
|
|
||
| Future<void> main() async { | ||
| final FlutterDriver driver = await FlutterDriver.connect(); | ||
|
|
||
| final String dataRequest = | ||
| await driver.requestData(null, timeout: const Duration(seconds: 1)); | ||
| await driver.close(); | ||
|
|
||
| exit(dataRequest == 'pass' ? 0 : 1); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,8 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { | |
| _surfaceStack.add(PersistedScene(_lastFrameScene)); | ||
| } | ||
|
|
||
| final List<PersistedContainerSurface> _surfaceStack = <PersistedContainerSurface>[]; | ||
| final List<PersistedContainerSurface> _surfaceStack = | ||
| <PersistedContainerSurface>[]; | ||
|
|
||
| /// The scene built by this scene builder. | ||
| /// | ||
|
|
@@ -19,7 +20,8 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { | |
| assert(() { | ||
| if (_surfaceStack.length != 1) { | ||
| final String surfacePrintout = _surfaceStack | ||
| .map<Type>((PersistedContainerSurface surface) => surface.runtimeType) | ||
| .map<Type>( | ||
| (PersistedContainerSurface surface) => surface.runtimeType) | ||
| .toList() | ||
| .join(', '); | ||
| throw Exception('Incorrect sequence of push/pop operations while ' | ||
|
|
@@ -42,7 +44,8 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { | |
| // the live tree. | ||
| if (surface.oldLayer != null) { | ||
| assert(surface.oldLayer.runtimeType == surface.runtimeType); | ||
| assert(debugAssertSurfaceState(surface.oldLayer, PersistedSurfaceState.active)); | ||
| assert(debugAssertSurfaceState( | ||
| surface.oldLayer, PersistedSurfaceState.active)); | ||
| surface.oldLayer.state = PersistedSurfaceState.pendingUpdate; | ||
| } | ||
| _adoptSurface(surface); | ||
|
|
@@ -97,6 +100,16 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { | |
| if (matrix4.length != 16) { | ||
| throw ArgumentError('"matrix4" must have 16 entries.'); | ||
| } | ||
| if (_surfaceStack.length == 1) { | ||
| // Top level transform contains view configuration to scale | ||
| // scene to devicepixelratio. Use identity instead since CSS uses | ||
| // logical device pixels. | ||
| if (!ui.debugEmulateFlutterTesterEnvironment) { | ||
| assert(matrix4[0] == window.devicePixelRatio && | ||
| matrix4[5] == window.devicePixelRatio); | ||
| } | ||
| matrix4 = Matrix4.identity().storage; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, now that I think about it, the correct fix would be to inject a transform that scales by 1/DPR. This way we don't have to rely on a particular framework behavior. To avoid extra div nodes we can do something like: In most cases we'll hit the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The framework itself injects pushTransform using ViewConfiguration. Ideally this code would not live in framework but in engine. See view.dart:39. Or instead of calling pushTransform , framework would call SceneBuilder.setViewConfiguration in engine.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but you can use |
||
| } | ||
| return _pushSurface(PersistedTransform(oldLayer, matrix4)); | ||
| } | ||
|
|
||
|
|
@@ -276,7 +289,8 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { | |
| void addRetained(ui.EngineLayer retainedLayer) { | ||
| final PersistedContainerSurface retainedSurface = retainedLayer; | ||
| if (assertionsEnabled) { | ||
| assert(debugAssertSurfaceState(retainedSurface, PersistedSurfaceState.active, PersistedSurfaceState.released)); | ||
| assert(debugAssertSurfaceState(retainedSurface, | ||
| PersistedSurfaceState.active, PersistedSurfaceState.released)); | ||
| } | ||
| retainedSurface.tryRetain(); | ||
| _adoptSurface(retainedSurface); | ||
|
|
@@ -319,7 +333,8 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { | |
| /// for more details. | ||
| @override | ||
| void addPerformanceOverlay(int enabledOptions, ui.Rect bounds) { | ||
| _addPerformanceOverlay(enabledOptions, bounds.left, bounds.right, bounds.top, bounds.bottom); | ||
| _addPerformanceOverlay( | ||
| enabledOptions, bounds.left, bounds.right, bounds.top, bounds.bottom); | ||
| } | ||
|
|
||
| /// Whether we've already warned the user about the lack of the performance | ||
|
|
@@ -337,7 +352,8 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { | |
| ) { | ||
| if (!_webOnlyDidWarnAboutPerformanceOverlay) { | ||
| _webOnlyDidWarnAboutPerformanceOverlay = true; | ||
| html.window.console.warn('The performance overlay isn\'t supported on the web'); | ||
| html.window.console | ||
| .warn('The performance overlay isn\'t supported on the web'); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -377,7 +393,8 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { | |
| _addTexture(offset.dx, offset.dy, width, height, textureId); | ||
| } | ||
|
|
||
| void _addTexture(double dx, double dy, double width, double height, int textureId) { | ||
| void _addTexture( | ||
| double dx, double dy, double width, double height, int textureId) { | ||
| // In test mode, allow this to be a no-op. | ||
| if (!ui.debugEmulateFlutterTesterEnvironment) { | ||
| throw UnimplementedError('Textures are not supported in Flutter Web'); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,19 +15,11 @@ class EngineWindow extends ui.Window { | |
| } | ||
|
|
||
| @override | ||
| double get devicePixelRatio { | ||
| if (_debugDevicePixelRatio != null) { | ||
| return _debugDevicePixelRatio; | ||
| } | ||
| double get devicePixelRatio => _debugDevicePixelRatio != null | ||
| ? _debugDevicePixelRatio | ||
| : browserDevicePixelRatio; | ||
|
|
||
| if (experimentalUseSkia) { | ||
| return browserDevicePixelRatio; | ||
| } else { | ||
| return 1.0; | ||
| } | ||
| } | ||
|
|
||
| /// Returns device pixel ratio returns by browser. | ||
| /// Returns device pixel ratio returned by browser. | ||
| static double get browserDevicePixelRatio { | ||
| double ratio = html.window.devicePixelRatio; | ||
| // Guard against WebOS returning 0. | ||
|
|
@@ -38,10 +30,7 @@ class EngineWindow extends ui.Window { | |
| /// | ||
| /// This is useful in tests to emulate screens of different dimensions. | ||
| void debugOverrideDevicePixelRatio(double value) { | ||
| assert(() { | ||
| _debugDevicePixelRatio = value; | ||
| return true; | ||
| }()); | ||
| _debugDevicePixelRatio = value; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change? It's still used only in tests, so the "debug" prefix and
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is now also used in integration test (the one that loads an image at framework level).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but that's still a test :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do integration tests strip out asserts?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If they don't, we should add the option to enable them. /cc @nturgut
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My comment is non-blocking though. I don't think there's major risk. It just doesn't feel right for a method that's not annotated as "debug" to manipulate a "debug" field without an assertion. |
||
| } | ||
|
|
||
| double _debugDevicePixelRatio; | ||
|
|
@@ -160,26 +149,38 @@ class EngineWindow extends ui.Window { | |
| case 'HapticFeedback.vibrate': | ||
| final String type = decoded.arguments; | ||
| domRenderer.vibrate(_getHapticFeedbackDuration(type)); | ||
| _replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); | ||
| return; | ||
| case 'SystemChrome.setApplicationSwitcherDescription': | ||
| final Map<String, dynamic> arguments = decoded.arguments; | ||
| domRenderer.setTitle(arguments['label']); | ||
| domRenderer.setThemeColor(ui.Color(arguments['primaryColor'])); | ||
| _replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); | ||
| return; | ||
| case 'SystemSound.play': | ||
| // There are no default system sounds on web. | ||
| _replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); | ||
| return; | ||
| case 'Clipboard.setData': | ||
| ClipboardMessageHandler().setDataMethodCall(decoded, callback); | ||
| _replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); | ||
| return; | ||
| case 'Clipboard.getData': | ||
| ClipboardMessageHandler().getDataMethodCall(callback); | ||
| _replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); | ||
| return; | ||
| } | ||
| break; | ||
|
|
||
| case 'flutter/textinput': | ||
| textEditing.channel.handleTextInput(data); | ||
| textEditing.channel.handleTextInput(data, callback); | ||
| return; | ||
|
|
||
| case 'flutter/web_test_e2e': | ||
| const MethodCodec codec = JSONMethodCodec(); | ||
| _replyToPlatformMessage(callback, codec.encodeSuccessEnvelope( | ||
| _handleWebTestEnd2EndMessage(codec, data) | ||
| )); | ||
| return; | ||
ferhatb marked this conversation as resolved.
Show resolved
Hide resolved
ferhatb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| case 'flutter/platform_views': | ||
|
|
@@ -192,7 +193,9 @@ class EngineWindow extends ui.Window { | |
|
|
||
| case 'flutter/accessibility': | ||
| // In widget tests we want to bypass processing of platform messages. | ||
| accessibilityAnnouncements.handleMessage(data); | ||
| final StandardMessageCodec codec = StandardMessageCodec(); | ||
| accessibilityAnnouncements.handleMessage(codec, data); | ||
| _replyToPlatformMessage(callback, codec.encodeMessage(true)); | ||
| return; | ||
|
|
||
| case 'flutter/navigation': | ||
|
|
@@ -204,9 +207,11 @@ class EngineWindow extends ui.Window { | |
| case 'routePushed': | ||
| case 'routeReplaced': | ||
| _browserHistory.setRouteName(message['routeName']); | ||
| _replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); | ||
| break; | ||
| case 'routePopped': | ||
| _browserHistory.setRouteName(message['previousRouteName']); | ||
| _replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); | ||
| break; | ||
| } | ||
| return; | ||
|
|
@@ -250,7 +255,9 @@ class EngineWindow extends ui.Window { | |
| ByteData data, | ||
| ) { | ||
| Future<void>.delayed(Duration.zero).then((_) { | ||
| callback(data); | ||
| if (callback != null) { | ||
| callback(data); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -315,6 +322,20 @@ class EngineWindow extends ui.Window { | |
| Rasterizer rasterizer = experimentalUseSkia ? Rasterizer(Surface()) : null; | ||
| } | ||
|
|
||
| bool _handleWebTestEnd2EndMessage(MethodCodec codec, ByteData data) { | ||
| final MethodCall decoded = codec.decodeMethodCall(data); | ||
| final Map<String, dynamic> message = decoded.arguments; | ||
| double ratio = double.parse(decoded.arguments); | ||
| bool result = false; | ||
| switch(decoded.method) { | ||
| case 'setDevicePixelRatio': | ||
| window.debugOverrideDevicePixelRatio(ratio); | ||
| window.onMetricsChanged(); | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| /// The window singleton. | ||
| /// | ||
| /// `dart:ui` window delegates to this value. However, this value has a wider | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.