No-op set Paint.enableDithering (it's deprecated)

@ds84182

EDIT as of 2023-08-09. Here is our proposal: #112498 (comment)

This flag was added in #44134, however from a quick investigation we can find no usages of this in google3. If this isn't being used, we should deprecate and remove it.

@ds84182 do you know what happened to the usage of dithering? Did this solve the problem it was designed to solve or did this get lost somehow. Or am I looking wrong?

See also: #109740

Unfortunately we found that dithering did not work as intended with overlapping translucent gradients.

Additionally we also ran into issues with screenshot tests with dithering enabled. It ended up not being worthwhile to enable because all of our goldens would need to be updated. It's also worth noting that it was originally a per-Paint opt-in, which would've been easier for us to integrate as only our gradients need dithering.

I've separately investigated smoothing out our gradients using blue noise in a shader, which turns out to be a lot more effective than Skia's dithering, even when blending translucent gradients together.

Kill it with fire!

@jonahwilliams

I use the enableDithering flag in my apps, because otherwise color banding occurs, for example between two shades of gray.

The gradient used in the screenshot:

Container(
  height: 200,
  decoration: BoxDecoration(
    gradient: !disableGradient
        ? LinearGradient(
            colors: [
              Color(0xff262626),
              Color(0xff0f0f0f),
            ],
            begin: Alignment.bottomLeft,
            end: Alignment.topRight,
            stops: [0.0, 0.4],
          )
        : null,
    borderRadius: BorderRadius.only(
      bottomLeft: Radius.circular(20),
    ),
  ),
);

To illustrate the problem here are two screenshots:

On an iPhone X with enableDithering = false strong color banding is visible. On the device it looks stronger than on the screenshot.

On an iPhone X with enableDithering = true no color banding is visible (on the screenshot you can see it easily though).

I would like this flag to remain, but maybe my use case is too rare?
@jonahwilliams

I would like this flag to remain, but maybe my use case is too rare?

I've personally run into this problem when working with a designer on another product. It isn't acceptable for Flutter developers to have to go back to their designers and tell them to rework the look because Flutter can't cleanly render their design. Popularity shouldn't dictate API. Let's keep it.

Alternatively, we could remove the flag and automatically do dithering. That's probably better since many users won't be savvy enough to know to look for it. The problem with that is the aforementioned golden image tests. We'd have to make sure it was deterministic so that that it doesn't fiddle with those.

I'm not sure why Skia's implementation is non deterministic.

Update: Engine team has consensus we'd like to remove the global/static Paint.enableDithering.

I've assigned this issue to myself to propose/recommend a path forward (with a realistic migration timeline for users). Likely outcomes (but not exhaustive) include (a) dithering by default, with a local API to opt-out, (b) not dithering by default, with a local API to opt-in, or (c) either (a) or (b) with a context-provided opt-in/opt-out (i.e. "dither this widget subtree").

I've investigated a bit, and here is what I think we should do:

Proposal

Add an instance property as bool ui.Paint.dither (engine)

Document that not every paint operation uses dithering, and there is no guarantee that every backend implements
dithering identically. It might be nice also to document a motivating example (why to use dithering), and alternatives (i.e.
custom shaders). If true, it supersedes the (static) variable Paint.enableDithering.
Add similar properties as bool painting.{Linear|Radial|Sweep}Gradient (framework)

If set, it plumbs through to dart:ui (instead of being a global) during the paint operations.
Deprecate ui.Paint.enableDithering (engine)

Updating the documentation, and announcing a short (?) deprecation period
Remove, as a breaking change, ui.Paint.enableDithering (engine).

Alternatives

These can sometimes be mixed-and-matched with the proposal above, and are not mutually exclusive.

Add dithering to specific UI objects, like Gradient.linear, instead of a Paint.dither flag.

If we don't expect dithering to apply to all/most paint operations, it feels strange to tie to the Paint object.
Provide an "easier" way to set dithering across a widget tree, i.e. as an InheritedWidget

This would be an easier migration path off Paint.enableDithering, but would have performance ramifications.
Enable dithering by default, and instead have the ability to disable dithering.

This would be a larger breaking change, and it's not clear that it would be well received.
Use some sort of heuristic, at least in Impeller, on providing dithering automatically

In this case, it becomes bool? dither, and if null uses heuristics. Interestingly enough, Paint.enableDithering
claims we do this (I'm guessing because Skia might?) by stating:

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.

I'm open to this, but would need some help figuring out a good approach (if done poorly it's another big breaking change).

Feel free to suggest different alternatives and/or problems :)

Forward-fix a test that will break with an engine roll. (#133619)

Update (flipping the default from false -> true) and deprecate Paint.…

After a short baking period I'll finalize cleaning this up.

Relaxed a test on `Paint.toString()` to ignore `dither: ...`. (#136302)

Remove support for `Paint.enableDithering=false` in `dart:ui`. (#46745)

[Impeller] Unconditionally dither in gradient shader fragments. (#46746)

Surgically remove `.*dither.*` from the Engine (#46750)

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

Surgically remove `.*dither.*` from the Engine (flutter-team-archive/…

Navigation Menu

Repository navigation

No-op `set Paint.enableDithering` (it's deprecated) #112498

Proposal

Alternatives

21 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

No-op set Paint.enableDithering (it's deprecated) #112498

Description

Activity

jonahwilliams commented on Sep 27, 2022

ds84182 commented on Sep 27, 2022

dnfield commented on Sep 27, 2022

b3nni97 commented on Jan 5, 2023

gaaclarke commented on Jan 6, 2023

gaaclarke commented on Jan 6, 2023

matanlurey commented on Aug 3, 2023

matanlurey commented on Aug 9, 2023

Proposal

Alternatives

21 remaining items

matanlurey commented on Sep 1, 2023

github-actions commented on Nov 14, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

No-op `set Paint.enableDithering` (it's deprecated) #112498