-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
ImageFilter default tile mode automatic selection breaking change notice #11338
Changes from 5 commits
5eaeb31
f50f152
3e78374
06dce06
bfa53e6
e92ca5f
3128374
c24df86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
--- | ||
title: ImageFilter.blur default tile mode automatic selection. | ||
description: >- | ||
If a tile mode wasn't specified in the constructor, ImageFilter.blur will | ||
select one based on the rendering context. | ||
--- | ||
|
||
## Summary | ||
|
||
The `ui.ImageFilter.blur`'s default tile mode is now automatically selected | ||
by the backend. Previously `TileMode.clamp` was used unless a different tile | ||
mode was specified. Passing a value of `null` to the filter's constructor | ||
specifies automatic selection. | ||
|
||
## Background | ||
|
||
`ImageFilter`'s _tile mode_ specifies what happens to edge pixels for the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add |
||
applied filter. There are four options, `TileMode.clamp` (the previous | ||
default), `Tilemode.repeated`, `TileMode.mirror`, and `TileMode.decal`. | ||
Previously, `ImageFilter` defaulted to `clamp` mode if the | ||
behavior wasn't specified, which sometimes surprised developers | ||
as it didn't always match expectations. | ||
|
||
As of this change, the filter automatically selects the following tile modes | ||
based on context: | ||
|
||
* `decal` with save layers and when applied to individual draws. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. drawShape? |
||
* `mirror` with backdrop filters. | ||
* `clamp` for drawImage. | ||
|
||
## Migration guide | ||
|
||
Only image filters that don't specify an explicit tile mode are | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blur |
||
impacted by this change. In these cases, you can specify an explicit tile | ||
mode to retain the prior behavior. However, we believe the automatic | ||
selection of behavior will be a superior choice in most cases. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
Code before migration: | ||
|
||
```dart | ||
var filter = ui.ImageFilter.blur(sigmaX: 4, sigmaY: 4); | ||
``` | ||
|
||
Code after migration: | ||
|
||
```dart | ||
// Set tile mode to old default. | ||
var filter = ui.ImageFilter.blur(sigmaX: 4, sigmaY: 4, tileMode: TileMode.clamp); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be turned into a lint with a fix to maintain consistent behavior? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want folks to use the automatic selection though and not provide a default. maybe this section isn't clear and we should just not include it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That intent wasn't obvious from my reading, nor was the work that has gone into creating this proposal. I'm happy to work with y'all to build a proposal that better communicates what you are attempting to achieve this, if that would be helpful. Otherwise I can suggest some minor edits? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mentioned this confusion in an earlier comment. I think this part of the section needs to indicate that it is optional and only needed if the developer preferred the previous behavior. I think it might be worthwhile to include it if the comment in the "after" section made it clear that nothing needs to be done, but if you really want the old behavior then this is the way to restore it. There has to be a concise way to express that the change is optional and probably not desired, but here it is if you need it. |
||
``` | ||
|
||
## Timeline | ||
|
||
Landed in version: xxx<br> | ||
In stable release: Not yet | ||
|
||
## References | ||
|
||
{% include docs/main-api.md %} | ||
|
||
API documentation: | ||
|
||
* [`ImageFilter`][] | ||
* [`TileMode`][] | ||
|
||
Relevant issues: | ||
|
||
* [Issue #154935][] | ||
* [Issue #110318][] | ||
* [Issue #157693][] | ||
|
||
Relevant PRs: | ||
|
||
* [Change default TileMode for blur ImageFilter objects to null][] | ||
|
||
|
||
[`ImageFilter`]: https://api.flutter.dev/flutter/dart-ui/ImageFilter-class.html | ||
[`ImageFilter.blur`]: https://api.flutter.dev/flutter/dart-ui/ImageFilter/ImageFilter.blur.html | ||
[`TileMode`]: https://api.flutter.dev/flutter/dart-ui/TileMode.html | ||
[Issue #154935]: https://github.com/flutter/flutter/issues/154935 | ||
[Issue #110318]: https://github.com/flutter/flutter/issues/110318 | ||
[Issue #157693]: https://github.com/flutter/flutter/issues/157693 | ||
[Change default TileMode for blur ImageFilter objects to null]: https://github.com/flutter/engine/pull/55552 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this imply that you need to manually pass a null to get the behavior? How about:
"Previously the default was
clamp
unless a different tile mode was specified. Now, the default isnull
and specifies automatic selection unless a specific tile mode is specified."