Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Reland enable DisplayList by default#27407

Merged
fluttergithubbot merged 1 commit into
flutter-team-archive:masterfrom
flar:enable-DisplayList-by-default
Jul 15, 2021
Merged

Reland enable DisplayList by default#27407
fluttergithubbot merged 1 commit into
flutter-team-archive:masterfrom
flar:enable-DisplayList-by-default

Conversation

@flar

@flar flar commented Jul 14, 2021

Copy link
Copy Markdown
Contributor

No description provided.

@flar
flar requested review from chinmaygarde and gw280 July 14, 2021 20:42
@google-cla google-cla Bot added the cla: yes label Jul 14, 2021
@gw280

gw280 commented Jul 14, 2021

Copy link
Copy Markdown
Contributor

I'm a little out of the loop, but how confident are we in the test coverage for this?

@chinmaygarde

Copy link
Copy Markdown
Contributor

but how confident are we in the test coverage for this?

I am adequately confident with the coverage in #26928. This was also attempted once before and findings have been incorporated into fixes in the engine.

@flar

flar commented Jul 14, 2021

Copy link
Copy Markdown
Contributor Author

I think it has survived the engine post-submit tests and the framework roll, except that I think there is an outstanding issue with the framework tests were the SVG tests do some golden tests that require exact results and we end up with some +/-1 errors. I was asking @dnfield about adjusting those SVG golden tests to have a build-in epsilon for comparison, but I don't think that conversation was resolved.

@flar
flar requested a review from dnfield July 14, 2021 22:24
@dnfield

dnfield commented Jul 14, 2021

Copy link
Copy Markdown
Contributor

In the past what we've done is just update the goldens in flutter_svg - is there some reason we can't do that now?

@flar

flar commented Jul 14, 2021

Copy link
Copy Markdown
Contributor Author

In the past what we've done is just update the goldens in flutter_svg - is there some reason we can't do that now?

If this were guaranteed to stick all the way up the pipeline then we might do that, but we may eventually perform similar optimizations as SkPicture used to do and so the rendering may fluctuate even if this PR does stick. Expect some volatility here over the next few weeks.

@flar

flar commented Jul 14, 2021

Copy link
Copy Markdown
Contributor Author

@flar

flar commented Jul 15, 2021

Copy link
Copy Markdown
Contributor Author

Waiting to land: flutter/tests#116

@flar flar added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jul 15, 2021
@fluttergithubbot
fluttergithubbot merged commit 7e252b5 into flutter-team-archive:master Jul 15, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 15, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 15, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 15, 2021
flar pushed a commit to flutter/flutter that referenced this pull request Jul 15, 2021
flar added a commit that referenced this pull request Jul 16, 2021
flar added a commit that referenced this pull request Jul 16, 2021
@flar
flar deleted the enable-DisplayList-by-default branch August 4, 2021 18:49
moffatman pushed a commit to moffatman/engine that referenced this pull request Aug 5, 2021
moffatman pushed a commit to moffatman/engine that referenced this pull request Aug 5, 2021
naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request Sep 2, 2021
naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Development

Successfully merging this pull request may close these issues.

5 participants