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

Reland again "Remove layer integral offset snapping #17112" #18160

Merged
liyuqian merged 4 commits into from
May 8, 2020
Merged

Reland again "Remove layer integral offset snapping #17112" #18160

liyuqian merged 4 commits into from
May 8, 2020

Conversation

@liyuqian
Copy link
Contributor

@liyuqian liyuqian commented May 5, 2020 •
edited
Loading

This reverts commit a7a25d3 and relands our reland #17915.

Additionally, we fixed the cull rect logic in OpacityLayer::Preroll which is the root cause of flutter/flutter#56298. We've always had that root problem before but it did not trigger performance issues because we were using the OpacityLayer's paint_bounds, instead of its child's paint_bounds for preparing the layer raster cache. A correct handling of the cull rect should allow us to cull at any level.

It also turns out that our ios32 (iPhone4s) performacne can regress a lot
without snapping. My theory is that although the picture has a
fractional top left corner, many drawing operations inside the picture
have integral coordinations. In older hardwares, keeping those
coordinates integral seems to be performance critical.

To avoid flutter/flutter#41654, the snapping
will still be disabled if the matrix has non-scale-translation
transformations.

@liyuqian liyuqian requested a review from flar May 5, 2020 21:43
@auto-assign auto-assign bot requested a review from chinmaygarde May 5, 2020 21:44
flar
flar reviewed May 5, 2020
View reviewed changes
@liyuqian
Restore snapping on scale/translate only pictures
c6bc494 
It turns out that our ios32 (iPhone4s) performacne can regress a lot
without snapping. My theory is that although the picture has a
fractional top left corner, many drawing operations inside the picture
have integral coordinations. In older hardwares, keeping those
coordinates integral seems to be performance critical.

To avoid flutter/flutter#41654, the snapping
will still be disabled if the matrix has non-scale-translation
transformations.
@liyuqian
Copy link
Contributor Author

liyuqian commented May 7, 2020

@flar : any more comment on this after the discussion with Skia?

flar
flar approved these changes May 7, 2020
View reviewed changes
Copy link
Contributor

@flar flar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@liyuqian liyuqian merged commit 28d9985 into flutter:master May 8, 2020
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 8, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2020
@engine-flutter-autoroll
Roll engine 3953c3c..e7ee47d (22 commits)
8500164 
git@github.com:flutter/engine.git/compare/3953c3ccd15a...e7ee47d

