Skip to content

Reland dispose images when done (#67100) #67177

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.

Sign up for GitHub

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

Jump to bottom
Merged
dnfield merged 4 commits into from
Oct 5, 2020
Merged

Reland dispose images when done (#67100) #67177

dnfield merged 4 commits into from
Oct 5, 2020

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