Refactor ShaderTarget to not explicitly mention impeller or Skia #141460

dnfield · 2024-01-12T17:58:44Z

Refactors ShaderTarget to make it opaque as to whether it's using Impeller or SkSL and instead has it focus on the target platform it's generating for.

ImpellerC includes SkSL right now whether you ask for it or not.

The tester target also might need SkSL or Vulkan depending on whether --enable-impeller is passed.

dnfield · 2024-01-12T22:58:26Z

packages/flutter_tools/test/general.shard/asset_bundle_test.dart

          command: <String>[
            impellerc,
-            '--sksl',
+            // --sksl is implicit.


For these - should I make it explicit in the tool? I'm not sure what will be easier to deal with going forward.

christopherfujino · 2024-01-16T19:54:36Z

packages/flutter_tools/lib/src/build_system/targets/ios.dart

      environment,
      assetDirectory,
      targetPlatform: TargetPlatform.ios,
      // Always specify an impeller shader target so that we support runtime toggling and


I'm guessing we can remove this comment?

christopherfujino · 2024-01-16T20:02:53Z

packages/flutter_tools/lib/src/build_system/targets/shader_compiler.dart

-  impelleriOS(<String>['--runtime-stage-metal']),
-  impellerSwiftShader(<String>['--runtime-stage-vulkan']),
-  sksl(<String>['--sksl']);
+  android(<String>['--runtime-stage-gles', '--runtime-stage-vulkan']),


Could this whole enum just be a function that maps TargetPlatform to List<String> stages?

which would then simplify all the callsites where we have to pass around both ShaderTarget AND TargetPlatform

Yeah, I think it should be safe to always include the SkSL target with any impeller target. At least for now.

It could. I'll do that.

christopherfujino

LGTM

…r_target

christopherfujino · 2024-01-28T07:20:38Z

dev/integration_tests/flutter_gallery/ios/Runner.xcodeproj/project.pbxproj

 			baseConfigurationReference = 7AFA3C8E1D35360C0083082E /* Release.xcconfig */;
 			buildSettings = {
 				ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon;
+				DEVELOPMENT_TEAM = S8QB4VV633;


I'm guessing we don't want to check this in?

yeah I'll fix this.

christopherfujino

LGTM!

…kia (flutter/flutter#141460)

Manual roll Flutter from c65ab4d to e02e207 (38 revisions) Manual roll requested by [email protected] flutter/flutter@c65ab4d...e02e207 2024-02-01 [email protected] Roll Flutter Engine from f4fbabf1eb9f to 68943afd62d1 (9 revisions) (flutter/flutter#142690) 2024-02-01 [email protected] Introduce tone-based surfaces and accent color add-ons - Part 1 (flutter/flutter#142654) 2024-02-01 [email protected] improve error message when `--base-href` argument does not start with `/` (flutter/flutter#142667) 2024-02-01 [email protected] Roll Flutter Engine from c4247c5e31ba to f4fbabf1eb9f (1 revision) (flutter/flutter#142675) 2024-02-01 [email protected] Roll Flutter Engine from c83617eee093 to c4247c5e31ba (3 revisions) (flutter/flutter#142662) 2024-02-01 [email protected] [Impeller] opt vulkan tests into GPU tracing. (flutter/flutter#142649) 2024-02-01 [email protected] Convert button `.icon` and `.tonalIcon` constructors to take nullable icons. (flutter/flutter#142644) 2024-02-01 [email protected] Fix token usages on Regular Chip and Action Chip (flutter/flutter#141701) 2024-02-01 [email protected] Added ButtonStyle.foregroundBuilder and ButtonStyle.backgroundBuilder (flutter/flutter#141818) 2024-01-31 [email protected] Roll Flutter Engine from 5b89189b8b5f to c83617eee093 (2 revisions) (flutter/flutter#142656) 2024-01-31 [email protected] [flutter_tools] add debugging to ios/core_devices.dart (flutter/flutter#142187) 2024-01-31 [email protected] Fix showDialog docs (flutter/flutter#142458) 2024-01-31 49699333+dependabot[bot]@users.noreply.github.com Bump peter-evans/create-pull-request from 5.0.2 to 6.0.0 (flutter/flutter#142650) 2024-01-31 [email protected] Roll Flutter Engine from 20e53614c16c to 5b89189b8b5f (2 revisions) (flutter/flutter#142640) 2024-01-31 [email protected] Refactor ShaderTarget to not explicitly mention impeller or Skia (flutter/flutter#141460) 2024-01-31 [email protected] Roll Flutter Engine from 9ccd81d7595b to 20e53614c16c (3 revisions) (flutter/flutter#142628) 2024-01-31 [email protected] Show Mac Designed For iPad in 'flutter devices' (flutter/flutter#141718) 2024-01-31 [email protected] Marks Mac_arm64_ios basic_material_app_ios__compile to be unflaky (flutter/flutter#142594) 2024-01-31 [email protected] Fix ParentDataWidget crash for multi view scenarios (flutter/flutter#142486) 2024-01-31 [email protected] Roll Flutter Engine from e0d8f472a1b6 to 9ccd81d7595b (1 revision) (flutter/flutter#142625) 2024-01-31 [email protected] Marks Mac_arm64 tool_tests_commands to be unflaky (flutter/flutter#142593) 2024-01-31 [email protected] "System back gesture" explanation (flutter/flutter#142254) 2024-01-31 [email protected] Marks Mac_x64 tool_tests_commands to be unflaky (flutter/flutter#142592) 2024-01-31 [email protected] Marks Mac_x64_ios integration_test_test_ios to be unflaky (flutter/flutter#142595) 2024-01-31 [email protected] Marks Mac_x64 native_ui_tests_macos to be unflaky (flutter/flutter#142598) 2024-01-31 [email protected] Marks Mac_x64_ios hot_mode_dev_cycle_ios__benchmark to be unflaky (flutter/flutter#142597) 2024-01-31 [email protected] Marks Mac_arm64 native_ui_tests_macos to be unflaky (flutter/flutter#142599) 2024-01-31 [email protected] Marks Windows_android hot_mode_dev_cycle_win__benchmark to be flaky (flutter/flutter#142609) 2024-01-31 [email protected] Marks Mac_arm64_ios integration_test_test_ios to be unflaky (flutter/flutter#142596) 2024-01-31 [email protected] Mark test that leaks image. (flutter/flutter#142539) 2024-01-31 [email protected] Roll Flutter Engine from b9bc256156b8 to e0d8f472a1b6 (1 revision) (flutter/flutter#142623) 2024-01-31 [email protected] Fix unresponsive mouse tooltip (flutter/flutter#142282) 2024-01-31 [email protected] Marks Linux_android_emu android_defines_test to be unflaky (flutter/flutter#142591) 2024-01-31 [email protected] Roll Flutter Engine from 447dd212447e to b9bc256156b8 (6 revisions) (flutter/flutter#142617) 2024-01-31 [email protected] Roll Packages from 25abb5d to 5b48c44 (4 revisions) (flutter/flutter#142616) 2024-01-31 [email protected] Fix null operator error when tapping on 'MenuItemButton' (flutter/flutter#142230) 2024-01-31 [email protected] Roll Flutter Engine from 8e7df85f7d11 to 447dd212447e (2 revisions) (flutter/flutter#142587) 2024-01-31 [email protected] Split out AppBar/SliverAppBar material tests (flutter/flutter#142560) 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 [email protected],[email protected] on the revert to ensure that a human is aware of the problem. ...

…kia (flutter/flutter#141460)

Refactor ShaderTarget to not explicitly mention impeller or Skia

a3baeba

dnfield requested a review from jonahwilliams January 12, 2024 17:58

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jan 12, 2024

dnfield added 2 commits January 12, 2024 10:28

Analysis

3c1c6fc

tests

8a0ff33

dnfield marked this pull request as ready for review January 12, 2024 22:56

dnfield commented Jan 12, 2024

View reviewed changes

dnfield requested a review from christopherfujino January 16, 2024 17:03

christopherfujino reviewed Jan 16, 2024

View reviewed changes

dnfield added 4 commits January 17, 2024 12:42

Merge remote-tracking branch 'upstream/main' into shader_target

cbb7e82

review

5ee3168

test args

f3ec78c

Merge branch 'master' into shader_target

fd4c582

dnfield requested a review from christopherfujino January 17, 2024 22:41

christopherfujino approved these changes Jan 25, 2024

View reviewed changes

dnfield added 2 commits January 27, 2024 11:14

merge

ad529f2

Merge branch 'shader_target' of github.com:dnfield/flutter into shade…

190cd7b

…r_target

christopherfujino reviewed Jan 28, 2024

View reviewed changes

fixes

5f98081

christopherfujino approved these changes Jan 31, 2024

View reviewed changes

dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 31, 2024

auto-submit bot merged commit c417c46 into flutter:master Jan 31, 2024

engine-flutter-autoroll mentioned this pull request Feb 1, 2024

Manual roll Flutter from c65ab4d513da to e02e2079bea7 (38 revisions) flutter/packages#6028

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 1, 2024

c417c46 Refactor ShaderTarget to not explicitly mention impeller or S…

a1b275b

…kia (flutter/flutter#141460)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024

c417c46 Refactor ShaderTarget to not explicitly mention impeller or S…

5b4bf36

…kia (flutter/flutter#141460)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor ShaderTarget to not explicitly mention impeller or Skia #141460

Refactor ShaderTarget to not explicitly mention impeller or Skia #141460

Uh oh!

dnfield commented Jan 12, 2024 •

edited

Loading

Uh oh!

dnfield Jan 12, 2024

Uh oh!

christopherfujino Jan 16, 2024

Uh oh!

dnfield Jan 17, 2024

Uh oh!

christopherfujino Jan 16, 2024

Uh oh!

christopherfujino Jan 16, 2024

Uh oh!

jonahwilliams Jan 16, 2024

Uh oh!

dnfield Jan 17, 2024

Uh oh!

christopherfujino left a comment

Uh oh!

christopherfujino Jan 28, 2024

Uh oh!

dnfield Jan 31, 2024

Uh oh!

christopherfujino left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Refactor ShaderTarget to not explicitly mention impeller or Skia #141460

Refactor ShaderTarget to not explicitly mention impeller or Skia #141460

Uh oh!

Conversation

dnfield commented Jan 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dnfield commented Jan 12, 2024 •

edited

Loading