Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Nov 20, 2024

Setting "enableGPUValidationMode = "1"" disables Metal API validation. I know, I know.

Fixes #159227

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 20, 2024
@github-actions github-actions bot added the platform-ios iOS applications specifically label Nov 20, 2024
@jonahwilliams jonahwilliams marked this pull request as ready for review November 20, 2024 23:08
@jonahwilliams
Copy link
Contributor Author

As a follow up, I need to opt all of our benchmarks out of this setting...

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually when we do a migration we also build (and therefore automigrate) all the example and integration test projects to prove out that it's working.

Example: #140256

@@ -0,0 +1,106 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

import 'package:test/fake.dart';

import '../../src/common.dart';

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Comment on lines +42 to +43
'debugServiceExtension = "internal"'
'\n enableGPUValidationMode = "1"'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check the entire XML file to make sure other whitespace doesn't get messed up.

'''
<?xml version="1.0" encoding="UTF-8"?>
  <LaunchAction
    buildConfiguration = "Debug"
    selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
    selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
    launchStyle = "0"
    useCustomWorkingDirectory = "NO"
    ignoresPersistentStateOnLaunch = "NO"
    debugDocumentVersioning = "YES"
    debugServiceExtension = "internal"
    enableGPUValidationMode = "1"
    allowLocationSimulation = "YES">
'''

Copy link
Contributor Author

@jonahwilliams jonahwilliams Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh the white space is definitely messed up, since the indentation varies. I'm wagering that most folks never actually open this file...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally given the normal generated template scheme that's never otherwise been updated, the migration adds the right whitespace. Otherwise Xcode will try to format it if the whitespace is off the first time it opens, which will dirty the working copy again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a real programmer and regex'd my way to a solution.

@jonahwilliams
Copy link
Contributor Author

Yeah I can migrate in this PR, wasn't sure if it was common to do it separately or not.

@jonahwilliams
Copy link
Contributor Author

is there a way to trigger the migrators that skips building?

RemoveBitcodeMigration(app.project, globals.logger),
XcodeThinBinaryBuildPhaseInputPathsMigration(app.project, globals.logger),
UIApplicationMainDeprecationMigration(app.project, globals.logger),
MetalAPIValidationMigrator(app.project, globals.logger),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only migrates iOS, you'll also want to add it here for macOS:

SwiftPackageManagerGitignoreMigration(flutterProject, globals.logger),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@jonahwilliams
Copy link
Contributor Author

oh, maybe --config-only

@jmagman
Copy link
Member

jmagman commented Nov 20, 2024

is there a way to trigger the migrators that skips building?

Not a nice way, though we should. You can get a lot of it by commenting out the Android part of test.dart:

flutter/dev/bots/test.dart

Lines 383 to 393 in 17e38a4

final bool verifyCaching = exampleDirectory.path.contains('flutter_gallery');
final String examplePath = path.relative(exampleDirectory.path, from: Directory.current.path);
final List<String> additionalArgs = <String>[
if (mainFile != null) path.relative(mainFile.path, from: exampleDirectory.absolute.path),
];
if (Directory(path.join(examplePath, 'android')).existsSync()) {
await _flutterBuildApk(examplePath, release: false, additionalArgs: additionalArgs, verifyCaching: verifyCaching);
await _flutterBuildApk(examplePath, release: true, additionalArgs: additionalArgs, verifyCaching: verifyCaching);
} else {
print('Example project ${path.basename(examplePath)} has no android directory, skipping apk');
}

add '--config-only' here

additionalArgs: <String>[...additionalArgs, '--no-codesign'],

and here
additionalArgs: additionalArgs

then

$ cd dev/bots
$ flutter pub get
$ SHARD=build_tests dart test.dart

Then I grep to see if there's anything unmigrated left and run build --config-only manually in those directories.

@github-actions github-actions bot added the a: desktop Running on desktop label Nov 21, 2024
@github-actions github-actions bot added the d: examples Sample code and demos label Nov 21, 2024
Comment on lines 50 to 54