git log 3953c3c..e7ee47d --first-parent --oneline
2020-05-08 ferhat@gmail.com [web] Implement matrix parameter for linear gradient (flutter/engine#18208)
2020-05-08 liyuqian@google.com Reland again "Remove layer integral offset snapping flutter#17112" (flutter/engine#18160)
2020-05-08 skia-flutter-autoroll@skia.org Roll src/third_party/skia c66cd987f7c0..0dc207f836da (5 commits) (flutter/engine#18212)
2020-05-08 stuartmorgan@google.com Refactor GLFW embedding to support headless mode (flutter/engine#18205)
2020-05-07 a-siva@users.noreply.github.com Manual Roll of Dart 39e0e75fcf...ce62ad2e8b (flutter/engine#18211)
2020-05-07 38722979+Kurun-pan@users.noreply.github.com Support EventChannel C++ plugin API for Linux/Windows (flutter/engine#17015)
2020-05-07 robert.ancell@canonical.com Handle leak of message handle when no engine present (flutter/engine#18157)
2020-05-07 xster@google.com add docs to platformviewios (and some drive-by changes) (flutter/engine#17593)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 0066adefa97d..c66cd987f7c0 (4 commits) (flutter/engine#18206)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia edea19858ccc..0066adefa97d (3 commits) (flutter/engine#18203)
2020-05-07 jason-simmons@users.noreply.github.com Remove the global engine entry timestamp (flutter/engine#18182)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart e86e4d61834a..ce62ad2e8b40 (13 commits) (flutter/engine#18201)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2871ab0727bf..edea19858ccc (3 commits) (flutter/engine#18198)
2020-05-07 dtiselice@google.com Fixed ChildSceneLayer elevation issue on Fuchsia. (flutter/engine#18144)
2020-05-07 iska.kaushik@gmail.com [profiling] CPU Profiling support for iOS (flutter/engine#18087)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia e3d1de7c5281..2871ab0727bf (1 commits) (flutter/engine#18197)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart 733153eb517c..e86e4d61834a (6 commits) (flutter/engine#18195)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 3b2db26c59d6..e3d1de7c5281 (4 commits) (flutter/engine#18192)
2020-05-07 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from jMJqf... to 1MVsE... (flutter/engine#18194)
2020-05-07 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from RpHTv... to MhpFP... (flutter/engine#18191)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 88d04cb51acf..3b2db26c59d6 (1 commits) (flutter/engine#18190)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart 4da5b40fb6dc..733153eb517c (23 commits) (flutter/engine#18188)

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 garyq@google.com 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

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 8, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2020
@engine-flutter-autoroll
Roll engine 3953c3c..403931f (24 commits)
b187369 
git@github.com:flutter/engine.git/compare/3953c3ccd15a...403931f

git log 3953c3c..403931f --first-parent --oneline
2020-05-08 robert.ancell@canonical.com Add FlValue (flutter/engine#18185)
2020-05-08 jason-simmons@users.noreply.github.com [SkParagraph] Copy text height behavior to the Skia paragraph style (flutter/engine#18178)
2020-05-08 ferhat@gmail.com [web] Implement matrix parameter for linear gradient (flutter/engine#18208)
2020-05-08 liyuqian@google.com Reland again "Remove layer integral offset snapping flutter#17112" (flutter/engine#18160)
2020-05-08 skia-flutter-autoroll@skia.org Roll src/third_party/skia c66cd987f7c0..0dc207f836da (5 commits) (flutter/engine#18212)
2020-05-08 stuartmorgan@google.com Refactor GLFW embedding to support headless mode (flutter/engine#18205)
2020-05-07 a-siva@users.noreply.github.com Manual Roll of Dart 39e0e75fcf...ce62ad2e8b (flutter/engine#18211)
2020-05-07 38722979+Kurun-pan@users.noreply.github.com Support EventChannel C++ plugin API for Linux/Windows (flutter/engine#17015)
2020-05-07 robert.ancell@canonical.com Handle leak of message handle when no engine present (flutter/engine#18157)
2020-05-07 xster@google.com add docs to platformviewios (and some drive-by changes) (flutter/engine#17593)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 0066adefa97d..c66cd987f7c0 (4 commits) (flutter/engine#18206)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia edea19858ccc..0066adefa97d (3 commits) (flutter/engine#18203)
2020-05-07 jason-simmons@users.noreply.github.com Remove the global engine entry timestamp (flutter/engine#18182)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart e86e4d61834a..ce62ad2e8b40 (13 commits) (flutter/engine#18201)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2871ab0727bf..edea19858ccc (3 commits) (flutter/engine#18198)
2020-05-07 dtiselice@google.com Fixed ChildSceneLayer elevation issue on Fuchsia. (flutter/engine#18144)
2020-05-07 iska.kaushik@gmail.com [profiling] CPU Profiling support for iOS (flutter/engine#18087)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia e3d1de7c5281..2871ab0727bf (1 commits) (flutter/engine#18197)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart 733153eb517c..e86e4d61834a (6 commits) (flutter/engine#18195)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 3b2db26c59d6..e3d1de7c5281 (4 commits) (flutter/engine#18192)
2020-05-07 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from jMJqf... to 1MVsE... (flutter/engine#18194)
2020-05-07 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from RpHTv... to MhpFP... (flutter/engine#18191)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 88d04cb51acf..3b2db26c59d6 (1 commits) (flutter/engine#18190)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart 4da5b40fb6dc..733153eb517c (23 commits) (flutter/engine#18188)

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 garyq@google.com 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

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 8, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2020
@engine-flutter-autoroll
Roll engine 3953c3c..9193d8f (25 commits)
e51a888 
git@github.com:flutter/engine.git/compare/3953c3ccd15a...9193d8f

git log 3953c3c..9193d8f --first-parent --oneline
2020-05-08 skia-flutter-autoroll@skia.org Roll src/third_party/skia 0dc207f836da..a14084ba1b41 (3 commits) (flutter/engine#18219)
2020-05-08 robert.ancell@canonical.com Add FlValue (flutter/engine#18185)
2020-05-08 jason-simmons@users.noreply.github.com [SkParagraph] Copy text height behavior to the Skia paragraph style (flutter/engine#18178)
2020-05-08 ferhat@gmail.com [web] Implement matrix parameter for linear gradient (flutter/engine#18208)
2020-05-08 liyuqian@google.com Reland again "Remove layer integral offset snapping flutter#17112" (flutter/engine#18160)
2020-05-08 skia-flutter-autoroll@skia.org Roll src/third_party/skia c66cd987f7c0..0dc207f836da (5 commits) (flutter/engine#18212)
2020-05-08 stuartmorgan@google.com Refactor GLFW embedding to support headless mode (flutter/engine#18205)
2020-05-07 a-siva@users.noreply.github.com Manual Roll of Dart 39e0e75fcf...ce62ad2e8b (flutter/engine#18211)
2020-05-07 38722979+Kurun-pan@users.noreply.github.com Support EventChannel C++ plugin API for Linux/Windows (flutter/engine#17015)
2020-05-07 robert.ancell@canonical.com Handle leak of message handle when no engine present (flutter/engine#18157)
2020-05-07 xster@google.com add docs to platformviewios (and some drive-by changes) (flutter/engine#17593)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 0066adefa97d..c66cd987f7c0 (4 commits) (flutter/engine#18206)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia edea19858ccc..0066adefa97d (3 commits) (flutter/engine#18203)
2020-05-07 jason-simmons@users.noreply.github.com Remove the global engine entry timestamp (flutter/engine#18182)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart e86e4d61834a..ce62ad2e8b40 (13 commits) (flutter/engine#18201)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2871ab0727bf..edea19858ccc (3 commits) (flutter/engine#18198)
2020-05-07 dtiselice@google.com Fixed ChildSceneLayer elevation issue on Fuchsia. (flutter/engine#18144)
2020-05-07 iska.kaushik@gmail.com [profiling] CPU Profiling support for iOS (flutter/engine#18087)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia e3d1de7c5281..2871ab0727bf (1 commits) (flutter/engine#18197)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart 733153eb517c..e86e4d61834a (6 commits) (flutter/engine#18195)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 3b2db26c59d6..e3d1de7c5281 (4 commits) (flutter/engine#18192)
2020-05-07 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from jMJqf... to 1MVsE... (flutter/engine#18194)
2020-05-07 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from RpHTv... to MhpFP... (flutter/engine#18191)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/skia 88d04cb51acf..3b2db26c59d6 (1 commits) (flutter/engine#18190)
2020-05-07 skia-flutter-autoroll@skia.org Roll src/third_party/dart 4da5b40fb6dc..733153eb517c (23 commits) (flutter/engine#18188)

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 garyq@google.com 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

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
@liyuqian liyuqian deleted the reland2 branch May 8, 2020 21:50
@liyuqian liyuqian mentioned this pull request Jul 18, 2020
Merged
liyuqian added a commit to liyuqian/flutter that referenced this pull request Jul 23, 2020
@liyuqian
Add a test for 3D transform with needsCompositing
64e578a 
The test was first written in
flutter#41654 (comment)

This will ensure that flutter#41654
won't have regressions.

This test would fail without flutter/engine#18160
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@chinmaygarde chinmaygarde Awaiting requested review from chinmaygarde

1 more reviewer

@flar flar flar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@liyuqian @flar @googlebot