Skip to content

Reland dispose images when done (#67100)#67177

Merged
dnfield merged 4 commits intoflutter:masterfrom
dnfield:reland_image_dispose
Oct 5, 2020
Merged

Reland dispose images when done (#67100)#67177
dnfield merged 4 commits intoflutter:masterfrom
dnfield:reland_image_dispose

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Oct 2, 2020

Fixes #56482 (again)

Changes since last time:

This reverts commit 473358d.

@dnfield
Reland dispose images when done (flutter#67100)
80c3d7b 
Changes since last time:

- Test for CanvasKit image rendering
  (flutter#67176)
- Fix CanvasKit dispose impl
  (flutter/engine#21555)
- Update internal google3 customer with a problematic ImageStream
  Listener impl (cl/335091311)

This reverts commit 473358d.
@dnfield dnfield requested a review from goderbauer October 2, 2020 23:13
@flutter-dashboard flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Oct 2, 2020
@dnfield
Copy link
Contributor Author

dnfield commented Oct 2, 2020

This will be in draft mode until the relevant CanvasKit change rolls into the framework - I'll rebase this and then it'll be ready for review.

@goderbauer
Copy link
Member

goderbauer commented Oct 2, 2020

Is this a 1:1 revert of 473358d or are there any additional changes in here?

@goderbauer
Copy link
Member

goderbauer commented Oct 2, 2020

Never mind, I didn't read the PR description carefully.

goderbauer
goderbauer approved these changes Oct 2, 2020
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@dnfield dnfield marked this pull request as ready for review October 5, 2020 15:56
@dnfield
Copy link
Contributor Author

dnfield commented Oct 5, 2020

Engine roll merged in, af ew updates for NNBD in the test, and the internal CL is submitting right now.

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Oct 5, 2020

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux web_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dnfield
Copy link
Contributor Author

dnfield commented Oct 5, 2020

Hm. There appears to be a new failure internally I have to check out. Moving this back to draft.

@dnfield dnfield marked this pull request as draft October 5, 2020 17:40
@flutter-dashboard
Copy link

flutter-dashboard bot commented Oct 5, 2020

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@dnfield
Copy link
Contributor Author

dnfield commented Oct 5, 2020

This now needs cl/335459002

@dnfield dnfield marked this pull request as ready for review October 5, 2020 18:36
@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Oct 5, 2020

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dnfield
Copy link
Contributor Author

dnfield commented Oct 5, 2020

(google testing hasn't caught up yet to the CL I just landed, the test will pass now)

@dnfield dnfield merged commit 2cdec25 into flutter:master Oct 5, 2020
@dnfield dnfield deleted the reland_image_dispose branch October 5, 2020 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goderbauer goderbauer goderbauer approved these changes

Assignees

No one assigned

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

removing Image widgets does not reduce Graphics memory allocation

4 participants

@dnfield @goderbauer @fluttergithubbot @googlebot