@override
String? migrateLine(String line) {
return line;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need to override this.

Suggested change
@override
String? migrateLine(String line) {
return line;
}

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jonahwilliams jonahwilliams changed the title [flutter_tools] opt new iOS/macOS apps out of Metal API validation. [flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. Nov 21, 2024
@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 21, 2024

auto label is removed for flutter/flutter/159228, Failed to enqueue flutter/flutter/159228 with HTTP 400: GraphQL mutate failed.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2024
@auto-submit auto-submit bot added this pull request to the merge queue Nov 21, 2024
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2024
@jonahwilliams jonahwilliams deleted the opt-Out-of-shader-validation branch November 21, 2024 01:39
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 21, 2024
Roll Flutter from 8536b96 to 93d772c (37 revisions)

flutter/flutter@8536b96...93d772c

2024-11-21 bkonyi@google.com Added additional logging to `_listCoreDevices` (flutter/flutter#159275)
2024-11-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 78b87f3fe023 to d1a08064e193 (1 revision) (flutter/flutter#159280)
2024-11-21 jason-simmons@users.noreply.github.com Shut down DevTools and DDS processes if flutter_tools is killed by a signal (flutter/flutter#159238)
2024-11-21 matanlurey@users.noreply.github.com Remove `RepaintBoundary` that is no longer needed. (flutter/flutter#159232)
2024-11-21 matanlurey@users.noreply.github.com Try a speculative fix for Gradle OOMs. (flutter/flutter#159234)
2024-11-21 21270878+elliette@users.noreply.github.com On-device Widget Inspector button exits widget selection (flutter/flutter#158219)
2024-11-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 523d381893c8 to 78b87f3fe023 (2 revisions) (flutter/flutter#159270)
2024-11-21 jessiewong401@gmail.com Remove `firebase_abstract_method_smoke_test` (flutter/flutter#159145)
2024-11-21 engine-flutter-autoroll@skia.org Roll Packages from e95f6d8 to 913b99e (7 revisions) (flutter/flutter#159268)
2024-11-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 69c325513a65 to 523d381893c8 (3 revisions) (flutter/flutter#159263)
2024-11-21 jonahwilliams@google.com [flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. (flutter/flutter#159228)
2024-11-21 jmccandless@google.com Scribe Android handwriting text input (flutter/flutter#148784)
2024-11-21 matanlurey@users.noreply.github.com Terminate non-detached test devices on `flutter run` completion (flutter/flutter#159170)
2024-11-21 matanlurey@users.noreply.github.com Un-skip tests that use `flutter build apk`. (flutter/flutter#159231)
2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2d32cf3a7971 to 69c325513a65 (1 revision) (flutter/flutter#159229)
2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3828681d1f86 to 2d32cf3a7971 (3 revisions) (flutter/flutter#159226)
2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3245c8976976 to 3828681d1f86 (1 revision) (flutter/flutter#159217)
2024-11-20 matanlurey@users.noreply.github.com Add `--dry-run` to `dev/bots/test.dart`. (flutter/flutter#158956)
2024-11-20 34871572+gmackall@users.noreply.github.com Add docs for setting up Android Studio to auto format Kotlin code (flutter/flutter#159209)
2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 80d77505fdde to 3245c8976976 (1 revision) (flutter/flutter#159214)
2024-11-20 stanleycocoa@gmail.com Fix: The enableFeedback property of InkWell cannot be set to a nullab� (flutter/flutter#158907)
2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3f19207e820e to 80d77505fdde (1 revision) (flutter/flutter#159210)
2024-11-20 engine-flutter-autoroll@skia.org Roll Packages from fc4adc7 to e95f6d8 (6 revisions) (flutter/flutter#159201)
2024-11-20 kustermann@google.com Remove dependency on [Target] and instead operate on [Architecture] (flutter/flutter#159196)
2024-11-20 63278936+hongeSunCoder@users.noreply.github.com Fix git command in Quality-Assurance.md (flutter/flutter#155146)
2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7eb87547cbc6 to 3f19207e820e (4 revisions) (flutter/flutter#159176)
2024-11-20 matanlurey@users.noreply.github.com Make `runner` non-nullable as it always is. (flutter/flutter#159156)
2024-11-19 tessertaha@gmail.com Update Material 3 `CircularProgressIndicator` for new visual style (flutter/flutter#158104)
2024-11-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4ff696b555dc to 7eb87547cbc6 (3 revisions) (flutter/flutter#159168)
2024-11-19 magder@google.com Add platform-android label for all flutter_tools *android* files (flutter/flutter#159166)
2024-11-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from d5820a638885 to 4ff696b555dc (1 revision) (flutter/flutter#159164)
2024-11-19 bernaferrari2@gmail.com Reland Add UI Benchmarks (flutter/flutter#153368)
2024-11-19 mohellebiabdessalem@gmail.com fix lint usage of `task` inside `resolve_dependecies.gradle` file (flutter/flutter#158022)
2024-11-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from cff1e751f853 to d5820a638885 (5 revisions) (flutter/flutter#159155)
2024-11-19 mohellebiabdessalem@gmail.com Removing redundant backticks in `flutter\packages\flutter_tools\gradle\gradle.kts` (flutter/flutter#159051)
2024-11-19 50643541+Mairramer@users.noreply.github.com Fixes initial validation with AutovalidateMode.always on first build (flutter/flutter#156708)
2024-11-19 tessertaha@gmail.com Introduce new Material 3 `Slider` shapes (flutter/flutter#152237)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC louisehsu@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

...
@reidbaker reidbaker mentioned this pull request Dec 13, 2024
11 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop d: examples Sample code and demos platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS/macOS templates default to enabling Metal API validation which causes a substantial regression on some Impeller workloads.

3 participants