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

ImageFilter default tile mode automatic selection breaking change notice #11338

Merged
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
Copy link
Contributor

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 is null and specifies automatic selection unless a specific tile mode is specified."


## Background

`ImageFilter`'s _tile mode_ specifies what happens to edge pixels for the
Copy link
Contributor

Choose a reason for hiding this comment

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

Add .blur to mentions of ImageFilter?

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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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);
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Loading