Fix pipeline library issue related to not tracking pending futures.

egramond · Apr 27, 2022 · 86c91dc · 86c91dc
1 parent 0b63e2c
commit 86c91dc
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 28 deletions.
diff --git a/impeller/display_list/display_list_dispatcher.cc b/impeller/display_list/display_list_dispatcher.cc
@@ -473,4 +473,8 @@ void DisplayListDispatcher::drawShadow(const SkPath& path,
   UNIMPLEMENTED;
 }
 
+Picture DisplayListDispatcher::EndRecordingAsPicture() {
+  return canvas_.EndRecordingAsPicture();
+}
+
 }  // namespace impeller
diff --git a/impeller/display_list/display_list_dispatcher.h b/impeller/display_list/display_list_dispatcher.h
@@ -18,6 +18,8 @@ class DisplayListDispatcher final : public flutter::Dispatcher {
 
   ~DisplayListDispatcher();
 
+  Picture EndRecordingAsPicture();
+
   // |flutter::Dispatcher|
   void setAntiAlias(bool aa) override;
 

diff --git a/impeller/entity/content_context.cc b/impeller/entity/content_context.cc
@@ -4,6 +4,8 @@
 
 #include "impeller/entity/content_context.h"
 
+#include <sstream>
+
 namespace impeller {
 
 ContentContext::ContentContext(std::shared_ptr<Context> context)
@@ -23,6 +25,7 @@ ContentContext::ContentContext(std::shared_ptr<Context> context)
   // Pipelines that are variants of the base pipelines with custom descriptors.
   if (auto solid_fill_pipeline = solid_fill_pipelines_[{}]->WaitAndGet()) {
     auto clip_pipeline_descriptor = solid_fill_pipeline->GetDescriptor();
+    clip_pipeline_descriptor.SetLabel("Clip Pipeline");
     // Write to the stencil buffer.
     StencilAttachmentDescriptor stencil0;
     stencil0.stencil_compare = CompareFunction::kGreaterEqual;

diff --git a/impeller/entity/content_context.h b/impeller/entity/content_context.h
@@ -111,14 +111,16 @@ class ContentContext {
       return found->second->WaitAndGet();
     }
 
-    auto found = container.find({});
+    auto prototype = container.find({});
 
     // The prototype must always be initialized in the constructor.
-    FML_CHECK(found != container.end());
+    FML_CHECK(prototype != container.end());
 
-    auto variant_future = found->second->WaitAndGet()->CreateVariant(
-        [&opts](PipelineDescriptor& desc) {
+    auto variant_future = prototype->second->WaitAndGet()->CreateVariant(
+        [&opts, variants_count = container.size()](PipelineDescriptor& desc) {
           ApplyOptionsToDescriptor(desc, opts);
+          desc.SetLabel(
+              SPrintF("%s V#%zu", desc.GetLabel().c_str(), variants_count));
         });
     auto variant = std::make_unique<TypedPipeline>(std::move(variant_future));
     auto variant_pipeline = variant->WaitAndGet();

diff --git a/impeller/renderer/backend/metal/pipeline_library_mtl.h b/impeller/renderer/backend/metal/pipeline_library_mtl.h
@@ -26,18 +26,16 @@ class PipelineLibraryMTL final : public PipelineLibrary {
  private:
   friend ContextMTL;
 
-  using Pipelines = std::unordered_map<PipelineDescriptor,
-                                       std::shared_ptr<const Pipeline>,
-                                       ComparableHash<PipelineDescriptor>,
-                                       ComparableEqual<PipelineDescriptor>>;
+  using Pipelines =
+      std::unordered_map<PipelineDescriptor,
+                         std::shared_future<std::shared_ptr<Pipeline>>,
+                         ComparableHash<PipelineDescriptor>,
+                         ComparableEqual<PipelineDescriptor>>;
   id<MTLDevice> device_ = nullptr;
   Pipelines pipelines_;
 
   PipelineLibraryMTL(id<MTLDevice> device);
 
-  void SavePipeline(PipelineDescriptor descriptor,
-                    std::shared_ptr<const Pipeline> pipeline);
-
   // |PipelineLibrary|
   PipelineFuture GetRenderPipeline(PipelineDescriptor descriptor) override;
 

diff --git a/impeller/renderer/backend/metal/pipeline_library_mtl.mm b/impeller/renderer/backend/metal/pipeline_library_mtl.mm
@@ -67,19 +67,16 @@
   return [device newDepthStencilStateWithDescriptor:descriptor];
 }
 
-std::future<std::shared_ptr<Pipeline>> PipelineLibraryMTL::GetRenderPipeline(
+PipelineFuture PipelineLibraryMTL::GetRenderPipeline(
     PipelineDescriptor descriptor) {
-  auto promise = std::make_shared<std::promise<std::shared_ptr<Pipeline>>>();
-  auto future = promise->get_future();
   if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) {
-    promise->set_value(nullptr);
-    return future;
+    return found->second;
   }
 
-  // TODO(csg): There is a bug here where multiple calls to GetRenderPipeline
-  // will result in multiple render pipelines of the same descriptor being
-  // created till the first instance of the creation invokes its completion
-  // callback.
+  auto promise = std::make_shared<std::promise<std::shared_ptr<Pipeline>>>();
+  auto future = PipelineFuture{promise->get_future()};
+
+  pipelines_[descriptor] = future;
 
   auto weak_this = weak_from_this();
 
@@ -100,25 +97,19 @@
           promise->set_value(nullptr);
           return;
         }
+
         auto new_pipeline = std::shared_ptr<PipelineMTL>(new PipelineMTL(
             weak_this,
             descriptor,                                        //
             render_pipeline_state,                             //
             CreateDepthStencilDescriptor(descriptor, device_)  //
             ));
         promise->set_value(new_pipeline);
-        this->SavePipeline(descriptor, new_pipeline);
       };
   [device_ newRenderPipelineStateWithDescriptor:GetMTLRenderPipelineDescriptor(
                                                     descriptor)
                               completionHandler:completion_handler];
   return future;
 }
 
-void PipelineLibraryMTL::SavePipeline(
-    PipelineDescriptor descriptor,
-    std::shared_ptr<const Pipeline> pipeline) {
-  pipelines_[descriptor] = std::move(pipeline);
-}
-
 }  // namespace impeller
diff --git a/impeller/renderer/pipeline.h b/impeller/renderer/pipeline.h
@@ -21,7 +21,7 @@ class Pipeline;
 // would be a concurrency pessimization.
 //
 // Use a struct that stores the future and the descriptor separately.
-using PipelineFuture = std::future<std::shared_ptr<Pipeline>>;
+using PipelineFuture = std::shared_future<std::shared_ptr<Pipeline>>;
 
 //------------------------------------------------------------------------------
 /// @brief      Describes the fixed function and programmable aspects of
@@ -48,6 +48,13 @@ class Pipeline {
 
   virtual bool IsValid() const = 0;
 
+  //----------------------------------------------------------------------------
+  /// @brief      Get the descriptor that was responsible for creating this
+  ///             pipeline. It may be copied and modified to create a pipeline
+  ///             variant.
+  ///
+  /// @return     The descriptor.
+  ///
   const PipelineDescriptor& GetDescriptor() const;
 
   PipelineFuture CreateVariant(