Skip to content

Commit 242d4f9

Browse files
gmackallmackall-work
gmackall
and
mackall-work
authored
Add a cli flag for toggling HCPP use, and enable it (#182516)
1. Adds the flag (`--enable-surface-control`) 2. Adds a driver test which tests that we can upgrade all 3 of the create calls (HC, TLHC w VD fallback, TLHC w HC fallback) 3. Respects its use in create call Does not yet rename it, will do that in a follow up as it's logically independent. --------- Co-authored-by: Gray Mackall <mackall@google.com>
1 parent 5f9dc04 commit 242d4f9

10 files changed

Lines changed: 303 additions & 17 deletions

File tree

‎dev/bots/suite_runners/run_android_engine_tests.dart‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Future<void> runAndroidEngineTests({required ImpellerBackend impellerBackend}) a
5757
// TODO(matanlurey): Enable once `flutter drive` retains error logs.
5858
// final RegExp impellerStdoutPattern = RegExp('Using the Imepller rendering backend (.*)');
5959

60-
Future<void> runTest(FileSystemEntity file) async {
60+
Future<void> runTest(FileSystemEntity file, {bool useHCPPFlag = false}) async {
6161
final CommandResult result = await runCommand(
6262
'flutter',
6363
<String>[
@@ -68,6 +68,7 @@ Future<void> runAndroidEngineTests({required ImpellerBackend impellerBackend}) a
6868
// make less things start up unnecessarily.
6969
'--no-dds',
7070
'--no-enable-dart-profiling',
71+
if (useHCPPFlag) '--enable-surface-control',
7172
'--test-arguments=test',
7273
'--test-arguments=--reporter=expanded',
7374
],
@@ -107,6 +108,18 @@ Future<void> runAndroidEngineTests({required ImpellerBackend impellerBackend}) a
107108

108109
// Test HCPP Platform Views on Vulkan.
109110
if (impellerBackend == ImpellerBackend.vulkan) {
111+
final runFirstTests = <String>[
112+
// Run upgrade_legacy_pv_types first, as it is testing the flag and not the manifest
113+
'upgrade_legacy_pv_types',
114+
];
115+
116+
for (final testName in runFirstTests) {
117+
await runTest(
118+
mains.firstWhere((FileSystemEntity file) => file.path.contains(testName)),
119+
useHCPPFlag: true,
120+
);
121+
}
122+
110123
androidManifestXml.writeAsStringSync(
111124
androidManifestXml.readAsStringSync().replaceFirst(
112125
kSurfaceControlMetadataDisabled,
@@ -116,8 +129,9 @@ Future<void> runAndroidEngineTests({required ImpellerBackend impellerBackend}) a
116129
for (final file in mains) {
117130
// This statement is attempting to catch all tests inside of the
118131
// dev/integration_tests/android_engine_test/lib/hcpp
119-
// directory.
120-
if (!file.path.contains('hcpp')) {
132+
// directory, except for upgrade_legacy_pv_types which we already ran.
133+
if (!file.path.contains('hcpp') ||
134+
runFirstTests.any((String name) => file.path.contains(name))) {
121135
continue;
122136
}
123137
await runTest(file);

‎dev/integration_tests/android_engine_test/lib/hcpp/upgrade_legacy_pv_types_main.dart‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:convert';
6+
7+
import 'package:android_driver_extensions/extension.dart';
8+
import 'package:flutter/foundation.dart';
9+
import 'package:flutter/gestures.dart';
10+
import 'package:flutter/material.dart';
11+
import 'package:flutter/rendering.dart';
12+
import 'package:flutter/services.dart';
13+
import 'package:flutter_driver/driver_extension.dart';
14+
15+
import '../src/allow_list_devices.dart';
16+
17+
void main() async {
18+
ensureAndroidDevice();
19+
enableFlutterDriverExtension(
20+
handler: (String? command) async {
21+
return json.encode(<String, Object?>{
22+
'supported': await HybridAndroidViewController.checkIfSupported(),
23+
});
24+
},
25+
commands: <CommandExtension>[nativeDriverCommands],
26+
);
27+
28+
runApp(const MyApp());
29+
}
30+
31+
class MyApp extends StatefulWidget {
32+
const MyApp({super.key});
33+
34+
@override
35+
State<MyApp> createState() => _MyAppState();
36+
}
37+
38+
class _MyAppState extends State<MyApp> {
39+
bool _showViews = true;
40+
41+
@override
42+
Widget build(BuildContext context) {
43+
return MaterialApp(
44+
debugShowCheckedModeBanner: false,
45+
home: Scaffold(
46+
appBar: AppBar(title: const Text('HCPP Upgrade Smoke Test')),
47+
body: Column(
48+
children: <Widget>[
49+
ElevatedButton(
50+
key: const ValueKey<String>('ToggleViews'),
51+
onPressed: () => setState(() => _showViews = !_showViews),
52+
child: Text(_showViews ? 'Remove Views' : 'Add Views'),
53+
),
54+
if (_showViews) ...<Widget>[
55+
const Expanded(child: _HCPlatformView(viewType: 'box_platform_view')),
56+
const Expanded(child: _TLHCWithHCFallbackPlatformView(viewType: 'box_platform_view')),
57+
const Expanded(child: _TLHCWithVDFallbackPlatformView(viewType: 'box_platform_view')),
58+
],
59+
],
60+
),
61+
),
62+
);
63+
}
64+
}
65+
66+
/// Uses initExpensiveAndroidView — the HC path.
67+
class _HCPlatformView extends StatelessWidget {
68+
const _HCPlatformView({required this.viewType});
69+
final String viewType;
70+
71+
@override
72+
Widget build(BuildContext context) {
73+
return PlatformViewLink(
74+
viewType: viewType,
75+
surfaceFactory: (BuildContext context, PlatformViewController controller) {
76+
return AndroidViewSurface(
77+
controller: controller as AndroidViewController,
78+
gestureRecognizers: const <Factory<OneSequenceGestureRecognizer>>{},
79+
hitTestBehavior: PlatformViewHitTestBehavior.opaque,
80+
);
81+
},
82+
onCreatePlatformView: (PlatformViewCreationParams params) {
83+
return PlatformViewsService.initExpensiveAndroidView(
84+
id: params.id,
85+
viewType: viewType,
86+
layoutDirection: TextDirection.ltr,
87+
creationParamsCodec: const StandardMessageCodec(),
88+
)
89+
..addOnPlatformViewCreatedListener(params.onPlatformViewCreated)
90+
..create();
91+
},
92+
);
93+
}
94+
}
95+
96+
/// Uses initSurfaceAndroidView — the TLHC (with HC fallback) path.
97+
class _TLHCWithHCFallbackPlatformView extends StatelessWidget {
98+
const _TLHCWithHCFallbackPlatformView({required this.viewType});
99+
final String viewType;
100+
101+
@override
102+
Widget build(BuildContext context) {
103+
return PlatformViewLink(
104+
viewType: viewType,
105+
surfaceFactory: (BuildContext context, PlatformViewController controller) {
106+
return AndroidViewSurface(
107+
controller: controller as AndroidViewController,
108+
gestureRecognizers: const <Factory<OneSequenceGestureRecognizer>>{},
109+
hitTestBehavior: PlatformViewHitTestBehavior.opaque,
110+
);
111+
},
112+
onCreatePlatformView: (PlatformViewCreationParams params) {
113+
return PlatformViewsService.initSurfaceAndroidView(
114+
id: params.id,
115+
viewType: viewType,
116+
layoutDirection: TextDirection.ltr,
117+
creationParamsCodec: const StandardMessageCodec(),
118+
)
119+
..addOnPlatformViewCreatedListener(params.onPlatformViewCreated)
120+
..create();
121+
},
122+
);
123+
}
124+
}
125+
126+
/// Uses initAndroidView — the TLHC/VD path.
127+
class _TLHCWithVDFallbackPlatformView extends StatelessWidget {
128+
const _TLHCWithVDFallbackPlatformView({required this.viewType});
129+
final String viewType;
130+
131+
@override
132+
Widget build(BuildContext context) {
133+
return PlatformViewLink(
134+
viewType: viewType,
135+
surfaceFactory: (BuildContext context, PlatformViewController controller) {
136+
return AndroidViewSurface(
137+
controller: controller as AndroidViewController,
138+
gestureRecognizers: const <Factory<OneSequenceGestureRecognizer>>{},
139+
hitTestBehavior: PlatformViewHitTestBehavior.opaque,
140+
);
141+
},
142+
onCreatePlatformView: (PlatformViewCreationParams params) {
143+
return PlatformViewsService.initAndroidView(
144+
id: params.id,
145+
viewType: viewType,
146+
layoutDirection: TextDirection.ltr,
147+
creationParamsCodec: const StandardMessageCodec(),
148+
)
149+
..addOnPlatformViewCreatedListener(params.onPlatformViewCreated)
150+
..create();
151+
},
152+
);
153+
}
154+
}

‎dev/integration_tests/android_engine_test/test_driver/hcpp/upgrade_legacy_pv_types_main_test.dart‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:convert';
6+
import 'dart:io' as io;
7+
8+
import 'package:android_driver_extensions/native_driver.dart';
9+
import 'package:flutter_driver/flutter_driver.dart';
10+
import 'package:test/test.dart';
11+
12+
/// Smoke test: verifies that when HCPP is enabled, all three
13+
/// legacy platform view creation APIs (HC via initExpensiveAndroidView,
14+
/// TLHC with HC fallback via initSurfaceAndroidView, and
15+
/// TLHC with VD fallback via initAndroidView) are upgraded to HCPP mode
16+
/// without crashing.
17+
void main() async {
18+
late final FlutterDriver flutterDriver;
19+
late final NativeDriver nativeDriver;
20+
21+
setUpAll(() async {
22+
// Clear logcat before the test so we only see logs from this run.
23+
await io.Process.run('adb', <String>['logcat', '-c']);
24+
25+
flutterDriver = await FlutterDriver.connect();
26+
nativeDriver = await AndroidNativeDriver.connect(flutterDriver);
27+
await flutterDriver.waitUntilFirstFrameRasterized();
28+
});
29+
30+
tearDownAll(() async {
31+
await nativeDriver.close();
32+
await flutterDriver.close();
33+
});
34+
35+
test('verify that HCPP is supported and enabled', () async {
36+
final response = json.decode(await flutterDriver.requestData('')) as Map<String, Object?>;
37+
expect(response['supported'], true);
38+
}, timeout: Timeout.none);
39+
40+
test('all three platform view types render without crashing', () async {
41+
final Health health = await flutterDriver.checkHealth();
42+
expect(health.status, HealthStatus.ok);
43+
}, timeout: Timeout.none);
44+
45+
test('all three platform view types dispose without crashing', () async {
46+
await flutterDriver.tap(find.byValueKey('ToggleViews'));
47+
await Future<void>.delayed(const Duration(seconds: 1));
48+
49+
final Health health = await flutterDriver.checkHealth();
50+
expect(health.status, HealthStatus.ok);
51+
}, timeout: Timeout.none);
52+
53+
test('verify HCPP was used for all views via logcat', () async {
54+
// Dump logcat filtered to the PlatformViewsChannel tag.
55+
final io.ProcessResult result = await io.Process.run('adb', <String>[
56+
'logcat',
57+
'-d',
58+
'-s',
59+
'PlatformViewsChannel:*',
60+
]);
61+
final logcat = result.stdout as String;
62+
63+
// We created 3 platform views — expect 3 HCPP log lines.
64+
final int hcppCount = 'Using HCPP platform view rendering strategy.'.allMatches(logcat).length;
65+
final int legacyCount = 'Using legacy platform view rendering strategy.'
66+
.allMatches(logcat)
67+
.length;
68+
69+
expect(
70+
hcppCount,
71+
3,
72+
reason:
73+
'Expected 3 HCPP creations (one per view type), '
74+
'got $hcppCount. Logcat:\n$logcat',
75+
);
76+
expect(
77+
legacyCount,
78+
0,
79+
reason:
80+
'Expected 0 legacy creations, '
81+
'got $legacyCount. Logcat:\n$logcat',
82+
);
83+
}, timeout: Timeout.none);
84+
}

‎engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ public class FlutterShellArgs {
5353
public static final String ARG_DISABLE_IMPELLER = "--enable-impeller=false";
5454
public static final String ARG_KEY_ENABLE_VULKAN_VALIDATION = "enable-vulkan-validation";
5555
public static final String ARG_ENABLE_VULKAN_VALIDATION = "--enable-vulkan-validation";
56+
public static final String ARG_KEY_TOGGLE_SURFACE_CONTROL = "enable-surface-control";
57+
public static final String ARG_ENABLE_SURFACE_CONTROL = "--enable-surface-control=true";
58+
public static final String ARG_DISABLE_SURFACE_CONTROL = "--enable-surface-control=false";
5659
public static final String ARG_KEY_DUMP_SHADER_SKP_ON_SHADER_COMPILATION =
5760
"dump-skp-on-shader-compilation";
5861
public static final String ARG_DUMP_SHADER_SKP_ON_SHADER_COMPILATION =
@@ -132,6 +135,14 @@ public static FlutterShellArgs fromIntent(@NonNull Intent intent) {
132135
if (intent.getBooleanExtra(ARG_KEY_ENABLE_VULKAN_VALIDATION, false)) {
133136
args.add(ARG_ENABLE_VULKAN_VALIDATION);
134137
}
138+
if (intent.hasExtra(ARG_KEY_TOGGLE_SURFACE_CONTROL)) {
139+
if (intent.getBooleanExtra(ARG_KEY_TOGGLE_SURFACE_CONTROL, false)) {
140+
args.add(ARG_ENABLE_SURFACE_CONTROL);
141+
} else {
142+
args.add(ARG_DISABLE_SURFACE_CONTROL);
143+
}
144+
}
145+
135146
if (intent.getBooleanExtra(ARG_KEY_DUMP_SHADER_SKP_ON_SHADER_COMPILATION, false)) {
136147
args.add(ARG_DUMP_SHADER_SKP_ON_SHADER_COMPILATION);
137148
}

‎engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/PlatformViewsChannel.java‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,19 @@ private void create(@NonNull MethodCall call, @NonNull MethodChannel.Result resu
9292
: null;
9393

9494
try {
95-
// TODO(gmackall): Enable hcpp path in a follow up PR to
96-
// https://github.com/flutter/flutter/pull/170553/.
97-
// with a new more externally friendly flag name.
98-
// if (handler.isHcppEnabled()) {
99-
// final PlatformViewCreationRequest request =
100-
// PlatformViewCreationRequest.createHCPPRequest(
101-
// (int) createArgs.get("id"),
102-
// (String) createArgs.get("viewType"),
103-
// (int) createArgs.get("direction"),
104-
// additionalParams);
105-
// handler.createPlatformViewHcpp(request);
106-
// result.success(null);
107-
// return;
108-
// }
95+
if (handler.isHcppEnabled()) {
96+
Log.i(TAG, "Using HCPP platform view rendering strategy.");
97+
final PlatformViewCreationRequest request =
98+
PlatformViewCreationRequest.createHCPPRequest(
99+
(int) createArgs.get("id"),
100+
(String) createArgs.get("viewType"),
101+
(int) createArgs.get("direction"),
102+
additionalParams);
103+
handler.createPlatformViewHcpp(request);
104+
result.success(null);
105+
return;
106+
}
107+
Log.i(TAG, "Using legacy platform view rendering strategy.");
109108
if (usesPlatformViewLayer) {
110109
final PlatformViewCreationRequest request =
111110
PlatformViewCreationRequest.createHybridCompositionRequest(

‎packages/flutter_tools/lib/src/android/android_device.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,11 @@ class AndroidDevice extends Device {
687687
'dart-flags',
688688
debuggingOptions.dartFlags,
689689
],
690+
if (debuggingOptions.enableSurfaceControl) ...<String>[
691+
'--ez',
692+
'enable-surface-control',
693+
'true',
694+
],
690695
if (debuggingOptions.useTestFonts) ...<String>['--ez', 'use-test-fonts', 'true'],
691696
if (debuggingOptions.verboseSystemLogs) ...<String>['--ez', 'verbose-logging', 'true'],
692697
if (userIdentifier != null) ...<String>['--user', userIdentifier],

‎packages/flutter_tools/lib/src/commands/run.dart‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
248248
addEnableFlutterGpuFlag(verboseHelp: verboseHelp);
249249
addEnableVulkanValidationFlag(verboseHelp: verboseHelp);
250250
addEnableEmbedderApiFlag(verboseHelp: verboseHelp);
251+
addEnableSurfaceControlFlag(verboseHelp: verboseHelp);
251252
}
252253

253254
bool get traceStartup => boolArg('trace-startup');
@@ -264,6 +265,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
264265
bool get enableVulkanValidation => boolArg('enable-vulkan-validation');
265266
bool get uninstallFirst => boolArg('uninstall-first');
266267
bool get enableEmbedderApi => boolArg('enable-embedder-api');
268+
bool get enableSurfaceControl => boolArg('enable-surface-control');
267269
bool get enableLocalDiscovery => boolArg(RunCommand.kEnableLocalDiscovery);
268270

269271
@override
@@ -329,6 +331,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
329331
usingCISystem: usingCISystem,
330332
debugLogsDirectoryPath: debugLogsDirectoryPath,
331333
webDevServerConfig: webDevServerConfig,
334+
enableSurfaceControl: enableSurfaceControl,
332335
enableLocalDiscovery: enableLocalDiscovery,
333336
);
334337
} else {
@@ -392,6 +395,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
392395
enableDevTools: boolArg(FlutterCommand.kEnableDevTools),
393396
ipv6: boolArg(FlutterCommand.ipv6Flag),
394397
printDtd: boolArg(FlutterGlobalOptions.kPrintDtd, global: true),
398+
enableSurfaceControl: enableSurfaceControl,
395399
enableLocalDiscovery: enableLocalDiscovery,
396400
webDevServerConfig: webDevServerConfig,
397401
);

‎packages/flutter_tools/lib/src/commands/test.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
8080
addEnableImpellerFlag(verboseHelp: verboseHelp);
8181
addMachineOutputFlag(verboseHelp: verboseHelp);
8282
addEnableFlutterGpuFlag(verboseHelp: verboseHelp);
83+
addEnableSurfaceControlFlag(verboseHelp: verboseHelp);
8384

8485
argParser
8586
..addFlag(

0 commit comments

Comments
 (0)