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

Handle SurfaceView in a VirtualDisplay #33599

Merged
fluttergithubbot merged 25 commits into from
Jun 23, 2022
Merged

Handle SurfaceView in a VirtualDisplay #33599

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0832efc
Handle SurfaceView and TextureView in a VirtualDisplay
May 24, 2022
e2b2b94
refactor
May 24, 2022
e2e2341
Fix licenses
May 25, 2022
a8675ec
Add logs
May 26, 2022
9d30502
comment
May 26, 2022
6981201
fix
May 26, 2022
b8e49a0
use info
May 26, 2022
583b597
remove nullable context
May 26, 2022
dde18d7
document code
Jun 15, 2022
7f87be7
fixes
Jun 16, 2022
5e924ab
fix test
Jun 16, 2022
7cb2919
fallback to VD if API level < 23
Jun 17, 2022
42adbc4
revert last pieces
Jun 18, 2022
cf3b220
add integration test
Jun 18, 2022
225f943
clean up
Jun 18, 2022
58e54df
add missing view_type
Jun 18, 2022
cffe771
revert
Jun 18, 2022
33877fa
fixes
Jun 19, 2022
e44e6cd
talkback issue
Jun 21, 2022
8f821f6
don't include TextureView since it breaks a11y in google maps
Jun 21, 2022
1cde9af
integration test (rotate activity + invalidate after frame)
Jun 22, 2022
7b4708a
feedback
Jun 22, 2022
f0d1029
dispose bug
Jun 22, 2022
110bdb5
fix comment line, so it fits in a line after formatting
Jun 22, 2022
5a2c81b
handle clamp
Jun 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
don't include TextureView since it breaks a11y in google maps
  • Loading branch information
Emmanuel Garcia committed Jun 21, 2022
commit 8f821f6f609e0d7c49a7ed3d0c12ee27d214edf9
11 changes: 8 additions & 3 deletions shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import android.util.SparseArray;
import android.view.MotionEvent;
import android.view.SurfaceView;
import android.view.TextureView;
import android.view.View;
import android.view.ViewGroup;
import android.widget.FrameLayout;
Expand Down Expand Up @@ -52,12 +51,18 @@
public class PlatformViewsController implements PlatformViewsAccessibilityDelegate {
private static final String TAG = "PlatformViewsController";

// These view types allow out-of-band drawing operation that don't notify the Android view
// These view types allow out-of-band drawing commands that don't notify the Android view
// hierarchy.
// To support these cases, Flutter hosts the embedded view in a VirtualDisplay,
// and binds the VirtualDisplay to a GL texture that is then composed by the engine.
// However, there are a few issues with Virtual Displays. For example, they don't fully support
// accessibility due to https://github.com/flutter/flutter/issues/29717,
// and keyboard interactions may have non-derterministic behavior.
Copy link
Member

@jason-simmons jason-simmons Jun 21, 2022

Choose a reason for hiding this comment

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

typo: "deterministic"

Copy link
Author

@blasten blasten Jun 22, 2022

Choose a reason for hiding this comment

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

done

// Views that issue out-of-band drawing commands that aren't included in this array are
// required to call `View#invalidate()` to notify Flutter about the update.
// This isn't ideal, but given all the other limitations it's a reasonable tradeoff.
// Related issue: https://github.com/flutter/flutter/issues/103630
private static Class[] VIEW_TYPES_REQUIRE_VD = {SurfaceView.class, TextureView.class};
private static Class[] VIEW_TYPES_REQUIRE_VD = {SurfaceView.class};
Copy link
Contributor

@dnfield dnfield Jun 21, 2022

Choose a reason for hiding this comment

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

These limitations seem pretty severe.

What are we solving with this? Just unbreaking mapbox? Are there lots of use cases for SurfaceView inside a platform view? Could we be doing something better here and just giving mapbox a GL context to work with?

What views need to call invalidate? Just TextureViews?

Copy link
Contributor

@dnfield dnfield Jun 21, 2022

Choose a reason for hiding this comment

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

Of course, if we did give plugins some way to directly talk to GL, we'd have to have them do some heavier lifting around a11y and input anyway...

Copy link
Author

@blasten blasten Jun 22, 2022

Choose a reason for hiding this comment

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

plugins can actually.
https://api.flutter.dev/flutter/widgets/Texture-class.html - this is how Camera is implemented under the hood.

What are we solving with this?

Being able to add an embedded SurfaceView into a Flutter app. Like mapbox.

Are there lots of use cases for SurfaceView inside a platform view?

There's a g3 customer that used a WebRTC view, but they are using hybrid composition. Hybrid composition implies jank in the main thread since all raster tasks are run there, slower platform channels, and the possibility of glitches that can jump to P0 when they hit an internal customer. The texture approach performs better generally.

What views need to call invalidate? Just TextureViews?

TextureViews yes. flutter/flutter#103686 for context. At this moment, it's the following tradeoff: would we be ok breaking accessibility?, so flutter/flutter#103686 can be closed? And in the future, maybe with enough resources fix accessibility just for virtual display. Is virtual display big enough to fix it?

Copy link
Contributor

@dnfield dnfield Jun 22, 2022

Choose a reason for hiding this comment

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

But rather than having SurfaceView or TextureView in a platform view, could we just give plugins a GL context to draw into?

Alternatively, could the plugin just use the TextureRegistry API instead? That should generally perform better than platform views - but again, I'm not sure I really understand all the requirements. For a WebRTC plugin, I'd imagine they want to treat that more like how exoplayer works rather than like a platform view though...

Copy link
Author

@blasten blasten Jun 22, 2022

Choose a reason for hiding this comment

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

I would actually love to see is an official API that allows to project a View into a GL Texture while the view is in the view hierarchy. Some challenges:

  1. it must support embedded TextureViews like the Google Maps View.
  2. it must support embedded SurfaceViews like mapbox or WebRTC views.
  3. WebView is a special view that does its own thing

Copy link

@josephcrowell josephcrowell Jun 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

MapView uses SurfaceView and TextureView https://github.com/mapbox/mapbox-maps-android/blob/main/sdk/src/main/java/com/mapbox/maps/MapView.kt

The SDK also includes a lot of other functionality which we would have to implement ourselves instead if we either rendered the MapView directly to a surface or used MapSurface, outside of an activity, I believe? Other Flutter Mapbox plugin projects don't necessarily use flutter-mapbox-gl. Navigation apps have their own implementations of flutter-mapbox-gl's functionality mainly because the Mapbox Navigation SDK implements the MapView directly and handles navigation activity through the view and expects other components to be available in the activity.

https://docs.mapbox.com/android/navigation/examples/turn-by-turn-experience/

Mapbox is pushing developers in the direction of using their activity as a drop in UI completely and not directly implementing individual components at all: https://docs.mapbox.com/android/navigation/guides/drop-in-ui/

Looking at the list of things which would need to be reimplemented if we used a texture registry separately and reimplemented the remaining components of the Navigation SDK, we might as well just write native applications because we would be doing the same amount of work either way. Maybe less work.

Am I reading this wrong or will we actually have to bypass the activity and all the other Mapbox Navigation widgets (which are not drawn on the GL surface) when pushing the GL surface, which is only part of the SDK functionality, through a texture view?

These aren't a play button and a seek bar. They're turn by turn directions (this thing is complcated with lane guidance and road signs and more) and distances and mute buttons and speeding indicators and navigation/overhead view switchers where the underlying code isn't directly exposed to us.

Copy link
Contributor

@dnfield dnfield Jun 23, 2022

Choose a reason for hiding this comment

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

MapView uses SurfaceView and TextureView https://github.com/mapbox/mapbox-maps-android/blob/main/sdk/src/main/java/com/mapbox/maps/MapView.kt

Right, it's not best option for flutter Olivia

Copy link
Contributor

@dnfield dnfield Jun 23, 2022

Choose a reason for hiding this comment

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

Haha Olivia was somehow added by autocorrect sorry :)

Copy link

@josephcrowell josephcrowell Jun 23, 2022

Choose a reason for hiding this comment

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

Would it be possible to pipe the entire activity including overlay widgets through using the texture registry?

Copy link
Contributor

@dnfield dnfield Jun 23, 2022

Choose a reason for hiding this comment

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

No. Just the surface


private final PlatformViewRegistryImpl registry;

Expand Down
1 change: 1 addition & 0 deletions ...nario_app/android/app/src/main/java/dev/flutter/scenarios/TexturePlatformViewFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public void onSurfaceTextureAvailable(SurfaceTexture surface, int width, int hei
paint.setColor(Color.GREEN);
canvas.drawCircle(canvas.getWidth() / 2, canvas.getHeight() / 2, 20, paint);
textureView.unlockCanvasAndPost(canvas);
textureView.invalidate();
}

@Override
Expand Down