Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

matthew-carroll
Copy link
Contributor

@matthew-carroll matthew-carroll commented Feb 12, 2020

Exposes FlutterView, FlutterSurfaceView, and FlutterTextureView to FlutterActivity and FlutterFragment. (#41984, #47557)

#41984 requires access to FlutterSurfaceView to set the Z media overlay

#47557 requires access to FlutterSurfaceView to enable secure rendering

While both of the referenced issues could be solved by revealing only FlutterSurfaceView, it seems reasonable that for symmetry we should reveal the alternative implementation, too, which is FlutterTextureView.

Additionally, the nature of the requirements that require access to FlutterSurfaceView indicate that it's likely that some level of Android View control will be required for FlutterView, itself. Therefore, it is exposed, too. (In review I removed this method until such a time that we require it)

The following changes were made:

  • Accept a FlutterSurfaceView or FlutterTextureView in the FlutterView constructor instead of internally instantiating one or the other.

  • Add FlutterActivityAndFragmentDelegate#Host callbacks that allow Hosts to customize their FlutterView, FlutterSurfaceView, and FlutterTextureView. (breaking change for any implementers of Host interface - the fix is to declare no-ops).

  • Move FlutterView.RenderMode to io.flutter.embedding.android.RenderMode (soft deprecation)

  • Move FlutterView.TransparencyMode to io.flutter.embedding.android.TransparencyMode (soft deprecation)

  • Adjusted all APIs to use io.flutter.embedding.android.RenderMode/TransparencyMode instead of io.flutter.embedding.android.FlutterView.RenderMode/TransparencyMode (breaking change for any API consumers - the fix is to change the classpath of the enums).

The reason for the move of RenderMode and TransparencyMode is that the fundamental API change made in this PR completely takes those configurations out of the purview of FlutterView. Therefore, those enums make no sense living in FlutterView.

Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. FYI it looks like there's some formatting issues in CI.

@@ -71,7 +71,7 @@
* FlutterEngine(context); flutterEngine .getDartExecutor()
* .executeDartEntrypoint(DartEntrypoint.createDefault());
*
* <p>// Cache the pre-warmed FlutterEngine in the FlutterEngineCache.
* <p><p>// Cache the pre-warmed FlutterEngine in the FlutterEngineCache.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this implementation comment is lost

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looks like the formatter already jumbled this. I just tried to fix the spacing, but I'm not sure how I'll get it by the linter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looks like the formatter already jumbled this. I just tried to fix the spacing, but I'm not sure how I'll get it by the linter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I missed that this was supposed to be a code sample, sorry. I thought this was an implementation comment meant for the actual code of the class. What you want here are actually <pre> tags, not paragraph (weird HTML convention). I'll put in a follow up CL to find and fix the instances of code eaten like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the linter wants to keep adding more <p> tags here. Really, we don't want any. Do you know if <pre> is supported in a way that would help here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we both had the same idea. Do you know if I should add a <pre> on the line above the whole block and a </pre> on the lien after the whole block?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it the "right" way to do this is <pre>{@code }</pre>. https://stackoverflow.com/a/52002609/3968079 has a good explanation. I put up #16565 to try and clean this up since this was a regression from the formatting change.

@matthew-carroll matthew-carroll force-pushed the 41984_47557_expose-FlutterSurfaceView-to-app-devs branch from 1940209 to 70e59c0 Compare February 12, 2020 23:13
@matthew-carroll
Copy link
Contributor Author

@mklim @dnfield @xster do you think this requires a breaking change notice?

@mklim
Copy link
Contributor

mklim commented Feb 12, 2020

This doesn't break anything in flutter/tests so it doesn't strictly require one. It also avoids any "real" breakages and just adds a deprecation warning to a public API, so I think that's OK.

@matthew-carroll
Copy link
Contributor Author

@mklim Agreed with those points - the only qualifier is that technically anyone who implements Host will get a compile breakage due to new interface methods. Is that a concern?

@matthew-carroll
Copy link
Contributor Author

I'm gonna go ahead and merge because I'm not aware of any external uses of the Host interface. Even if there are, it is a trivial correction as far as compile issues go.

@matthew-carroll matthew-carroll merged commit f39bc73 into flutter:master Feb 13, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 14, 2020
dnfield pushed a commit to flutter/flutter that referenced this pull request 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)
NoamDev pushed a commit to NoamDev/engine that referenced this pull request Feb 27, 2020
NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020
@louyaming
Copy link

louyaming commented May 13, 2020

I have a question as :https://stackoverflow.com/questions/61743018/the-nullpointerexception-crash-that-occur-in-the-flutter-under-rendermode-of-ren

I found a problem when I reviewed the code,as:
FlutterActivityAndFragmentDelegate.java:
flutterView = new FlutterView(host.getActivity(), flutterTextureView); L272

FlutterView.java: this.renderSurface = flutterSurfaceView; L267 flutterSurfaceView is null
FlutterView.java: renderSurface.attachToRenderer(flutterRenderer); L699
so this line will occur NullPointerException.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants