diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 81a34dbb7c256..dbc6fec50527e 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -219,11 +219,9 @@ ContentContext::ContentContext(std::shared_ptr context) yuv_to_rgb_filter_pipelines_[{}] = CreateDefaultPipeline(*context_); - // Pipelines that are variants of the base pipelines with custom descriptors. - // TODO(98684): Rework this API to allow fetching the descriptor without - // waiting for the pipeline to build. - if (auto solid_fill_pipeline = solid_fill_pipelines_[{}]->WaitAndGet()) { - auto clip_pipeline_descriptor = solid_fill_pipeline->GetDescriptor(); + if (solid_fill_pipelines_[{}]->GetDescriptor().has_value()) { + auto clip_pipeline_descriptor = + solid_fill_pipelines_[{}]->GetDescriptor().value(); clip_pipeline_descriptor.SetLabel("Clip Pipeline"); // Disable write to all color attachments. auto color_attachments = diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc index 4cd1de4225b6d..21ef0193d197c 100644 --- a/impeller/entity/contents/runtime_effect_contents.cc +++ b/impeller/entity/contents/runtime_effect_contents.cc @@ -126,7 +126,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, options.primitive_type = geometry_result.type; options.ApplyToPipelineDescriptor(desc); - auto pipeline = context->GetPipelineLibrary()->GetPipeline(desc).get(); + auto pipeline = context->GetPipelineLibrary()->GetPipeline(desc).Get(); if (!pipeline) { VALIDATION_LOG << "Failed to get or create runtime effect pipeline."; return false; diff --git a/impeller/playground/imgui/imgui_impl_impeller.cc b/impeller/playground/imgui/imgui_impl_impeller.cc index 74ea9d10b04aa..d93a7369e7524 100644 --- a/impeller/playground/imgui/imgui_impl_impeller.cc +++ b/impeller/playground/imgui/imgui_impl_impeller.cc @@ -102,7 +102,7 @@ bool ImGui_ImplImpeller_Init( } bd->pipeline = - context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get(); + context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); IM_ASSERT(bd->pipeline != nullptr && "Could not create ImGui pipeline."); bd->sampler = context->GetSamplerLibrary()->GetSampler({}); diff --git a/impeller/renderer/backend/gles/pipeline_library_gles.cc b/impeller/renderer/backend/gles/pipeline_library_gles.cc index 6bdeaba85d386..25fa0ab632304 100644 --- a/impeller/renderer/backend/gles/pipeline_library_gles.cc +++ b/impeller/renderer/backend/gles/pipeline_library_gles.cc @@ -180,8 +180,9 @@ PipelineFuture PipelineLibraryGLES::GetPipeline( } if (!reactor_) { - return RealizedFuture>>( - nullptr); + return { + descriptor, + RealizedFuture>>(nullptr)}; } auto vert_function = descriptor.GetEntrypointForStage(ShaderStage::kVertex); @@ -190,14 +191,16 @@ PipelineFuture PipelineLibraryGLES::GetPipeline( if (!vert_function || !frag_function) { VALIDATION_LOG << "Could not find stage entrypoint functions in pipeline descriptor."; - return RealizedFuture>>( - nullptr); + return { + descriptor, + RealizedFuture>>(nullptr)}; } auto promise = std::make_shared< std::promise>>>(); - auto future = PipelineFuture{promise->get_future()}; - pipelines_[descriptor] = future; + auto pipeline_future = + PipelineFuture{descriptor, promise->get_future()}; + pipelines_[descriptor] = pipeline_future; auto weak_this = weak_from_this(); auto result = reactor_->AddOperation( @@ -243,7 +246,7 @@ PipelineFuture PipelineLibraryGLES::GetPipeline( }); FML_CHECK(result); - return future; + return pipeline_future; } // |PipelineLibrary| @@ -251,11 +254,9 @@ PipelineFuture PipelineLibraryGLES::GetPipeline( ComputePipelineDescriptor descriptor) { auto promise = std::make_shared< std::promise>>>(); - auto future = - PipelineFuture{promise->get_future()}; // TODO(dnfield): implement compute for GLES. promise->set_value(nullptr); - return future; + return {descriptor, promise->get_future()}; } // |PipelineLibrary| diff --git a/impeller/renderer/backend/metal/pipeline_library_mtl.mm b/impeller/renderer/backend/metal/pipeline_library_mtl.mm index 8339bf4e1bd3d..85272ef16fad5 100644 --- a/impeller/renderer/backend/metal/pipeline_library_mtl.mm +++ b/impeller/renderer/backend/metal/pipeline_library_mtl.mm @@ -93,14 +93,16 @@ } if (!IsValid()) { - return RealizedFuture>>( - nullptr); + return { + descriptor, + RealizedFuture>>(nullptr)}; } auto promise = std::make_shared< std::promise>>>(); - auto future = PipelineFuture{promise->get_future()}; - pipelines_[descriptor] = future; + auto pipeline_future = + PipelineFuture{descriptor, promise->get_future()}; + pipelines_[descriptor] = pipeline_future; auto weak_this = weak_from_this(); auto completion_handler = @@ -132,7 +134,7 @@ [device_ newRenderPipelineStateWithDescriptor:GetMTLRenderPipelineDescriptor( descriptor) completionHandler:completion_handler]; - return future; + return pipeline_future; } PipelineFuture PipelineLibraryMTL::GetPipeline( @@ -143,15 +145,17 @@ } if (!IsValid()) { - return RealizedFuture>>( - nullptr); + return { + descriptor, + RealizedFuture>>( + nullptr)}; } auto promise = std::make_shared< std::promise>>>(); - auto future = - PipelineFuture{promise->get_future()}; - compute_pipelines_[descriptor] = future; + auto pipeline_future = PipelineFuture{ + descriptor, promise->get_future()}; + compute_pipelines_[descriptor] = pipeline_future; auto weak_this = weak_from_this(); auto completion_handler = @@ -185,7 +189,7 @@ new ComputePipelineMTL(weak_this, descriptor) options:MTLPipelineOptionNone completionHandler:completion_handler]; - return future; + return pipeline_future; } // |PipelineLibrary| diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index b282b4fdd9d38..6f29c25252f82 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -61,14 +61,16 @@ PipelineFuture PipelineLibraryVK::GetPipeline( } if (!IsValid()) { - return RealizedFuture>>( - nullptr); + return { + descriptor, + RealizedFuture>>(nullptr)}; } auto promise = std::make_shared< std::promise>>>(); - auto future = PipelineFuture{promise->get_future()}; - pipelines_[descriptor] = future; + auto pipeline_future = + PipelineFuture{descriptor, promise->get_future()}; + pipelines_[descriptor] = pipeline_future; auto weak_this = weak_from_this(); @@ -86,7 +88,7 @@ PipelineFuture PipelineLibraryVK::GetPipeline( weak_this, descriptor, std::move(pipeline_create_info))); }); - return future; + return pipeline_future; } // |PipelineLibrary| @@ -94,11 +96,9 @@ PipelineFuture PipelineLibraryVK::GetPipeline( ComputePipelineDescriptor descriptor) { auto promise = std::make_shared< std::promise>>>(); - auto future = - PipelineFuture{promise->get_future()}; // TODO(dnfield): implement compute for GLES. promise->set_value(nullptr); - return future; + return {descriptor, promise->get_future()}; } static vk::AttachmentDescription CreatePlaceholderAttachmentDescription( diff --git a/impeller/renderer/compute_pipeline_descriptor.h b/impeller/renderer/compute_pipeline_descriptor.h index 3b71d05f3b24e..582844db7dd3c 100644 --- a/impeller/renderer/compute_pipeline_descriptor.h +++ b/impeller/renderer/compute_pipeline_descriptor.h @@ -53,10 +53,4 @@ class ComputePipelineDescriptor final std::shared_ptr entrypoint_; }; -using ComputePipelineMap = std::unordered_map< - ComputePipelineDescriptor, - std::shared_future>>, - ComparableHash, - ComparableEqual>; - } // namespace impeller diff --git a/impeller/renderer/compute_unittests.cc b/impeller/renderer/compute_unittests.cc index 7017364903e82..5da7fe133ec6b 100644 --- a/impeller/renderer/compute_unittests.cc +++ b/impeller/renderer/compute_unittests.cc @@ -29,7 +29,7 @@ TEST_P(ComputeTest, CanCreateComputePass) { SamplePipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_desc.has_value()); auto compute_pipeline = - context->GetPipelineLibrary()->GetPipeline(pipeline_desc).get(); + context->GetPipelineLibrary()->GetPipeline(pipeline_desc).Get(); ASSERT_TRUE(compute_pipeline); auto cmd_buffer = context->CreateCommandBuffer(); diff --git a/impeller/renderer/pipeline.cc b/impeller/renderer/pipeline.cc index 4399b2707259d..a71b4bd97425c 100644 --- a/impeller/renderer/pipeline.cc +++ b/impeller/renderer/pipeline.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/renderer/pipeline.h" +#include #include "compute_pipeline_descriptor.h" #include "impeller/base/promise.h" @@ -24,8 +25,8 @@ PipelineFuture CreatePipelineFuture( const Context& context, std::optional desc) { if (!context.IsValid()) { - return RealizedFuture>>( - nullptr); + return {desc, RealizedFuture>>( + nullptr)}; } return context.GetPipelineLibrary()->GetPipeline(std::move(desc)); @@ -35,8 +36,10 @@ PipelineFuture CreatePipelineFuture( const Context& context, std::optional desc) { if (!context.IsValid()) { - return RealizedFuture>>( - nullptr); + return { + desc, + RealizedFuture>>( + nullptr)}; } return context.GetPipelineLibrary()->GetPipeline(std::move(desc)); @@ -51,7 +54,8 @@ template PipelineFuture Pipeline::CreateVariant( std::function descriptor_callback) const { if (!descriptor_callback) { - return RealizedFuture>>(nullptr); + return {std::nullopt, + RealizedFuture>>(nullptr)}; } auto copied_desc = desc_; @@ -62,7 +66,7 @@ PipelineFuture Pipeline::CreateVariant( if (!library) { VALIDATION_LOG << "The library from which this pipeline was created was " "already collected."; - return RealizedFuture>>(nullptr); + return {desc_, RealizedFuture>>(nullptr)}; } return library->GetPipeline(std::move(copied_desc)); diff --git a/impeller/renderer/pipeline.h b/impeller/renderer/pipeline.h index a38821a33f0a7..cd900b2f5dd47 100644 --- a/impeller/renderer/pipeline.h +++ b/impeller/renderer/pipeline.h @@ -20,14 +20,15 @@ class PipelineLibrary; template class Pipeline; -// TODO(csg): Using a simple future is sub-optimal since callers that want to -// eagerly create and cache pipeline variants will have to await on the future -// to get its pipeline descriptor (unless they have explicitly cached it). This -// would be a concurrency pessimization. -// -// Use a struct that stores the future and the descriptor separately. -template -using PipelineFuture = std::shared_future>>; +template +struct PipelineFuture { + std::optional descriptor; + std::shared_future>> future; + + const std::shared_ptr> Get() const { return future.get(); } + + bool IsValid() const { return future.valid(); } +}; //------------------------------------------------------------------------------ /// @brief Describes the fixed function and programmable aspects of @@ -107,12 +108,16 @@ class RenderPipelineT { return pipeline_; } did_wait_ = true; - if (pipeline_future_.valid()) { - pipeline_ = pipeline_future_.get(); + if (pipeline_future_.IsValid()) { + pipeline_ = pipeline_future_.Get(); } return pipeline_; } + std::optional GetDescriptor() const { + return pipeline_future_.descriptor; + } + private: PipelineFuture pipeline_future_; std::shared_ptr> pipeline_; @@ -145,8 +150,8 @@ class ComputePipelineT { return pipeline_; } did_wait_ = true; - if (pipeline_future_.valid()) { - pipeline_ = pipeline_future_.get(); + if (pipeline_future_.IsValid()) { + pipeline_ = pipeline_future_.Get(); } return pipeline_; } diff --git a/impeller/renderer/pipeline_descriptor.h b/impeller/renderer/pipeline_descriptor.h index 5ab8fb67fd082..7b1ed5aa00a18 100644 --- a/impeller/renderer/pipeline_descriptor.h +++ b/impeller/renderer/pipeline_descriptor.h @@ -138,10 +138,4 @@ class PipelineDescriptor final : public Comparable { PrimitiveType primitive_type_ = PrimitiveType::kTriangle; }; -using PipelineMap = std::unordered_map< - PipelineDescriptor, - std::shared_future>>, - ComparableHash, - ComparableEqual>; - } // namespace impeller diff --git a/impeller/renderer/pipeline_library.cc b/impeller/renderer/pipeline_library.cc index 8b0732d6cb019..816d95508a2a9 100644 --- a/impeller/renderer/pipeline_library.cc +++ b/impeller/renderer/pipeline_library.cc @@ -18,7 +18,7 @@ PipelineFuture PipelineLibrary::GetPipeline( auto promise = std::make_shared< std::promise>>>(); promise->set_value(nullptr); - return promise->get_future(); + return {descriptor, promise->get_future()}; } PipelineFuture PipelineLibrary::GetPipeline( @@ -29,7 +29,7 @@ PipelineFuture PipelineLibrary::GetPipeline( auto promise = std::make_shared< std::promise>>>(); promise->set_value(nullptr); - return promise->get_future(); + return {descriptor, promise->get_future()}; } } // namespace impeller diff --git a/impeller/renderer/pipeline_library.h b/impeller/renderer/pipeline_library.h index a2ad4b304c1fe..ff53b8a0dee06 100644 --- a/impeller/renderer/pipeline_library.h +++ b/impeller/renderer/pipeline_library.h @@ -15,6 +15,17 @@ namespace impeller { class Context; +using PipelineMap = std::unordered_map, + ComparableHash, + ComparableEqual>; + +using ComputePipelineMap = + std::unordered_map, + ComparableHash, + ComparableEqual>; + class PipelineLibrary : public std::enable_shared_from_this { public: virtual ~PipelineLibrary(); diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 26220c7e76b12..cfd3637f9a8c2 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -55,7 +55,7 @@ TEST_P(RendererTest, CanCreateBoxPrimitive) { ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); auto box_pipeline = - context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get(); + context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(box_pipeline); // Vertex buffer. @@ -119,7 +119,7 @@ TEST_P(RendererTest, CanRenderPerspectiveCube) { desc->SetCullMode(CullMode::kBackFace); desc->SetSampleCount(SampleCount::kCount4); auto pipeline = - context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get(); + context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(pipeline); struct Cube { @@ -208,7 +208,7 @@ TEST_P(RendererTest, CanRenderMultiplePrimitives) { ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); auto box_pipeline = - context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get(); + context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(box_pipeline); // Vertex buffer. @@ -279,7 +279,7 @@ TEST_P(RendererTest, CanRenderToTexture) { BoxPipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_desc.has_value()); auto box_pipeline = - context->GetPipelineLibrary()->GetPipeline(pipeline_desc).get(); + context->GetPipelineLibrary()->GetPipeline(pipeline_desc).Get(); ASSERT_TRUE(box_pipeline); VertexBufferBuilder vertex_builder; @@ -409,7 +409,7 @@ TEST_P(RendererTest, CanRenderInstanced) { ->GetPipeline(PipelineBuilder::MakeDefaultPipelineDescriptor( *GetContext()) ->SetSampleCount(SampleCount::kCount4)) - .get(); + .Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); Command cmd; @@ -449,7 +449,7 @@ TEST_P(RendererTest, CanBlitTextureToTexture) { ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); auto mipmaps_pipeline = - context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get(); + context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(mipmaps_pipeline); TextureDescriptor texture_desc; @@ -559,7 +559,7 @@ TEST_P(RendererTest, CanBlitTextureToBuffer) { ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); auto mipmaps_pipeline = - context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get(); + context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(mipmaps_pipeline); auto bridge = CreateTextureForFixture("bay_bridge.jpg"); @@ -690,7 +690,7 @@ TEST_P(RendererTest, CanGenerateMipmaps) { ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); auto mipmaps_pipeline = - context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get(); + context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(mipmaps_pipeline); auto boston = CreateTextureForFixture("boston.jpg", true); @@ -807,7 +807,7 @@ TEST_P(RendererTest, TheImpeller) { ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); auto pipeline = - context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).get(); + context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); auto blue_noise = CreateTextureForFixture("blue_noise.png"); @@ -867,7 +867,7 @@ TEST_P(RendererTest, ArrayUniforms) { ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); auto pipeline = - context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).get(); + context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); SinglePassCallback callback = [&](RenderPass& pass) { @@ -923,7 +923,7 @@ TEST_P(RendererTest, InactiveUniforms) { ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); auto pipeline = - context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).get(); + context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); SinglePassCallback callback = [&](RenderPass& pass) { diff --git a/impeller/runtime_stage/runtime_stage_unittests.cc b/impeller/runtime_stage/runtime_stage_unittests.cc index 15b585a817f7f..1c4a9acf5e1e3 100644 --- a/impeller/runtime_stage/runtime_stage_unittests.cc +++ b/impeller/runtime_stage/runtime_stage_unittests.cc @@ -248,7 +248,7 @@ TEST_P(RuntimeStageTest, CanCreatePipelineFromRuntimeStage) { desc.SetColorAttachmentDescriptor(0u, color0); desc.SetStencilAttachmentDescriptors(stencil0); desc.SetStencilPixelFormat(PixelFormat::kDefaultStencil); - auto pipeline = GetContext()->GetPipelineLibrary()->GetPipeline(desc).get(); + auto pipeline = GetContext()->GetPipelineLibrary()->GetPipeline(desc).Get(); ASSERT_NE(pipeline, nullptr); }