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

Conversation

jason-simmons
Copy link
Member

Including images in the size of a Picture may overstate the picture's
memory usage. The picture may not be the only object holding a
reference to the image (possibly because the image is owned by the
framework's image cache).

If Dart thinks that the size of the picture includes the image's size,
then Dart will more aggressively garbage collect the picture. But
deleting the picture will not delete the image, and the extra GCs may
cause jank.

Fixes flutter/flutter#63876

Including images in the size of a Picture may overstate the picture's
memory usage.  The picture may not be the only object holding a
reference to the image (possibly because the image is owned by the
framework's image cache).

If Dart thinks that the size of the picture includes the image's size,
then Dart will more aggressively garbage collect the picture.  But
deleting the picture will not delete the image, and the extra GCs may
cause jank.

Fixes flutter/flutter#63876
@@ -327,7 +327,6 @@ void Canvas::drawImage(const CanvasImage* image,
ToDart("Canvas.drawImage called with non-genuine Image."));
return;
}
external_allocation_size_ += image->GetAllocationSize();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this means I'll have to re-work the way the hint_freed patch was plumbing through that an image was freed up.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM

@jason-simmons jason-simmons added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 21, 2020
@fluttergithubbot fluttergithubbot merged commit e09af86 into flutter:master Aug 21, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 22, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 22, 2020
christopherfujino added a commit that referenced this pull request Aug 27, 2020
* update dart revision
* Fix EGL_BAD_SURFACE when app is paused (#20735)
* The `ForwardingGestureRecognizers` to have back reference to the `PlatformViewsController` so it can access `FlutterViewController` when its available  (#20708)
* Remove image sizes from Picture::GetAllocationSize (#20673)
* update licenses golden

Co-authored-by: Emmanuel Garcia <[email protected]>
Co-authored-by: Chris Yang <[email protected]>
Co-authored-by: Jason Simmons <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image animation performance regression on Android
4 participants