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

Position overlay layer views in PlatformViewsController.onDisplayOverlaySurface #19295

Merged

Conversation

jason-simmons
Copy link
Member

No description provided.

@fluttergithubbot
Copy link
Contributor

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.

},
1000);
public void acquireLatestImageViewFrame() {
flutterImageView.acquireLatestImage();
Copy link

Choose a reason for hiding this comment

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

minor nit: flutterImageView is nullable

int key = overlayLayerViews.keyAt(i);
FlutterImageView overlayView = overlayLayerViews.valueAt(i);
if (currentFrameUsedOverlayLayerIds.contains(key)) {
overlayView.acquireLatestImage();
Copy link

@blasten blasten Jun 25, 2020

Choose a reason for hiding this comment

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

The Android team suggested to acquire the image off the UI thread. This can be done later, just fyi.

Copy link

Choose a reason for hiding this comment

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

Same for the one above, but I guess that could go in a single PR.

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

LGTM + nit

@jason-simmons jason-simmons force-pushed the hybrid_display_overlay_surface branch from c70fce4 to 0d9339b Compare June 25, 2020 01:37
@blasten blasten merged commit 2b74e8a into flutter:master Jun 25, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants