From 3fc04290cd9dd9d7a4eb2e88d192ab7973af1cd3 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 8 Nov 2022 17:05:36 +0800 Subject: [PATCH 1/3] [Impeller] Support access the descriptor of PipelineFuture directly --- impeller/entity/contents/content_context.cc | 8 ++--- .../contents/runtime_effect_contents.cc | 2 +- .../playground/imgui/imgui_impl_impeller.cc | 5 +-- .../backend/gles/pipeline_library_gles.cc | 23 +++++++------- .../backend/metal/pipeline_library_mtl.mm | 30 ++++++++++-------- .../backend/vulkan/pipeline_library_vk.cc | 18 +++++------ impeller/renderer/compute_unittests.cc | 2 +- impeller/renderer/pipeline.cc | 16 ++++++---- impeller/renderer/pipeline.h | 25 ++++++++------- impeller/renderer/pipeline_library.cc | 4 +-- impeller/renderer/renderer_unittests.cc | 31 ++++++++++--------- .../runtime_stage/runtime_stage_unittests.cc | 3 +- 12 files changed, 90 insertions(+), 77 deletions(-) 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..18bec50d1a8b9 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).future.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..1388faa9c4222 100644 --- a/impeller/playground/imgui/imgui_impl_impeller.cc +++ b/impeller/playground/imgui/imgui_impl_impeller.cc @@ -101,8 +101,9 @@ bool ImGui_ImplImpeller_Init( desc->SetStencilAttachmentDescriptors(stencil.value()); } - bd->pipeline = - context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get(); + bd->pipeline = context->GetPipelineLibrary() + ->GetPipeline(std::move(desc)) + .future.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..945be18630948 100644 --- a/impeller/renderer/backend/gles/pipeline_library_gles.cc +++ b/impeller/renderer/backend/gles/pipeline_library_gles.cc @@ -176,12 +176,13 @@ bool PipelineLibraryGLES::IsValid() const { PipelineFuture PipelineLibraryGLES::GetPipeline( PipelineDescriptor descriptor) { if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { - return found->second; + return {descriptor, found->second}; } 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.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..a4258edce410b 100644 --- a/impeller/renderer/backend/metal/pipeline_library_mtl.mm +++ b/impeller/renderer/backend/metal/pipeline_library_mtl.mm @@ -89,18 +89,20 @@ PipelineFuture PipelineLibraryMTL::GetPipeline( PipelineDescriptor descriptor) { if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { - return found->second; + return {descriptor, found->second}; } 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.future; auto weak_this = weak_from_this(); auto completion_handler = @@ -132,26 +134,28 @@ [device_ newRenderPipelineStateWithDescriptor:GetMTLRenderPipelineDescriptor( descriptor) completionHandler:completion_handler]; - return future; + return pipeline_future; } PipelineFuture PipelineLibraryMTL::GetPipeline( ComputePipelineDescriptor descriptor) { if (auto found = compute_pipelines_.find(descriptor); found != compute_pipelines_.end()) { - return found->second; + return {descriptor, found->second}; } 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.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..de2a3cbfe57f9 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -57,18 +57,20 @@ PipelineFuture PipelineLibraryVK::GetPipeline( PipelineDescriptor descriptor) { Lock lock(pipelines_mutex_); if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { - return found->second; + return {descriptor, found->second}; } 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.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_unittests.cc b/impeller/renderer/compute_unittests.cc index 7017364903e82..6c5ab8ae9372f 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).future.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..ab74488593434 100644 --- a/impeller/renderer/pipeline.h +++ b/impeller/renderer/pipeline.h @@ -20,14 +20,11 @@ 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; +}; //------------------------------------------------------------------------------ /// @brief Describes the fixed function and programmable aspects of @@ -107,12 +104,16 @@ class RenderPipelineT { return pipeline_; } did_wait_ = true; - if (pipeline_future_.valid()) { - pipeline_ = pipeline_future_.get(); + if (pipeline_future_.future.valid()) { + pipeline_ = pipeline_future_.future.get(); } return pipeline_; } + std::optional GetDescriptor() const { + return pipeline_future_.descriptor; + } + private: PipelineFuture pipeline_future_; std::shared_ptr> pipeline_; @@ -145,8 +146,8 @@ class ComputePipelineT { return pipeline_; } did_wait_ = true; - if (pipeline_future_.valid()) { - pipeline_ = pipeline_future_.get(); + if (pipeline_future_.future.valid()) { + pipeline_ = pipeline_future_.future.get(); } return pipeline_; } 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/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 26220c7e76b12..9744145b2d991 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)).future.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)).future.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)).future.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).future.get(); ASSERT_TRUE(box_pipeline); VertexBufferBuilder vertex_builder; @@ -409,7 +409,7 @@ TEST_P(RendererTest, CanRenderInstanced) { ->GetPipeline(PipelineBuilder::MakeDefaultPipelineDescriptor( *GetContext()) ->SetSampleCount(SampleCount::kCount4)) - .get(); + .future.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)).future.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)).future.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)).future.get(); ASSERT_TRUE(mipmaps_pipeline); auto boston = CreateTextureForFixture("boston.jpg", true); @@ -806,8 +806,9 @@ TEST_P(RendererTest, TheImpeller) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); - auto pipeline = - context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).get(); + auto pipeline = context->GetPipelineLibrary() + ->GetPipeline(pipeline_descriptor) + .future.get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); auto blue_noise = CreateTextureForFixture("blue_noise.png"); @@ -866,8 +867,9 @@ TEST_P(RendererTest, ArrayUniforms) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); - auto pipeline = - context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).get(); + auto pipeline = context->GetPipelineLibrary() + ->GetPipeline(pipeline_descriptor) + .future.get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); SinglePassCallback callback = [&](RenderPass& pass) { @@ -922,8 +924,9 @@ TEST_P(RendererTest, InactiveUniforms) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); - auto pipeline = - context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).get(); + auto pipeline = context->GetPipelineLibrary() + ->GetPipeline(pipeline_descriptor) + .future.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..233c246d3d998 100644 --- a/impeller/runtime_stage/runtime_stage_unittests.cc +++ b/impeller/runtime_stage/runtime_stage_unittests.cc @@ -248,7 +248,8 @@ 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).future.get(); ASSERT_NE(pipeline, nullptr); } From cf3a26c5f2c64a17765998c1ca85bf32bbc132b6 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 8 Nov 2022 20:22:10 +0800 Subject: [PATCH 2/3] Tweak code --- .../renderer/backend/gles/pipeline_library_gles.cc | 4 ++-- .../renderer/backend/metal/pipeline_library_mtl.mm | 8 ++++---- .../renderer/backend/vulkan/pipeline_library_vk.cc | 4 ++-- impeller/renderer/compute_pipeline_descriptor.h | 6 ------ impeller/renderer/pipeline_descriptor.h | 6 ------ impeller/renderer/pipeline_library.h | 11 +++++++++++ 6 files changed, 19 insertions(+), 20 deletions(-) diff --git a/impeller/renderer/backend/gles/pipeline_library_gles.cc b/impeller/renderer/backend/gles/pipeline_library_gles.cc index 945be18630948..25fa0ab632304 100644 --- a/impeller/renderer/backend/gles/pipeline_library_gles.cc +++ b/impeller/renderer/backend/gles/pipeline_library_gles.cc @@ -176,7 +176,7 @@ bool PipelineLibraryGLES::IsValid() const { PipelineFuture PipelineLibraryGLES::GetPipeline( PipelineDescriptor descriptor) { if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { - return {descriptor, found->second}; + return found->second; } if (!reactor_) { @@ -200,7 +200,7 @@ PipelineFuture PipelineLibraryGLES::GetPipeline( std::promise>>>(); auto pipeline_future = PipelineFuture{descriptor, promise->get_future()}; - pipelines_[descriptor] = pipeline_future.future; + pipelines_[descriptor] = pipeline_future; auto weak_this = weak_from_this(); auto result = reactor_->AddOperation( diff --git a/impeller/renderer/backend/metal/pipeline_library_mtl.mm b/impeller/renderer/backend/metal/pipeline_library_mtl.mm index a4258edce410b..85272ef16fad5 100644 --- a/impeller/renderer/backend/metal/pipeline_library_mtl.mm +++ b/impeller/renderer/backend/metal/pipeline_library_mtl.mm @@ -89,7 +89,7 @@ PipelineFuture PipelineLibraryMTL::GetPipeline( PipelineDescriptor descriptor) { if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { - return {descriptor, found->second}; + return found->second; } if (!IsValid()) { @@ -102,7 +102,7 @@ std::promise>>>(); auto pipeline_future = PipelineFuture{descriptor, promise->get_future()}; - pipelines_[descriptor] = pipeline_future.future; + pipelines_[descriptor] = pipeline_future; auto weak_this = weak_from_this(); auto completion_handler = @@ -141,7 +141,7 @@ ComputePipelineDescriptor descriptor) { if (auto found = compute_pipelines_.find(descriptor); found != compute_pipelines_.end()) { - return {descriptor, found->second}; + return found->second; } if (!IsValid()) { @@ -155,7 +155,7 @@ std::promise>>>(); auto pipeline_future = PipelineFuture{ descriptor, promise->get_future()}; - compute_pipelines_[descriptor] = pipeline_future.future; + compute_pipelines_[descriptor] = pipeline_future; auto weak_this = weak_from_this(); auto completion_handler = diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index de2a3cbfe57f9..6f29c25252f82 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -57,7 +57,7 @@ PipelineFuture PipelineLibraryVK::GetPipeline( PipelineDescriptor descriptor) { Lock lock(pipelines_mutex_); if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { - return {descriptor, found->second}; + return found->second; } if (!IsValid()) { @@ -70,7 +70,7 @@ PipelineFuture PipelineLibraryVK::GetPipeline( std::promise>>>(); auto pipeline_future = PipelineFuture{descriptor, promise->get_future()}; - pipelines_[descriptor] = pipeline_future.future; + pipelines_[descriptor] = pipeline_future; auto weak_this = weak_from_this(); 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/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.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(); From 493d49156d29b61a6121e0774b8d540fb474f12a Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 9 Nov 2022 15:02:10 +0800 Subject: [PATCH 3/3] Clean code --- .../contents/runtime_effect_contents.cc | 2 +- .../playground/imgui/imgui_impl_impeller.cc | 5 ++- impeller/renderer/compute_unittests.cc | 2 +- impeller/renderer/pipeline.h | 12 ++++--- impeller/renderer/renderer_unittests.cc | 31 +++++++++---------- .../runtime_stage/runtime_stage_unittests.cc | 3 +- 6 files changed, 27 insertions(+), 28 deletions(-) diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc index 18bec50d1a8b9..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).future.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 1388faa9c4222..d93a7369e7524 100644 --- a/impeller/playground/imgui/imgui_impl_impeller.cc +++ b/impeller/playground/imgui/imgui_impl_impeller.cc @@ -101,9 +101,8 @@ bool ImGui_ImplImpeller_Init( desc->SetStencilAttachmentDescriptors(stencil.value()); } - bd->pipeline = context->GetPipelineLibrary() - ->GetPipeline(std::move(desc)) - .future.get(); + bd->pipeline = + 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/compute_unittests.cc b/impeller/renderer/compute_unittests.cc index 6c5ab8ae9372f..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).future.get(); + context->GetPipelineLibrary()->GetPipeline(pipeline_desc).Get(); ASSERT_TRUE(compute_pipeline); auto cmd_buffer = context->CreateCommandBuffer(); diff --git a/impeller/renderer/pipeline.h b/impeller/renderer/pipeline.h index ab74488593434..cd900b2f5dd47 100644 --- a/impeller/renderer/pipeline.h +++ b/impeller/renderer/pipeline.h @@ -24,6 +24,10 @@ 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(); } }; //------------------------------------------------------------------------------ @@ -104,8 +108,8 @@ class RenderPipelineT { return pipeline_; } did_wait_ = true; - if (pipeline_future_.future.valid()) { - pipeline_ = pipeline_future_.future.get(); + if (pipeline_future_.IsValid()) { + pipeline_ = pipeline_future_.Get(); } return pipeline_; } @@ -146,8 +150,8 @@ class ComputePipelineT { return pipeline_; } did_wait_ = true; - if (pipeline_future_.future.valid()) { - pipeline_ = pipeline_future_.future.get(); + if (pipeline_future_.IsValid()) { + pipeline_ = pipeline_future_.Get(); } return pipeline_; } diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 9744145b2d991..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)).future.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)).future.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)).future.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).future.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)) - .future.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)).future.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)).future.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)).future.get(); + context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(mipmaps_pipeline); auto boston = CreateTextureForFixture("boston.jpg", true); @@ -806,9 +806,8 @@ TEST_P(RendererTest, TheImpeller) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); - auto pipeline = context->GetPipelineLibrary() - ->GetPipeline(pipeline_descriptor) - .future.get(); + auto pipeline = + context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); auto blue_noise = CreateTextureForFixture("blue_noise.png"); @@ -867,9 +866,8 @@ TEST_P(RendererTest, ArrayUniforms) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); - auto pipeline = context->GetPipelineLibrary() - ->GetPipeline(pipeline_descriptor) - .future.get(); + auto pipeline = + context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); SinglePassCallback callback = [&](RenderPass& pass) { @@ -924,9 +922,8 @@ TEST_P(RendererTest, InactiveUniforms) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); - auto pipeline = context->GetPipelineLibrary() - ->GetPipeline(pipeline_descriptor) - .future.get(); + auto pipeline = + 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 233c246d3d998..1c4a9acf5e1e3 100644 --- a/impeller/runtime_stage/runtime_stage_unittests.cc +++ b/impeller/runtime_stage/runtime_stage_unittests.cc @@ -248,8 +248,7 @@ TEST_P(RuntimeStageTest, CanCreatePipelineFromRuntimeStage) { desc.SetColorAttachmentDescriptor(0u, color0); desc.SetStencilAttachmentDescriptors(stencil0); desc.SetStencilPixelFormat(PixelFormat::kDefaultStencil); - auto pipeline = - GetContext()->GetPipelineLibrary()->GetPipeline(desc).future.get(); + auto pipeline = GetContext()->GetPipelineLibrary()->GetPipeline(desc).Get(); ASSERT_NE(pipeline, nullptr); }