[Impeller] Wire-up AndroidSurfaceImpellerVulkan #37249

iskakaushik · 2022-11-02T17:29:56Z

This enables preliminary support for Impeller + Vulkan on Android.
Pending multiple fidelity fixes and adding a preference for Vulkan.
Also noteworthy is [Impeller] Bundling Vulkan support for Impeller on Android is expected to cause a size regression flutter#114531

- This enables preliminary support for Impeller + Vulkan on Android. - Pending multiple fidelity fixes and adding a preference for Vulkan. - Also noteworthy is flutter/flutter#114531

flutter-dashboard · 2022-11-02T17:30:00Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

chinmaygarde · 2022-11-02T18:29:46Z

shell/platform/android/platform_view_android.cc

@@ -43,8 +44,15 @@ std::unique_ptr<AndroidSurface> AndroidSurfaceFactoryImpl::CreateSurface() {
                                                      jni_facade_);
    case AndroidRenderingAPI::kOpenGLES:
      if (enable_impeller_) {
+// TODO(kaushikiska@): Enable this after wiring a preference for Vulkan backend.
+#if false


Hmm, false is a preprocessor define? TIL

chinmaygarde · 2022-11-02T18:30:09Z

tools/gn

@@ -436,10 +436,10 @@ def to_gn_args(args):
    gn_args['skia_use_metal'] = True
    gn_args['shell_enable_metal'] = True

-  # Enable Vulkan on all platforms except for Android and iOS. This is just
+  # Enable Vulkan on all platforms except for iOS. This is just


…gine#37249)

dnfield · 2022-11-03T20:48:59Z

before: https://storage.googleapis.com/flutter_infra_release/flutter/edbba91086317574ae116482841633912245fb86/android-arm64-release/sizes/index.html

after: https://storage.googleapis.com/flutter_infra_release/flutter/c7b2230e98d28c8538faeabed42c27145ae30a68/android-arm64-release/sizes/index.html

Seems like a bunch of things went up by ~1kb, but the treemap doesn't seem to reflec tthe size increase you were mentioning...

…114640) * 27972e7d5 [fuchsia] mouse-input test (flutter/engine#37221) * 491032cfc Update docs to mention felt build --host (flutter/engine#37224) * 51089422d [macOS, multiwindow] Compositor gets FlutterView lazily (flutter/engine#36392) * 2e89bce45 Enter a scope before calling Dart APIs in ThrowIfUIOperationsProhibited (flutter/engine#37226) * 7843ae80c [Impeller] Correct the ordering of filters in 'Paint::WithFilters' (flutter/engine#37239) * 2cbe38be0 Produce both ddc and dart2js platform files. (flutter/engine#37162) * 2e3bc8052 Apply internal cl for C++20 prep (flutter/engine#37266) * c183e7701 Roll Dart SDK from 883ab3f70e3d to 94ac8f6cc756 (1 revision) (flutter/engine#37267) * d21f34673 Roll Fuchsia Mac SDK from BPxzJkBzD8R9GFg1n... to 8OZH-l7aK1-73Hyrf... (flutter/engine#37270) * edbba9108 Roll Fuchsia Linux SDK from 9P-WnaDSnineZtFz0... to np4MU3wmDOuhlg6CR... (flutter/engine#37269) * c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/engine#37249) * 73c588119 [Impeller] dont call SPIRV_CROSS_THROW in SkSL backend (flutter/engine#37273) * 893f5cd30 Roll Skia from f41fa8bffd58 to 78927395bf5c (20 revisions) (flutter/engine#37275) * 2a7f3d0c1 Roll Skia from 78927395bf5c to fe751b616832 (1 revision) (flutter/engine#37276) * fbe98c079 Roll Dart SDK from 94ac8f6cc756 to 8e089c61be58 (2 revisions) (flutter/engine#37277) * 95b9b5d83 [Impeller] Add blit command to copy texture to buffer (flutter/engine#37198) * 50c0fbc39 Roll Dart SDK from 8e089c61be58 to 866f5cfad18a (1 revision) (flutter/engine#37278) * 138acebc6 Roll Skia from fe751b616832 to fdfa00287cff (1 revision) (flutter/engine#37279) * 224b4013f Roll Fuchsia Mac SDK from 8OZH-l7aK1-73Hyrf... to mOXbRSWGSdWRXIefR... (flutter/engine#37282) * 74f021922 Announce alerts through SemanticsService on Windows (flutter/engine#37173) * a828fbb4a Roll Dart SDK from 866f5cfad18a to 433f075a852b (1 revision) (flutter/engine#37284) * 49165f1b0 Roll Fuchsia Linux SDK from np4MU3wmDOuhlg6CR... to -0Xq1c-TncmWBWzqg... (flutter/engine#37285) * d06616ecf Roll Skia from fdfa00287cff to cf3fa752a958 (2 revisions) (flutter/engine#37288) * 0b79b5c3f Roll Skia from cf3fa752a958 to af0582c7b223 (5 revisions) (flutter/engine#37290) * 66b244d9f [Impeller] validate calls to texture in SkSL (flutter/engine#37289)

chinmaygarde · 2022-11-03T21:28:40Z

Seems like a bunch of things went up by ~1kb, but the treemap doesn't seem to reflec tthe size increase you were mentioning...

Perhaps the compression is less effective?

chinmaygarde · 2022-11-03T21:32:17Z

Umm, is that the right treemap? I don't see impeller_entity_shaders_vk_data (or something similar) in the large symbols list in the after trace. I do see impeller_entity_shaders_gles_data though.

There is also no VK backend code:

iskakaushik · 2022-11-03T21:40:37Z

@chinmaygarde could it be the case that this could have gotten tree-shaken away because it's not actually wired yet? The
entry point is behind #if false. https://github.com/flutter/engine/pull/37249/files#diff-033d9ee4b0ca06755bdc597c70b48f7e46d73a5234f9964a982cb383046c0ab4R48

chinmaygarde · 2022-11-03T21:42:05Z

Oh, right. Yeah. That makes sense. We should see the size increases after we actually start using it.

dnfield · 2022-11-03T22:04:30Z

Probably it.

zanderso · 2022-11-04T20:24:58Z

SkiaPerf is also reporting no size increase https://flutter-flutter-perf.skia.org/e/?queries=sub_result%3Dlibflutter_compressed_bytes%26test%3Dhello_world_android__compile

…lutter#114640) * 27972e7d5 [fuchsia] mouse-input test (flutter/engine#37221) * 491032cfc Update docs to mention felt build --host (flutter/engine#37224) * 51089422d [macOS, multiwindow] Compositor gets FlutterView lazily (flutter/engine#36392) * 2e89bce45 Enter a scope before calling Dart APIs in ThrowIfUIOperationsProhibited (flutter/engine#37226) * 7843ae80c [Impeller] Correct the ordering of filters in 'Paint::WithFilters' (flutter/engine#37239) * 2cbe38be0 Produce both ddc and dart2js platform files. (flutter/engine#37162) * 2e3bc8052 Apply internal cl for C++20 prep (flutter/engine#37266) * c183e7701 Roll Dart SDK from 883ab3f70e3d to 94ac8f6cc756 (1 revision) (flutter/engine#37267) * d21f34673 Roll Fuchsia Mac SDK from BPxzJkBzD8R9GFg1n... to 8OZH-l7aK1-73Hyrf... (flutter/engine#37270) * edbba9108 Roll Fuchsia Linux SDK from 9P-WnaDSnineZtFz0... to np4MU3wmDOuhlg6CR... (flutter/engine#37269) * c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/engine#37249) * 73c588119 [Impeller] dont call SPIRV_CROSS_THROW in SkSL backend (flutter/engine#37273) * 893f5cd30 Roll Skia from f41fa8bffd58 to 78927395bf5c (20 revisions) (flutter/engine#37275) * 2a7f3d0c1 Roll Skia from 78927395bf5c to fe751b616832 (1 revision) (flutter/engine#37276) * fbe98c079 Roll Dart SDK from 94ac8f6cc756 to 8e089c61be58 (2 revisions) (flutter/engine#37277) * 95b9b5d83 [Impeller] Add blit command to copy texture to buffer (flutter/engine#37198) * 50c0fbc39 Roll Dart SDK from 8e089c61be58 to 866f5cfad18a (1 revision) (flutter/engine#37278) * 138acebc6 Roll Skia from fe751b616832 to fdfa00287cff (1 revision) (flutter/engine#37279) * 224b4013f Roll Fuchsia Mac SDK from 8OZH-l7aK1-73Hyrf... to mOXbRSWGSdWRXIefR... (flutter/engine#37282) * 74f021922 Announce alerts through SemanticsService on Windows (flutter/engine#37173) * a828fbb4a Roll Dart SDK from 866f5cfad18a to 433f075a852b (1 revision) (flutter/engine#37284) * 49165f1b0 Roll Fuchsia Linux SDK from np4MU3wmDOuhlg6CR... to -0Xq1c-TncmWBWzqg... (flutter/engine#37285) * d06616ecf Roll Skia from fdfa00287cff to cf3fa752a958 (2 revisions) (flutter/engine#37288) * 0b79b5c3f Roll Skia from cf3fa752a958 to af0582c7b223 (5 revisions) (flutter/engine#37290) * 66b244d9f [Impeller] validate calls to texture in SkSL (flutter/engine#37289)

Wire-up AndroidSurfaceImpellerVulkan

970e9cd

- This enables preliminary support for Impeller + Vulkan on Android. - Pending multiple fidelity fixes and adding a preference for Vulkan. - Also noteworthy is flutter/flutter#114531

iskakaushik requested a review from chinmaygarde November 2, 2022 17:29

flutter-dashboard bot added the platform-android label Nov 2, 2022

flutter-dashboard bot added the needs tests label Nov 2, 2022

iskakaushik changed the title ~~Wire-up AndroidSurfaceImpellerVulkan~~ [Impeller] Wire-up AndroidSurfaceImpellerVulkan Nov 2, 2022

iskakaushik added the e: impeller label Nov 2, 2022

iskakaushik self-assigned this Nov 2, 2022

chinmaygarde approved these changes Nov 2, 2022

View reviewed changes

fix licenses

84d7a59

iskakaushik added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2022

auto-submit bot merged commit c7b2230 into flutter:main Nov 3, 2022

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to c7b2230e98d2 (11 revisions) flutter/flutter#114574

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

c93b940

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 73c588119139 (12 revisions) flutter/flutter#114575

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

a714f2e

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 893f5cd30dc0 (13 revisions) flutter/flutter#114576

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

c692b03

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 893f5cd30dc0 (13 revisions) flutter/flutter#114581

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

d69cd12

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 2a7f3d0c14ee (14 revisions) flutter/flutter#114584

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

5dc2d92

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to fbe98c079b2e (15 revisions) flutter/flutter#114585

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

6c2293a

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 95b9b5d8373a (16 revisions) flutter/flutter#114589

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

039eaf3

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 95b9b5d8373a (16 revisions) flutter/flutter#114598

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

3a64e11

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 50c0fbc390ca (17 revisions) flutter/flutter#114599

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

2bd8067

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 138acebc6aa5 (18 revisions) flutter/flutter#114604

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

7b85724

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to d06616ecfb80 (23 revisions) flutter/flutter#114638

Closed

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

daa2248

…gine#37249)

engine-flutter-autoroll mentioned this pull request Nov 3, 2022

Roll Flutter Engine from 840a7b346216 to 66b244d9fa0a (25 revisions) flutter/flutter#114640

Merged

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

c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/en…

2c443ed

…gine#37249)

iskakaushik deleted the wire-android-surface-impeller-1 branch November 4, 2022 10:57

schwa423 pushed a commit to schwa423/engine that referenced this pull request Nov 16, 2022

[Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter#37249)

c7426be

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Impeller] Wire-up AndroidSurfaceImpellerVulkan #37249

[Impeller] Wire-up AndroidSurfaceImpellerVulkan #37249

Uh oh!

iskakaushik commented Nov 2, 2022

Uh oh!

flutter-dashboard bot commented Nov 2, 2022

Uh oh!

chinmaygarde Nov 2, 2022

Uh oh!

chinmaygarde Nov 2, 2022

Uh oh!

dnfield commented Nov 3, 2022

Uh oh!

chinmaygarde commented Nov 3, 2022

Uh oh!

chinmaygarde commented Nov 3, 2022 •

edited

Loading

Uh oh!

iskakaushik commented Nov 3, 2022

Uh oh!

chinmaygarde commented Nov 3, 2022

Uh oh!

dnfield commented Nov 3, 2022

Uh oh!

zanderso commented Nov 4, 2022

Uh oh!

Uh oh!

[Impeller] Wire-up AndroidSurfaceImpellerVulkan #37249

[Impeller] Wire-up AndroidSurfaceImpellerVulkan #37249

Uh oh!

Conversation

iskakaushik commented Nov 2, 2022

Uh oh!

flutter-dashboard bot commented Nov 2, 2022

Uh oh!

chinmaygarde Nov 2, 2022

Choose a reason for hiding this comment

Uh oh!

chinmaygarde Nov 2, 2022

Choose a reason for hiding this comment

Uh oh!

dnfield commented Nov 3, 2022

Uh oh!

chinmaygarde commented Nov 3, 2022

Uh oh!

chinmaygarde commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iskakaushik commented Nov 3, 2022

Uh oh!

chinmaygarde commented Nov 3, 2022

Uh oh!

dnfield commented Nov 3, 2022

Uh oh!

zanderso commented Nov 4, 2022

Uh oh!

Uh oh!

chinmaygarde commented Nov 3, 2022 •

edited

Loading