-
Notifications
You must be signed in to change notification settings - Fork 29.2k
Manually Roll engine to f001ea2...7ef88f8 (34 commits) to resolve goldens #48598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
A Skia roll contained some AA algorithm changes that has imperceptible differences on a few goldens. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM
Hey @GaryQian just confirming, this affected which golden files? I'm monitoring a potential triage bug. |
The breaking change policy doesn't mention goldens anymore. Although, updating like this does break existing tests. @Hixie should golden changes just be listed in the changelog then? |
Skia makes routine AA algorithm changes all the time. These changes are usually imperceptible (single digit RGB value diffs) and don't change the final look of the widgets. Counting these as breaking would make it very difficult to roll skia consistently. |
Oh for sure! I just meant handling it as far as just putting it in the change log. While imperceptible, if anyone has golden file test out there that will be broken by this, they won't know why. If their golden file tests break for no apparent reason, folks may be less inclined to use them. :) |
@Piinks upstream changes (Skia, Dart) are exempt from the policy for now. That said, we shouldn't ever have to roll the engine manually. Why couldn't the goldens be updated on the Skia Gold dashboard and then the bot do the roll? |
@Hixie right now, changes to goldens are compiled into a tryjob for approval during pre-submit checks for the associated pull request. The change cannot land without approving the new images, so changes originating from the engine need to be manually rolled in. |
We could run the gold tryjob on engine PRs too, that way any engine-originating golden changes are pre-approved/resolved? But the changes should not reflect in the framework until the engine commit is rolled in. Would probably have to maintain two separate sets of goldens (engine tot and framework tot) and merge them on rolls. Conflicts may occur but I suspect they will be really really rare. |
The changes could totally be done in the autoroller commit, it's just that the autoroller has a tendency to instantly close and start a new roll when the goldens first fail before we have a change to triage. Was just a bit saner to pause the roller and do one manually. |
That's what I was hoping we could do. I had thought engine changes were already running framework tests. I think it could work without conflict, or the need for separate sets of golden files. |
Ah! I think I may just need to update the engine's cirrus file. It doesn't have goldctl, so it doesn't run tryjobs. |
flutter/engine@f001ea2...7ef88f8
git log f001ea2..7ef88f8 --first-parent --oneline
2020-01-10 [email protected] Fixes leak of url in FlutterObservatoryPublisher (flutter/engine#14822)
2020-01-10 [email protected] [fuchsia] Add diagnostics directory to the remote dirs and ensure entry exists (flutter/engine#15258)
2020-01-09 [email protected] Roll src/third_party/skia 04e77813f8f4..67d0f3fd725c (41 commits) (flutter/engine#15377)
2020-01-09 [email protected] modify test_runner.dart for windows to fix test build errors (flutter/engine#15326)
2020-01-09 [email protected] Roll fuchsia/sdk/core/linux-amd64 from _gFH2... to 75iyW... (flutter/engine#15373)
2020-01-09 [email protected] [web] Tests for browser history implementation (flutter/engine#15324)
2020-01-09 [email protected] Roll src/third_party/dart c0ca187f2699..395daaa3ecfc (84 commits) (flutter/engine#15374)
2020-01-09 [email protected] Revert "Roll src/third_party/skia 04e77813f8f4..92ca3baba6a5 (15 commits) (#15330)" (flutter/engine#15376)
2020-01-09 [email protected] Roll src/third_party/skia 92ca3baba6a5..a92320d4e6a5 (18 commits) (flutter/engine#15375)
2020-01-09 [email protected] Revert "Add support for on/off switch labels when built on iOS 13. (#12467)" (flutter/engine#15370)
2020-01-09 [email protected] Make BackgroundMode public. (#45747) (flutter/engine#14673)
2020-01-09 [email protected] Add shell api to set default for windows data (flutter/engine#14002)
2020-01-09 [email protected] Roll src/third_party/skia 04e77813f8f4..92ca3baba6a5 (15 commits) (flutter/engine#15330)
2020-01-09 [email protected] Add support for on/off switch labels when built on iOS 13. (flutter/engine#12467)
2020-01-09 [email protected] Add missing super.onNewIntent() call (flutter/engine#15328)
2020-01-09 [email protected] Revert "Add a deprecation javadoc note to the old FlutterActivity (#15156)" (flutter/engine#15331)
2020-01-09 [email protected] Fix devicepixel ratio reset problem when replaying clip stack (flutter/engine#15327)
2020-01-09 [email protected] Enable BackdropFilter for most browsers (flutter/engine#15321)
2020-01-09 [email protected] add warning message for clipboard api (flutter/engine#15304)
2020-01-08 [email protected] [SkParagraph] Cache the font collection created for use by the Skia text shaper library (flutter/engine#15317)
2020-01-08 [email protected] Reland "[web] Calculate align offset for each paragraph line (LineMetrics.left)" (#14537) (flutter/engine#15151)
2020-01-08 [email protected] Roll src/third_party/skia f811fc331a14..04e77813f8f4 (7 commits) (flutter/engine#15312)
2020-01-08 [email protected] Roll fuchsia/sdk/core/linux-amd64 from oNKzr... to _gFH2... (flutter/engine#15311)
2020-01-08 [email protected] Move html window.devicePixelRatio access to EngineWindow and fix WebOS issue (flutter/engine#15315)
2020-01-08 [email protected] Fix Path.from. (flutter/engine#15268)
2020-01-08 [email protected] Reland "Bump simulator version in IosUnitTests & scenario app in preparation for luci xcode 11 migration (#15316)", Reverted in #15313
2020-01-08 [email protected] Throw an exception in Image.toByteData rather than hang forever. (flutter/engine#15152)
2020-01-08 [email protected] Implement Path.from in the CanvasKit backend (flutter/engine#14468)
2020-01-08 [email protected] Revert "Bump simulator version in IosUnitTests & scenario app in preparation for luci xcode 11 migration (#15154)" (flutter/engine#15313)
2020-01-08 [email protected] Made it so you can specify the old gen heap size. (flutter/engine#15259)
2020-01-08 [email protected] Roll fuchsia/sdk/core/mac-amd64 from gL-XG... to rTJJV... (flutter/engine#15308)
2020-01-08 [email protected] Add a deprecation javadoc note to the old FlutterActivity (flutter/engine#15156)
2020-01-08 [email protected] Bump simulator version in IosUnitTests & scenario app in preparation for luci xcode 11 migration (flutter/engine#15154)
2020-01-08 [email protected] Roll src/third_party/skia 91e0d7526944..f811fc331a14 (32 commits) (flutter/engine#15306)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.
To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug