Flutter External Texture Based On OpenGL Texture Underlying Share Context #11819

kaisa695275735 · 2019-09-02T01:57:25Z

This is a new PR about External Texture, the old one(#11276) is closed because of my mistaken git operation.

kaisa695275735 · 2019-09-12T02:42:38Z

Hi @dnfield @gaaclarke We've added the unit test code in testing/scenarios_app for IOS, which is similar to the platform view example.
For Android ,we've finished the demo code to be tested, but did not find any good way for testing benchmark

dnfield · 2019-09-12T03:33:56Z

We don't have as much wired up for Android on CI just yet. Before we look at this, can you work on the merge conflicts? @gw280 just landed a patch that is causing some conflicts with this one.

# Conflicts: # shell/platform/darwin/ios/platform_view_ios.h # testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj

kaisa695275735 · 2019-09-12T06:16:47Z

We don't have as much wired up for Android on CI just yet. Before we look at this, can you work on the merge conflicts? @gw280 just landed a patch that is causing some conflicts with this one.

Conflicts are resolved already.

gw280 · 2019-09-13T19:37:44Z

We don't have as much wired up for Android on CI just yet. Before we look at this, can you work on the merge conflicts? @gw280 just landed a patch that is causing some conflicts with this one.

Conflicts are resolved already.

My change got backed out because it was failing LUCI, so I'm currently working on reworking that. When are you hoping to merge this change?

shell/platform/android/android_external_texture_gl_share_context.cc

shell/platform/android/android_external_texture_gl_share_context.h

shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java

shell/platform/android/io/flutter/view/FlutterView.java

shell/platform/android/platform_view_android_jni.cc

shell/platform/darwin/ios/ios_external_texture_gl_share_context.h

# Conflicts: # shell/platform/darwin/ios/platform_view_ios.h # testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m # testing/scenario_app/lib/main.dart # testing/scenario_app/lib/src/scenario.dart

kaisa695275735 · 2019-09-16T06:39:22Z

We don't have as much wired up for Android on CI just yet. Before we look at this, can you work on the merge conflicts? @gw280 just landed a patch that is causing some conflicts with this one.

Conflicts are resolved already.

My change got backed out because it was failing LUCI, so I'm currently working on reworking that. When are you hoping to merge this change?

We may not merge this change until the code reviews are all resolved.

# Conflicts: # shell/platform/darwin/ios/ios_gl_context.h # shell/platform/darwin/ios/platform_view_ios.h

kangwang1988 · 2019-09-18T10:02:09Z

@dnfield
@kaisa695275735 is my colleague focusing on video related works.
Should we wait for more members to review it or it's about ready to be merged?

dnfield · 2019-09-18T14:23:22Z

Thanks! It needs to have a LGTM before meeting. I can try to take a look today as well but it looks like @gaaclarke is on it.

Conflicts: shell/platform/darwin/ios/platform_view_ios.h

kaisa695275735 · 2019-09-25T02:27:18Z

@gaaclarke we have make our changes.
please take a look! Thank you very much!

gaaclarke

Thanks for cleaning this up. It made it easier to review.

I've reviewed all the iOS code. One problem with it is that you are leaking OpenGL into interfaces that are renderer agnostic. Remember that we also support Metal and these changes don't make sense for that renderer.

The crux of your problem is FlutterTexture::copyPixelBuffer, right? I think there may be a way you can get the performance you want by using FlutterTexture and internally using CVOpenGLESTextureCache to get CPU/GPU images. See https://developer.apple.com/documentation/corevideo/cvopenglestexturecache?language=objc for more information.

gaaclarke · 2019-09-25T21:08:57Z

shell/platform/darwin/ios/platform_view_ios.h

  fml::scoped_nsprotocol<FlutterTextInputPlugin*> GetTextInputPlugin() const;

  void SetTextInputPlugin(fml::scoped_nsprotocol<FlutterTextInputPlugin*> plugin);

  // |PlatformView|
  void SetSemanticsEnabled(bool enabled) override;

+  EAGLSharegroup* GetGLShareGroup();


Leaking OpenGL in renderer agnostic code.

gaaclarke · 2019-09-25T21:09:27Z

shell/platform/darwin/ios/ios_surface.h

@@ -39,6 +39,8 @@ class IOSSurface {
  virtual std::unique_ptr<Surface> CreateGPUSurface(
      GrContext* gr_context = nullptr) = 0;

+  virtual EAGLSharegroup* GetGLShareGroup() = 0;


Leaking OpenGL into renderer agnostic code.

gaaclarke · 2019-09-25T21:10:18Z

shell/platform/darwin/ios/framework/Headers/FlutterTexture.h

 - (void)textureFrameAvailable:(int64_t)textureId;
 - (void)unregisterTexture:(int64_t)textureId;
+- (EAGLSharegroup*)getShareGroup;


Leaking OpenGL into renderer agnostic code.

gaaclarke · 2019-09-25T21:11:15Z

shell/platform/darwin/ios/ios_external_texture_gl_share_context.h

+// Unlike the original external texture which copies pixel buffer from an object that implements
+// FlutterTexture protocol, and creates a texture reference using the pixel buffer, this solution
+// will copy the OpenGL texture directly from the object implementing FlutterShareTexture protocol.
+class IOSExternalTextureShareContext : public flutter::Texture {


This should probably be named IOSExternalShareContextTexture. The type is usually placed at the end of the name.

gaaclarke · 2019-09-25T21:22:56Z

I drew up the iOS portion of the PR:

cbracken · 2019-10-07T17:44:38Z

@kaisa695275735 just checking in to see if you're still working on this PR. If so, no worries!

zhangruiyu · 2019-10-12T02:41:39Z

????

cbracken · 2019-10-21T18:04:51Z

@kaisa695275735 is PR still work-in-progress? Looks like we're waiting on fixes to address @gaaclarke 's comments.

kaisa695275735 · 2019-10-23T07:46:37Z

@kaisa695275735 is PR still work-in-progress? Looks like we're waiting on fixes to address @gaaclarke 's comments.

sorry for my later reply. I took a long vocation last month.
we are trying to use CVOpenGLESTextureCache to implement a filter camera demo, and we find this scheme does have the same CPU level with our share texture scheme.
So all the code for iOS is not needed.
We are trying to use SurfaceTexture scheme for Android, if they have the same CPU level,
this PR may be closed.

demo git: git@github.com:kaisa695275735/filter_camera.git

Flutter External Texture Based On Share Texture

55933e2

googlebot added cla: yes labels Sep 2, 2019

kaisa695275735 mentioned this pull request Sep 2, 2019

Flutter External Texture Based On Share Texture Underlying Share Con… #11276

Closed

code format

b5b19c7

iskakaushik requested review from dnfield and iskakaushik September 3, 2019 15:30

dnfield requested a review from gaaclarke September 3, 2019 16:31

kaisa695275735 added 6 commits September 6, 2019 14:47

Merge branch 'external_texture_merge_master' into external_texture_new

c3c0dfd

Add testing code for new external texture

8c2beb2

Code format

618094a

Merge branch 'external_texture_new_bkp4' into external_texture_new

adb2f13

iOS update texture id

0052f47

Code format

6dfa303

kaisa695275735 added 2 commits September 12, 2019 12:36

Merge branch 'upstream_master' into external_texture_new

9471f5c

# Conflicts: # shell/platform/darwin/ios/platform_view_ios.h # testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj

Code format

7e297d8

gaaclarke reviewed Sep 13, 2019

View reviewed changes

Merge branch 'origin_master' into external_texture_new

3e4196d

# Conflicts: # shell/platform/darwin/ios/platform_view_ios.h # testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m # testing/scenario_app/lib/main.dart # testing/scenario_app/lib/src/scenario.dart

kaisa695275735 added 7 commits September 16, 2019 14:46

Code format

f964c8a

Codereview

1a2be7a

Code format

6da0037

Hard code remove

464de18

Merge branch 'origin_master' into external_texture_new

6190ac2

# Conflicts: # shell/platform/darwin/ios/ios_gl_context.h # shell/platform/darwin/ios/platform_view_ios.h

code format

87bd692

Merge compile error modify

a1ef963

kaisa695275735 added 4 commits September 17, 2019 10:13

Delete unused code

7675cdc

Check SKImage null

0c9a675

Add texture_id == 0 debug log

78089db

Move docstring to header file

17185dc

kangwang1988 added the customer: alibaba label Sep 18, 2019

kaisa695275735 requested a review from gaaclarke September 19, 2019 06:32

kaisa695275735 added 5 commits September 20, 2019 10:03

Merge branch 'external_texture_backup' into external_texture_new

2310264

Merge branch 'master' into external_texture_new

ebba66c

Conflicts: shell/platform/darwin/ios/platform_view_ios.h

code format

3073e06

Merge branch 'external_texture_bkp3' into external_texture_new

37feaa4

doc string

ada6565

gaaclarke reviewed Sep 25, 2019

View reviewed changes

cbracken added the waiting for customer response label Oct 21, 2019

gaaclarke closed this Oct 23, 2019

Flutter External Texture Based On OpenGL Texture Underlying Share Context #11819

Flutter External Texture Based On OpenGL Texture Underlying Share Context #11819

Uh oh!

Conversation

kaisa695275735 commented Sep 2, 2019

Uh oh!

kaisa695275735 commented Sep 12, 2019

Uh oh!

dnfield commented Sep 12, 2019

Uh oh!

kaisa695275735 commented Sep 12, 2019

Uh oh!

gw280 commented Sep 13, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kaisa695275735 commented Sep 16, 2019

Uh oh!

kangwang1988 commented Sep 18, 2019

Uh oh!

dnfield commented Sep 18, 2019

Uh oh!

kaisa695275735 commented Sep 25, 2019

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

gaaclarke Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

gaaclarke Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

gaaclarke Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Sep 25, 2019

Uh oh!

cbracken commented Oct 7, 2019

Uh oh!

zhangruiyu commented Oct 12, 2019

Uh oh!

cbracken commented Oct 21, 2019

Uh oh!

kaisa695275735 commented Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kaisa695275735 commented Oct 23, 2019 •

edited

Loading