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

Conversation

@flar
Copy link
Contributor

@flar flar commented Jul 14, 2021

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

gw280 commented Jul 14, 2021

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

@chinmaygarde
Copy link
Member

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

flar commented Jul 14, 2021

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

dnfield commented Jul 14, 2021

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

flar commented Jul 14, 2021

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

flar commented Jul 14, 2021

@flar
Copy link
Contributor Author

flar commented Jul 15, 2021

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: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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants