Only clear GL context after changing the thread configuration (flutter#20965)

Author: Emmanuel Garcia

fml/raster_thread_merger.cc

@@ -21,6 +21,10 @@ RasterThreadMerger::RasterThreadMerger(fml::TaskQueueId platform_queue_id,
   FML_CHECK(!task_queues_->Owns(platform_queue_id_, gpu_queue_id_));
 }
 
+void RasterThreadMerger::SetMergeUnmergeCallback(const fml::closure& callback) {
+  merge_unmerge_callback_ = callback;
+}
+
 void RasterThreadMerger::MergeWithLease(size_t lease_term) {
   std::scoped_lock lock(lease_term_mutex_);
   if (TaskQueuesAreSame()) {
@@ -30,11 +34,19 @@ void RasterThreadMerger::MergeWithLease(size_t lease_term) {
     return;
   }
   FML_DCHECK(lease_term > 0) << "lease_term should be positive.";
-  if (!IsMergedUnSafe()) {
-    bool success = task_queues_->Merge(platform_queue_id_, gpu_queue_id_);
-    FML_CHECK(success) << "Unable to merge the raster and platform threads.";
-    lease_term_ = lease_term;
+
+  if (IsMergedUnSafe()) {
+    merged_condition_.notify_one();
+    return;
+  }
+
+  bool success = task_queues_->Merge(platform_queue_id_, gpu_queue_id_);
+  if (success && merge_unmerge_callback_ != nullptr) {
+    merge_unmerge_callback_();
   }
+  FML_CHECK(success) << "Unable to merge the raster and platform threads.";
+  lease_term_ = lease_term;
+
   merged_condition_.notify_one();
 }
 
@@ -48,6 +60,9 @@ void RasterThreadMerger::UnMergeNow() {
   }
   lease_term_ = 0;
   bool success = task_queues_->Unmerge(platform_queue_id_);
+  if (success && merge_unmerge_callback_ != nullptr) {
+    merge_unmerge_callback_();
+  }
   FML_CHECK(success) << "Unable to un-merge the raster and platform threads.";
 }
 

fml/raster_thread_merger.h

@@ -85,6 +85,13 @@ class RasterThreadMerger
   // noop.
   bool IsEnabled();
 
+  // Registers a callback that can be used to clean up global state right after
+  // the thread configuration has changed.
+  //
+  // For example, it can be used to clear the GL context so it can be used in
+  // the next task from a different thread.
+  void SetMergeUnmergeCallback(const fml::closure& callback);
+
  private:
   static const int kLeaseNotSet;
   fml::TaskQueueId platform_queue_id_;
@@ -93,6 +100,7 @@ class RasterThreadMerger
   std::atomic_int lease_term_;
   std::condition_variable merged_condition_;
   std::mutex lease_term_mutex_;
+  fml::closure merge_unmerge_callback_;
   bool enabled_;
 
   bool IsMergedUnSafe() const;

fml/raster_thread_merger_unittests.cc

@@ -579,5 +579,53 @@ TEST(RasterThreadMerger, RunExpiredTasksWhileFirstTaskUnMergesThreads) {
   thread_raster.join();
 }
 
+TEST(RasterThreadMerger, SetMergeUnmergeCallback) {
+  fml::MessageLoop* loop1 = nullptr;
+  fml::AutoResetWaitableEvent latch1;
+  fml::AutoResetWaitableEvent term1;
+  std::thread thread1([&loop1, &latch1, &term1]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    loop1 = &fml::MessageLoop::GetCurrent();
+    latch1.Signal();
+    term1.Wait();
+  });
+
+  fml::MessageLoop* loop2 = nullptr;
+  fml::AutoResetWaitableEvent latch2;
+  fml::AutoResetWaitableEvent term2;
+  std::thread thread2([&loop2, &latch2, &term2]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    loop2 = &fml::MessageLoop::GetCurrent();
+    latch2.Signal();
+    term2.Wait();
+  });
+
+  latch1.Wait();
+  latch2.Wait();
+
+  fml::TaskQueueId qid1 = loop1->GetTaskRunner()->GetTaskQueueId();
+  fml::TaskQueueId qid2 = loop2->GetTaskRunner()->GetTaskQueueId();
+
+  const auto raster_thread_merger =
+      fml::MakeRefCounted<fml::RasterThreadMerger>(qid1, qid2);
+
+  int callbacks = 0;
+  raster_thread_merger->SetMergeUnmergeCallback(
+      [&callbacks]() { callbacks++; });
+
+  ASSERT_EQ(0, callbacks);
+
+  raster_thread_merger->MergeWithLease(1);
+  ASSERT_EQ(1, callbacks);
+
+  raster_thread_merger->DecrementLease();
+  ASSERT_EQ(2, callbacks);
+
+  term1.Signal();
+  term2.Signal();
+  thread1.join();
+  thread2.join();
+}
+
 }  // namespace testing
 }  // namespace fml

shell/common/rasterizer.cc

@@ -89,6 +89,12 @@ void Rasterizer::Setup(std::unique_ptr<Surface> surface) {
         delegate_.GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId();
     raster_thread_merger_ =
         fml::MakeRefCounted<fml::RasterThreadMerger>(platform_id, gpu_id);
+    raster_thread_merger_->SetMergeUnmergeCallback([=]() {
+      // Clear the GL context after the thread configuration has changed.
+      if (surface_) {
+        surface_->ClearRenderContext();
+      }
+    });
   }
 #endif
 }
@@ -102,6 +108,7 @@ void Rasterizer::Teardown() {
       raster_thread_merger_.get()->IsMerged()) {
     FML_DCHECK(raster_thread_merger_->IsEnabled());
     raster_thread_merger_->UnMergeNow();
+    raster_thread_merger_->SetMergeUnmergeCallback(nullptr);
   }
 }
 
@@ -467,14 +474,6 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
 
     FireNextFrameCallbackIfPresent();
 
-    // Clear the render context after submitting the frame.
-    // This ensures that the GL context is released after drawing to the
-    // surface.
-    //
-    // The GL context must be clear before performing Gr context deferred
-    // cleanup.
-    surface_->ClearRenderContext();
-
     if (surface_->GetContext()) {
       TRACE_EVENT0("flutter", "PerformDeferredSkiaCleanup");
       surface_->GetContext()->performDeferredCleanup(kSkiaCleanupExpiration);