Creates a new RenderMode for FlutterView #19143
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| @Nullable private Image nextImage; | ||
| private Image currentImage; | ||
|
|
||
| public FlutterImageView(Context context, ImageReader imageReader) { |
There was a problem hiding this comment.
nit: @NonNull annotation in both arguments
| this.imageReader = imageReader; | ||
| } | ||
|
|
||
| @RequiresApi(api = Build.VERSION_CODES.KITKAT) |
There was a problem hiding this comment.
nit: It looks like this entire class requires KITKAT due to the dependency on ImageReader.
| return; | ||
| } | ||
|
|
||
| currentImage.close(); |
| } | ||
|
|
||
| @RequiresApi(api = Build.VERSION_CODES.P) | ||
| private void drawImageBuffer(Canvas canvas) { |
There was a problem hiding this comment.
Here and below Canvas arguments need @NonNull
| import androidx.annotation.Nullable; | ||
| import androidx.annotation.RequiresApi; | ||
|
|
||
| class FlutterImageView extends View implements RenderSurface { |
|
Direction looks 👍 |
| invalidate(); | ||
| } | ||
|
|
||
| @RequiresApi(api = Build.VERSION_CODES.P) |
There was a problem hiding this comment.
nit: Only a codepath in onDraw requires this API.
| drawImageBuffer(canvas); | ||
| } else if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { | ||
| drawImagePlane(canvas); | ||
| } |
There was a problem hiding this comment.
else: throw unsupported Android version
There was a problem hiding this comment.
Since, I moved the @RequiresApi(api = Build.VERSION_CODES.KITKAT) to the top of the class due to your other comment. I don't think it needs that anymore, right?
bparrishMines
changed the title
bparrishMines
changed the title
| renderSurface = flutterTextureView; | ||
| } else { | ||
| throw new IllegalArgumentException( | ||
| String.format("RenderMode not supported with this constructor: ", renderMode)); |
There was a problem hiding this comment.
Hi, the missing format arguments break Google test (here and below line 268). If we have not yet, can we fix this?
5 participants
Description
Creates an image
RenderModethat paints a Flutter UI provided by anImageReaderonto aCanvas. This is needed for hybrid platform views on Android.Since this adds a new
RenderMode, this slightly changes the behavior in a deprecated constructor that defaulted toRenderMode.textureifRenderMode.surfacewas not selected.Related Issues
flutter/flutter#58289
Tests
I added the following tests:
This PR only creates a new
RenderMode, but doesn't actually set it up to be used. This is part of a larger project to support hybrid views: https://github.com/flutter/flutter/projects/138I added a trivial test to verify that a call to
FlutterImageView.acquireLatestImagecallsImageReader.acquireLatestImage()andinvalidate().Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.