From e97d42602ca09f8e165a3fa255bfe27538ace583 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 30 Sep 2020 13:57:14 -0700 Subject: [PATCH 1/7] only need catransactions when there are platform views --- .../framework/Source/FlutterPlatformViews.mm | 3 ++- shell/platform/darwin/ios/ios_surface.mm | 17 ++--------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 7f732f7b4fd4e..75ae1d1e0b6b1 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -279,6 +279,7 @@ CancelFrame(); return PostPrerollResult::kSkipAndRetryFrame; } + [CATransaction begin]; raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); return PostPrerollResult::kSuccess; } @@ -547,7 +548,7 @@ composition_order_.clear(); did_submit &= frame->Submit(); - + [CATransaction commit]; return did_submit; } diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index 1583e0c41c6a3..976c71ee45a49 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -71,9 +71,6 @@ TRACE_EVENT0("flutter", "IOSSurface::CancelFrame"); FML_CHECK(platform_views_controller_ != nullptr); platform_views_controller_->CancelFrame(); - // Committing the current transaction as |BeginFrame| will create a nested - // CATransaction otherwise. - [CATransaction commit]; } // |ExternalViewEmbedder| @@ -84,7 +81,6 @@ TRACE_EVENT0("flutter", "IOSSurface::BeginFrame"); FML_CHECK(platform_views_controller_ != nullptr); platform_views_controller_->SetFrameSize(frame_size); - [CATransaction begin]; } // |ExternalViewEmbedder| @@ -102,10 +98,6 @@ TRACE_EVENT0("flutter", "IOSSurface::PostPrerollAction"); FML_CHECK(platform_views_controller_ != nullptr); PostPrerollResult result = platform_views_controller_->PostPrerollAction(raster_thread_merger); - if (result == PostPrerollResult::kSkipAndRetryFrame) { - // Commit the current transaction if the frame is dropped. - [CATransaction commit]; - } return result; } @@ -126,13 +118,8 @@ void IOSSurface::SubmitFrame(GrDirectContext* context, std::unique_ptr frame) { TRACE_EVENT0("flutter", "IOSSurface::SubmitFrame"); FML_CHECK(platform_views_controller_ != nullptr); - bool submitted = - platform_views_controller_->SubmitFrame(std::move(context), ios_context_, std::move(frame)); - - if (submitted) { - TRACE_EVENT0("flutter", "IOSSurface::DidSubmitFrame"); - [CATransaction commit]; - } + platform_views_controller_->SubmitFrame(std::move(context), ios_context_, std::move(frame)); + TRACE_EVENT0("flutter", "IOSSurface::DidSubmitFrame"); } // |ExternalViewEmbedder| From 744e93fc35648f139eac4ff7b88d6b44b6c0dbec Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 1 Oct 2020 17:55:08 -0700 Subject: [PATCH 2/7] add comments --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 75ae1d1e0b6b1..e31e780dfbb8e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -279,6 +279,10 @@ CancelFrame(); return PostPrerollResult::kSkipAndRetryFrame; } + // If the post preroll action is successful, we will display platform views in the current frame. + // In order to sync the rendering of the platform views (quartz) with skia's rendering, + // We need to begin an explicit CATransaction. This transaction needs to be submitted + // after the current frame is submitted. [CATransaction begin]; raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); return PostPrerollResult::kSuccess; @@ -548,6 +552,10 @@ composition_order_.clear(); did_submit &= frame->Submit(); + + // The frame is submitted with embedded platform views. + // There should be a |[CATransaction begin]| call in this frame prior to all the drawing. + // Now we need to commit the transaction. [CATransaction commit]; return did_submit; } From 39697a72c1c15784947035e965f09b90392c4baf Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 1 Oct 2020 18:00:28 -0700 Subject: [PATCH 3/7] add dchecks --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 8 +++++++- .../ios/framework/Source/FlutterPlatformViews_Internal.h | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index e31e780dfbb8e..7414deca461bf 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -283,7 +283,9 @@ // In order to sync the rendering of the platform views (quartz) with skia's rendering, // We need to begin an explicit CATransaction. This transaction needs to be submitted // after the current frame is submitted. + FML_DCHECK([[NSThread currentThread] isMainThread]); [CATransaction begin]; + catransaction_added_ = true; raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); return PostPrerollResult::kSuccess; } @@ -556,7 +558,11 @@ // The frame is submitted with embedded platform views. // There should be a |[CATransaction begin]| call in this frame prior to all the drawing. // Now we need to commit the transaction. - [CATransaction commit]; + if (catransaction_added_) { + FML_DCHECK([[NSThread currentThread] isMainThread]); + [CATransaction commit]; + catransaction_added_ = false; + } return did_submit; } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index f0867bc4bbcc8..54e4c225bf57e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -245,6 +245,8 @@ class FlutterPlatformViewsController { std::unique_ptr> weak_factory_; + bool catransaction_added_ = false; + void OnCreate(FlutterMethodCall* call, FlutterResult& result); void OnDispose(FlutterMethodCall* call, FlutterResult& result); void OnAcceptGesture(FlutterMethodCall* call, FlutterResult& result); From 7b1b0f74624a8b9ac146912998a019bc8094bc4b Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 2 Oct 2020 11:38:13 -0700 Subject: [PATCH 4/7] reviews --- .../framework/Source/FlutterPlatformViews.mm | 32 ++++++++++++------- .../Source/FlutterPlatformViews_Internal.h | 7 ++++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 7414deca461bf..4cc092ed944c9 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -283,9 +283,7 @@ // In order to sync the rendering of the platform views (quartz) with skia's rendering, // We need to begin an explicit CATransaction. This transaction needs to be submitted // after the current frame is submitted. - FML_DCHECK([[NSThread currentThread] isMainThread]); - [CATransaction begin]; - catransaction_added_ = true; + BeginCATransaction(); raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); return PostPrerollResult::kSuccess; } @@ -293,6 +291,8 @@ void FlutterPlatformViewsController::PrerollCompositeEmbeddedView( int view_id, std::unique_ptr params) { + // No CATransactions should be added in the begining of the frame. + FML_DCHECK(!catransaction_added_); picture_recorders_[view_id] = std::make_unique(); auto rtree_factory = RTreeFactory(); @@ -555,14 +555,10 @@ did_submit &= frame->Submit(); - // The frame is submitted with embedded platform views. - // There should be a |[CATransaction begin]| call in this frame prior to all the drawing. - // Now we need to commit the transaction. - if (catransaction_added_) { - FML_DCHECK([[NSThread currentThread] isMainThread]); - [CATransaction commit]; - catransaction_added_ = false; - } + // If the frame is submitted with embedded platform views, + // there should be a |[CATransaction begin]| call in this frame prior to all the drawing. + // If that case, we need to commit the transaction. + CommitCATransactionIfNeeded(); return did_submit; } @@ -677,6 +673,20 @@ views_to_dispose_.clear(); } +void FlutterPlatformViewsController::BeginCATransaction() { + FML_DCHECK([[NSThread currentThread] isMainThread]); + [CATransaction begin]; + catransaction_added_ = true; +} + +void FlutterPlatformViewsController::CommitCATransactionIfNeeded() { + if (catransaction_added_) { + FML_DCHECK([[NSThread currentThread] isMainThread]); + [CATransaction commit]; + catransaction_added_ = false; + } +} + } // namespace flutter // This recognizers delays touch events from being dispatched to the responder chain until it failed diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index 54e4c225bf57e..4fcaaf3874212 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -296,6 +296,13 @@ class FlutterPlatformViewsController { // order. void BringLayersIntoView(LayersMap layer_map); + // Begin a CATransaction. + // This transaction needs to be balanced with |CommitCATransactionIfNeeded|. + void BeginCATransaction(); + + // Commit a CATransaction if |BeginCATransaction| has been called during the frame. + void CommitCATransactionIfNeeded(); + FML_DISALLOW_COPY_AND_ASSIGN(FlutterPlatformViewsController); }; From 53c5ac7da419431a75fdd50d5eb2632abb0d11fe Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 2 Oct 2020 11:40:17 -0700 Subject: [PATCH 5/7] edit comments --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 4cc092ed944c9..59ecc9e59fa80 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -291,7 +291,8 @@ void FlutterPlatformViewsController::PrerollCompositeEmbeddedView( int view_id, std::unique_ptr params) { - // No CATransactions should be added in the begining of the frame. + // All the CATransactions should be committed by the end of the last frame, + // so catransaction_added_ must be false. FML_DCHECK(!catransaction_added_); picture_recorders_[view_id] = std::make_unique(); From 9ceced8026d4effbdc2453050d7703bc656cefd9 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 2 Oct 2020 13:48:02 -0700 Subject: [PATCH 6/7] format --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 59ecc9e59fa80..9a09f870fac16 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -292,7 +292,7 @@ int view_id, std::unique_ptr params) { // All the CATransactions should be committed by the end of the last frame, - // so catransaction_added_ must be false. + // so catransaction_added_ must be false. FML_DCHECK(!catransaction_added_); picture_recorders_[view_id] = std::make_unique(); From 5565220fd164e0f9b37775cefeb5f6605d17acf9 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 2 Oct 2020 13:53:06 -0700 Subject: [PATCH 7/7] more dchecks --- .../platform/darwin/ios/framework/Source/FlutterPlatformViews.mm | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 9a09f870fac16..c0c185c6ee4b7 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -676,6 +676,7 @@ void FlutterPlatformViewsController::BeginCATransaction() { FML_DCHECK([[NSThread currentThread] isMainThread]); + FML_DCHECK(!catransaction_added_); [CATransaction begin]; catransaction_added_ = true; }