Move the mutators stack handling to preroll #9651

cyanglaz · 2019-07-02T21:49:21Z

Move the mutator stack handling to tree preroll. And the platform view is going to decide if it should composite or not, and set a flag.
In paint (Composite), based on the flag, we composite with the cached embedded view params.

iskakaushik

some minor changes. looks good overall.

iskakaushik · 2019-07-02T21:53:05Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

+    return;
+  }
+  current_composition_params_[view_id] = EmbeddedViewParams(*params.get());
+  views_need_recomposite.insert(view_id);


nit: better name would be view_needs_recomposition_

done, kept is as views_need_recomposition_ since it is a set.

Maybe views_to_recomposite_?

iskakaushik · 2019-07-02T21:53:27Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h

  std::vector<int64_t> active_composition_order_;

+  // Only compoiste platform views in this set.
+  std::unordered_set<int64_t> views_need_recomposite;


private variable, end with underscore.

iskakaushik · 2019-07-02T21:54:32Z

flow/layers/platform_view_layer.cc

    return;
  }
-  context->view_embedder->PrerollCompositeEmbeddedView(view_id_);
+  std::unique_ptr<EmbeddedViewParams> params =


this doesn't need to be a unique_ptr. Just make this a stack variable.

This is a refactoring just landed in a previous PR. #9640

cyanglaz

Updated with @iskakaushik's comment

cyanglaz · 2019-07-02T21:58:31Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h

  std::vector<int64_t> active_composition_order_;

+  // Only compoiste platform views in this set.
+  std::unordered_set<int64_t> views_need_recomposite;


cyanglaz · 2019-07-02T22:00:20Z

flow/layers/platform_view_layer.cc

    return;
  }
-  context->view_embedder->PrerollCompositeEmbeddedView(view_id_);
+  std::unique_ptr<EmbeddedViewParams> params =


This is a refactoring just landed in a previous PR. #9640

cyanglaz · 2019-07-02T22:00:50Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

+    return;
+  }
+  current_composition_params_[view_id] = EmbeddedViewParams(*params.get());
+  views_need_recomposite.insert(view_id);


done, kept is as views_need_recomposition_ since it is a set.

iskakaushik · 2019-07-02T22:03:34Z

LGTM

amirh

LGTM modulo nits

amirh · 2019-07-02T22:28:27Z

flow/layers/clip_path_layer.cc

 void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
  SkRect previous_cull_rect = context->cull_rect;
  SkRect clip_path_bounds = clip_path_.getBounds();
+  context->mutators_stack.pushClipPath(clip_path_);


nit: move these inside the condition (if the clip is empty we're not painting the subtree), it's going to have the same effect only that the intention is more explicit.

Same for the other clip layers.

amirh · 2019-07-02T22:34:10Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

+    return;
+  }
+  current_composition_params_[view_id] = EmbeddedViewParams(*params.get());
+  views_need_recomposite.insert(view_id);


Maybe views_to_recomposite_?

cyanglaz

Updated with @amirh's comment

cyanglaz · 2019-07-02T22:43:53Z

flow/layers/clip_path_layer.cc

 void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
  SkRect previous_cull_rect = context->cull_rect;
  SkRect clip_path_bounds = clip_path_.getBounds();
+  context->mutators_stack.pushClipPath(clip_path_);


cyanglaz · 2019-07-02T22:44:56Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

+    return;
+  }
+  current_composition_params_[view_id] = EmbeddedViewParams(*params.get());
+  views_need_recomposite.insert(view_id);


)

