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

Conversation

@long1eu
Copy link
Contributor

@long1eu long1eu commented Jan 13, 2019

@dnfield
Copy link
Contributor

dnfield commented Jan 14, 2019

Thank you for doing this! I've been thinking about this as well, it would help the process along for implementing filter effects for flutter_svg.

I think the approach overall makes sense, just a few small things on it.

@long1eu
Copy link
Contributor Author

long1eu commented Jan 14, 2019

I'll make the required changes. My idea of how we can improve this is to pass a list of ColorFilters so we can combine them. What do you think?

@dnfield
Copy link
Contributor

dnfield commented Jan 14, 2019

Great!

@jason-simmons would you mind taking a look at this and seeing if I missed anything?

@dnfield
Copy link
Contributor

dnfield commented Jan 14, 2019

Oh, and combining them is something we can probably handle at the framework level right? It's just vector math stuff?

@long1eu
Copy link
Contributor Author

long1eu commented Jan 14, 2019

Yes. And also we could remove or deprecate invertColors or at least use the ColorFilter.matrix to provide this functionality.

@long1eu
Copy link
Contributor Author

long1eu commented Jan 14, 2019

Copy link
Member

@jason-simmons jason-simmons 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
Copy link
Contributor

dnfield commented Jan 14, 2019

I would probably lean towards creating a package out of that, or creating something in the framework for it rather than increasing the engine API surface.

@long1eu
Copy link
Contributor Author

long1eu commented Jan 14, 2019

@dnfield of course

@dnfield dnfield merged commit 2820157 into flutter:master Jan 14, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 14, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 14, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 15, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants