Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add texture support for macOS shell. #8507
Add texture support for macOS shell. #8507
Changes from 16 commits
5302cf8
2953e13
4b422b3
fa3f8b0
a222a4a
668e656
97f1e91
cd636b4
799e9c1
52b1417
e49c549
7589f46
2c60631
42b7502
dbcdf16
9b15427
e4ff652
ba2efa3
9c19bac
6ff0a76
ec8083a
ff49e14
ee09e69
0f06004
09156bf
235314a
650c00a
d4c0be8
cda947c
e78d2e1
eb3bf39
d622f01
ec70d4a
2871956
9b63c65
4962f91
b20be2c
067e8a1
27c6f94
5f6833f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be identical to FlutterTexture.h other than the prefixes; now that macOS is in the same repo as iOS the goal is to share code whenever possible (and move away from the FLE prefix). You can just add that file to the
frameworkSshared.gni
list, and use the existing protocols in the macOS code.(If you want to add these comments to FlutterTexture.h, ideally as a separate PR, that would be great, since the existing file isn't commented at all.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chinmaygarde @stuartmorgan
I found in the macos test that CVOpenGLTextureCacheCreateTextureFromImage creates a GL_TEXTURE_RECTANGLE object instead of GL_TEXTURE_2D, which causes the texture to not be properly scaled to fill the flutter widget when rendering, so I used width/height in texture->copyPixelBuffer when the plugin receives the request, it scales the texture to the corresponding size according to the widget size provided by the engine, and then becomes normal when rendering.
I'm not sure to add width/height to ios/framework/Headers/FlutterTexture.h, or continue to use FLETexture.h, or should report a new issue in flutter engine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to specify that the texture target is GL_TEXTURE_RECTANGLE by specifying the target property of FlutterOpenGLTexture. All the copying ought to be unnecessarily inefficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're moving away from using FLE as a prefix, and this code isn't part of an unstable public API surface, you can use Flutter rather than FLE as the prefix for this class, so that it doesn't need to be renamed later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cloudwebrtc Are you saying that CVOpenGLTextureGetTarget is not returning GL_TEXTURE_RECTANGLE? I am a little bit confused. There should be no issues with having to convert to GL_TEXTURE_2D here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chinmaygarde See CVOpenGLTextureGetTarget from the code context
It should be the returned GL_TEXTURE_RECTANGLE, but the problem is with the engine's texture rendering.
The whole picture of the problem is:
The following error will appear in the console at runtime, without displaying any texture frames.
I solved it by patching skia
Then I can see the texture display, but the picture is not correct.
shell/platform/embedder/embedder.h#L214
Looks like this:
plugins/flutter_webrtc/macos/FlutterRTCVideoRenderer.m#L52
I am not sure that the type of dim in SpvDimRect is a bug in skia on macos.