flutter/engine@7d3e722...3c51a7b git log 7d3e722..3c51a7b --no-merges --oneline 3c51a7b Roll src/third_party/skia 11eb847a2080..857c9f955edb (2 commits) (flutter/engine#9676) 7f828dd Raster now returns an enum rather than boolean (flutter/engine#9661) 11b6afe Roll src/third_party/dart 67ab3be10d...b5aeaa6796 (flutter/engine#9675) 3c4dbe2 Revert " Roll src/third_party/dart 67ab3be10d...43891316ca (#9670)" (flutter/engine#9673) 5e596f2 Roll src/third_party/dart 67ab3be10d...43891316ca (flutter/engine#9670) 46a2239 Roll src/third_party/skia 5b52c52141ac..11eb847a2080 (6 commits) (flutter/engine#9671) b84f89b Allow embedders to add callbacks for responses to platform messages from the framework. (flutter/engine#9655) 8dac2e9 Begin separating macOS engine from view controller (flutter/engine#9654) d3616c7 Roll src/third_party/skia 0e0113dcbd9a..5b52c52141ac (8 commits) (flutter/engine#9665) cea2c36 Mutators Stack refactoring (flutter/engine#9663) 6a8782f Roll src/third_party/skia 93eeff578b08..0e0113dcbd9a (9 commits) (flutter/engine#9662) 791143f ExternalViewEmbedder can CancelFrame after pre-roll (flutter/engine#9660) d637f29 External view embedder can tell if embedded views have mutated (flutter/engine#9653) ceee3d7 Roll src/third_party/skia 38ae3f42fec1..93eeff578b08 (1 commits) (flutter/engine#9659) d757290 Roll src/third_party/skia febc162c7898..38ae3f42fec1 (3 commits) (flutter/engine#9658) 58133ab Roll src/third_party/skia 3de5c6388142..febc162c7898 (2 commits) (flutter/engine#9657) 29342dd Roll src/third_party/skia 1e2cb444e0c1..3de5c6388142 (8 commits) (flutter/engine#9656) b547356 Pipeline allows continuations that can produce to front (flutter/engine#9652) 8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651) 511b9f2 Roll src/third_party/skia 215ff3325230..1e2cb444e0c1 (12 commits) (flutter/engine#9650) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (jsimmons@google.com), and stop the roller if necessary.

flutter/engine@7d3e722...3c51a7b git log 7d3e722..3c51a7b --no-merges --oneline 3c51a7b Roll src/third_party/skia 11eb847a2080..857c9f955edb (2 commits) (flutter/engine#9676) 7f828dd Raster now returns an enum rather than boolean (flutter/engine#9661) 11b6afe Roll src/third_party/dart 67ab3be10d...b5aeaa6796 (flutter/engine#9675) 3c4dbe2 Revert &flutter#34; Roll src/third_party/dart 67ab3be10d...43891316ca (flutter#9670)&flutter#34; (flutter/engine#9673) 5e596f2 Roll src/third_party/dart 67ab3be10d...43891316ca (flutter/engine#9670) 46a2239 Roll src/third_party/skia 5b52c52141ac..11eb847a2080 (6 commits) (flutter/engine#9671) b84f89b Allow embedders to add callbacks for responses to platform messages from the framework. (flutter/engine#9655) 8dac2e9 Begin separating macOS engine from view controller (flutter/engine#9654) d3616c7 Roll src/third_party/skia 0e0113dcbd9a..5b52c52141ac (8 commits) (flutter/engine#9665) cea2c36 Mutators Stack refactoring (flutter/engine#9663) 6a8782f Roll src/third_party/skia 93eeff578b08..0e0113dcbd9a (9 commits) (flutter/engine#9662) 791143f ExternalViewEmbedder can CancelFrame after pre-roll (flutter/engine#9660) d637f29 External view embedder can tell if embedded views have mutated (flutter/engine#9653) ceee3d7 Roll src/third_party/skia 38ae3f42fec1..93eeff578b08 (1 commits) (flutter/engine#9659) d757290 Roll src/third_party/skia febc162c7898..38ae3f42fec1 (3 commits) (flutter/engine#9658) 58133ab Roll src/third_party/skia 3de5c6388142..febc162c7898 (2 commits) (flutter/engine#9657) 29342dd Roll src/third_party/skia 1e2cb444e0c1..3de5c6388142 (8 commits) (flutter/engine#9656) b547356 Pipeline allows continuations that can produce to front (flutter/engine#9652) 8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651) 511b9f2 Roll src/third_party/skia 215ff3325230..1e2cb444e0c1 (12 commits) (flutter/engine#9650) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (jsimmons@google.com), and stop the roller if necessary.

Chris Yang added 2 commits July 2, 2019 14:27

refactoring to move the mutator stack handling to preroll

cb61803

merge

d1d557c

googlebot added the cla: yes label Jul 2, 2019

cyanglaz requested review from amirh and iskakaushik July 2, 2019 21:49

cyanglaz changed the title ~~Move the mutator stack handling to preroll~~ Move the mutators stack handling to preroll Jul 2, 2019

iskakaushik reviewed Jul 2, 2019

View reviewed changes

cyanglaz commented Jul 2, 2019

View reviewed changes

iskakaushik approved these changes Jul 2, 2019

View reviewed changes

amirh approved these changes Jul 2, 2019

View reviewed changes

cyanglaz commented Jul 2, 2019

View reviewed changes

more review fixes

cd048b3

iskakaushik merged commit 8306ee6 into flutter:master Jul 2, 2019

cyanglaz deleted the mutator_stack_to_preroll branch July 3, 2019 00:25

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

4951cd8

)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..8306ee68c751 (2 commits) flutter/flutter#35498

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

3197e8d

)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..b5473560db44 (3 commits) flutter/flutter#35501

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

f7a80ab

)

This was referenced Jul 3, 2019

Roll engine 7d3e722349e7..29342dd30111 (4 commits) flutter/flutter#35504

Closed

Roll engine 7d3e722349e7..58133abced04 (5 commits) flutter/flutter#35511

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

80b07e5

)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..d75729006bba (6 commits) flutter/flutter#35519

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

c23daed

)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..ceee3d733921 (7 commits) flutter/flutter#35527

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

5d10957

)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..d637f291a96f (8 commits) flutter/flutter#35533

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

d6c4e42

)

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

482f2a5

)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..791143f17281 (9 commits) flutter/flutter#35538

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

ef7a84a

)

This was referenced Jul 3, 2019

Roll engine 7d3e722349e7..6a8782fe0162 (10 commits) flutter/flutter#35540

Closed

Roll engine 7d3e722349e7..cea2c3617f61 (11 commits) flutter/flutter#35546

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

c905a3a

)

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

6f30c31

)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..d3616c7a31de (12 commits) flutter/flutter#35552

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

87ffcfd

)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..8dac2e979697 (13 commits) flutter/flutter#35553

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

dfea5e0

)

This was referenced Jul 3, 2019

Roll engine 7d3e722349e7..b84f89b44b75 (14 commits) flutter/flutter#35557

Closed

Roll engine 7d3e722349e7..46a2239b4240 (15 commits) flutter/flutter#35562

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

b9dd42f

)

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

dfa5094

)

This was referenced Jul 4, 2019

Roll engine 7d3e722349e7..3c4dbe223cfe (17 commits) flutter/flutter#35565

Closed

Roll engine 7d3e722349e7..11b6afe00ca5 (18 commits) flutter/flutter#35569

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

1ff5f7c

)

engine-flutter-autoroll mentioned this pull request Jul 4, 2019

Roll engine 7d3e722349e7..7f828dd3e3d5 (19 commits) flutter/flutter#35571

Closed

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

2bf474f

)

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

8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651

71bf507

)

engine-flutter-autoroll mentioned this pull request Jul 4, 2019

Roll engine 7d3e722349e7..3c51a7bfff77 (20 commits) flutter/flutter#35572

Merged

Move the mutators stack handling to preroll #9651

Move the mutators stack handling to preroll #9651

Uh oh!

Conversation

cyanglaz commented Jul 2, 2019

Uh oh!

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

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!

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

iskakaushik commented Jul 2, 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!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants