iOS,macOS: Add Obj-C cflags to all Obj-C targets by cbracken · Pull Request #56386 · flutter/engine

cbracken · 2024-11-05T21:04:03Z

Ensure that all Objective-C code in the codebase is being built with the standard set of Flutter Objective-C compiler flags with ARC enabled.

Also bumps the cflags config up to the top of the first block within each target in which Objective-C sources appear, so that the location is consistent.

Migrates The following targets to ARC, which had been missed in previous passes since they didn't declare the standard Flutter Obj-C[++] cflags:

//flutter/fml:fml_unittests
//flutter/impeller/golden_tests:metal_screenshot
//flutter/impeller/playground:playground
//flutter/impeller/backend/metal:metal
//flutter/impeller/backend/metal:metal_unittests
//shell/gpu:gpu_surface_metal_unittests
//flutter/shell/platform/embedder:embedder_unittests

This patch includes no semantic changes.

Issue: flutter/flutter#137801

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

cbracken · 2024-11-05T21:07:26Z

fml/BUILD.gn

    ]

-    if (is_mac) {
+    if (is_mac || is_ios) {


Note: making consistent with the associated conditional in the target above that this one is testing.

cbracken · 2024-11-05T21:08:07Z

shell/platform/embedder/tests/embedder_metal_unittests.mm

                                              void* texture) {
  GrMtlTextureInfo info;
-  info.fTexture.reset([(id<MTLTexture>)texture retain]);
+  info.fTexture.retain(texture);


Found the non-arc code! Doesn't need a bridge cast since this is already void*.

cbracken · 2024-11-05T22:22:46Z

impeller/BUILD.gn


+  if (impeller_enable_metal) {
+    deps += [ "//flutter/impeller/renderer/backend/metal:metal_unittests" ]
+  }


No need to build these guys on windows, linux, or custom embedders for platforms without metal enabled.

jmagman · 2024-11-05T22:55:53Z

impeller/BUILD.gn

    "core:allocator_unittests",
    "display_list:skia_conversions_unittests",
    "geometry:geometry_unittests",
-    "renderer/backend/metal:metal_unittests",


How was this passing if impeller_enable_metal was false?

auto-submit · 2024-11-05T23:07:55Z

auto label is removed for flutter/engine/56386, due to - The status or check suite Mac mac_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

impeller/renderer/backend/metal/BUILD.gn

Ensure that all Objective-C code in the codebase are being built with the standard set of Flutter Objective-C compiler flags with ARC enabled. Also bumps the cflags config up to the top of the first block within each target in which Objective-C sources appear, so that the location is consistent. Issue: flutter/flutter#137801

…ine#56386)

flutter/engine@58ac1da...b36ca33 2024-11-06 [email protected] [skwasm] Fix empty backdrop drawing. (flutter/engine#56385) 2024-11-06 [email protected] [Impeller] generate stroke vertices into point arena. (flutter/engine#56390) 2024-11-06 [email protected] [Impeller] Do not capture the temporary ImpellerMapping struct pointer when storing release callbacks in libImpeller (flutter/engine#56411) 2024-11-06 6844906[email protected] Roll ICU from 9408c6fd4a39 to 4239b1559d11 (2 revisions) (flutter/engine#56407) 2024-11-06 [email protected] iOS,macOS: Add Obj-C cflags to all Obj-C targets (flutter/engine#56386) 2024-11-06 [email protected] Roll Skia from afaed8923682 to cf33c4e96e81 (5 revisions) (flutter/engine#56408) 2024-11-06 [email protected] Manual roll Dart SDK from 1c1d0420539f to d456f613465a (6 revisions) (flutter/engine#56406) 2024-11-06 [email protected] [Impeller] Avoid errors due to triangle fans usage on Molten. (flutter/engine#56321) 2024-11-06 [email protected] Roll Skia from b4df8dda7ffc to afaed8923682 (14 revisions) (flutter/engine#56404) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Ensure that all Objective-C code in the codebase is being built with the standard set of Flutter Objective-C compiler flags with ARC enabled. Also bumps the cflags config up to the top of the first block within each target in which Objective-C sources appear, so that the location is consistent. Migrates The following targets to ARC, which had been missed in previous passes since they didn't declare the standard Flutter Obj-C[++] cflags: * `//flutter/fml:fml_unittests` * `//flutter/impeller/golden_tests:metal_screenshot` * `//flutter/impeller/playground:playground` * `//flutter/impeller/backend/metal:metal` * `//flutter/impeller/backend/metal:metal_unittests` * `//shell/gpu:gpu_surface_metal_unittests` * `//flutter/shell/platform/embedder:embedder_unittests` This patch includes no semantic changes. Issue: flutter#137801 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

cbracken requested review from hellohuanlin, jmagman and stuartmorgan-g November 5, 2024 21:04

github-actions bot added platform-ios affects: desktop platform-macos e: impeller labels Nov 5, 2024

cbracken commented Nov 5, 2024

View reviewed changes

hellohuanlin approved these changes Nov 5, 2024

View reviewed changes

cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 5, 2024

jmagman approved these changes Nov 5, 2024

View reviewed changes

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 5, 2024

stuartmorgan-g reviewed Nov 6, 2024

View reviewed changes

impeller/renderer/backend/metal/BUILD.gn Outdated Show resolved Hide resolved

cbracken added 2 commits November 6, 2024 10:06

revert impeller ARC migration

e2d0e04

cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2024

auto-submit bot merged commit 3df1546 into flutter:main Nov 6, 2024

engine-flutter-autoroll mentioned this pull request Nov 6, 2024

Roll Flutter Engine from 58ac1dadd69d to 3df1546471f2 (5 revisions) flutter/flutter#158290

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2024

3df1546 iOS,macOS: Add Obj-C cflags to all Obj-C targets (flutter/eng…

a8341e1

…ine#56386)

engine-flutter-autoroll mentioned this pull request Nov 6, 2024

Roll Flutter Engine from 58ac1dadd69d to 48c10deccff1 (7 revisions) flutter/flutter#158294

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2024

3df1546 iOS,macOS: Add Obj-C cflags to all Obj-C targets (flutter/eng…

f128235

…ine#56386)

engine-flutter-autoroll mentioned this pull request Nov 6, 2024

Roll Flutter Engine from 58ac1dadd69d to b36ca3319825 (9 revisions) flutter/flutter#158295

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2024

3df1546 iOS,macOS: Add Obj-C cflags to all Obj-C targets (flutter/eng…

979866c

…ine#56386)

cbracken deleted the arc-gn-config branch November 28, 2024 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

iOS,macOS: Add Obj-C cflags to all Obj-C targets#56386

iOS,macOS: Add Obj-C cflags to all Obj-C targets#56386
auto-submit[bot] merged 2 commits intoflutter:mainfrom
cbracken:arc-gn-config

cbracken commented Nov 5, 2024 •

edited

Loading

Uh oh!

cbracken Nov 5, 2024 •

edited

Loading

Uh oh!

cbracken Nov 5, 2024 •

edited

Loading

Uh oh!

cbracken Nov 5, 2024

Uh oh!

jmagman Nov 5, 2024

Uh oh!

auto-submit bot commented Nov 5, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

cbracken commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

cbracken Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

jmagman Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Nov 5, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cbracken commented Nov 5, 2024 •

edited

Loading

cbracken Nov 5, 2024 •

edited

Loading

cbracken Nov 5, 2024 •

edited

Loading