Make sure root surface transformations survive resetting the matrix directly in Flow. #13405

chinmaygarde · 2019-10-29T00:05:06Z

This used to only be handled correctly for non-root layer backing stores. This
was mostly a side effect of the fact that we used recording canvases instead of
rendering directly into the backing store. We now use recording canvases
consistently.

Fixes b/143464703

chinmaygarde · 2019-10-29T00:05:15Z

xref b/143464703

…irectly in Flow. This used to only be handled correctly for non-root layer backing stores. This was mostly a side effect of the fact that we used recording canvases instead of rendering directly into the backing store. We now use recording canvases consistently. Fixes b/143464703

chinmaygarde · 2019-10-29T23:24:19Z

Fixes flutter/flutter#43732

amirh

LGTM modulo the canvas->clear (unless we have a good answer there)

amirh · 2019-10-29T23:49:50Z

shell/platform/embedder/embedder_external_view_embedder.cc

-  // the rasterizer before the submit call layer to access the surface surface
-  // canvas.
+  // If there is no root render target, create one now.
+  // TODO(chinmaygarde): This can now be moved to be later in the submit call.


nit: If it's something you think we should do let's have a tracking bug and change the language from "can" to "should"

Done and filed flutter/flutter#43778

amirh · 2019-10-29T23:55:13Z

shell/platform/embedder/embedder_external_view_embedder.cc

+  // while making sure to take into account any surface transformations.
  if (auto root_canvas = root_render_target_->GetRenderSurface()->getCanvas()) {
+    root_canvas->setMatrix(pending_surface_transformation_);
+    root_canvas->clear(SK_ColorTRANSPARENT);


Isn't this redundant for the root canvas? do you know if there's a performance cost for doing it twice?

No. The embedder may have unnecessary stuff on the render target that we need to get rid of.

amirh · 2019-10-29T23:56:29Z

shell/platform/embedder/fixtures/main.dart

 }
+
+@pragma('vm:entry-point')
+void verify_b143464703() {


Maybe include a link to a GitHub issue if you're referring a bug, this number is pretty mysterious for non googlers

Done. There was no GitHub issue when I began work on this.

chinmaygarde

Addressed all comments.

chinmaygarde · 2019-10-30T00:00:18Z

shell/platform/embedder/embedder_external_view_embedder.cc

-  // the rasterizer before the submit call layer to access the surface surface
-  // canvas.
+  // If there is no root render target, create one now.
+  // TODO(chinmaygarde): This can now be moved to be later in the submit call.


Done and filed flutter/flutter#43778

chinmaygarde · 2019-10-30T00:00:45Z

shell/platform/embedder/embedder_external_view_embedder.cc

+  // while making sure to take into account any surface transformations.
  if (auto root_canvas = root_render_target_->GetRenderSurface()->getCanvas()) {
+    root_canvas->setMatrix(pending_surface_transformation_);
+    root_canvas->clear(SK_ColorTRANSPARENT);


No. The embedder may have unnecessary stuff on the render target that we need to get rid of.

chinmaygarde · 2019-10-30T00:01:35Z

shell/platform/embedder/fixtures/main.dart

 }
+
+@pragma('vm:entry-point')
+void verify_b143464703() {


Done. There was no GitHub issue when I began work on this.

…matrix directly in Flow. (flutter/engine#13405)

git@github.com:flutter/engine.git/compare/36303c61b63f...5051bef git log 36303c6..5051bef --no-merges --oneline 2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia ebdae1144978..7da048b5e8f1 (1 commits) (flutter/engine#13438) 2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from DfLN8... to p7XYM... (flutter/engine#13437) 2019-10-30 bkonyi@google.com Roll src/third_party/dart eb4ab61349..d3a5b82355 (4 commits) 2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from goLxs... to nhX22... (flutter/engine#13435) 2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia c3a51a5e47ba..ebdae1144978 (1 commits) (flutter/engine#13434) 2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 1ae54bc22fc2..c3a51a5e47ba (1 commits) (flutter/engine#13433) 2019-10-30 bkonyi@google.com Roll src/third_party/dart ae5a86d790..eb4ab61349 (7 commits) 2019-10-30 dnfield@google.com Set the install name at link time for darwin dylibs (flutter/engine#13428) 2019-10-30 mehmetf@users.noreply.github.com Add isRunningInRobolectricTest back (flutter/engine#13424) 2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 76d22d1ee8cb..1ae54bc22fc2 (26 commits) (flutter/engine#13430) 2019-10-30 bkonyi@google.com Roll src/third_party/dart ebd059030b..ae5a86d790 (3 commits) 2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/clang/linux-amd64 from Vghc_... to WxGHg... (flutter/engine#13420) 2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from mc3jR... to DfLN8... (flutter/engine#13413) 2019-10-30 bkonyi@google.com Roll src/third_party/dart 780af0fcfc..ebd059030b (17 commits) 2019-10-30 chinmaygarde@google.com Make sure root surface transformations survive resetting the matrix directly in Flow. (flutter/engine#13405) 2019-10-30 iska.kaushik@gmail.com [fuchsia] [packaging] Create a script to upload debug symbols to CIPD (flutter/engine#13422) 2019-10-29 47866232+chunhtai@users.noreply.github.com Revert "fix fml_unittes is not run during presubmit (#13395)" (flutter/engine#13425) 2019-10-29 30870216+gaaclarke@users.noreply.github.com Made it so we clean up gl resources when view controllers get deleted. (flutter/engine#13396) 2019-10-29 30870216+gaaclarke@users.noreply.github.com Added back in empty lifecycle events so we don't break people that used to call `super`. (flutter/engine#13421) 2019-10-29 47866232+chunhtai@users.noreply.github.com fix fml_unittes is not run during presubmit (flutter/engine#13395) 2019-10-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia f22dda960136..76d22d1ee8cb (2 commits) (flutter/engine#13411) 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 cbracken@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

…matrix directly in Flow. (flutter#13405)" This reverts commit e3aff2c. Fixes b/144093523 Reopens b/143464703 Reopens flutter/flutter#43732

git@github.com:flutter/engine.git/compare/36303c61b63f...5051bef git log 36303c6..5051bef --no-merges --oneline 2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia ebdae1144978..7da048b5e8f1 (1 commits) (flutter/engine#13438) 2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from DfLN8... to p7XYM... (flutter/engine#13437) 2019-10-30 bkonyi@google.com Roll src/third_party/dart eb4ab61349..d3a5b82355 (4 commits) 2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from goLxs... to nhX22... (flutter/engine#13435) 2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia c3a51a5e47ba..ebdae1144978 (1 commits) (flutter/engine#13434) 2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 1ae54bc22fc2..c3a51a5e47ba (1 commits) (flutter/engine#13433) 2019-10-30 bkonyi@google.com Roll src/third_party/dart ae5a86d790..eb4ab61349 (7 commits) 2019-10-30 dnfield@google.com Set the install name at link time for darwin dylibs (flutter/engine#13428) 2019-10-30 mehmetf@users.noreply.github.com Add isRunningInRobolectricTest back (flutter/engine#13424) 2019-10-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 76d22d1ee8cb..1ae54bc22fc2 (26 commits) (flutter/engine#13430) 2019-10-30 bkonyi@google.com Roll src/third_party/dart ebd059030b..ae5a86d790 (3 commits) 2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/clang/linux-amd64 from Vghc_... to WxGHg... (flutter/engine#13420) 2019-10-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from mc3jR... to DfLN8... (flutter/engine#13413) 2019-10-30 bkonyi@google.com Roll src/third_party/dart 780af0fcfc..ebd059030b (17 commits) 2019-10-30 chinmaygarde@google.com Make sure root surface transformations survive resetting the matrix directly in Flow. (flutter/engine#13405) 2019-10-30 iska.kaushik@gmail.com [fuchsia] [packaging] Create a script to upload debug symbols to CIPD (flutter/engine#13422) 2019-10-29 47866232+chunhtai@users.noreply.github.com Revert "fix fml_unittes is not run during presubmit (flutter#13395)" (flutter/engine#13425) 2019-10-29 30870216+gaaclarke@users.noreply.github.com Made it so we clean up gl resources when view controllers get deleted. (flutter/engine#13396) 2019-10-29 30870216+gaaclarke@users.noreply.github.com Added back in empty lifecycle events so we don't break people that used to call `super`. (flutter/engine#13421) 2019-10-29 47866232+chunhtai@users.noreply.github.com fix fml_unittes is not run during presubmit (flutter/engine#13395) 2019-10-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia f22dda960136..76d22d1ee8cb (2 commits) (flutter/engine#13411) 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 cbracken@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

chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label Oct 29, 2019

googlebot added cla: yes labels Oct 29, 2019

chinmaygarde force-pushed the verifyb143464703 branch from a8884a0 to 00eed48 Compare October 29, 2019 23:20

chinmaygarde changed the title ~~Ensure that container layers preserve CTM for rendering into subsequent backing stores.~~ Make sure root surface transformations survive resetting the matrix directly in Flow. Oct 29, 2019

chinmaygarde requested review from amirh, cbracken, jason-simmons and kf6gpe October 29, 2019 23:22

chinmaygarde removed the Work in progress (WIP) Not ready (yet) for review! label Oct 29, 2019

chinmaygarde mentioned this pull request Oct 29, 2019

Parts of browser window animate vertically instead of horizontally flutter/flutter#43732

Closed

Licenses

e78c3ae

amirh approved these changes Oct 29, 2019

View reviewed changes

chinmaygarde mentioned this pull request Oct 29, 2019

Delay root surface acquisition in the embedder till we are ready to submit the frame. flutter/flutter#43778

Closed

PR

d916609

chinmaygarde commented Oct 30, 2019

View reviewed changes

chinmaygarde merged commit e3aff2c into flutter:master Oct 30, 2019

chinmaygarde deleted the verifyb143464703 branch October 30, 2019 00:45

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 30, 2019

e3aff2c Make sure root surface transformations survive resetting the …

bbdf177

…matrix directly in Flow. (flutter/engine#13405)

engine-flutter-autoroll mentioned this pull request Oct 30, 2019

Roll engine 36303c61b63f..5051bef17bdc (21 commits) flutter/flutter#43828

Merged

Make sure root surface transformations survive resetting the matrix directly in Flow. #13405

Make sure root surface transformations survive resetting the matrix directly in Flow. #13405

Uh oh!

Conversation

chinmaygarde commented Oct 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chinmaygarde commented Oct 29, 2019

Uh oh!

chinmaygarde commented Oct 29, 2019

Uh oh!

amirh 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!

Choose a reason for hiding this comment

Uh oh!

chinmaygarde 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chinmaygarde commented Oct 29, 2019 •

edited

Loading