Update (flipping the default from false -> true) and deprecate Paint.enableDithering. (flutter/engine#44705)

Update: Blocked on https://github.com/flutter/engine/pull/44912 landing,
and merging into google3.

---

Partial work towards https://github.com/flutter/flutter/issues/112498.

_**tl;dr**: In Impeller's backend we intend to _always_ dither
gradients, and never allow any changes long-term (i.e., write your own
shader if you want different behavior) with the assumption that dithered
gradients look better most of the time, and don't typically hurt
elsewhere._

Note that, at the time of this writing, I couldn't find a single case of
this being set explicitly to `true` inside Google, and there are between
[100 and 200 publicly accessible on
GitHub](https://github.com/search?q=%22Paint.enableDithering%22+language%3ADart&type=code&l=Dart),
of which virtually all are setting it explicitly to `true`.

There are some (valid) concerns this would cause a lot of golden-file
image diffs, so I'm going to seek explicit approval from the Google
testing team as well as someone from the framework team before landing
this commit.
This commit is contained in:
Matan Lurey 2023-09-01 10:19:59 -07:00 committed by GitHub
parent baa288096b
commit a330c4ff97
10 changed files with 21 additions and 16 deletions

View File

@ -45,7 +45,9 @@ Future<void> main(List<String> args) async {
]));
final ProcessResult runResult = Process.runSync(dart, <String>[regularDill]);
checkProcessResult(runResult);
String paintString = '"Paint.toString":"Paint(Color(0xffffffff))"';
// TODO(matanlurey): "dither: true" is now present by default, until it is
// remove entirely. See https://github.com/flutter/flutter/issues/112498.
String paintString = '"Paint.toString":"Paint(Color(0xffffffff); dither: true)"';
if (buildDir.contains('release')) {
paintString = '"Paint.toString":"Instance of \'Paint\'"';
}

View File

@ -1487,22 +1487,19 @@ class Paint {
_data.setInt32(_kDitherOffset, value ? 1 : 0, _kFakeHostEndian);
}
/// Whether to dither the output when drawing images.
/// Whether to dither the output when drawing some elements such as gradients.
///
/// If false, the default value, dithering will be enabled when the input
/// color depth is higher than the output color depth. For example,
/// drawing an RGB8 image onto an RGB565 canvas.
///
/// This value also controls dithering of [shader]s, which can make
/// gradients appear smoother.
///
/// Whether or not dithering affects the output is implementation defined.
/// Some implementations may choose to ignore this completely, if they're
/// unable to control dithering.
///
/// To ensure that dithering is consistently enabled for your entire
/// application, set this to true before invoking any drawing related code.
static bool enableDithering = false;
/// It is not expected that this flag will be used in the future; please leave
/// feedback in <https://github.com/flutter/flutter/issues/112498> if there is
/// a use case for this flag to remain long term.
@Deprecated(
'Dithering is now enabled by default on some elements (such as gradients) '
'and further support for dithering is expected to be handled by custom '
'shaders, so this flag is being removed: '
'https://github.com/flutter/flutter/issues/112498.'
'This feature was deprecated after 3.14.0-0.1.pre.'
)
static bool enableDithering = true;
@override
String toString() {

Binary file not shown.

Before

Width:  |  Height:  |  Size: 35 KiB

After

Width:  |  Height:  |  Size: 62 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 34 KiB

After

Width:  |  Height:  |  Size: 56 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 32 KiB

After

Width:  |  Height:  |  Size: 70 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 33 KiB

After

Width:  |  Height:  |  Size: 71 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 6.8 KiB

After

Width:  |  Height:  |  Size: 14 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 35 KiB

After

Width:  |  Height:  |  Size: 62 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 32 KiB

After

Width:  |  Height:  |  Size: 70 KiB

View File

@ -203,6 +203,10 @@ void main() {
}
test('Simple gradient', () async {
// TODO(matanl): While deprecated, we still don't want to accidentally
// change the behavior of the old API,
// https://github.com/flutter/flutter/issues/112498.
// ignore: deprecated_member_use
Paint.enableDithering = false;
final Image image = await toImage((Canvas canvas) {
final Paint paint = Paint()..shader = makeGradient();
@ -217,6 +221,8 @@ void main() {
}, skip: !Platform.isLinux); // https://github.com/flutter/flutter/issues/53784
test('Simple dithered gradient', () async {
// TODO(matanl): Reword this test once we remove the deprecated API.
// ignore: deprecated_member_use
Paint.enableDithering = true;
final Image image = await toImage((Canvas canvas) {
final Paint paint = Paint()..shader = makeGradient();