Always use texture layer when displaying an Android view by blasten · Pull Request #100091 · flutter/flutter

blasten · 2022-03-14T22:19:36Z

Ensures that Android views are always rendered using a texture layer.

Related issue: #96679

blasten · 2022-03-15T01:18:45Z

packages/flutter/lib/src/rendering/platform_view.dart

 ///  * [AndroidView] which is a widget that is used to show an Android view.
 ///  * [PlatformViewsService] which is a service for controlling platform views.
-class RenderAndroidView extends RenderBox with _PlatformViewGestureMixin {
+class RenderAndroidView extends PlatformViewRenderBox {


Making RenderAndroidView extend PlatformViewRenderBox allows AndroidViewSurface to continue to extend PlatformViewSurface due to the updateRenderObject override.

blasten · 2022-03-15T01:19:42Z

packages/flutter/lib/src/rendering/platform_view.dart

+    (controller as AndroidViewController).removeOnPlatformViewCreatedListener(_onPlatformViewCreated);
+    controller = viewController;
+    viewController.addOnPlatformViewCreatedListener(_onPlatformViewCreated);
+    viewController.pointTransformer = (Offset offset) => globalToLocal(offset);


This seems to be bug currently. pointTransformer is set in the controller during initialization, but it's not set again if the controller is changed.

blasten · 2022-03-15T01:20:46Z

packages/flutter/lib/src/rendering/platform_view.dart

-    _viewController = viewController;
+
+    (controller as AndroidViewController).removeOnPlatformViewCreatedListener(_onPlatformViewCreated);
+    controller = viewController;


this calls the setter in the parent class. I don't like this API, but I'm trying to make this PR a non-breaking change.

cyanglaz · 2022-03-15T04:18:42Z

packages/flutter/lib/src/rendering/platform_view.dart

-    _viewController.removeOnPlatformViewCreatedListener(_onPlatformViewCreated);
-    _viewController = viewController;
+
+    (controller as AndroidViewController).removeOnPlatformViewCreatedListener(_onPlatformViewCreated);


Is (controller as AndroidViewController) just the getter viewController? (Not the parameter)

I cleaned this up. I removed viewController and overrode controller.

cyanglaz · 2022-03-15T04:22:20Z

packages/flutter/lib/src/rendering/platform_view.dart

  /// Sets a new Android view controller.
  ///
  /// `viewController` must not be null.
  set viewController(AndroidViewController viewController) {


Prob better name this paramter so it doesn't conflict with the getter viewController

I removed viewController and overrode controller instead.

cyanglaz · 2022-03-15T04:24:24Z

packages/flutter/lib/src/rendering/platform_view.dart

+
+    (controller as AndroidViewController).removeOnPlatformViewCreatedListener(_onPlatformViewCreated);
+    controller = viewController;
+    viewController.addOnPlatformViewCreatedListener(_onPlatformViewCreated);


It looks a little confusing changing the property after sets the ivar. How about:

this. viewController.removeOnPlatformViewCreatedListener(_onPlatformViewCreated); controller = viewController controller.addOnPlatformViewCreatedListener(_onPlatformViewCreated); controller.pointTransformer = (Offset offset) => globalToLocal(offset); ...

cyanglaz · 2022-03-15T04:45:58Z

packages/flutter/lib/src/services/platform_views.dart

  @override
  Future<void> _sendCreateMessage() async {
-    assert(!_initialSize.isEmpty, 'trying to create $TextureAndroidViewController without setting a valid size.');
+    assert(_initialSize != null, 'trying to create $TextureAndroidViewController without setting an initial size.');


can we test these 2 asserts?

Done. This is guarded by the create() method, which doesn't call super.create unless _initialSize is set. These are tested in services/platform_views_test.dart.

cyanglaz · 2022-03-15T04:47:07Z

packages/flutter/lib/src/rendering/platform_view.dart

-    _viewController.removeOnPlatformViewCreatedListener(_onPlatformViewCreated);
-    _viewController = viewController;
+
+    (controller as AndroidViewController).removeOnPlatformViewCreatedListener(_onPlatformViewCreated);


can this be tested? e.g. the listener is removed from the old controller. and added to the new controller

Done in a7eeb3a

cyanglaz

LGTM

…tter/flutter#100091)

…0091)" This reverts commit 509ddfd.

…0091)" (#100222) This reverts commit 509ddfd.

…iew (flutter#100091)" (flutter#100222)" This reverts commit 1c2c942.

Always use texture layer when displaying an Android view

a5d515e

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Mar 14, 2022

blasten requested a review from cyanglaz March 14, 2022 22:20

blasten marked this pull request as ready for review March 14, 2022 22:20

Emmanuel Garcia added 3 commits March 14, 2022 15:42

Fix deprecation

906f893

Fix tests

55cb16d

API level comment

36d363b

blasten commented Mar 15, 2022

View reviewed changes

cyanglaz reviewed Mar 15, 2022

View reviewed changes

Emmanuel Garcia added 5 commits March 15, 2022 12:33

Fixes

96890fa

Fix deprecation pattern

244bd70

Types

17aa843

Test create() noop in initAndroidView

37005a4

Test callback reset

a7eeb3a

blasten requested a review from cyanglaz March 15, 2022 20:43

sort

b46f5fd

cyanglaz approved these changes Mar 15, 2022

View reviewed changes

Emmanuel Garcia added 3 commits March 15, 2022 13:57

attach check

0116e65

docs

c6e3a27

Naming

e6d2cbc

blasten mentioned this pull request Mar 15, 2022

Set new minimum height for an Android platform view googleads/googleads-mobile-flutter#532

Merged

blasten added the waiting for tree to go green label Mar 15, 2022

fluttergithubbot merged commit 509ddfd into flutter:master Mar 16, 2022

engine-flutter-autoroll mentioned this pull request Mar 16, 2022

Roll Flutter from fd25dc848324 to 509ddfda5c1b (5 revisions) flutter/packages#1230

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2022

509ddfd Always use texture layer when displaying an Android view (flu…

e2abd77

…tter/flutter#100091)

engine-flutter-autoroll mentioned this pull request Mar 16, 2022

Roll Flutter from fd25dc848324 to 509ddfda5c1b (5 revisions) flutter/plugins#5066

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 16, 2022

509ddfd Always use texture layer when displaying an Android view (flu…

4387df5

…tter/flutter#100091)

blasten deleted the switch_to_tx branch March 16, 2022 04:54

This was referenced Mar 16, 2022

Android platform view performance issues #96679

Closed

Android inline ad performance googleads/googleads-mobile-flutter#269

Closed

engine-flutter-autoroll mentioned this pull request Mar 16, 2022

Roll Flutter from fd25dc848324 to 509ddfda5c1b (5 revisions) flutter/packages#1231

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2022

509ddfd Always use texture layer when displaying an Android view (flu…

cabf3d2

…tter/flutter#100091)

zanderso added a commit that referenced this pull request Mar 16, 2022

Revert "Always use texture layer when displaying an Android view (#10…

8fc4dab

…0091)" This reverts commit 509ddfd.

zanderso mentioned this pull request Mar 16, 2022

Revert "Always use texture layer when displaying an Android view" #100222

Merged

zanderso added a commit that referenced this pull request Mar 16, 2022

Revert "Always use texture layer when displaying an Android view (#10…

1c2c942

…0091)" (#100222) This reverts commit 509ddfd.

blasten pushed a commit to blasten/flutter that referenced this pull request Mar 16, 2022

Revert "Revert "Always use texture layer when displaying an Android v…

89558f1

…iew (flutter#100091)" (flutter#100222)" This reverts commit 1c2c942.

blasten mentioned this pull request Mar 16, 2022

Reland: "Always use texture layer when displaying an Android view" #100237

Merged

jjliu15 mentioned this pull request Jul 7, 2022

Ads placed in ListView is getting impression before they appear on the screen in Flutter 3 googleads/googleads-mobile-flutter#580

Closed

Conversation

blasten commented Mar 14, 2022

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!

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!

cyanglaz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants