Make impeller::Paint's default isAntiAlias false, matching DlPaint, SkPaint, and DlBuilder by b-luk · Pull Request #193971 · flutter/flutter

b-luk · 2026-10-07T21:44:14Z

The existing code has a mismatch with the default isAntiAlias value of various initial Paint properties used throughout the code.

DisplayListBuilder uses a default DlPaint, which has a default isAntiAlias=false.
Skia's DlSkPaintDispatchHelper use a default SkPaint, which has a default isAntiAlias=false.
Impeller's DlOpReceivers use a default impeller::Paint, which has a default isAntiAlias=true.

The mismatch of impeller::Paint and DisplayListBuilder's DlPaint causes issues. DisplayListBuilder's setAntiAlias no-op exits if setting the same value as its internal DlPaint state:

flutter/engine/src/flutter/display_list/dl_builder.h

Lines 277 to 281 in d454b1b

    
           void setAntiAlias(bool aa) override { 
        
             if (current_.isAntiAlias() != aa) { 
        
               onSetAntiAlias(aa); 
        
             } 
        
           }

That works correctly if all its downstream DlOpReceivers have matching initial values for their Paint state. But this is not the case for impeller::Paint. If we get a Paint with setAntiAlias(false), DisplayListBuilder won't pass this setting to its downstream DlOpReceivers, instead incorrectly assuming that the downstream receivers already have a default false value.

All of these defaults need to agree. We can either change them all to true or all to false. This changes them all to false. impeller::Paint is the only one that previously was default true, so it is the only one that changes. Note that this default is different from the default isAntiAlias=true used for dart:ui's Paint.

Adds a new kDefaultAntiAlias = false const to DlPaint, which it uses when initializing itself.
Impeller's impeller::Paint sets its default with DlPaint::kDefaultAntiAlias instead of using a true literal. The default is now false instead of true, and is set to explicitly match DlPaint's default.
Updates tests which relied on DlPaint having AA=true to explicitly use AA=true (just SDF-related tests). Adds new tests to ensure the default paint in DlSkPaintDispatchHelper and the default impeller::Paint match DlPaint.

Fixes #193388

This is an alternate approach to #193939, which fixes the issue by aligning all the classes to use AA=true instead of AA=false. This AA=false better preserves existing behavior, because it does not change the value for engine-internal uses of DlPaint (e.g. ShaderMaskLayer, RasterCache).

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

…lias of false

gemini-code-assist

Code Review

This pull request updates the default anti-alias setting in DlPaint and Impeller's Paint to false (using DlPaint::kDefaultAntiAlias), aligning the default paint attributes with DlPaint::kDefault. It also adds documentation to DlOpReceiver regarding attribute initialization, updates existing tests to explicitly enable anti-aliasing where required, and introduces new unit tests to verify that initial paint states match DlPaint defaults. There are no review comments, and I have no feedback to provide.

flar · 2026-10-07T22:48:15Z

This is simpler than the alternative PR, hopefully we won't see too many golden/scuba diffs.

b-luk · 2026-10-07T22:50:51Z

This is simpler than the alternative PR, hopefully we won't see too many golden/scuba diffs.

I'm expecting a ton of them. All our Impeller golden tests rely on the default DlPaint having AA=true. This changes the default to false. So all goldens on SDF backends will not use UberSDF anymore. We could change every test that declares a DlPaint to explicitly set it to AA=true. That's many hundreds of lines of changes though. I'm seeing if there's an easier more central change we could make.

b-luk · 2026-10-07T23:10:32Z

This is simpler than the alternative PR, hopefully we won't see too many golden/scuba diffs.

I'm expecting a ton of them. All our Impeller golden tests rely on the default DlPaint having AA=true. This changes the default to false. So all goldens on SDF backends will not use UberSDF anymore. We could change every test that declares a DlPaint to explicitly set it to AA=true. That's many hundreds of lines of changes though. I'm seeing if there's an easier more central change we could make.

A bunch of changes for the SDF backend goldens, as expected: https://flutter-gold.skia.org/search?issue=193971&crs=github&patchsets=1&corpus=flutter

I added commit c7c93ff, which is an attempt to make our golden tests default to DlPaint with AA=true without explicitly setting it in every test. It has a new DefaultAntiAliasedDlPaint class in impeller/display_list/testing/default_anti_aliased_dl_paint.h. Our tests can import this and use using DlPaint = DefaultAntiAliasedDlPaint; to shadow their DlPaints with this one. It's kind of hacky though. Let me know what you think.

Will wait and see if this eliminates all the golden changes once the tests finish.

flar · 2026-10-08T00:00:45Z

Another solution is to have the paint used for impeller dispatch default to AA=false. Impeller paint will default to true and the one we use for the dispatcher will just override that to false.

flar · 2026-10-08T00:03:51Z

dl_dispatcher.h line 67 - (move to dl_dispatcher.cc and) set the paint_.aa = false.

b-luk · 2026-10-08T00:17:10Z

Another solution is to have the paint used for impeller dispatch default to AA=false. Impeller paint will default to true and the one we use for the dispatcher will just override that to false.

We'd still need something like my DefaultAntiAliasedDlPaint change to make all the golden tests use AA=true. Without it, the golden tests use DlPaint with default AA=false, and the dispatcher would override the Impeller paint to false. So the Impeller goldens will render with AA=false instead of true, and the SDF paths won't be tested.

b-luk · 2026-10-08T00:29:04Z

With the DefaultAntiAliasedDlPaint change, google testing is still in progress, but the skia-gold tests should be completed and they have no changes: https://flutter-gold.skia.org/search?issue=193971&crs=github&patchsets=2.

b-luk · 2026-10-08T16:48:18Z

Google testing finished. It has 3 failed tests, and none of them are related to this change.

flar

I like the simplicity of the fix here, but I think we may be asking for trouble in how the unit tests are being changed to match the new default.

flar · 2026-10-09T18:07:03Z

+/// impeller::testing::DefaultAntiAliasedDlPaint;`) so that the SDF playground
+/// backends exercise the SDF path without every test opting in explicitly.
+/// Only the default changes; setAntiAlias(false) works as usual.
+class DefaultAntiAliasedDlPaint : public flutter::DlPaint {


I do like how this class reduces the number of changes needed to the unit tests, but I also feel that it is hiding a defaulting issue as it does so, especially with the way that many of the files use using DlPaint = ... as the way to deploy it. That hides what is going on and makes understanding the tests obscure. One concern is that test code gets copied a lot to make new tests and if someone creates the new test in a different file it might get a different definition of what "DlPaint" means without anything obvious to tip them off.

Some thoughts:

Just bite the bullet and have all tests that need AA set it manually directly in the test.

change to something more transparent in the test files such as using DlPaintAA = or using DlAAPaint = so that at least the tests that are using it have evidence that they are using the AA variant in their declaration.

I think you're right that this makes understanding the tests a little obscure.

Changing to using DlPaintAA still requires changing every DlPaint instantiation to DlPaintAA, so it doesn't save us much compared to just explicitly calling paint.setAntiAlias(true) at every instantiation. And it still requires the DefaultAntiAliasedDlPaint class, which we don't need if we are just explicit with setAntiAlias.

I changed all the tests to explicitly use setAntiAlias. it's a lot of changed lines, but it's more clear about what is actually happening in the test. The downside to watch out for is that people may make tests in the future and might forget to explicitly set AA=true, so the test on the SDF backend will not actually test the SDF path.

If we think this approach is the way forward, maybe I can make changing the tests a separate PR that I'll get merged first, so this PR will just be about changing the default value. What do you think?

flar · 2026-10-09T18:09:07Z

  };
  // clang-format on
-  auto paint = flutter::DlPaint(flutter::DlColor::kDarkGrey());
+  auto paint = DlPaint(flutter::DlColor::kDarkGrey());


This is kind of an example of the obscurity. Someone might scratch their heads in the future wondering why this test now uses DlPaint directly whereas another test is using "flutter::DlPaint" and think it was just a change in policy regarding full naming, not realizing that the base name DlPaint is not the same as the qualified name "flutter::DlPaint"...

flar · 2026-10-09T18:11:19Z

+      : flutter::DlPaint(color) {
+    setAntiAlias(true);
+  }
+  // Implicit so that chained setters, which return flutter::DlPaint&, can


More obscurity and more workarounds to just having the unit tests simply initialize the AA flag as they desire.

flar

I like the explicit nature. Some of the paint objects probably don't need AA if they aren't going to be doing geometry fill/strokes, but they don't hurt anything.

At this point, does this seem advantageous compared to the other PR which switched all defaults (except Skia) to be AA=true?

flar · 2026-10-09T22:04:54Z

  DisplayListBuilder builder;

-  builder.DrawPaint(DlPaint().setColor(DlColor::kWhite()));
+  builder.DrawPaint(DlPaint().setAntiAlias(true).setColor(DlColor::kWhite()));


