Skip to content
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.

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

ImageShader should have a dispose method, and it should be called by the framework. #82832

Closed
dnfield opened this issue May 18, 2021 · 6 comments · Fixed by flutter/engine#35640
Assignees
Labels
a: images Loading, displaying, rendering images c: performance Relates to speed or footprint issues (see "perf:" labels) engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list perf: memory Performance issues related to memory

Comments

@dnfield
Copy link
Contributor

dnfield commented May 18, 2021

Image Shaders, particularly those that end up creating an SkImageShader, can hold substantial amounts of memory.

As we wire up methods to make sure we dispose of Layer objects and related memory, we should make sure we dispose of these as well.

see also #81514, #57746

@dnfield dnfield added framework flutter/packages/flutter repository. See also f: labels. engine flutter/engine repository. See also e: labels. c: performance Relates to speed or footprint issues (see "perf:" labels) a: images Loading, displaying, rendering images perf: memory Performance issues related to memory P1 High-priority issues at the top of the work list labels May 18, 2021
@dnfield dnfield self-assigned this May 18, 2021
@dnfield
Copy link
Contributor Author

dnfield commented May 18, 2021

/cc @flar @LongCatIsLooong who might have good opinions on this

@dnfield
Copy link
Contributor Author

dnfield commented May 18, 2021

While we're at it, we should fix _ImageFilter so that it doesn't call two native methods to initialize itself.

@dnfield
Copy link
Contributor Author

dnfield commented Aug 10, 2022

I think what we really want is for ImageShader to have a dispose method. ImageFilter was the wrong thing here. I will re-purpose this bug for that, since AFAICT we don't really tie much int he way of native resources to ImageFilters.

@dnfield dnfield changed the title ImageFilter should have a dispose method, and it should be called by the framework. ImageShader should have a dispose method, and it should be called by the framework. Aug 23, 2022
@dnfield
Copy link
Contributor Author

dnfield commented Aug 23, 2022

I was just wrong about filters. They're really pretty light weight and they can't really dispose anything - this only applies to image shaders.

@dnfield
Copy link
Contributor Author

dnfield commented Aug 23, 2022

And the framework does not use image shader anywhere AFAICT.

@dnfield dnfield removed the framework flutter/packages/flutter repository. See also f: labels. label Aug 23, 2022
@github-actions
Copy link

github-actions bot commented Sep 6, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: images Loading, displaying, rendering images c: performance Relates to speed or footprint issues (see "perf:" labels) engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list perf: memory Performance issues related to memory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants