Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] delete Impeller sim opt out. #56706

Merged
merged 13 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,10 @@ struct Settings {
#if FML_OS_ANDROID || FML_OS_IOS || FML_OS_IOS_SIMULATOR
// On iOS devices, Impeller is the default with no opt-out and this field is
// const.
#if FML_OS_IOS && !FML_OS_IOS_SIMULATOR
#if FML_OS_IOS || FML_OS_IOS_SIMULATOR
static constexpr const
#endif // FML_OS_IOS && !FML_OS_IOS_SIMULATOR
bool enable_impeller = true;
#endif // FML_OS_IOS && !FML_OS_IOS_SIMULATOR
bool enable_impeller = true; // NOLINT(readability-identifier-naming)
#else
bool enable_impeller = false;
#endif
Expand Down
2 changes: 1 addition & 1 deletion shell/common/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) {
settings.use_asset_fonts =
!command_line.HasOption(FlagForSwitch(Switch::DisableAssetFonts));

#if FML_OS_IOS && !FML_OS_IOS_SIMULATOR
#if FML_OS_IOS || FML_OS_IOS_SIMULATOR
// On these configurations, the Impeller flags are completely ignored with the
// default taking hold.
#else // FML_OS_IOS && !FML_OS_IOS_SIMULATOR
Expand Down
15 changes: 0 additions & 15 deletions shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm
Original file line number Diff line number Diff line change
Expand Up @@ -177,21 +177,6 @@ static BOOL DoesHardwareSupportWideGamut() {
settings.enable_wide_gamut = enableWideGamut;
#endif

#if FML_OS_IOS_SIMULATOR
if (!command_line.HasOption("enable-impeller")) {
// Next, look in the app bundle.
NSNumber* enableImpeller = [bundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
if (enableImpeller == nil) {
// If it isn't in the app bundle, look in the main bundle.
enableImpeller = [mainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
}
// Change the default only if the option is present.
if (enableImpeller != nil) {
settings.enable_impeller = enableImpeller.boolValue;
}
}
#endif // FML_OS_IOS_SIMULATOR

settings.warn_on_impeller_opt_out = true;

NSNumber* enableTraceSystrace = [mainBundle objectForInfoDictionaryKey:@"FLTTraceSystrace"];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,16 +225,6 @@ - (void)testLookUpForAssetsFromPackageFromBundle {
}
}

- (void)testDisableImpellerSettingIsCorrectlyParsed {
id mockMainBundle = OCMPartialMock([NSBundle mainBundle]);
OCMStub([mockMainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"NO");

auto settings = FLTDefaultSettingsForBundle();
// Check settings.enable_impeller value is same as the value defined in Info.plist.
XCTAssertEqual(settings.enable_impeller, NO);
[mockMainBundle stopMocking];
}

- (void)testRequestsWarningWhenImpellerOptOut {
auto settings = FLTDefaultSettingsForBundle();
XCTAssertEqual(settings.warn_on_impeller_opt_out, YES);
Expand Down Expand Up @@ -277,49 +267,6 @@ - (void)testEnableDartAssertsCommandLineArgument {
[mockMainBundle stopMocking];
}

- (void)testDisableImpellerSettingIsCorrectlyOverriddenByCommandLine {
id mockMainBundle = OCMPartialMock([NSBundle mainBundle]);
OCMStub([mockMainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"YES");
id mockProcessInfo = OCMPartialMock([NSProcessInfo processInfo]);
NSArray* arguments = @[ @"process_name", @"--enable-impeller=false" ];
OCMStub([mockProcessInfo arguments]).andReturn(arguments);

auto settings = FLTDefaultSettingsForBundle(nil, mockProcessInfo);
// Check settings.enable_impeller value is same as the value on command line.
XCTAssertEqual(settings.enable_impeller, NO);
[mockMainBundle stopMocking];
}

- (void)testDisableImpellerAppBundleSettingIsCorrectlyParsed {
NSString* bundleId = [FlutterDartProject defaultBundleIdentifier];
id mockAppBundle = OCMClassMock([NSBundle class]);
OCMStub([mockAppBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"NO");
OCMStub([mockAppBundle bundleWithIdentifier:bundleId]).andReturn(mockAppBundle);

auto settings = FLTDefaultSettingsForBundle();
// Check settings.enable_impeller value is same as the value defined in Info.plist.
XCTAssertEqual(settings.enable_impeller, NO);

[mockAppBundle stopMocking];
}

- (void)testEnableImpellerAppBundleSettingIsCorrectlyParsed {
NSString* bundleId = [FlutterDartProject defaultBundleIdentifier];
id mockAppBundle = OCMClassMock([NSBundle class]);
OCMStub([mockAppBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"YES");
OCMStub([mockAppBundle bundleWithIdentifier:bundleId]).andReturn(mockAppBundle);

// Since FLTEnableImpeller is set to false in the main bundle, this is also
// testing that setting FLTEnableImpeller in the app bundle takes
// precedence over setting it in the root bundle.

auto settings = FLTDefaultSettingsForBundle();
// Check settings.enable_impeller value is same as the value defined in Info.plist.
XCTAssertEqual(settings.enable_impeller, YES);

[mockAppBundle stopMocking];
}

- (void)testEnableTraceSystraceSettingIsCorrectlyParsed {
NSBundle* mainBundle = [NSBundle mainBundle];
NSNumber* enableTraceSystrace = [mainBundle objectForInfoDictionaryKey:@"FLTTraceSystrace"];
Expand Down
51 changes: 1 addition & 50 deletions testing/scenario_app/bin/run_ios_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ void main(List<String> args) async {
osRuntime: results.option('os-runtime')!,
osVersion: results.option('os-version')!,
withImpeller: results.flag('with-impeller'),
withSkia: results.flag('with-skia'),
dumpXcresultOnFailure: dumpXcresultOnFailurePath,
);
completer.complete();
Expand Down Expand Up @@ -110,7 +109,6 @@ Future<void> _run(
required String osRuntime,
required String osVersion,
required bool withImpeller,
required bool withSkia,
required String? dumpXcresultOnFailure,
}) async {
// Terminate early on SIGINT.
Expand All @@ -135,45 +133,6 @@ Future<void> _run(

cleanup.add(() => _deleteIfPresent(resultBundle));

if (withSkia) {
io.stderr.writeln('Running simulator tests with Skia');
io.stderr.writeln();
final process = await _runTests(
outScenariosPath: scenarioPath,
resultBundlePath: resultBundle.path,
osVersion: osVersion,
deviceName: deviceName,
iosEngineVariant: iosEngineVariant,
xcodeBuildExtraArgs: [
// Plist with `FTEEnableImpeller=NO`; all projects in the workspace require this file.
// For example, `FlutterAppExtensionTestHost` has a dummy file under the below directory.
r'INFOPLIST_FILE=$(TARGET_NAME)/Info_Skia.plist',
],
);
cleanup.add(process.kill);

// Create a temporary directory, if needed.
var storePath = dumpXcresultOnFailure;
if (storePath == null) {
final dumpDir = io.Directory.systemTemp.createTempSync();
storePath = dumpDir.path;
cleanup.add(() => dumpDir.delete(recursive: true));
}

if (await process.exitCode != 0) {
final String outputPath = _zipAndStoreFailedTestResults(
iosEngineVariant: iosEngineVariant,
resultBundle: resultBundle,
storePath: storePath,
);
io.stderr.writeln('Failed test results are stored at $outputPath');
throw _ToolFailure('test failed.');
} else {
io.stderr.writeln('test succcess.');
}
_deleteIfPresent(resultBundle);
}

if (withImpeller) {
final process = await _runTests(
outScenariosPath: scenarioPath,
Expand Down Expand Up @@ -247,15 +206,7 @@ final _args = ArgParser()
)
..addFlag(
'with-impeller',
help: 'Whether to use the Impeller backend to run the tests.\n\nCan be '
'combined with --with-skia to run the test suite with both backends.',
defaultsTo: true,
)
..addFlag(
'with-skia',
help:
'Whether to use the Skia backend to run the tests.\n\nCan be combined '
'with --with-impeller to run the test suite with both backends.',
help: 'Whether to use the Impeller backend to run the tests.',
defaultsTo: true,
)
..addOption(
Expand Down