cases like DrawPaint (and DrawColor) don't use the AA flag. Just FYI...

In fact, these might be more easily expressed as calls to DrawColor anyway.

As stated in my other comment, it seems simpler to me to change all DlPaint instantiations rather than go through them and determine whether we need an explicit AA=true for each case.

Changing DrawPaints to DrawColors is outside the scope of this PR.

b-luk · 2026-10-09T22:16:36Z

I like the explicit nature. Some of the paint objects probably don't need AA if they aren't going to be doing geometry fill/strokes, but they don't hurt anything.

Right, it doesn't hurt anything to set AA=true for calls which don't need it. But it seems like a lot of effort to look through every call case and determine whether we need it to set AA or not. And it increases the likelihood that someone in the future with copy/paste a test which doesn't set AA, but forget that their new test might need AA. It'll be a silent false positive golden for the SDF backend. So I think it's clearer to just always set AA=true for every paint, which matches with the default AA=true made by every call from dart:ui.

At this point, does this seem advantageous compared to the other PR which switched all defaults (except Skia) to be AA=true?

I don't know which I prefer. Maybe I prefer this default-false PR a little bit. This PR is a lot more boilerplate-verbose, and requires all new tests in the future to be slightly more verbose. But it preserves existing behavior better and more safely. The default-true PR would need to exhaustively track down all internal not-from-dart instantiations of DlPaint and explicitly set AA=false for them in order to maintain existing behavior (for Skia, at least).

b-luk · 2026-10-09T22:22:10Z

I don't know which I prefer. Maybe I prefer this default-false PR a little bit. This PR is a lot more boilerplate-verbose, and requires all new tests in the future to be slightly more verbose. But it preserves existing behavior better and more safely. The default-true PR would need to exhaustively track down all internal not-from-dart instantiations of DlPaint and explicitly set AA=false for them in order to maintain existing behavior (for Skia, at least).

If we are okay with behavior changes, then I think I like the original default-true approach with no special casing for internal DlPaint instantiations. It means some small behavior changes for some cases on Skia, which we see in the google testing scuba goldens. The changes mean rendering is slightly different, but not incorrect and arguably more correct. If we're okay with that, I think that would be the best approach out of all the ones we considered.

b-luk · 2026-10-12T21:51:50Z

Closed in favor of the default true approach: #193939

Make DlPaint, SkPaint, and impeller::Paint agree on a default isAntiA…

846504c

…lias of false

flutter-dashboard Bot added the CICD Run CI/CD label Oct 7, 2026

github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Oct 7, 2026

gemini-code-assist Bot reviewed Oct 7, 2026

View reviewed changes

default_anti_aliased_dl_paint

c7c93ff

github-actions Bot added the a: text input Entering text in a text field or keyboard related problems label Oct 7, 2026

b-luk mentioned this pull request Oct 8, 2026

Make DisplayLists' use of DlPaint, SkPaint, and impeller::Paint agree on the default isAntiAlias #193939

Merged

10 of 11 tasks

b-luk requested a review from flar October 8, 2026 16:48

Merge branch 'master' into paint-antialias-default-false

609712d

flar reviewed Oct 9, 2026

View reviewed changes

b-luk added 2 commits October 9, 2026 14:33

Remove DefaultAntiAliasedDlPaint

0aeaf4e

Explicitly enable anti-aliasing in Impeller rendering tests

aa0b98b

b-luk requested a review from flar October 9, 2026 22:04

flar reviewed Oct 9, 2026

View reviewed changes

b-luk requested a review from flar October 9, 2026 22:18

b-luk closed this Oct 12, 2026

	void setAntiAlias(bool aa) override {
	if (current_.isAntiAlias() != aa) {
	onSetAntiAlias(aa);
	}
	}

Conversation

b-luk commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

flar commented Oct 7, 2026

Uh oh!

b-luk commented Oct 7, 2026

Uh oh!

b-luk commented Oct 7, 2026

Uh oh!

flar commented Oct 8, 2026

Uh oh!

flar commented Oct 8, 2026

Uh oh!

b-luk commented Oct 8, 2026

Uh oh!

b-luk commented Oct 8, 2026

Uh oh!

b-luk commented Oct 8, 2026

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b-luk commented Oct 9, 2026

Uh oh!

b-luk commented Oct 9, 2026

Uh oh!

b-luk commented Oct 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

b-luk commented Oct 7, 2026 •

edited

Loading