Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose setZOrderMediaOverlay in the new embedding #41984

Closed
kf6gpe opened this issue Oct 4, 2019 · 5 comments
Closed

Expose setZOrderMediaOverlay in the new embedding #41984

kf6gpe opened this issue Oct 4, 2019 · 5 comments
Assignees
Labels
a: existing-apps Integration with existing apps via the add-to-app flow customer: dream (g3) engine flutter/engine repository. See also e: labels. platform-android Android applications specifically
Milestone

Comments

@kf6gpe
Copy link
Contributor

kf6gpe commented Oct 4, 2019

Requested by customer: dream. Mirror issue for https://b.corp.google.com/issues/142107954. Conversation should occur there.

@kf6gpe kf6gpe added engine flutter/engine repository. See also e: labels. e: embedder Users of the Embedder API customer: dream (g3) labels Oct 4, 2019
@chinmaygarde
Copy link
Member

I am removing the embedder tag as this is Android related and not using the embedder API.

@chinmaygarde chinmaygarde removed the e: embedder Users of the Embedder API label Oct 18, 2019
@mklim mklim added platform-android Android applications specifically a: existing-apps Integration with existing apps via the add-to-app flow labels Nov 7, 2019
@jmagman jmagman added this to Awaiting triage in Add-to-app - Android engine review Jan 9, 2020
@matthew-carroll matthew-carroll moved this from Awaiting triage to Engineer reviewed in Add-to-app - Android engine review Jan 17, 2020
@matthew-carroll matthew-carroll added this to On Radar in Matt's WIP Jan 17, 2020
@matthew-carroll
Copy link
Contributor

@mklim I'm curious if you have any thoughts on the API for this.

The embedding API at the Activity/Fragment/FlutterView level is meant to be generally configurable. However, the z-index only seems relevant for a FlutterSurfaceView, which may or may not be used.

I'm thinking we can expose that configuration at each relevant point and then document that it only applies when a RenderMode.surface is used. This comes with the downside that we have APIs that sometimes don't do anything and also increase the set of things that developers need to read about and consider. Do you have any opinions on the API?

@matthew-carroll matthew-carroll self-assigned this Jan 30, 2020
@matthew-carroll matthew-carroll added this to the March 2020 milestone Jan 30, 2020
@matthew-carroll matthew-carroll moved this from On Radar to In progress in Matt's WIP Jan 30, 2020
@matthew-carroll matthew-carroll moved this from In progress to Waiting for customer in Matt's WIP Jan 30, 2020
@mklim
Copy link
Contributor

mklim commented Feb 3, 2020

@matthew-carroll I don't think I'm as keyed in on this as you are at this point, but my instinct here would actually be to expose the underlying SurfaceView at some appropriate point for advanced users who really need to call SurfaceView specific APIs like this instead of trying to plumb calls all the way through the Android embedding that are ultimately view specific like this one to whatever view is attached. I get that we don't want to provide a false sense of security that any given view will remain associated with a particular engine, which as I understand it was the main problem with the v1 API exposing the view through some of its getters. But I also think wrapping plain old View APIs at a higher level would also be a mistake because then we'd be forcing clients to learn our wrapper layer in addition to Android, and it would inherently be a little confusing where all the API calls wouldn't work in all cases like you've mentioned, and we'd always be lagging in cases like this where we wouldn't be exposing the full API surface some users may want or need.

So ultimately I'd think of exposing a getView method on FlutterActivityAndFragmentDelegate with some clear warnings that there's no guarantees that the view is going to stay current for any amount of time after the API call. Maybe expose it only through some new callback registration so API users who care are forced to register onAdded/onRemoved listeners and so it's obvious through the API that the view is impermanent? That would match the rest of the flow. Could also give the Flutter view a specific ID and tell users who really want it to go find it within the layout directly, but it would be harder to do this since we don't distribute the engine as an AAR right now and so wouldn't be able to distribute the ID as an XML resource.

@matthew-carroll matthew-carroll moved this from Waiting for customer to On Radar in Matt's WIP Feb 10, 2020
@matthew-carroll matthew-carroll moved this from On Radar to In progress in Matt's WIP Feb 11, 2020
@matthew-carroll matthew-carroll moved this from In progress to In Review in Matt's WIP Feb 12, 2020
@matthew-carroll
Copy link
Contributor

The ability to access the FlutterSurfaceView and configure it is now facilitated via the PR here:
flutter/engine#16552

That engine change will need to roll to the framework before becoming general available on master.

Closing this issue as resolved.

@matthew-carroll matthew-carroll moved this from In Review to Done in Matt's WIP Feb 13, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this issue Feb 14, 2020
dnfield pushed a commit that referenced this issue Feb 14, 2020
* f49a8b6 Roll src/third_party/skia c03e6982f96f..465864cad5d2 (14 commits) (flutter/engine#16524)

* c477c06 Enable verbose logging for shell unittests on Fuchsia (flutter/engine#16526)

* a662579 Clear frame references at the end of every CanvasKit frame (flutter/engine#16525)

* 3f31ea3 Roll src/third_party/skia 465864cad5d2..21f382c19d76 (6 commits) (flutter/engine#16528)

* 38fb6b1 Roll fuchsia/sdk/core/linux-amd64 from 8L7NY... to Bmq1m... (flutter/engine#16529)

* 9c0168a Roll fuchsia/sdk/core/mac-amd64 from PMcw3... to 7JkB7... (flutter/engine#16530)

* e8a888d Roll src/third_party/skia 21f382c19d76..f83d0346c06a (2 commits) (flutter/engine#16532)

* 1e8b331 Roll src/third_party/dart 5244d99a5d4e..5fc031ebc1d7 (42 commits) (flutter/engine#16533)

* c4e3ae6 Roll src/third_party/skia f83d0346c06a..88c3793a4eaa (1 commits) (flutter/engine#16534)

* 6cdb14e Roll src/third_party/skia 88c3793a4eaa..abefc9c170c9 (1 commits) (flutter/engine#16535)

* 975acd8 Roll src/third_party/skia abefc9c170c9..4fe89b4d871d (2 commits) (flutter/engine#16536)

* b7424d0 Roll src/third_party/dart 5fc031ebc1d7..30151a654151 (2 commits) (flutter/engine#16537)

* 25e8127 Roll src/third_party/skia 4fe89b4d871d..dc2782c380f6 (1 commits) (flutter/engine#16538)

* 74fa10c Roll src/third_party/dart 30151a654151..76b18c455e2c (1 commits) (flutter/engine#16539)

* 91b8e40 Roll src/third_party/skia dc2782c380f6..cdf2491afa04 (1 commits) (flutter/engine#16540)

* 5acf9b1 Roll src/third_party/skia cdf2491afa04..50a490a1a4fb (2 commits) (flutter/engine#16541)

* 9897777 Roll src/third_party/skia 50a490a1a4fb..c3b67eb988c8 (4 commits) (flutter/engine#16542)

* 78a8909 Use os_log instead of syslog on Apple platforms (flutter/engine#13487)

* ea56ad2 libtxt: use a fixture in the benchmarks (flutter/engine#16531)

* a61dbf2 Revert "Use os_log instead of syslog on Apple platforms (#13487)" (flutter/engine#16546)

* 539f64f [fuchsia] Disable retained layers (flutter/engine#16548)

* c3b5072 Expose DPI helper functions for Runner apps to use (flutter/engine#16313)

* 5041ff1 support endless trace buffer (flutter/engine#16520)

* 6aacf5e Re-land: Use os_log instead of syslog on Apple platforms (flutter/engine#16549)

* a5736b8 Roll src/third_party/skia c3b67eb988c8..b1525c721ea6 (4 commits) (flutter/engine#16543)

* 49a370f Roll src/third_party/dart 76b18c455e2c..e4c39721c473 (6 commits) (flutter/engine#16544)

* 270421c Fix ensureInitializationCompleteAsync callback when already initialized. (#39675) (flutter/engine#16503)

* ca02b91 Prevent long flash when switching to Flutter app. (#47903) (flutter/engine#16527)

* 44e80fd skiping tests in Safari. LUCI recipe for Mac is ready. this is the only step left for stopping us running unit tests in Safari (flutter/engine#16550)

* 5fb0116 iOS platform view gesture blocking policy. (flutter/engine#15940)

* e0ebaea Revert "Re-land: Use os_log instead of syslog on Apple platforms (#16549)" (flutter/engine#16558)

* 8a6b949 [Fuchsia] Dump syslog output after tests have run (flutter/engine#16561)

* bca879c Roll src/third_party/dart e4c39721c473..0299903f3e78 (31 commits) (flutter/engine#16553)

* cd11d7a Roll fuchsia/sdk/core/mac-amd64 from 7JkB7... to t4kck... (flutter/engine#16555)

* 99a265b [web] Fix edge cases in Paragraph.getPositionForOffset to match Flutter (flutter/engine#16557)

* 8f8af1f Update felt documentation (flutter/engine#16559)

* 13dce50 Roll src/third_party/skia b1525c721ea6..67da665c27ff (32 commits) (flutter/engine#16562)

* 7c67573 Fix multiline Javadoc code blocks (flutter/engine#16565)

* aece5ad Move log_listener call into the reboot trap (flutter/engine#16564)

* 42f18d9 Roll src/third_party/skia 67da665c27ff..886e8500a9f2 (3 commits) (flutter/engine#16566)

* c4c6ef6 Samsung keyboard duplication workaround: updateSelection (flutter/engine#16547)

* 15062ca Revert "Re-arm timer as necessary in MessageLoopFuchsia" (flutter/engine#16568)

* 8802a1d Roll src/third_party/skia 886e8500a9f2..9102c86a81ad (1 commits) (flutter/engine#16570)

* dbdcae4 Roll src/third_party/skia 9102c86a81ad..6029cbd560b7 (2 commits) (flutter/engine#16575)

* f39bc73 Exposes FlutterSurfaceView, and FlutterTextureView to FlutterActivity and FlutterFragment. (#41984, #47557) (flutter/engine#16552)

* db030ec Roll src/third_party/skia 6029cbd560b7..1a733b5b760a (1 commits) (flutter/engine#16577)

* 050d29d Roll src/third_party/skia 1a733b5b760a..1d1333fcedf8 (3 commits) (flutter/engine#16578)

* 97fd898 Roll fuchsia/sdk/core/mac-amd64 from t4kck... to oHa-O... (flutter/engine#16581)

* 2e67866 Roll src/third_party/skia 1d1333fcedf8..3bf3b92dfab0 (1 commits) (flutter/engine#16584)
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: existing-apps Integration with existing apps via the add-to-app flow customer: dream (g3) engine flutter/engine repository. See also e: labels. platform-android Android applications specifically
Projects
No open projects
Matt's WIP
  
Done
Development

No branches or pull requests

4 participants