[Impeller] [Android] Refactor the Android context/surface implementation to work more like Skia. #41059

dnfield · 2023-04-11T00:14:29Z

Noticed this while working on flutter/flutter#124181

In Impeller right now, Android creates a context for each surface - but in Skia, Android creates one context and shares it across surfaces. For the multiple-engine scenario this matters. This also makes it possible for Android to provide the impeller::Context independently of surface creation.

Makes the impeller::Context available to the rasterizer ASAP, so that even if it has not received a surface yet calls to `toImage` and `toImageSync` can still produce a texture.

dnfield · 2023-04-11T18:09:46Z

(Looking into the test failure)

dnfield · 2023-04-11T19:05:59Z

I'm going to wire up an Impeller variant of test_surface_gl

jason-simmons · 2023-04-11T21:48:59Z

I'm seeing errors at startup when I try to run a demo of multiple engines created from a FlutterEngineGroup with this PR:

E/flutter ( 9053): [ERROR:flutter/impeller/toolkit/egl/egl.cc(56)] EGL Error: Bad Access (12290) in ../../flutter/impeller/toolkit/egl/context.cc:55
E/flutter ( 9053): [ERROR:flutter/shell/platform/android/android_surface_gl_impeller.cc(24)] Could not make offscreen context current.
F/flutter ( 9053): [FATAL:flutter/shell/platform/android/platform_view_android.cc(124)] Check failed: android_surface_ && android_surface_->IsValid(). Could not create an OpenGL, Vulkan or Software surface to set up rendering.

dnfield · 2023-04-11T22:11:30Z

I've updated the test to use the Metal backend. The GL backend has an additional complication, which I described in flutter/flutter#124635.

We should fix that separately IMHO.

dnfield · 2023-04-11T23:06:01Z

I'm seeing errors at startup when I try to run a demo of multiple engines created from a FlutterEngineGroup with this PR:

E/flutter ( 9053): [ERROR:flutter/impeller/toolkit/egl/egl.cc(56)] EGL Error: Bad Access (12290) in ../../flutter/impeller/toolkit/egl/context.cc:55
E/flutter ( 9053): [ERROR:flutter/shell/platform/android/android_surface_gl_impeller.cc(24)] Could not make offscreen context current.
F/flutter ( 9053): [FATAL:flutter/shell/platform/android/platform_view_android.cc(124)] Check failed: android_surface_ && android_surface_->IsValid(). Could not create an OpenGL, Vulkan or Software surface to set up rendering.

d570bbb fixes that locally for me.

dnfield · 2023-04-13T20:18:39Z

From triage: I'm going to make a smaller version of this patch that doesn't touch Android as much yet.

…mplementation to work more like Skia. (flutter/engine#41059)

…126714) flutter/engine@c784d6d...502e1eb 2023-05-12 [email protected] [Impeller] starts running vulkan golden image tests with swiftshader (flutter/engine#41958) 2023-05-12 [email protected] [web] Fix crash on old versions of Chrome (Intl.Segmenter missing) (flutter/engine#41986) 2023-05-12 [email protected] Move clang tidy v2 build to prod. (flutter/engine#41985) 2023-05-12 [email protected] Reland "Move linux_license to engine v2."" (flutter/engine#41983) 2023-05-12 [email protected] Roll Dart SDK from 6cd6e54f280d to 858f83d6d993 (2 revisions) (flutter/engine#41980) 2023-05-12 [email protected] Roll Skia from 0b929ada9cc6 to 539fb10d7cfb (2 revisions) (flutter/engine#41979) 2023-05-12 [email protected] Roll Fuchsia Mac SDK from MjcKzcsqMDkuRKopu... to rO5qs4MpN6WfJcu64... (flutter/engine#41975) 2023-05-12 [email protected] Roll Dart SDK from 4028c7d8b87c to 6cd6e54f280d (1 revision) (flutter/engine#41974) 2023-05-12 [email protected] Roll Fuchsia Linux SDK from lf8VcONMWlne4oa3H... to HMLNsG7w9sTDpW2Ge... (flutter/engine#41973) 2023-05-12 [email protected] Roll Skia from 208a07f71701 to 0b929ada9cc6 (2 revisions) (flutter/engine#41971) 2023-05-12 [email protected] [Impeller] [Android] Refactor the Android context/surface implementation to work more like Skia. (flutter/engine#41059) 2023-05-12 [email protected] Roll Skia from ccec5093ca38 to 208a07f71701 (1 revision) (flutter/engine#41970) 2023-05-12 [email protected] [Impeller] Dont synthesize fake index buffer if none was provided to VBB (flutter/engine#41969) 2023-05-12 [email protected] [Impeller] Make index buffer optional (flutter/engine#41962) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from lf8VcONMWlne to HMLNsG7w9sTD fuchsia/sdk/core/mac-amd64 from MjcKzcsqMDku to rO5qs4MpN6Wf 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],[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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…lutter#126714) flutter/engine@c784d6d...502e1eb 2023-05-12 [email protected] [Impeller] starts running vulkan golden image tests with swiftshader (flutter/engine#41958) 2023-05-12 [email protected] [web] Fix crash on old versions of Chrome (Intl.Segmenter missing) (flutter/engine#41986) 2023-05-12 [email protected] Move clang tidy v2 build to prod. (flutter/engine#41985) 2023-05-12 [email protected] Reland "Move linux_license to engine v2."" (flutter/engine#41983) 2023-05-12 [email protected] Roll Dart SDK from 6cd6e54f280d to 858f83d6d993 (2 revisions) (flutter/engine#41980) 2023-05-12 [email protected] Roll Skia from 0b929ada9cc6 to 539fb10d7cfb (2 revisions) (flutter/engine#41979) 2023-05-12 [email protected] Roll Fuchsia Mac SDK from MjcKzcsqMDkuRKopu... to rO5qs4MpN6WfJcu64... (flutter/engine#41975) 2023-05-12 [email protected] Roll Dart SDK from 4028c7d8b87c to 6cd6e54f280d (1 revision) (flutter/engine#41974) 2023-05-12 [email protected] Roll Fuchsia Linux SDK from lf8VcONMWlne4oa3H... to HMLNsG7w9sTDpW2Ge... (flutter/engine#41973) 2023-05-12 [email protected] Roll Skia from 208a07f71701 to 0b929ada9cc6 (2 revisions) (flutter/engine#41971) 2023-05-12 [email protected] [Impeller] [Android] Refactor the Android context/surface implementation to work more like Skia. (flutter/engine#41059) 2023-05-12 [email protected] Roll Skia from ccec5093ca38 to 208a07f71701 (1 revision) (flutter/engine#41970) 2023-05-12 [email protected] [Impeller] Dont synthesize fake index buffer if none was provided to VBB (flutter/engine#41969) 2023-05-12 [email protected] [Impeller] Make index buffer optional (flutter/engine#41962) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from lf8VcONMWlne to HMLNsG7w9sTD fuchsia/sdk/core/mac-amd64 from MjcKzcsqMDku to rO5qs4MpN6Wf 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],[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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…and its swapchain In flutter#41059 the Android context and surface were changed to share one impeller::ContextVK across all instances of AndroidSurfaceVulkanImpeller. However, the ContextVK holds a reference to the current swapchain. If an app uses multiple AndroidSurfaceVulkanImpeller instances (e.g. for platform views), then one surface may overwrite the ContextVK's swapchain while another surface is trying to render. This patch allows each surface to get its own impeller::Context instance holding a swapchain tied to that surface. The per-surface contexts will delegate most operations to the shared parent ContextVK.

…and its swapchain (#44171) In #41059 the Android context and surface were changed to share one impeller::ContextVK across all instances of AndroidSurfaceVulkanImpeller. However, the ContextVK holds a reference to the current swapchain. If an app uses multiple AndroidSurfaceVulkanImpeller instances (e.g. for platform views), then one surface may overwrite the ContextVK's swapchain while another surface is trying to render. This patch allows each surface to get its own impeller::Context instance holding a swapchain tied to that surface. The per-surface contexts will delegate most operations to the shared parent ContextVK.

…and its swapchain (flutter#44171) In flutter#41059 the Android context and surface were changed to share one impeller::ContextVK across all instances of AndroidSurfaceVulkanImpeller. However, the ContextVK holds a reference to the current swapchain. If an app uses multiple AndroidSurfaceVulkanImpeller instances (e.g. for platform views), then one surface may overwrite the ContextVK's swapchain while another surface is trying to render. This patch allows each surface to get its own impeller::Context instance holding a swapchain tied to that surface. The per-surface contexts will delegate most operations to the shared parent ContextVK.

Refactor AndroidContext/Surface, particularly for Impeller.

960ccb8

flutter-dashboard bot added the platform-android label Apr 11, 2023

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

a261062

chinmaygarde added e: impeller and removed platform-android labels Apr 11, 2023

chinmaygarde assigned dnfield Apr 11, 2023

Fix toImage/toImageSync.

77b8916

Makes the impeller::Context available to the rasterizer ASAP, so that even if it has not received a surface yet calls to `toImage` and `toImageSync` can still produce a texture.

dnfield marked this pull request as ready for review April 11, 2023 17:14

dnfield requested review from bdero, chinmaygarde and jason-simmons April 11, 2023 17:19

flutter-dashboard bot added the platform-android label Apr 11, 2023

Use Metal instead

3fa00ae

dnfield mentioned this pull request Apr 11, 2023

[Impeller] GL backend has a race around surface setup and toImage/toImageSync flutter/flutter#124635

Closed

dnfield added 2 commits April 11, 2023 15:19

fix Fuchsia builds

a682562

Fix multi-engine impeller

d570bbb

dnfield added 2 commits April 11, 2023 17:39

Fix lint issue, nogncheck

d8a8ec2

explicit

7b61b22

dnfield marked this pull request as draft April 13, 2023 20:18

dnfield mentioned this pull request Apr 13, 2023

[Impeller] Allow image rasterization/decoding before/without surface acquisition #41168

Merged

dnfield and others added 2 commits April 13, 2023 20:09

Merge branch 'main' into imp_img_sync

28155cb

Update rasterizer.cc

17564ee

dnfield changed the title ~~[Impeller] Make toImageSync use the context instead of requiring a surface.~~ [Impeller] [Android] Refactor the Android context/surface implementation to work more like Skia. Apr 14, 2023

dnfield marked this pull request as ready for review April 14, 2023 03:12

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from ef5c349f04d8 to d49f2ce83fb1 (7 revisions) flutter/flutter#126659

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

f395152

…mplementation to work more like Skia. (flutter/engine#41059)

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from ef5c349f04d8 to 60dc1385dd2a (8 revisions) flutter/flutter#126663

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

450df75

…mplementation to work more like Skia. (flutter/engine#41059)

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from ef5c349f04d8 to 8a95a0ea2699 (9 revisions) flutter/flutter#126664

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

7e08487

…mplementation to work more like Skia. (flutter/engine#41059)

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from ef5c349f04d8 to 8a95a0ea2699 (9 revisions) flutter/flutter#126672

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

40a57d6

…mplementation to work more like Skia. (flutter/engine#41059)

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from ef5c349f04d8 to 70f665779d99 (10 revisions) flutter/flutter#126675

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

26fb607

…mplementation to work more like Skia. (flutter/engine#41059)

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from ef5c349f04d8 to bb366befbe9d (11 revisions) flutter/flutter#126681

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

ca329d6

…mplementation to work more like Skia. (flutter/engine#41059)

dnfield deleted the imp_img_sync branch May 12, 2023 16:15

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from c784d6d4137d to bb366befbe9d (10 revisions) flutter/flutter#126697

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

91f967e

…mplementation to work more like Skia. (flutter/engine#41059)

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from c784d6d4137d to 3b9ff4118eac (12 revisions) flutter/flutter#126704

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

57ba01f

…mplementation to work more like Skia. (flutter/engine#41059)

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from c784d6d4137d to 2ac855263742 (13 revisions) flutter/flutter#126709

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

3aa63e9

…mplementation to work more like Skia. (flutter/engine#41059)

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from c784d6d4137d to 502e1eb591d9 (14 revisions) flutter/flutter#126714

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

554787f6c [Impeller] [Android] Refactor the Android context/surface i…

5f2fb26

…mplementation to work more like Skia. (flutter/engine#41059)

jason-simmons mentioned this pull request Jul 31, 2023

[Impeller] Create a wrapper Impeller context for each Vulkan surface and its swapchain #44171

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Impeller] [Android] Refactor the Android context/surface implementation to work more like Skia. #41059

[Impeller] [Android] Refactor the Android context/surface implementation to work more like Skia. #41059

Uh oh!

dnfield commented Apr 11, 2023 •

edited

Loading

Uh oh!

dnfield commented Apr 11, 2023

Uh oh!

dnfield commented Apr 11, 2023

Uh oh!

jason-simmons commented Apr 11, 2023

Uh oh!

dnfield commented Apr 11, 2023

Uh oh!

dnfield commented Apr 11, 2023

Uh oh!

dnfield commented Apr 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[Impeller] [Android] Refactor the Android context/surface implementation to work more like Skia. #41059

[Impeller] [Android] Refactor the Android context/surface implementation to work more like Skia. #41059

Uh oh!

Conversation

dnfield commented Apr 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnfield commented Apr 11, 2023

Uh oh!

dnfield commented Apr 11, 2023

Uh oh!

jason-simmons commented Apr 11, 2023

Uh oh!

dnfield commented Apr 11, 2023

Uh oh!

dnfield commented Apr 11, 2023

Uh oh!

dnfield commented Apr 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dnfield commented Apr 11, 2023 •

edited

Loading