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

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Aug 23, 2022

Image shaders can contain images and should be disposed.

Fixes flutter/flutter#82832

Once this lands we should update vector_graphics to dispose the image shaders it creates.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Aug 23, 2022
@dnfield dnfield marked this pull request as ready for review August 23, 2022 16:37
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

Won't web need a debugDisposed + test as well?

@dnfield
Copy link
Contributor Author

dnfield commented Aug 23, 2022

Yeah I missed that on web. Will fix.

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 23, 2022
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

@auto-submit auto-submit bot merged commit 15cf53a into flutter:main Aug 23, 2022
expect(shader.debugDisposed, true);
}

image.dispose();
Copy link
Member

Choose a reason for hiding this comment

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

A test that tries to attach a disposed shader to a Paint and then use the Paint could be useful. It should not segfault or abort, and should rather throw a Dart exception.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 23, 2022
zanderso pushed a commit to flutter/flutter that referenced this pull request Aug 23, 2022
…110121)

* 94f97c337 Roll Skia from b91dbe274717 to 94602de608e0 (4 revisions) (flutter/engine#35634)

* 6ec08c50e Roll Skia from 94602de608e0 to 8c32b0148770 (7 revisions) (flutter/engine#35636)

* 9195b7a2f Roll Dart SDK from 03506589f2d0 to c064b9bacc4d (1 revision) (flutter/engine#35637)

* f7ec70782 Roll Skia from 8c32b0148770 to a9a05d0bf0c5 (3 revisions) (flutter/engine#35639)

* 15cf53acc Image shader dispose (flutter/engine#35640)

* ee3cd0d8a Roll Skia from a9a05d0bf0c5 to 147232a3cebc (2 revisions) (flutter/engine#35641)

* 5f53ee3b2 [Impeller] Use linear sampling for reads in the texture atlas (flutter/engine#35618)

* 4ff7f7b9a Roll Fuchsia Mac SDK from JocShsJvXYeSVjgvW... to Es8SXkqDOYZ5dgYTq... (flutter/engine#35642)

* 565cc861e Roll Skia from 147232a3cebc to cc63d5dd13b6 (1 revision) (flutter/engine#35643)

* b4af69f02 Roll Dart SDK from c064b9bacc4d to 3042d6196824 (1 revision) (flutter/engine#35644)

* 8cc02dc73 Roll Skia from cc63d5dd13b6 to a4744aa054cf (2 revisions) (flutter/engine#35645)
@dnfield dnfield deleted the image_shader_dispose branch August 24, 2022 00:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants