From 3c1aa033db48b74b2324502964fa2f3df64ef5b3 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 2 Mar 2020 20:05:39 -0800 Subject: [PATCH 01/24] first fix --- fml/memory/weak_ptr.h | 10 ++-- shell/common/pipeline.h | 38 ++++++++++++++ shell/common/rasterizer.cc | 14 +++-- shell/common/rasterizer.h | 2 +- shell/common/shell.cc | 2 +- .../ios/framework/Source/FlutterEngine.mm | 51 +++++-------------- .../darwin/ios/ios_surface_software.h | 2 + .../darwin/ios/ios_surface_software.mm | 8 +++ 8 files changed, 78 insertions(+), 49 deletions(-) diff --git a/fml/memory/weak_ptr.h b/fml/memory/weak_ptr.h index 190865b93013f..71199f68bd8fa 100644 --- a/fml/memory/weak_ptr.h +++ b/fml/memory/weak_ptr.h @@ -75,12 +75,12 @@ class WeakPtr { // "originating" |WeakPtrFactory|. explicit operator bool() const { - FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); return flag_ && flag_->is_valid(); } T* get() const { - FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); return *this ? ptr_ : nullptr; } @@ -97,13 +97,13 @@ class WeakPtr { } T& operator*() const { - FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); FML_DCHECK(*this); return *get(); } T* operator->() const { - FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); FML_DCHECK(*this); return get(); } @@ -177,7 +177,7 @@ class WeakPtrFactory { } ~WeakPtrFactory() { - FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); flag_->Invalidate(); } diff --git a/shell/common/pipeline.h b/shell/common/pipeline.h index 2bf3b19c4b06f..c4adc8e9f3a2d 100644 --- a/shell/common/pipeline.h +++ b/shell/common/pipeline.h @@ -164,6 +164,44 @@ class Pipeline : public fml::RefCountedThreadSafe> { : PipelineConsumeResult::Done; } + /// @note Procedure doesn't copy all closures. + FML_WARN_UNUSED_RESULT + PipelineConsumeResult ConsumeWithoutSingal(const Consumer& consumer) { + if (consumer == nullptr) { + return PipelineConsumeResult::NoneAvailable; + } + + if (!available_.TryWait()) { + return PipelineConsumeResult::NoneAvailable; + } + + ResourcePtr resource; + size_t trace_id = 0; + size_t items_count = 0; + + { + std::scoped_lock lock(queue_mutex_); + std::tie(resource, trace_id) = std::move(queue_.front()); + queue_.pop_front(); + items_count = queue_.size(); + } + + { + TRACE_EVENT0("flutter", "PipelineConsume"); + consumer(std::move(resource)); + } + + --inflight_; + + TRACE_FLOW_END("flutter", "PipelineItem", trace_id); + TRACE_EVENT_ASYNC_END0("flutter", "PipelineItem", trace_id); + + return items_count > 0 ? PipelineConsumeResult::MoreAvailable + : PipelineConsumeResult::Done; + } + + + private: const uint32_t depth_; fml::Semaphore empty_; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 7d6a7a74e8a91..1ca064a59d22a 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -109,7 +109,7 @@ void Rasterizer::DrawLastLayerTree() { DrawToSurface(*last_layer_tree_); } -void Rasterizer::Draw(fml::RefPtr> pipeline) { +void Rasterizer::Draw(fml::RefPtr> pipeline, bool resubmit) { TRACE_EVENT0("flutter", "GPURasterizer::Draw"); if (gpu_thread_merger_ && !gpu_thread_merger_->IsOnRasterizingThread()) { // we yield and let this frame be serviced on the right thread. @@ -122,8 +122,12 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { [&](std::unique_ptr layer_tree) { raster_status = DoDraw(std::move(layer_tree)); }; - - PipelineConsumeResult consume_result = pipeline->Consume(consumer); + PipelineConsumeResult consume_result; + if (resubmit) { + consume_result = pipeline->ConsumeWithoutSingal(consumer); + } else { + consume_result = pipeline->Consume(consumer); + } // if the raster status is to resubmit the frame, we push the frame to the // front of the queue and also change the consume status to more available. if (raster_status == RasterStatus::kResubmit) { @@ -139,9 +143,9 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { switch (consume_result) { case PipelineConsumeResult::MoreAvailable: { task_runners_.GetGPUTaskRunner()->PostTask( - [weak_this = weak_factory_.GetWeakPtr(), pipeline]() { + [weak_this = weak_factory_.GetWeakPtr(), pipeline, raster_status]() { if (weak_this) { - weak_this->Draw(pipeline); + weak_this->Draw(pipeline, raster_status == RasterStatus::kResubmit); } }); break; diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index fcc1e356c6399..965c0b1bd8086 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -247,7 +247,7 @@ class Rasterizer final : public SnapshotDelegate { /// @param[in] pipeline The layer tree pipeline to take the next layer tree /// to render from. /// - void Draw(fml::RefPtr> pipeline); + void Draw(fml::RefPtr> pipeline, bool resubmit); //---------------------------------------------------------------------------- /// @brief The type of the screenshot to obtain of the previously diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 3a3f9cfa901ef..e7c1f230ef7ca 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -937,7 +937,7 @@ void Shell::OnAnimatorDraw(fml::RefPtr> pipeline) { rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline)]() { if (rasterizer) { - rasterizer->Draw(pipeline); + rasterizer->Draw(pipeline, false); if (waiting_for_first_frame.load()) { waiting_for_first_frame.store(false); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 4ed3ece0b98dc..318bccd7fc459 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -445,43 +445,20 @@ - (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryURI { return std::make_unique(shell, shell.GetTaskRunners()); }; - if (flutter::IsIosEmbeddedViewsPreviewEnabled()) { - // Embedded views requires the gpu and the platform views to be the same. - // The plan is to eventually dynamically merge the threads when there's a - // platform view in the layer tree. - // For now we use a fixed thread configuration with the same thread used as the - // gpu and platform task runner. - // TODO(amirh/chinmaygarde): remove this, and dynamically change the thread configuration. - // https://github.com/flutter/flutter/issues/23975 - - flutter::TaskRunners task_runners(threadLabel.UTF8String, // label - fml::MessageLoop::GetCurrent().GetTaskRunner(), // platform - fml::MessageLoop::GetCurrent().GetTaskRunner(), // gpu - _threadHost.ui_thread->GetTaskRunner(), // ui - _threadHost.io_thread->GetTaskRunner() // io - ); - // Create the shell. This is a blocking operation. - _shell = flutter::Shell::Create(std::move(task_runners), // task runners - std::move(windowData), // window data - std::move(settings), // settings - on_create_platform_view, // platform view creation - on_create_rasterizer // rasterzier creation - ); - } else { - flutter::TaskRunners task_runners(threadLabel.UTF8String, // label - fml::MessageLoop::GetCurrent().GetTaskRunner(), // platform - _threadHost.gpu_thread->GetTaskRunner(), // gpu - _threadHost.ui_thread->GetTaskRunner(), // ui - _threadHost.io_thread->GetTaskRunner() // io - ); - // Create the shell. This is a blocking operation. - _shell = flutter::Shell::Create(std::move(task_runners), // task runners - std::move(windowData), // window data - std::move(settings), // settings - on_create_platform_view, // platform view creation - on_create_rasterizer // rasterzier creation - ); - } + flutter::TaskRunners task_runners(threadLabel.UTF8String, // label + fml::MessageLoop::GetCurrent().GetTaskRunner(), // platform + _threadHost.gpu_thread->GetTaskRunner(), // gpu + _threadHost.ui_thread->GetTaskRunner(), // ui + _threadHost.io_thread->GetTaskRunner() // io + ); + // Create the shell. This is a blocking operation. + _shell = flutter::Shell::Create(std::move(task_runners), // task runners + std::move(windowData), // window data + std::move(settings), // settings + on_create_platform_view, // platform view creation + on_create_rasterizer // rasterzier creation + ); + if (_shell == nullptr) { FML_LOG(ERROR) << "Could not start a shell FlutterEngine with entrypoint: " diff --git a/shell/platform/darwin/ios/ios_surface_software.h b/shell/platform/darwin/ios/ios_surface_software.h index daac2ffc77231..6a3f843aee8fe 100644 --- a/shell/platform/darwin/ios/ios_surface_software.h +++ b/shell/platform/darwin/ios/ios_surface_software.h @@ -58,6 +58,8 @@ class IOSSurfaceSoftware final : public IOSSurface, void PrerollCompositeEmbeddedView(int view_id, std::unique_ptr params) override; + PostPrerollResult PostPrerollAction(fml::RefPtr gpu_thread_merger) override; + // |ExternalViewEmbedder| std::vector GetCurrentCanvases() override; diff --git a/shell/platform/darwin/ios/ios_surface_software.mm b/shell/platform/darwin/ios/ios_surface_software.mm index ab5490cf25140..68267882b57d5 100644 --- a/shell/platform/darwin/ios/ios_surface_software.mm +++ b/shell/platform/darwin/ios/ios_surface_software.mm @@ -166,6 +166,14 @@ platform_views_controller->PrerollCompositeEmbeddedView(view_id, std::move(params)); } +// |ExternalViewEmbedder| +PostPrerollResult IOSSurfaceSoftware::PostPrerollAction( + fml::RefPtr gpu_thread_merger) { + FlutterPlatformViewsController* platform_views_controller = GetPlatformViewsController(); + FML_CHECK(platform_views_controller != nullptr); + return platform_views_controller->PostPrerollAction(gpu_thread_merger); +} + // |ExternalViewEmbedder| std::vector IOSSurfaceSoftware::GetCurrentCanvases() { FlutterPlatformViewsController* platform_views_controller = GetPlatformViewsController(); From a5a3a8b387f9f0ee907bbcb6c8db748ca84f5944 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 3 Mar 2020 15:37:16 -0800 Subject: [PATCH 02/24] fix thread merging crash --- flow/embedded_views.h | 7 ++++++ fml/gpu_thread_merger.cc | 2 ++ shell/common/rasterizer.cc | 6 +++++ .../framework/Source/FlutterPlatformViews.mm | 22 ++++++++++++++++--- .../Source/FlutterPlatformViews_Internal.h | 13 +++++++++++ shell/platform/darwin/ios/ios_surface_gl.h | 3 +++ shell/platform/darwin/ios/ios_surface_gl.mm | 8 +++++++ .../darwin/ios/ios_surface_software.h | 4 ++++ .../darwin/ios/ios_surface_software.mm | 9 ++++++++ 9 files changed, 71 insertions(+), 3 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 030eb88c8a06d..27642b81e7db2 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -250,6 +250,13 @@ class ExternalViewEmbedder { virtual bool SubmitFrame(GrContext* context); + // Caller should make sure to call this after |SubmitFrame|. + // Embedder that implements this method to do additional tasks after |SubmitFrame|. + // One of such examples is threading merging on iOS. + // + // After invoking this method, the current task on the TaskRunner should end immediately. + virtual void EndFrame(fml::RefPtr gpu_thread_merger) {} + FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); }; // ExternalViewEmbedder diff --git a/fml/gpu_thread_merger.cc b/fml/gpu_thread_merger.cc index 9d0c2415b6a09..6b25322ea3ee9 100644 --- a/fml/gpu_thread_merger.cc +++ b/fml/gpu_thread_merger.cc @@ -21,6 +21,7 @@ GpuThreadMerger::GpuThreadMerger(fml::TaskQueueId platform_queue_id, } void GpuThreadMerger::MergeWithLease(size_t lease_term) { + FML_DLOG(ERROR) << "threads are merged"; FML_DCHECK(lease_term > 0) << "lease_term should be positive."; if (!is_merged_) { is_merged_ = task_queues_->Merge(platform_queue_id_, gpu_queue_id_); @@ -63,6 +64,7 @@ GpuThreadStatus GpuThreadMerger::DecrementLease() { lease_term_--; if (lease_term_ == 0) { bool success = task_queues_->Unmerge(platform_queue_id_); + FML_DLOG(ERROR) << "threads are unmerged"; FML_CHECK(success) << "Unable to un-merge the GPU and platform threads."; is_merged_ = false; return GpuThreadStatus::kUnmergedNow; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 1ca064a59d22a..efd773b9c033f 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -138,6 +138,12 @@ void Rasterizer::Draw(fml::RefPtr> pipeline, bool r consume_result = PipelineConsumeResult::MoreAvailable; } + // Merging the thread as we know the next `Draw` should be run on the platform thread. + if (raster_status == RasterStatus::kResubmit) { + auto* external_view_embedder = surface_->GetExternalViewEmbedder(); + external_view_embedder->EndFrame(gpu_thread_merger_); + } + // Consume as many pipeline items as possible. But yield the event loop // between successive tries. switch (consume_result) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index e7a30d61add75..56ab0c50936df 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -165,7 +165,7 @@ } void FlutterPlatformViewsController::CancelFrame() { - composition_order_.clear(); + composition_order_ = active_composition_order_; } bool FlutterPlatformViewsController::HasPendingViewOperations() { @@ -184,8 +184,9 @@ if (gpu_thread_merger->IsMerged()) { gpu_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); } else { + // Wait until |EndFrame| to perform thread merging. + will_merge_threads_ = true; CancelFrame(); - gpu_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); return PostPrerollResult::kResubmitFrame; } } @@ -387,6 +388,7 @@ composition_order_.clear(); return did_submit; } + DetachUnusedLayers(); active_composition_order_.clear(); UIView* flutter_view = flutter_view_.get(); @@ -414,7 +416,20 @@ return did_submit; } +void FlutterPlatformViewsController::EndFrame(fml::RefPtr gpu_thread_merger) { + if (will_merge_threads_) { + gpu_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); + will_merge_threads_ = false; + } +} + void FlutterPlatformViewsController::DetachUnusedLayers() { + if (active_composition_order_.empty()) { + return; + } + + FML_DCHECK([NSThread currentThread] == [NSThread mainThread]); + std::unordered_set composition_order_set; for (int64_t view_id : composition_order_) { @@ -441,6 +456,8 @@ return; } + FML_DCHECK([NSThread currentThread] == [NSThread mainThread]); + for (int64_t viewId : views_to_dispose_) { UIView* root_view = root_views_[viewId].get(); [root_view removeFromSuperview]; @@ -467,7 +484,6 @@ auto overlay_it = overlays_.find(overlay_id); if (!gr_context) { - FML_DLOG(ERROR) << "No GrContext"; if (overlays_.count(overlay_id) != 0) { return; } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index bdd013c98e07f..755c16f4f06ca 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -90,6 +90,8 @@ class FlutterPlatformViewsController { void SetFrameSize(SkISize frame_size); + // Indicates that we don't compisite any platform views or overlays during this frame. + // Also reverts the composite_order_ to its original state at the begining of the frame. void CancelFrame(); void PrerollCompositeEmbeddedView(int view_id, @@ -113,6 +115,10 @@ class FlutterPlatformViewsController { bool SubmitFrame(GrContext* gr_context, std::shared_ptr gl_context); + // Invoked at the very end of a frame. + // After invoking this method, nothing should happen on the current TaskRunner during the same frame. + void EndFrame(fml::RefPtr gpu_thread_merger); + void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); private: @@ -169,6 +175,8 @@ class FlutterPlatformViewsController { void OnAcceptGesture(FlutterMethodCall* call, FlutterResult& result); void OnRejectGesture(FlutterMethodCall* call, FlutterResult& result); + // Remove PlatformViews and Overlays that is in `active_composition_order_` but not `composition_order_`. + // Must run on main thread. void DetachUnusedLayers(); // Dispose the views in `views_to_dispose_`. void DisposeViews(); @@ -214,6 +222,11 @@ class FlutterPlatformViewsController { void ApplyMutators(const MutatorsStack& mutators_stack, UIView* embedded_view); void CompositeWithParams(int view_id, const EmbeddedViewParams& params); + // Default to `false`. + // If `true`, gpu thread and platform thread should be merged during |EndFrame|. + // Always resets to `false` right after the threads are merged. + bool will_merge_threads_; + FML_DISALLOW_COPY_AND_ASSIGN(FlutterPlatformViewsController); }; diff --git a/shell/platform/darwin/ios/ios_surface_gl.h b/shell/platform/darwin/ios/ios_surface_gl.h index 906f3be92b2d8..888fd39aed69c 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.h +++ b/shell/platform/darwin/ios/ios_surface_gl.h @@ -78,6 +78,9 @@ class IOSSurfaceGL final : public IOSSurface, // |ExternalViewEmbedder| bool SubmitFrame(GrContext* context) override; + // |ExternalViewEmbedder| + void EndFrame(fml::RefPtr gpu_thread_merger) override; + private: std::shared_ptr context_; std::unique_ptr render_target_; diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index 4f1da5a593a02..6804845f4bb80 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -152,4 +152,12 @@ return submitted; } +// |ExternalViewEmbedder| +void IOSSurfaceGL::EndFrame(fml::RefPtr gpu_thread_merger) { + FlutterPlatformViewsController* platform_views_controller = GetPlatformViewsController(); + if (platform_views_controller == nullptr) { + return; + } + return platform_views_controller->EndFrame(gpu_thread_merger); +} } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_surface_software.h b/shell/platform/darwin/ios/ios_surface_software.h index 6a3f843aee8fe..e376dad386ff6 100644 --- a/shell/platform/darwin/ios/ios_surface_software.h +++ b/shell/platform/darwin/ios/ios_surface_software.h @@ -69,6 +69,10 @@ class IOSSurfaceSoftware final : public IOSSurface, // |ExternalViewEmbedder| bool SubmitFrame(GrContext* context) override; + // |ExternalViewEmbedder| + void EndFrame(fml::RefPtr gpu_thread_merger) override; + + private: fml::scoped_nsobject layer_; sk_sp sk_surface_; diff --git a/shell/platform/darwin/ios/ios_surface_software.mm b/shell/platform/darwin/ios/ios_surface_software.mm index 68267882b57d5..b5d7fca0be92e 100644 --- a/shell/platform/darwin/ios/ios_surface_software.mm +++ b/shell/platform/darwin/ios/ios_surface_software.mm @@ -197,4 +197,13 @@ return platform_views_controller->SubmitFrame(nullptr, nullptr); } +// |ExternalViewEmbedder| +void IOSSurfaceSoftware::EndFrame(fml::RefPtr gpu_thread_merger) { + FlutterPlatformViewsController* platform_views_controller = GetPlatformViewsController(); + if (platform_views_controller == nullptr) { + return; + } + return platform_views_controller->EndFrame(gpu_thread_merger); +} + } // namespace flutter From 4f14cf874c97d41e3d35328c130541d4e73dd0c4 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 4 Mar 2020 10:21:13 -0800 Subject: [PATCH 03/24] formatting and remove testing code --- flow/embedded_views.h | 7 +-- fml/gpu_thread_merger.cc | 2 - fml/memory/weak_ptr.h | 10 ++-- shell/common/pipeline.h | 38 -------------- shell/common/rasterizer.cc | 15 +++--- shell/common/rasterizer.h | 2 +- .../ios/framework/Source/FlutterEngine.mm | 51 ++++++++++++++----- .../Source/FlutterPlatformViews_Internal.h | 7 +-- .../darwin/ios/ios_surface_software.h | 1 - 9 files changed, 57 insertions(+), 76 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 27642b81e7db2..51475ac26ca90 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -251,10 +251,11 @@ class ExternalViewEmbedder { virtual bool SubmitFrame(GrContext* context); // Caller should make sure to call this after |SubmitFrame|. - // Embedder that implements this method to do additional tasks after |SubmitFrame|. - // One of such examples is threading merging on iOS. + // Embedder that implements this method to do additional tasks after + // |SubmitFrame|. One of such examples is threading merging on iOS. // - // After invoking this method, the current task on the TaskRunner should end immediately. + // After invoking this method, the current task on the TaskRunner should end + // immediately. virtual void EndFrame(fml::RefPtr gpu_thread_merger) {} FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); diff --git a/fml/gpu_thread_merger.cc b/fml/gpu_thread_merger.cc index 6b25322ea3ee9..9d0c2415b6a09 100644 --- a/fml/gpu_thread_merger.cc +++ b/fml/gpu_thread_merger.cc @@ -21,7 +21,6 @@ GpuThreadMerger::GpuThreadMerger(fml::TaskQueueId platform_queue_id, } void GpuThreadMerger::MergeWithLease(size_t lease_term) { - FML_DLOG(ERROR) << "threads are merged"; FML_DCHECK(lease_term > 0) << "lease_term should be positive."; if (!is_merged_) { is_merged_ = task_queues_->Merge(platform_queue_id_, gpu_queue_id_); @@ -64,7 +63,6 @@ GpuThreadStatus GpuThreadMerger::DecrementLease() { lease_term_--; if (lease_term_ == 0) { bool success = task_queues_->Unmerge(platform_queue_id_); - FML_DLOG(ERROR) << "threads are unmerged"; FML_CHECK(success) << "Unable to un-merge the GPU and platform threads."; is_merged_ = false; return GpuThreadStatus::kUnmergedNow; diff --git a/fml/memory/weak_ptr.h b/fml/memory/weak_ptr.h index 71199f68bd8fa..190865b93013f 100644 --- a/fml/memory/weak_ptr.h +++ b/fml/memory/weak_ptr.h @@ -75,12 +75,12 @@ class WeakPtr { // "originating" |WeakPtrFactory|. explicit operator bool() const { - //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); return flag_ && flag_->is_valid(); } T* get() const { - //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); return *this ? ptr_ : nullptr; } @@ -97,13 +97,13 @@ class WeakPtr { } T& operator*() const { - //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); FML_DCHECK(*this); return *get(); } T* operator->() const { - //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); FML_DCHECK(*this); return get(); } @@ -177,7 +177,7 @@ class WeakPtrFactory { } ~WeakPtrFactory() { - //FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); + FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker); flag_->Invalidate(); } diff --git a/shell/common/pipeline.h b/shell/common/pipeline.h index c4adc8e9f3a2d..2bf3b19c4b06f 100644 --- a/shell/common/pipeline.h +++ b/shell/common/pipeline.h @@ -164,44 +164,6 @@ class Pipeline : public fml::RefCountedThreadSafe> { : PipelineConsumeResult::Done; } - /// @note Procedure doesn't copy all closures. - FML_WARN_UNUSED_RESULT - PipelineConsumeResult ConsumeWithoutSingal(const Consumer& consumer) { - if (consumer == nullptr) { - return PipelineConsumeResult::NoneAvailable; - } - - if (!available_.TryWait()) { - return PipelineConsumeResult::NoneAvailable; - } - - ResourcePtr resource; - size_t trace_id = 0; - size_t items_count = 0; - - { - std::scoped_lock lock(queue_mutex_); - std::tie(resource, trace_id) = std::move(queue_.front()); - queue_.pop_front(); - items_count = queue_.size(); - } - - { - TRACE_EVENT0("flutter", "PipelineConsume"); - consumer(std::move(resource)); - } - - --inflight_; - - TRACE_FLOW_END("flutter", "PipelineItem", trace_id); - TRACE_EVENT_ASYNC_END0("flutter", "PipelineItem", trace_id); - - return items_count > 0 ? PipelineConsumeResult::MoreAvailable - : PipelineConsumeResult::Done; - } - - - private: const uint32_t depth_; fml::Semaphore empty_; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index efd773b9c033f..ea9241eea8144 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -109,7 +109,7 @@ void Rasterizer::DrawLastLayerTree() { DrawToSurface(*last_layer_tree_); } -void Rasterizer::Draw(fml::RefPtr> pipeline, bool resubmit) { +void Rasterizer::Draw(fml::RefPtr> pipeline) { TRACE_EVENT0("flutter", "GPURasterizer::Draw"); if (gpu_thread_merger_ && !gpu_thread_merger_->IsOnRasterizingThread()) { // we yield and let this frame be serviced on the right thread. @@ -122,12 +122,8 @@ void Rasterizer::Draw(fml::RefPtr> pipeline, bool r [&](std::unique_ptr layer_tree) { raster_status = DoDraw(std::move(layer_tree)); }; - PipelineConsumeResult consume_result; - if (resubmit) { - consume_result = pipeline->ConsumeWithoutSingal(consumer); - } else { - consume_result = pipeline->Consume(consumer); - } + + PipelineConsumeResult consume_result = pipeline->Consume(consumer); // if the raster status is to resubmit the frame, we push the frame to the // front of the queue and also change the consume status to more available. if (raster_status == RasterStatus::kResubmit) { @@ -138,7 +134,8 @@ void Rasterizer::Draw(fml::RefPtr> pipeline, bool r consume_result = PipelineConsumeResult::MoreAvailable; } - // Merging the thread as we know the next `Draw` should be run on the platform thread. + // Merging the thread as we know the next `Draw` should be run on the platform + // thread. if (raster_status == RasterStatus::kResubmit) { auto* external_view_embedder = surface_->GetExternalViewEmbedder(); external_view_embedder->EndFrame(gpu_thread_merger_); @@ -151,7 +148,7 @@ void Rasterizer::Draw(fml::RefPtr> pipeline, bool r task_runners_.GetGPUTaskRunner()->PostTask( [weak_this = weak_factory_.GetWeakPtr(), pipeline, raster_status]() { if (weak_this) { - weak_this->Draw(pipeline, raster_status == RasterStatus::kResubmit); + weak_this->Draw(pipeline); } }); break; diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 965c0b1bd8086..fcc1e356c6399 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -247,7 +247,7 @@ class Rasterizer final : public SnapshotDelegate { /// @param[in] pipeline The layer tree pipeline to take the next layer tree /// to render from. /// - void Draw(fml::RefPtr> pipeline, bool resubmit); + void Draw(fml::RefPtr> pipeline); //---------------------------------------------------------------------------- /// @brief The type of the screenshot to obtain of the previously diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 318bccd7fc459..4ed3ece0b98dc 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -445,20 +445,43 @@ - (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryURI { return std::make_unique(shell, shell.GetTaskRunners()); }; - flutter::TaskRunners task_runners(threadLabel.UTF8String, // label - fml::MessageLoop::GetCurrent().GetTaskRunner(), // platform - _threadHost.gpu_thread->GetTaskRunner(), // gpu - _threadHost.ui_thread->GetTaskRunner(), // ui - _threadHost.io_thread->GetTaskRunner() // io - ); - // Create the shell. This is a blocking operation. - _shell = flutter::Shell::Create(std::move(task_runners), // task runners - std::move(windowData), // window data - std::move(settings), // settings - on_create_platform_view, // platform view creation - on_create_rasterizer // rasterzier creation - ); - + if (flutter::IsIosEmbeddedViewsPreviewEnabled()) { + // Embedded views requires the gpu and the platform views to be the same. + // The plan is to eventually dynamically merge the threads when there's a + // platform view in the layer tree. + // For now we use a fixed thread configuration with the same thread used as the + // gpu and platform task runner. + // TODO(amirh/chinmaygarde): remove this, and dynamically change the thread configuration. + // https://github.com/flutter/flutter/issues/23975 + + flutter::TaskRunners task_runners(threadLabel.UTF8String, // label + fml::MessageLoop::GetCurrent().GetTaskRunner(), // platform + fml::MessageLoop::GetCurrent().GetTaskRunner(), // gpu + _threadHost.ui_thread->GetTaskRunner(), // ui + _threadHost.io_thread->GetTaskRunner() // io + ); + // Create the shell. This is a blocking operation. + _shell = flutter::Shell::Create(std::move(task_runners), // task runners + std::move(windowData), // window data + std::move(settings), // settings + on_create_platform_view, // platform view creation + on_create_rasterizer // rasterzier creation + ); + } else { + flutter::TaskRunners task_runners(threadLabel.UTF8String, // label + fml::MessageLoop::GetCurrent().GetTaskRunner(), // platform + _threadHost.gpu_thread->GetTaskRunner(), // gpu + _threadHost.ui_thread->GetTaskRunner(), // ui + _threadHost.io_thread->GetTaskRunner() // io + ); + // Create the shell. This is a blocking operation. + _shell = flutter::Shell::Create(std::move(task_runners), // task runners + std::move(windowData), // window data + std::move(settings), // settings + on_create_platform_view, // platform view creation + on_create_rasterizer // rasterzier creation + ); + } if (_shell == nullptr) { FML_LOG(ERROR) << "Could not start a shell FlutterEngine with entrypoint: " diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index 755c16f4f06ca..effccd5d2a192 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -116,7 +116,8 @@ class FlutterPlatformViewsController { bool SubmitFrame(GrContext* gr_context, std::shared_ptr gl_context); // Invoked at the very end of a frame. - // After invoking this method, nothing should happen on the current TaskRunner during the same frame. + // After invoking this method, nothing should happen on the current TaskRunner during the same + // frame. void EndFrame(fml::RefPtr gpu_thread_merger); void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); @@ -175,8 +176,8 @@ class FlutterPlatformViewsController { void OnAcceptGesture(FlutterMethodCall* call, FlutterResult& result); void OnRejectGesture(FlutterMethodCall* call, FlutterResult& result); - // Remove PlatformViews and Overlays that is in `active_composition_order_` but not `composition_order_`. - // Must run on main thread. + // Remove PlatformViews and Overlays that is in `active_composition_order_` but not + // `composition_order_`. Must run on main thread. void DetachUnusedLayers(); // Dispose the views in `views_to_dispose_`. void DisposeViews(); diff --git a/shell/platform/darwin/ios/ios_surface_software.h b/shell/platform/darwin/ios/ios_surface_software.h index e376dad386ff6..748d487afec58 100644 --- a/shell/platform/darwin/ios/ios_surface_software.h +++ b/shell/platform/darwin/ios/ios_surface_software.h @@ -72,7 +72,6 @@ class IOSSurfaceSoftware final : public IOSSurface, // |ExternalViewEmbedder| void EndFrame(fml::RefPtr gpu_thread_merger) override; - private: fml::scoped_nsobject layer_; sk_sp sk_surface_; From 25a9bac14fcddfe15c9ba07cbeca831acce9064e Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 4 Mar 2020 10:28:22 -0800 Subject: [PATCH 04/24] remove extra testing code --- shell/common/rasterizer.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index ea9241eea8144..d97c566e97884 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -146,7 +146,7 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { switch (consume_result) { case PipelineConsumeResult::MoreAvailable: { task_runners_.GetGPUTaskRunner()->PostTask( - [weak_this = weak_factory_.GetWeakPtr(), pipeline, raster_status]() { + [weak_this = weak_factory_.GetWeakPtr(), pipeline]() { if (weak_this) { weak_this->Draw(pipeline); } From d903fbe715ae0dbb4b3681d18b6ddd95ccf7dc0a Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 4 Mar 2020 10:39:30 -0800 Subject: [PATCH 05/24] remove extra stuff --- shell/common/shell.cc | 2 +- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index e7c1f230ef7ca..3a3f9cfa901ef 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -937,7 +937,7 @@ void Shell::OnAnimatorDraw(fml::RefPtr> pipeline) { rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline)]() { if (rasterizer) { - rasterizer->Draw(pipeline, false); + rasterizer->Draw(pipeline); if (waiting_for_first_frame.load()) { waiting_for_first_frame.store(false); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 56ab0c50936df..cccce831c7be8 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -388,7 +388,6 @@ composition_order_.clear(); return did_submit; } - DetachUnusedLayers(); active_composition_order_.clear(); UIView* flutter_view = flutter_view_.get(); From 89dde6e7b734ab812d05397b98c257b56622eb14 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 9 Mar 2020 15:39:49 -0700 Subject: [PATCH 06/24] fix dispose views race condition --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index cccce831c7be8..533701e3f0505 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -369,6 +369,12 @@ bool FlutterPlatformViewsController::SubmitFrame(GrContext* gr_context, std::shared_ptr gl_context) { + // As we are merging the threads, we bail here and let the next frame handle this. + if (will_merge_threads_) { + picture_recorders_.clear(); + composition_order_.clear(); + return true; + } DisposeViews(); bool did_submit = true; From d7ec7f6f29d0f6e7d6fa65d0891aebb9a7def44e Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 13 Mar 2020 11:10:22 -0700 Subject: [PATCH 07/24] remove extra lines generated in merge --- shell/platform/darwin/ios/ios_surface_gl.mm | 8 -------- 1 file changed, 8 deletions(-) diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index 46ec70c69c27c..b3aa25a4e66cd 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -83,12 +83,4 @@ return GetExternalViewEmbedderIfEnabled(); } -// |ExternalViewEmbedder| -void IOSSurfaceGL::EndFrame(fml::RefPtr gpu_thread_merger) { - FlutterPlatformViewsController* platform_views_controller = GetPlatformViewsController(); - if (platform_views_controller == nullptr) { - return; - } - return platform_views_controller->EndFrame(gpu_thread_merger); -} } // namespace flutter From 3d2efcdb872285f10d95cf1bafd748ceedee8167 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 13 Mar 2020 11:11:39 -0700 Subject: [PATCH 08/24] remove extra line --- shell/platform/darwin/ios/ios_surface_software.mm | 9 --------- 1 file changed, 9 deletions(-) diff --git a/shell/platform/darwin/ios/ios_surface_software.mm b/shell/platform/darwin/ios/ios_surface_software.mm index 80a4c9cdfb416..06abe29c661e8 100644 --- a/shell/platform/darwin/ios/ios_surface_software.mm +++ b/shell/platform/darwin/ios/ios_surface_software.mm @@ -127,13 +127,4 @@ return GetExternalViewEmbedderIfEnabled(); } -// |ExternalViewEmbedder| -void IOSSurfaceSoftware::EndFrame(fml::RefPtr gpu_thread_merger) { - FlutterPlatformViewsController* platform_views_controller = GetPlatformViewsController(); - if (platform_views_controller == nullptr) { - return; - } - return platform_views_controller->EndFrame(gpu_thread_merger); -} - } // namespace flutter From 64f8e9c41224bd7b41299199a08f790c0d79c144 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 13 Mar 2020 11:12:56 -0700 Subject: [PATCH 09/24] catransaction commit only on main thread --- shell/platform/darwin/ios/ios_surface.mm | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index 000f6dc4ed29d..19ccd4e346396 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -90,7 +90,9 @@ bool IsIosEmbeddedViewsPreviewEnabled() { platform_views_controller_->CancelFrame(); // Committing the current transaction as |BeginFrame| will create a nested // CATransaction otherwise. - [CATransaction commit]; + if ([NSThread currentThread] == [NSThread mainThread]) { + [CATransaction commit]; + } } // |ExternalViewEmbedder| @@ -98,7 +100,9 @@ bool IsIosEmbeddedViewsPreviewEnabled() { TRACE_EVENT0("flutter", "IOSSurface::BeginFrame"); FML_CHECK(platform_views_controller_ != nullptr); platform_views_controller_->SetFrameSize(frame_size); - [CATransaction begin]; + if ([NSThread currentThread] == [NSThread mainThread]) { + [CATransaction begin]; + } } // |ExternalViewEmbedder| @@ -136,7 +140,9 @@ bool IsIosEmbeddedViewsPreviewEnabled() { TRACE_EVENT0("flutter", "IOSSurface::SubmitFrame"); FML_CHECK(platform_views_controller_ != nullptr); bool submitted = platform_views_controller_->SubmitFrame(std::move(context), ios_context_); - [CATransaction commit]; + if ([NSThread currentThread] == [NSThread mainThread]) { + [CATransaction commit]; + } return submitted; } From 8015dc60c5d3793007ceee298662a0541e9a3aaa Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 13 Mar 2020 11:27:45 -0700 Subject: [PATCH 10/24] EndFrame force check platform view controller --- shell/platform/darwin/ios/ios_surface.mm | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index 19ccd4e346396..480cec7c466a7 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -148,11 +148,8 @@ bool IsIosEmbeddedViewsPreviewEnabled() { // |ExternalViewEmbedder| void IOSSurface::EndFrame(fml::RefPtr gpu_thread_merger) { - FlutterPlatformViewsController* platform_views_controller = GetPlatformViewsController(); - if (platform_views_controller == nullptr) { - return; - } - return platform_views_controller->EndFrame(gpu_thread_merger); + FML_CHECK(platform_views_controller_ != nullptr); + return platform_views_controller_->EndFrame(gpu_thread_merger); } } // namespace flutter From 9a914a9fc5a34669c92b6edfb4dbf30a1d32f4c3 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 24 Mar 2020 10:50:21 -0700 Subject: [PATCH 11/24] review fixes --- flow/embedded_views.h | 12 +++++++----- .../ios/framework/Source/FlutterPlatformViews.mm | 12 +++++++----- .../framework/Source/FlutterPlatformViews_Internal.h | 5 +++-- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 87d311e62db0c..0e87278d3ea1b 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -253,12 +253,14 @@ class ExternalViewEmbedder { // This is called after submitting the embedder frame and the surface frame. virtual void FinishFrame(); - // Caller should make sure to call this after |SubmitFrame|. - // Embedder that implements this method to do additional tasks after - // |SubmitFrame|. One of such examples is threading merging on iOS. + // This should only be called after |SubmitFrame|. + // This method provides the embedder a way to do additional tasks after |SubmitFrame|. + // After invoking this method, the current task on the TaskRunner should end immediately. // - // After invoking this method, the current task on the TaskRunner should end - // immediately. + // For example on the iOS embedder, threads are merged in this call. + // A new frame on the platform thread starts immediately. If the GPU thread still has some + // task running, there could be two frames being rendered concurrently, which causes undefined + // behaviors. virtual void EndFrame(fml::RefPtr gpu_thread_merger) {} FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 08f30dd827952..a402b45dc8ad4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -248,8 +248,8 @@ if (gpu_thread_merger->IsMerged()) { gpu_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); } else { - // Wait until |EndFrame| to perform thread merging. - will_merge_threads_ = true; + // Wait until |EndFrame| to merge the threads. + merge_threads_ = true; CancelFrame(); return PostPrerollResult::kResubmitFrame; } @@ -451,7 +451,9 @@ bool FlutterPlatformViewsController::SubmitFrame(GrContext* gr_context, std::shared_ptr ios_context, SkCanvas* background_canvas) { - if (will_merge_threads_) { + if (merge_threads_) { + // Threads are about to be merged, we drop everything from this frame + // and possibly resubmit the same layer tree in the next frame. picture_recorders_.clear(); composition_order_.clear(); return true; @@ -560,9 +562,9 @@ } void FlutterPlatformViewsController::EndFrame(fml::RefPtr gpu_thread_merger) { - if (will_merge_threads_) { + if (merge_threads_) { gpu_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); - will_merge_threads_ = false; + merge_threads_ = false; } } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index 3d3c0b9160d2e..88c3b0e5c79df 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -133,7 +133,7 @@ class FlutterPlatformViewsController { void SetFrameSize(SkISize frame_size); // Indicates that we don't compisite any platform views or overlays during this frame. - // Also reverts the composite_order_ to its original state at the begining of the frame. + // Also reverts the composition_order_ to its original state at the begining of the frame. void CancelFrame(); void PrerollCompositeEmbeddedView(int view_id, @@ -282,7 +282,7 @@ class FlutterPlatformViewsController { // Default to `false`. // If `true`, gpu thread and platform thread should be merged during |EndFrame|. // Always resets to `false` right after the threads are merged. - bool will_merge_threads_; + bool merge_threads_; // Allocates a new FlutterPlatformViewLayer if needed, draws the pixels within the rect from // the picture on the layer's canvas. std::shared_ptr GetLayer(GrContext* gr_context, @@ -292,6 +292,7 @@ class FlutterPlatformViewsController { int64_t view_id, int64_t overlay_id); // Removes overlay views and platform views that aren't needed in the current frame. + // Must run on the platform thread. void RemoveUnusedLayers(); // Appends the overlay views and platform view and sets their z index based on the composition // order. From 08dfdfd6243c75ef42ae88ebe40c110f3696751c Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 24 Mar 2020 10:51:00 -0700 Subject: [PATCH 12/24] formatting --- flow/embedded_views.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 0e87278d3ea1b..3cc35b5bb1824 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -254,13 +254,14 @@ class ExternalViewEmbedder { virtual void FinishFrame(); // This should only be called after |SubmitFrame|. - // This method provides the embedder a way to do additional tasks after |SubmitFrame|. - // After invoking this method, the current task on the TaskRunner should end immediately. + // This method provides the embedder a way to do additional tasks after + // |SubmitFrame|. After invoking this method, the current task on the + // TaskRunner should end immediately. // // For example on the iOS embedder, threads are merged in this call. - // A new frame on the platform thread starts immediately. If the GPU thread still has some - // task running, there could be two frames being rendered concurrently, which causes undefined - // behaviors. + // A new frame on the platform thread starts immediately. If the GPU thread + // still has some task running, there could be two frames being rendered + // concurrently, which causes undefined behaviors. virtual void EndFrame(fml::RefPtr gpu_thread_merger) {} FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); From 3bba8216166442fd51db35e3d770df7f2e7830b9 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 6 Apr 2020 11:44:46 -0700 Subject: [PATCH 13/24] add test view embedder --- shell/common/BUILD.gn | 2 ++ shell/common/shell_test_external_view_embedder.cc | 0 shell/common/shell_test_external_view_embedder.h | 0 3 files changed, 2 insertions(+) create mode 100644 shell/common/shell_test_external_view_embedder.cc create mode 100644 shell/common/shell_test_external_view_embedder.h diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index 66684679c3de3..b849e8675e315 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -191,6 +191,8 @@ if (enable_unittests) { "pipeline_unittests.cc", "shell_test.cc", "shell_test.h", + "shell_test_external_view_embedder.h", + "shell_test_external_view_embedder.cc", "shell_test_platform_view.cc", "shell_test_platform_view.h", "shell_unittests.cc", diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h new file mode 100644 index 0000000000000..e69de29bb2d1d From 423d116a9241b4f4e410c2a652773ced3dbbb4a5 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 7 Apr 2020 11:18:23 -0700 Subject: [PATCH 14/24] rename gpu thread merger --- flow/embedded_views.h | 2 +- shell/common/rasterizer.cc | 2 +- .../shell_test_external_view_embedder.h | 66 +++++++++++++++++++ .../framework/Source/FlutterPlatformViews.mm | 4 +- .../Source/FlutterPlatformViews_Internal.h | 2 +- shell/platform/darwin/ios/ios_surface.h | 2 +- shell/platform/darwin/ios/ios_surface.mm | 4 +- 7 files changed, 74 insertions(+), 8 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 1a36483dbc927..d1bcf0b98c0b7 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -262,7 +262,7 @@ class ExternalViewEmbedder { // A new frame on the platform thread starts immediately. If the GPU thread // still has some task running, there could be two frames being rendered // concurrently, which causes undefined behaviors. - virtual void EndFrame(fml::RefPtr gpu_thread_merger) {} + virtual void EndFrame(fml::RefPtr raster_thread_merger) {} FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 34483261023d2..24eb70103e375 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -140,7 +140,7 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { // thread. if (raster_status == RasterStatus::kResubmit) { auto* external_view_embedder = surface_->GetExternalViewEmbedder(); - external_view_embedder->EndFrame(gpu_thread_merger_); + external_view_embedder->EndFrame(raster_thread_merger_); } // Consume as many pipeline items as possible. But yield the event loop diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index e69de29bb2d1d..26637d7ee0d9b 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -0,0 +1,66 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_TEST_EXTERNAL_VIEW_EMBEDDER_H_ +#define FLUTTER_SHELL_TEST_EXTERNAL_VIEW_EMBEDDER_H_ + +#include "flutter/flow/embedded_views.h" +#include "flutter/fml/raster_thread_merger.h" + +namespace flutter { + +//------------------------------------------------------------------------------ +/// @brief The external view embedder used by |ShellTestPlatformViewGL| +/// +class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { + public: + using EndFrameCallBack = + std::function; + + EmbedderExternalViewEmbedder( + const EndFrameCallBack& end_frame_call_back):end_frame_call_back_(end_frame_call_back); + + ~EmbedderExternalViewEmbedder() override; + + private: + // |ExternalViewEmbedder| + void CancelFrame() override; + + // |ExternalViewEmbedder| + void BeginFrame(SkISize frame_size, + GrContext* context, + double device_pixel_ratio) override; + + // |ExternalViewEmbedder| + void PrerollCompositeEmbeddedView( + int view_id, + std::unique_ptr params) override; + + // |ExternalViewEmbedder| + std::vector GetCurrentCanvases() override; + + // |ExternalViewEmbedder| + SkCanvas* CompositeEmbeddedView(int view_id) override; + + // |ExternalViewEmbedder| + bool SubmitFrame(GrContext* context, SkCanvas* background_canvas) override; + + // |ExternalViewEmbedder| + void FinishFrame() override; + + // |ExternalViewEmbedder| + void EndFrame(fml::RefPtr raster_thread_merger) override; + + // |ExternalViewEmbedder| + SkCanvas* GetRootCanvas() override; + + private: + const EndFrameCallBack end_frame_call_back_; + + FML_DISALLOW_COPY_AND_ASSIGN(EmbedderExternalViewEmbedder); +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_TEST_EXTERNAL_VIEW_EMBEDDER_H_ diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 52446ada6a81b..b03c1a9ed801b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -587,9 +587,9 @@ } } -void FlutterPlatformViewsController::EndFrame(fml::RefPtr gpu_thread_merger) { +void FlutterPlatformViewsController::EndFrame(fml::RefPtr raster_thread_merger) { if (merge_threads_) { - gpu_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); + raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); merge_threads_ = false; } } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index 3f11da7721909..e97720a711fd5 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -168,7 +168,7 @@ class FlutterPlatformViewsController { // Invoked at the very end of a frame. // After invoking this method, nothing should happen on the current TaskRunner during the same // frame. - void EndFrame(fml::RefPtr gpu_thread_merger); + void EndFrame(fml::RefPtr raster_thread_merger); void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); diff --git a/shell/platform/darwin/ios/ios_surface.h b/shell/platform/darwin/ios/ios_surface.h index e3fe6a057bde2..ed890c7687d33 100644 --- a/shell/platform/darwin/ios/ios_surface.h +++ b/shell/platform/darwin/ios/ios_surface.h @@ -84,7 +84,7 @@ class IOSSurface : public ExternalViewEmbedder { void FinishFrame() override; // |ExternalViewEmbedder| - void EndFrame(fml::RefPtr gpu_thread_merger) override; + void EndFrame(fml::RefPtr raster_thread_merger) override; public: FML_DISALLOW_COPY_AND_ASSIGN(IOSSurface); diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index 4eaa09290f793..3eebf666e78ba 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -145,10 +145,10 @@ bool IsIosEmbeddedViewsPreviewEnabled() { } // |ExternalViewEmbedder| -void IOSSurface::EndFrame(fml::RefPtr gpu_thread_merger) { +void IOSSurface::EndFrame(fml::RefPtr raster_thread_merger) { TRACE_EVENT0("flutter", "IOSSurface::EndFrame"); FML_CHECK(platform_views_controller_ != nullptr); - return platform_views_controller_->EndFrame(gpu_thread_merger); + return platform_views_controller_->EndFrame(raster_thread_merger); } // |ExternalViewEmbedder| From 9c749cdbd9d0ef5e30570546c42463c4a01b8a6b Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 7 Apr 2020 14:40:12 -0700 Subject: [PATCH 15/24] test for end frame call --- flow/embedded_views.h | 3 +- shell/common/animator_unittests.cc | 2 +- shell/common/shell_test.cc | 16 ++++-- shell/common/shell_test.h | 10 +++- .../shell_test_external_view_embedder.cc | 55 +++++++++++++++++++ .../shell_test_external_view_embedder.h | 24 +++++--- shell/common/shell_test_platform_view.cc | 10 +++- shell/common/shell_test_platform_view.h | 5 +- shell/common/shell_test_platform_view_gl.cc | 9 ++- shell/common/shell_test_platform_view_gl.h | 8 ++- shell/common/shell_unittests.cc | 49 ++++++++++++++++- .../framework/Source/FlutterPlatformViews.mm | 3 +- 12 files changed, 164 insertions(+), 30 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index d1bcf0b98c0b7..14dd00cf5e8c8 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -262,7 +262,8 @@ class ExternalViewEmbedder { // A new frame on the platform thread starts immediately. If the GPU thread // still has some task running, there could be two frames being rendered // concurrently, which causes undefined behaviors. - virtual void EndFrame(fml::RefPtr raster_thread_merger) {} + virtual void EndFrame( + fml::RefPtr raster_thread_merger) {} FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder); diff --git a/shell/common/animator_unittests.cc b/shell/common/animator_unittests.cc index ed54ea5272be2..bf2ac9f5a9cf1 100644 --- a/shell/common/animator_unittests.cc +++ b/shell/common/animator_unittests.cc @@ -55,7 +55,7 @@ TEST_F(ShellTest, VSyncTargetTime) { return ShellTestPlatformView::Create( shell, shell.GetTaskRunners(), vsync_clock, std::move(create_vsync_waiter), - ShellTestPlatformView::BackendType::kDefaultBackend); + ShellTestPlatformView::BackendType::kDefaultBackend, nullptr); }, [](Shell& shell) { return std::make_unique(shell, shell.GetTaskRunners()); diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 378375c5eac1e..cf7f9ed8d801b 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -260,9 +260,12 @@ std::unique_ptr ShellTest::CreateShell(Settings settings, simulate_vsync); } -std::unique_ptr ShellTest::CreateShell(Settings settings, - TaskRunners task_runners, - bool simulate_vsync) { +std::unique_ptr ShellTest::CreateShell( + Settings settings, + TaskRunners task_runners, + bool simulate_vsync, + std::shared_ptr + shell_test_external_view_embedder) { const auto vsync_clock = std::make_shared(); CreateVsyncWaiter create_vsync_waiter = [&]() { if (simulate_vsync) { @@ -275,17 +278,18 @@ std::unique_ptr ShellTest::CreateShell(Settings settings, }; return Shell::Create( task_runners, settings, - [vsync_clock, &create_vsync_waiter](Shell& shell) { + [vsync_clock, &create_vsync_waiter, + shell_test_external_view_embedder](Shell& shell) { return ShellTestPlatformView::Create( shell, shell.GetTaskRunners(), vsync_clock, std::move(create_vsync_waiter), - ShellTestPlatformView::BackendType::kDefaultBackend); + ShellTestPlatformView::BackendType::kDefaultBackend, + shell_test_external_view_embedder); }, [](Shell& shell) { return std::make_unique(shell, shell.GetTaskRunners()); }); } - void ShellTest::DestroyShell(std::unique_ptr shell) { DestroyShell(std::move(shell), GetTaskRunnersForFixture()); } diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index 680db7d349c1d..9f02aa35e68b3 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -15,6 +15,7 @@ #include "flutter/lib/ui/window/platform_message.h" #include "flutter/shell/common/run_configuration.h" #include "flutter/shell/common/shell.h" +#include "flutter/shell/common/shell_test_external_view_embedder.h" #include "flutter/shell/common/thread_host.h" #include "flutter/shell/common/vsync_waiters_test.h" #include "flutter/testing/elf_loader.h" @@ -31,9 +32,12 @@ class ShellTest : public ThreadTest { Settings CreateSettingsForFixture(); std::unique_ptr CreateShell(Settings settings, bool simulate_vsync = false); - std::unique_ptr CreateShell(Settings settings, - TaskRunners task_runners, - bool simulate_vsync = false); + std::unique_ptr CreateShell( + Settings settings, + TaskRunners task_runners, + bool simulate_vsync = false, + std::shared_ptr + shell_test_external_view_embedder = nullptr); void DestroyShell(std::unique_ptr shell); void DestroyShell(std::unique_ptr shell, TaskRunners task_runners); TaskRunners GetTaskRunnersForFixture(); diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index e69de29bb2d1d..683e21ad73c91 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -0,0 +1,55 @@ +#include "shell_test_external_view_embedder.h" + +namespace flutter { + +// |ExternalViewEmbedder| +void ShellTestExternalViewEmbedder::CancelFrame() {} + +// |ExternalViewEmbedder| +void ShellTestExternalViewEmbedder::BeginFrame(SkISize frame_size, + GrContext* context, + double device_pixel_ratio) {} + +// |ExternalViewEmbedder| +void ShellTestExternalViewEmbedder::PrerollCompositeEmbeddedView( + int view_id, + std::unique_ptr params) {} + +// |ExternalViewEmbedder| +PostPrerollResult ShellTestExternalViewEmbedder::PostPrerollAction( + fml::RefPtr raster_thread_merger) { + FML_DCHECK(raster_thread_merger); + return post_preroll_result_; +} + +// |ExternalViewEmbedder| +std::vector ShellTestExternalViewEmbedder::GetCurrentCanvases() { + return {}; +} + +// |ExternalViewEmbedder| +SkCanvas* ShellTestExternalViewEmbedder::CompositeEmbeddedView(int view_id) { + return nullptr; +} + +// |ExternalViewEmbedder| +bool ShellTestExternalViewEmbedder::SubmitFrame(GrContext* context, + SkCanvas* background_canvas) { + return true; +} + +// |ExternalViewEmbedder| +void ShellTestExternalViewEmbedder::FinishFrame() {} + +// |ExternalViewEmbedder| +void ShellTestExternalViewEmbedder::EndFrame( + fml::RefPtr raster_thread_merger) { + end_frame_call_back_(); +} + +// |ExternalViewEmbedder| +SkCanvas* ShellTestExternalViewEmbedder::GetRootCanvas() { + return nullptr; +} + +} // namespace flutter diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 26637d7ee0d9b..56d9123ea8a7a 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -13,15 +13,16 @@ namespace flutter { //------------------------------------------------------------------------------ /// @brief The external view embedder used by |ShellTestPlatformViewGL| /// -class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { +class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { public: - using EndFrameCallBack = - std::function; + using EndFrameCallBack = std::function; - EmbedderExternalViewEmbedder( - const EndFrameCallBack& end_frame_call_back):end_frame_call_back_(end_frame_call_back); + ShellTestExternalViewEmbedder(const EndFrameCallBack& end_frame_call_back, + PostPrerollResult post_preroll_result) + : end_frame_call_back_(end_frame_call_back), + post_preroll_result_(post_preroll_result) {} - ~EmbedderExternalViewEmbedder() override; + ~ShellTestExternalViewEmbedder() = default; private: // |ExternalViewEmbedder| @@ -37,6 +38,10 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { int view_id, std::unique_ptr params) override; + // |ExternalViewEmbedder| + PostPrerollResult PostPrerollAction( + fml::RefPtr raster_thread_merger) override; + // |ExternalViewEmbedder| std::vector GetCurrentCanvases() override; @@ -50,15 +55,16 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { void FinishFrame() override; // |ExternalViewEmbedder| - void EndFrame(fml::RefPtr raster_thread_merger) override; + void EndFrame( + fml::RefPtr raster_thread_merger) override; // |ExternalViewEmbedder| SkCanvas* GetRootCanvas() override; - private: const EndFrameCallBack end_frame_call_back_; + const PostPrerollResult post_preroll_result_; - FML_DISALLOW_COPY_AND_ASSIGN(EmbedderExternalViewEmbedder); + FML_DISALLOW_COPY_AND_ASSIGN(ShellTestExternalViewEmbedder); }; } // namespace flutter diff --git a/shell/common/shell_test_platform_view.cc b/shell/common/shell_test_platform_view.cc index cfd43308bfc10..29bd24d3134ff 100644 --- a/shell/common/shell_test_platform_view.cc +++ b/shell/common/shell_test_platform_view.cc @@ -19,7 +19,9 @@ std::unique_ptr ShellTestPlatformView::Create( TaskRunners task_runners, std::shared_ptr vsync_clock, CreateVsyncWaiter create_vsync_waiter, - BackendType backend) { + BackendType backend, + std::shared_ptr + shell_test_external_view_embedder) { // TODO(gw280): https://github.com/flutter/flutter/issues/50298 // Make this fully runtime configurable switch (backend) { @@ -27,12 +29,14 @@ std::unique_ptr ShellTestPlatformView::Create( #ifdef SHELL_ENABLE_GL case BackendType::kGLBackend: return std::make_unique( - delegate, task_runners, vsync_clock, create_vsync_waiter); + delegate, task_runners, vsync_clock, create_vsync_waiter, + shell_test_external_view_embedder); #endif // SHELL_ENABLE_GL #ifdef SHELL_ENABLE_VULKAN case BackendType::kVulkanBackend: return std::make_unique( - delegate, task_runners, vsync_clock, create_vsync_waiter); + delegate, task_runners, vsync_clock, create_vsync_waiter, + shell_test_external_view_embedder); #endif // SHELL_ENABLE_VULKAN default: FML_LOG(FATAL) << "No backends supported for ShellTestPlatformView"; diff --git a/shell/common/shell_test_platform_view.h b/shell/common/shell_test_platform_view.h index c7aefe86b33bc..9a850b1d481d6 100644 --- a/shell/common/shell_test_platform_view.h +++ b/shell/common/shell_test_platform_view.h @@ -6,6 +6,7 @@ #define FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_H_ #include "flutter/shell/common/platform_view.h" +#include "flutter/shell/common/shell_test_external_view_embedder.h" #include "flutter/shell/common/vsync_waiters_test.h" namespace flutter { @@ -24,7 +25,9 @@ class ShellTestPlatformView : public PlatformView { TaskRunners task_runners, std::shared_ptr vsync_clock, CreateVsyncWaiter create_vsync_waiter, - BackendType backend); + BackendType backend, + std::shared_ptr + shell_test_external_view_embedder); virtual void SimulateVSync() = 0; diff --git a/shell/common/shell_test_platform_view_gl.cc b/shell/common/shell_test_platform_view_gl.cc index 2bd597575e523..c980c53168645 100644 --- a/shell/common/shell_test_platform_view_gl.cc +++ b/shell/common/shell_test_platform_view_gl.cc @@ -12,11 +12,14 @@ ShellTestPlatformViewGL::ShellTestPlatformViewGL( PlatformView::Delegate& delegate, TaskRunners task_runners, std::shared_ptr vsync_clock, - CreateVsyncWaiter create_vsync_waiter) + CreateVsyncWaiter create_vsync_waiter, + std::shared_ptr + shell_test_external_view_embedder) : ShellTestPlatformView(delegate, std::move(task_runners)), gl_surface_(SkISize::Make(800, 600)), create_vsync_waiter_(std::move(create_vsync_waiter)), - vsync_clock_(vsync_clock) {} + vsync_clock_(vsync_clock), + shell_test_external_view_embedder_(shell_test_external_view_embedder) {} ShellTestPlatformViewGL::~ShellTestPlatformViewGL() = default; @@ -70,7 +73,7 @@ ShellTestPlatformViewGL::GetGLProcResolver() const { // |GPUSurfaceGLDelegate| ExternalViewEmbedder* ShellTestPlatformViewGL::GetExternalViewEmbedder() { - return nullptr; + return shell_test_external_view_embedder_.get(); } } // namespace testing diff --git a/shell/common/shell_test_platform_view_gl.h b/shell/common/shell_test_platform_view_gl.h index 03db7910873e0..cac2fdc649781 100644 --- a/shell/common/shell_test_platform_view_gl.h +++ b/shell/common/shell_test_platform_view_gl.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_GL_H_ #define FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_GL_H_ +#include "flutter/shell/common/shell_test_external_view_embedder.h" #include "flutter/shell/common/shell_test_platform_view.h" #include "flutter/shell/gpu/gpu_surface_gl_delegate.h" #include "flutter/testing/test_gl_surface.h" @@ -18,7 +19,9 @@ class ShellTestPlatformViewGL : public ShellTestPlatformView, ShellTestPlatformViewGL(PlatformView::Delegate& delegate, TaskRunners task_runners, std::shared_ptr vsync_clock, - CreateVsyncWaiter create_vsync_waiter); + CreateVsyncWaiter create_vsync_waiter, + std::shared_ptr + shell_test_external_view_embedder); virtual ~ShellTestPlatformViewGL() override; @@ -31,6 +34,9 @@ class ShellTestPlatformViewGL : public ShellTestPlatformView, std::shared_ptr vsync_clock_; + std::shared_ptr + shell_test_external_view_embedder_; + // |PlatformView| std::unique_ptr CreateRenderingSurface() override; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index c9c1345541fcc..cd863f1771cbe 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -23,6 +23,7 @@ #include "flutter/shell/common/platform_view.h" #include "flutter/shell/common/rasterizer.h" #include "flutter/shell/common/shell_test.h" +#include "flutter/shell/common/shell_test_external_view_embedder.h" #include "flutter/shell/common/shell_test_platform_view.h" #include "flutter/shell/common/switches.h" #include "flutter/shell/common/thread_host.h" @@ -139,7 +140,7 @@ TEST_F(ShellTest, return static_cast>( std::make_unique(task_runners)); }, - ShellTestPlatformView::BackendType::kDefaultBackend); + ShellTestPlatformView::BackendType::kDefaultBackend, nullptr); }, [](Shell& shell) { return std::make_unique(shell, shell.GetTaskRunners()); @@ -466,6 +467,52 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { DestroyShell(std::move(shell)); } +// chris +TEST_F(ShellTest, + ExternalEmbedderEndFrameIsCalledWhenPostPrerollResultIsResubmit) { + auto settings = CreateSettingsForFixture(); + fml::AutoResetWaitableEvent endFrameLatch; + bool end_frame_called = false; + auto end_frame_callback = [&] { + end_frame_called = true; + endFrameLatch.Signal(); + }; + auto external_view_embedder = std::make_shared( + end_frame_callback, PostPrerollResult::kResubmitFrame); + auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(), + false, external_view_embedder); + + // Create the surface needed by rasterizer + PlatformViewNotifyCreated(shell.get()); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("emptyMain"); + + RunEngine(shell.get(), std::move(configuration)); + + LayerTreeBuilder builder = [&](std::shared_ptr root) { + SkPictureRecorder recorder; + SkCanvas* recording_canvas = + recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80)); + recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80), + SkPaint(SkColor4f::FromColor(SK_ColorRED))); + auto sk_picture = recorder.finishRecordingAsPicture(); + fml::RefPtr queue = fml::MakeRefCounted( + this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); + auto picture_layer = std::make_shared( + SkPoint::Make(10, 10), + flutter::SkiaGPUObject({sk_picture, queue}), false, false); + root->Add(picture_layer); + }; + + PumpOneFrame(shell.get(), 100, 100, builder); + endFrameLatch.Wait(); + + ASSERT_TRUE(end_frame_called); + + DestroyShell(std::move(shell)); +} + TEST(SettingsTest, FrameTimingSetsAndGetsProperly) { // Ensure that all phases are in kPhases. ASSERT_EQ(sizeof(FrameTiming::kPhases), diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index b03c1a9ed801b..eb70e9c51fc21 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -587,7 +587,8 @@ } } -void FlutterPlatformViewsController::EndFrame(fml::RefPtr raster_thread_merger) { +void FlutterPlatformViewsController::EndFrame( + fml::RefPtr raster_thread_merger) { if (merge_threads_) { raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); merge_threads_ = false; From b1747f9c32ae8ce0dd6b591657b43802b89b116f Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 7 Apr 2020 14:58:38 -0700 Subject: [PATCH 16/24] remove unnecessary comment --- shell/common/shell_unittests.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index cd863f1771cbe..720666ae1bb69 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -467,7 +467,6 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) { DestroyShell(std::move(shell)); } -// chris TEST_F(ShellTest, ExternalEmbedderEndFrameIsCalledWhenPostPrerollResultIsResubmit) { auto settings = CreateSettingsForFixture(); From 640f2890050327f74c1f6bafa6a8fa038d56ca34 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 7 Apr 2020 15:17:32 -0700 Subject: [PATCH 17/24] review fixes --- shell/common/rasterizer.cc | 3 +++ shell/platform/darwin/ios/ios_surface.mm | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 24eb70103e375..a2d254660d5a4 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -140,6 +140,9 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { // thread. if (raster_status == RasterStatus::kResubmit) { auto* external_view_embedder = surface_->GetExternalViewEmbedder(); + // We know only the `external_view_embedder` can + // causes|RasterStatus::kResubmit|. Check to make sure. + FML_DCheck(external_view_embedder != nullptr); external_view_embedder->EndFrame(raster_thread_merger_); } diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index 3eebf666e78ba..42fffa2a5c384 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -91,6 +91,9 @@ bool IsIosEmbeddedViewsPreviewEnabled() { // Committing the current transaction as |BeginFrame| will create a nested // CATransaction otherwise. if ([NSThread currentThread] == [NSThread mainThread]) { + // The only time we need to commit the `CATranscation` is when + // there are platform views in the scene, which has to be run on the + // main thread. [CATransaction commit]; } } @@ -101,6 +104,9 @@ bool IsIosEmbeddedViewsPreviewEnabled() { FML_CHECK(platform_views_controller_ != nullptr); platform_views_controller_->SetFrameSize(frame_size); if ([NSThread currentThread] == [NSThread mainThread]) { + // The only time we need to commit the `CATranscation` is when + // there are platform views in the scene, which has to be run on the + // main thread. [CATransaction begin]; } } @@ -157,6 +163,9 @@ bool IsIosEmbeddedViewsPreviewEnabled() { if ([NSThread currentThread] != [NSThread mainThread]) { return; } + // The only time we need to commit the `CATranscation` is when + // there are platform views in the scene, which has to be run on the + // main thread. [CATransaction commit]; } } // namespace flutter From d3649027c56a50b8b9cc08320abb8519c4a66dd8 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 7 Apr 2020 15:47:54 -0700 Subject: [PATCH 18/24] review fixes --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index eb70e9c51fc21..8c606b17e4cec 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -472,11 +472,18 @@ if (merge_threads_) { // Threads are about to be merged, we drop everything from this frame // and possibly resubmit the same layer tree in the next frame. + // Before merging thread, we know the code is not running on the main thread. Assert that + FML_DCHECK([NSThread currentThread] != [NSThread mainThread]) picture_recorders_.clear(); composition_order_.clear(); return true; } + // Any UIKit related code has to run on main thread. + // When on a non-main thread, we only allow the rest of the method to run if there is no platform + // view. + FML_DCHECK([NSThread currentThread] == [NSThread mainThread] || views_to_dispose_.empty()) + DisposeViews(); // Resolve all pending GPU operations before allocating a new surface. From aa4064e4fe2b2c68f3696b4c114b3e2d8618f873 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 7 Apr 2020 15:58:15 -0700 Subject: [PATCH 19/24] review fixes --- shell/common/rasterizer.cc | 2 +- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 6 +++--- shell/platform/darwin/ios/ios_surface.mm | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index a2d254660d5a4..132e6a91815e4 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -142,7 +142,7 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { auto* external_view_embedder = surface_->GetExternalViewEmbedder(); // We know only the `external_view_embedder` can // causes|RasterStatus::kResubmit|. Check to make sure. - FML_DCheck(external_view_embedder != nullptr); + FML_DCHECK(external_view_embedder != nullptr); external_view_embedder->EndFrame(raster_thread_merger_); } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 8c606b17e4cec..a42ab260c12c3 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -473,7 +473,7 @@ // Threads are about to be merged, we drop everything from this frame // and possibly resubmit the same layer tree in the next frame. // Before merging thread, we know the code is not running on the main thread. Assert that - FML_DCHECK([NSThread currentThread] != [NSThread mainThread]) + FML_DCHECK(![[NSThread currentThread] isMainThread]) picture_recorders_.clear(); composition_order_.clear(); return true; @@ -482,7 +482,7 @@ // Any UIKit related code has to run on main thread. // When on a non-main thread, we only allow the rest of the method to run if there is no platform // view. - FML_DCHECK([NSThread currentThread] == [NSThread mainThread] || views_to_dispose_.empty()) + FML_DCHECK([[NSThread currentThread] isMainThread] || views_to_dispose_.empty()) DisposeViews(); @@ -666,7 +666,7 @@ return; } - FML_DCHECK([NSThread currentThread] == [NSThread mainThread]); + FML_DCHECK([[NSThread currentThread] isMainThread]); for (int64_t viewId : views_to_dispose_) { UIView* root_view = root_views_[viewId].get(); diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index 42fffa2a5c384..ebba65dd05324 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -90,7 +90,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() { platform_views_controller_->CancelFrame(); // Committing the current transaction as |BeginFrame| will create a nested // CATransaction otherwise. - if ([NSThread currentThread] == [NSThread mainThread]) { + if ([NSThread currentThread] isMainThread]) { // The only time we need to commit the `CATranscation` is when // there are platform views in the scene, which has to be run on the // main thread. @@ -103,7 +103,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() { TRACE_EVENT0("flutter", "IOSSurface::BeginFrame"); FML_CHECK(platform_views_controller_ != nullptr); platform_views_controller_->SetFrameSize(frame_size); - if ([NSThread currentThread] == [NSThread mainThread]) { + if ([NSThread currentThread] isMainThread]) { // The only time we need to commit the `CATranscation` is when // there are platform views in the scene, which has to be run on the // main thread. @@ -160,7 +160,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() { // |ExternalViewEmbedder| void IOSSurface::FinishFrame() { TRACE_EVENT0("flutter", "IOSSurface::DidSubmitFrame"); - if ([NSThread currentThread] != [NSThread mainThread]) { + if (![NSThread currentThread] isMainThread]) { return; } // The only time we need to commit the `CATranscation` is when From 203840918ab019af4f64685f987829a6af40a030 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 7 Apr 2020 16:37:50 -0700 Subject: [PATCH 20/24] review fixes --- shell/common/shell_test_platform_view.cc | 3 +-- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 6 +++--- .../ios/framework/Source/FlutterPlatformViews_Internal.h | 2 +- shell/platform/darwin/ios/ios_surface.h | 2 +- shell/platform/darwin/ios/ios_surface.mm | 8 ++++---- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/shell/common/shell_test_platform_view.cc b/shell/common/shell_test_platform_view.cc index 29bd24d3134ff..624dd443f7aec 100644 --- a/shell/common/shell_test_platform_view.cc +++ b/shell/common/shell_test_platform_view.cc @@ -35,8 +35,7 @@ std::unique_ptr ShellTestPlatformView::Create( #ifdef SHELL_ENABLE_VULKAN case BackendType::kVulkanBackend: return std::make_unique( - delegate, task_runners, vsync_clock, create_vsync_waiter, - shell_test_external_view_embedder); + delegate, task_runners, vsync_clock, create_vsync_waiter); #endif // SHELL_ENABLE_VULKAN default: FML_LOG(FATAL) << "No backends supported for ShellTestPlatformView"; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index a42ab260c12c3..2c2a91f4836a6 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -473,7 +473,7 @@ // Threads are about to be merged, we drop everything from this frame // and possibly resubmit the same layer tree in the next frame. // Before merging thread, we know the code is not running on the main thread. Assert that - FML_DCHECK(![[NSThread currentThread] isMainThread]) + FML_DCHECK(![[NSThread currentThread] isMainThread]); picture_recorders_.clear(); composition_order_.clear(); return true; @@ -482,7 +482,7 @@ // Any UIKit related code has to run on main thread. // When on a non-main thread, we only allow the rest of the method to run if there is no platform // view. - FML_DCHECK([[NSThread currentThread] isMainThread] || views_to_dispose_.empty()) + FML_DCHECK([[NSThread currentThread] isMainThread] || views_to_dispose_.empty()); DisposeViews(); @@ -595,7 +595,7 @@ } void FlutterPlatformViewsController::EndFrame( - fml::RefPtr raster_thread_merger) { + fml::RefPtr raster_thread_merger) { if (merge_threads_) { raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); merge_threads_ = false; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index e97720a711fd5..6532be8ca14c2 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -168,7 +168,7 @@ class FlutterPlatformViewsController { // Invoked at the very end of a frame. // After invoking this method, nothing should happen on the current TaskRunner during the same // frame. - void EndFrame(fml::RefPtr raster_thread_merger); + void EndFrame(fml::RefPtr raster_thread_merger); void OnMethodCall(FlutterMethodCall* call, FlutterResult& result); diff --git a/shell/platform/darwin/ios/ios_surface.h b/shell/platform/darwin/ios/ios_surface.h index ed890c7687d33..eae39031d77ce 100644 --- a/shell/platform/darwin/ios/ios_surface.h +++ b/shell/platform/darwin/ios/ios_surface.h @@ -84,7 +84,7 @@ class IOSSurface : public ExternalViewEmbedder { void FinishFrame() override; // |ExternalViewEmbedder| - void EndFrame(fml::RefPtr raster_thread_merger) override; + void EndFrame(fml::RefPtr raster_thread_merger) override; public: FML_DISALLOW_COPY_AND_ASSIGN(IOSSurface); diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index ebba65dd05324..5f2725273e0dd 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -90,7 +90,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() { platform_views_controller_->CancelFrame(); // Committing the current transaction as |BeginFrame| will create a nested // CATransaction otherwise. - if ([NSThread currentThread] isMainThread]) { + if ([[NSThread currentThread] isMainThread]) { // The only time we need to commit the `CATranscation` is when // there are platform views in the scene, which has to be run on the // main thread. @@ -103,7 +103,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() { TRACE_EVENT0("flutter", "IOSSurface::BeginFrame"); FML_CHECK(platform_views_controller_ != nullptr); platform_views_controller_->SetFrameSize(frame_size); - if ([NSThread currentThread] isMainThread]) { + if ([[NSThread currentThread] isMainThread]) { // The only time we need to commit the `CATranscation` is when // there are platform views in the scene, which has to be run on the // main thread. @@ -151,7 +151,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() { } // |ExternalViewEmbedder| -void IOSSurface::EndFrame(fml::RefPtr raster_thread_merger) { +void IOSSurface::EndFrame(fml::RefPtr raster_thread_merger) { TRACE_EVENT0("flutter", "IOSSurface::EndFrame"); FML_CHECK(platform_views_controller_ != nullptr); return platform_views_controller_->EndFrame(raster_thread_merger); @@ -160,7 +160,7 @@ bool IsIosEmbeddedViewsPreviewEnabled() { // |ExternalViewEmbedder| void IOSSurface::FinishFrame() { TRACE_EVENT0("flutter", "IOSSurface::DidSubmitFrame"); - if (![NSThread currentThread] isMainThread]) { + if (![[NSThread currentThread] isMainThread]) { return; } // The only time we need to commit the `CATranscation` is when From 49cd78f74219dbcacaf6243003b80876aab6c4e3 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 8 Apr 2020 10:31:55 -0700 Subject: [PATCH 21/24] gn format --- shell/common/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index b849e8675e315..3fab3317b094b 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -191,8 +191,8 @@ if (enable_unittests) { "pipeline_unittests.cc", "shell_test.cc", "shell_test.h", - "shell_test_external_view_embedder.h", "shell_test_external_view_embedder.cc", + "shell_test_external_view_embedder.h", "shell_test_platform_view.cc", "shell_test_platform_view.h", "shell_unittests.cc", From 4b6362504825f51036a0797fa647ab5b6d13220f Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 8 Apr 2020 11:53:39 -0700 Subject: [PATCH 22/24] license golden --- ci/licenses_golden/licenses_flutter | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 08d65c11b2a61..398966f4019d4 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -592,6 +592,8 @@ FILE: ../../../flutter/shell/common/shell_io_manager.cc FILE: ../../../flutter/shell/common/shell_io_manager.h FILE: ../../../flutter/shell/common/shell_test.cc FILE: ../../../flutter/shell/common/shell_test.h +FILE: ../../../flutter/shell/common/shell_test_external_view_embedder.cc +FILE: ../../../flutter/shell/common/shell_test_external_view_embedder.h FILE: ../../../flutter/shell/common/shell_test_platform_view.cc FILE: ../../../flutter/shell/common/shell_test_platform_view.h FILE: ../../../flutter/shell/common/shell_test_platform_view_gl.cc From bf067abf63b6d241116401926a89fe1ea23cbfd9 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 8 Apr 2020 15:25:10 -0700 Subject: [PATCH 23/24] add external view embedder in fuschia tests --- shell/common/shell_test_platform_view.cc | 3 ++- .../common/shell_test_platform_view_vulkan.cc | 19 +++++++++++++++---- .../common/shell_test_platform_view_vulkan.h | 16 ++++++++++++++-- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/shell/common/shell_test_platform_view.cc b/shell/common/shell_test_platform_view.cc index 624dd443f7aec..29bd24d3134ff 100644 --- a/shell/common/shell_test_platform_view.cc +++ b/shell/common/shell_test_platform_view.cc @@ -35,7 +35,8 @@ std::unique_ptr ShellTestPlatformView::Create( #ifdef SHELL_ENABLE_VULKAN case BackendType::kVulkanBackend: return std::make_unique( - delegate, task_runners, vsync_clock, create_vsync_waiter); + delegate, task_runners, vsync_clock, create_vsync_waiter, + shell_test_external_view_embedder); #endif // SHELL_ENABLE_VULKAN default: FML_LOG(FATAL) << "No backends supported for ShellTestPlatformView"; diff --git a/shell/common/shell_test_platform_view_vulkan.cc b/shell/common/shell_test_platform_view_vulkan.cc index e9f41484d9881..1c234a44b9767 100644 --- a/shell/common/shell_test_platform_view_vulkan.cc +++ b/shell/common/shell_test_platform_view_vulkan.cc @@ -11,11 +11,14 @@ ShellTestPlatformViewVulkan::ShellTestPlatformViewVulkan( PlatformView::Delegate& delegate, TaskRunners task_runners, std::shared_ptr vsync_clock, - CreateVsyncWaiter create_vsync_waiter) + CreateVsyncWaiter create_vsync_waiter, + std::shared_ptr + shell_test_external_view_embedder) : ShellTestPlatformView(delegate, std::move(task_runners)), create_vsync_waiter_(std::move(create_vsync_waiter)), vsync_clock_(vsync_clock), - proc_table_(fml::MakeRefCounted()) {} + proc_table_(fml::MakeRefCounted()), + shell_test_external_view_embedder_(shell_test_external_view_embedder) {} ShellTestPlatformViewVulkan::~ShellTestPlatformViewVulkan() = default; @@ -29,7 +32,8 @@ void ShellTestPlatformViewVulkan::SimulateVSync() { // |PlatformView| std::unique_ptr ShellTestPlatformViewVulkan::CreateRenderingSurface() { - return std::make_unique(proc_table_); + return std::make_unique(proc_table_, + shell_test_external_view_embedder_); } // |PlatformView| @@ -45,7 +49,9 @@ PointerDataDispatcherMaker ShellTestPlatformViewVulkan::GetDispatcherMaker() { // https://github.com/flutter/flutter/issues/51132 ShellTestPlatformViewVulkan::OffScreenSurface::OffScreenSurface( fml::RefPtr vk) - : valid_(false), vk_(std::move(vk)) { + : valid_(false), + vk_(std::move(vk)), + shell_test_external_view_embedder_(shell_test_external_view_embedder) { if (!vk_ || !vk_->HasAcquiredMandatoryProcAddresses()) { FML_DLOG(ERROR) << "Proc table has not acquired mandatory proc addresses."; return; @@ -170,5 +176,10 @@ SkMatrix ShellTestPlatformViewVulkan::OffScreenSurface::GetRootTransformation() return matrix; } +flutter::ExternalViewEmbedder* +ShellTestPlatformViewVulkan::OffScreenSurface::GetExternalViewEmbedder() { + return shell_test_external_view_embedder_.get(); +} + } // namespace testing } // namespace flutter diff --git a/shell/common/shell_test_platform_view_vulkan.h b/shell/common/shell_test_platform_view_vulkan.h index 38bce87fa5414..31757f39a30b0 100644 --- a/shell/common/shell_test_platform_view_vulkan.h +++ b/shell/common/shell_test_platform_view_vulkan.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_VULKAN_H_ #define FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_VULKAN_H_ +#include "flutter/shell/common/shell_test_external_view_embedder.h" #include "flutter/shell/common/shell_test_platform_view.h" #include "flutter/shell/gpu/gpu_surface_vulkan_delegate.h" #include "flutter/vulkan/vulkan_application.h" @@ -18,7 +19,9 @@ class ShellTestPlatformViewVulkan : public ShellTestPlatformView { ShellTestPlatformViewVulkan(PlatformView::Delegate& delegate, TaskRunners task_runners, std::shared_ptr vsync_clock, - CreateVsyncWaiter create_vsync_waiter); + CreateVsyncWaiter create_vsync_waiter, + std::shared_ptr + shell_test_external_view_embedder); ~ShellTestPlatformViewVulkan() override; @@ -27,7 +30,9 @@ class ShellTestPlatformViewVulkan : public ShellTestPlatformView { private: class OffScreenSurface : public flutter::Surface { public: - OffScreenSurface(fml::RefPtr vk); + OffScreenSurface(fml::RefPtr vk, + std::shared_ptr + shell_test_external_view_embedder); ~OffScreenSurface() override; @@ -42,9 +47,13 @@ class ShellTestPlatformViewVulkan : public ShellTestPlatformView { // |Surface| GrContext* GetContext() override; + flutter::ExternalViewEmbedder* GetExternalViewEmbedder() override; + private: bool valid_; fml::RefPtr vk_; + std::shared_ptr + shell_test_external_view_embedder_; std::unique_ptr application_; std::unique_ptr logical_device_; sk_sp context_; @@ -61,6 +70,9 @@ class ShellTestPlatformViewVulkan : public ShellTestPlatformView { fml::RefPtr proc_table_; + std::shared_ptr + shell_test_external_view_embedder_; + // |PlatformView| std::unique_ptr CreateRenderingSurface() override; From c23e5f7ecc7b35463a362b81281f5f9a7d058c13 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 8 Apr 2020 16:22:51 -0700 Subject: [PATCH 24/24] fix complier error --- shell/common/shell_test_platform_view_vulkan.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/shell/common/shell_test_platform_view_vulkan.cc b/shell/common/shell_test_platform_view_vulkan.cc index 1c234a44b9767..4c21c2dfba36e 100644 --- a/shell/common/shell_test_platform_view_vulkan.cc +++ b/shell/common/shell_test_platform_view_vulkan.cc @@ -48,7 +48,9 @@ PointerDataDispatcherMaker ShellTestPlatformViewVulkan::GetDispatcherMaker() { // We need to merge this functionality back into //vulkan. // https://github.com/flutter/flutter/issues/51132 ShellTestPlatformViewVulkan::OffScreenSurface::OffScreenSurface( - fml::RefPtr vk) + fml::RefPtr vk, + std::shared_ptr + shell_test_external_view_embedder) : valid_(false), vk_(std::move(vk)), shell_test_external_view_embedder_(shell_test_external_view_embedder) {