From bd46b7b392822ebaf12afe19f1a34ff6c0c9e1c0 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 18 Apr 2024 14:16:53 -0700 Subject: [PATCH 1/5] [Impeller] cleanup RenderPipelineT --- impeller/entity/contents/content_context.h | 201 +++++++++++---------- impeller/renderer/pipeline.h | 18 +- impeller/scene/scene_context.h | 4 +- 3 files changed, 120 insertions(+), 103 deletions(-) diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index ecebcccf3bc77..af403e66811a4 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -96,149 +96,165 @@ namespace impeller { #ifdef IMPELLER_DEBUG using CheckerboardPipeline = - RenderPipelineT; + RenderPipelineHandle; #endif // IMPELLER_DEBUG using LinearGradientFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using SolidFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using RadialGradientFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using ConicalGradientFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using SweepGradientFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using LinearGradientSSBOFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using ConicalGradientSSBOFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using RadialGradientSSBOFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using SweepGradientSSBOFillPipeline = - RenderPipelineT; + RenderPipelineHandle; using RRectBlurPipeline = - RenderPipelineT; -using BlendPipeline = RenderPipelineT; + RenderPipelineHandle; +using BlendPipeline = + RenderPipelineHandle; using TexturePipeline = - RenderPipelineT; + RenderPipelineHandle; using TextureStrictSrcPipeline = - RenderPipelineT; -using PositionUVPipeline = - RenderPipelineT; + RenderPipelineHandle; +using PositionUVPipeline = RenderPipelineHandle; using TiledTexturePipeline = - RenderPipelineT; + RenderPipelineHandle; using KernelDecalPipeline = - RenderPipelineT; + RenderPipelineHandle; using KernelPipeline = - RenderPipelineT; + RenderPipelineHandle; using BorderMaskBlurPipeline = - RenderPipelineT; + RenderPipelineHandle; using MorphologyFilterPipeline = - RenderPipelineT; + RenderPipelineHandle; using ColorMatrixColorFilterPipeline = - RenderPipelineT; + RenderPipelineHandle; using LinearToSrgbFilterPipeline = - RenderPipelineT; + RenderPipelineHandle; using SrgbToLinearFilterPipeline = - RenderPipelineT; + RenderPipelineHandle; using GlyphAtlasPipeline = - RenderPipelineT; + RenderPipelineHandle; using GlyphAtlasColorPipeline = - RenderPipelineT; + RenderPipelineHandle; using PorterDuffBlendPipeline = - RenderPipelineT; -using ClipPipeline = RenderPipelineT; + RenderPipelineHandle; +using ClipPipeline = RenderPipelineHandle; using GeometryColorPipeline = - RenderPipelineT; + RenderPipelineHandle; using YUVToRGBFilterPipeline = - RenderPipelineT; + RenderPipelineHandle; // Advanced blends -using BlendColorPipeline = - RenderPipelineT; +using BlendColorPipeline = RenderPipelineHandle; using BlendColorBurnPipeline = - RenderPipelineT; + RenderPipelineHandle; using BlendColorDodgePipeline = - RenderPipelineT; -using BlendDarkenPipeline = - RenderPipelineT; + RenderPipelineHandle; +using BlendDarkenPipeline = RenderPipelineHandle; using BlendDifferencePipeline = - RenderPipelineT; + RenderPipelineHandle; using BlendExclusionPipeline = - RenderPipelineT; + RenderPipelineHandle; using BlendHardLightPipeline = - RenderPipelineT; -using BlendHuePipeline = - RenderPipelineT; -using BlendLightenPipeline = - RenderPipelineT; + RenderPipelineHandle; +using BlendHuePipeline = RenderPipelineHandle; +using BlendLightenPipeline = RenderPipelineHandle; using BlendLuminosityPipeline = - RenderPipelineT; -using BlendMultiplyPipeline = - RenderPipelineT; -using BlendOverlayPipeline = - RenderPipelineT; + RenderPipelineHandle; +using BlendMultiplyPipeline = RenderPipelineHandle; +using BlendOverlayPipeline = RenderPipelineHandle; using BlendSaturationPipeline = - RenderPipelineT; -using BlendScreenPipeline = - RenderPipelineT; + RenderPipelineHandle; +using BlendScreenPipeline = RenderPipelineHandle; using BlendSoftLightPipeline = - RenderPipelineT; + RenderPipelineHandle; // Framebuffer Advanced Blends using FramebufferBlendColorPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendColorBurnPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendColorDodgePipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendDarkenPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendDifferencePipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendExclusionPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendHardLightPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendHuePipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendLightenPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendLuminosityPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendMultiplyPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendOverlayPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendSaturationPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendScreenPipeline = - RenderPipelineT; + RenderPipelineHandle; using FramebufferBlendSoftLightPipeline = - RenderPipelineT; + RenderPipelineHandle; /// Geometry Pipelines using PointsComputeShaderPipeline = ComputePipelineBuilder; @@ -246,11 +262,12 @@ using UvComputeShaderPipeline = ComputePipelineBuilder; #ifdef IMPELLER_ENABLE_OPENGLES using TextureExternalPipeline = - RenderPipelineT; + RenderPipelineHandle; using TiledTextureExternalPipeline = - RenderPipelineT; + RenderPipelineHandle; #endif // IMPELLER_ENABLE_OPENGLES // A struct used to isolate command buffer storage from the content diff --git a/impeller/renderer/pipeline.h b/impeller/renderer/pipeline.h index 7bed0902d2b48..36b3597130449 100644 --- a/impeller/renderer/pipeline.h +++ b/impeller/renderer/pipeline.h @@ -89,7 +89,7 @@ PipelineFuture CreatePipelineFuture( std::optional desc); template -class RenderPipelineT { +class RenderPipelineHandle { static_assert( ShaderStageCompatibilityChecker::Check(), "The output slots for the fragment shader don't have matches in the " @@ -100,16 +100,16 @@ class RenderPipelineT { using FragmentShader = FragmentShader_; using Builder = PipelineBuilder; - explicit RenderPipelineT(const Context& context) - : RenderPipelineT(CreatePipelineFuture( + explicit RenderPipelineHandle(const Context& context) + : RenderPipelineHandle(CreatePipelineFuture( context, Builder::MakeDefaultPipelineDescriptor(context))) {} - explicit RenderPipelineT(const Context& context, - std::optional desc) - : RenderPipelineT(CreatePipelineFuture(context, desc)) {} + explicit RenderPipelineHandle(const Context& context, + std::optional desc) + : RenderPipelineHandle(CreatePipelineFuture(context, desc)) {} - explicit RenderPipelineT(PipelineFuture future) + explicit RenderPipelineHandle(PipelineFuture future) : pipeline_future_(std::move(future)) {} std::shared_ptr> WaitAndGet() { @@ -132,9 +132,9 @@ class RenderPipelineT { std::shared_ptr> pipeline_; bool did_wait_ = false; - RenderPipelineT(const RenderPipelineT&) = delete; + RenderPipelineHandle(const RenderPipelineHandle&) = delete; - RenderPipelineT& operator=(const RenderPipelineT&) = delete; + RenderPipelineHandle& operator=(const RenderPipelineHandle&) = delete; }; template diff --git a/impeller/scene/scene_context.h b/impeller/scene/scene_context.h index 4c99e99867213..48ff7b8ec9458 100644 --- a/impeller/scene/scene_context.h +++ b/impeller/scene/scene_context.h @@ -125,13 +125,13 @@ class SceneContext { /// If a pipeline could not be created, returns nullptr. std::unique_ptr MakePipelineVariants(Context& context) { auto pipeline = - PipelineVariantsT>( + PipelineVariantsT>( context); if (!pipeline.IsValid()) { return nullptr; } return std::make_unique< - PipelineVariantsT>>( + PipelineVariantsT>>( std::move(pipeline)); } From 2c459e6ff0e462318249dfc527b63bae1d1560ce Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 18 Apr 2024 14:19:08 -0700 Subject: [PATCH 2/5] ComputeShaderT --- impeller/renderer/pipeline.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/impeller/renderer/pipeline.h b/impeller/renderer/pipeline.h index 36b3597130449..29c6635b6dba7 100644 --- a/impeller/renderer/pipeline.h +++ b/impeller/renderer/pipeline.h @@ -138,22 +138,23 @@ class RenderPipelineHandle { }; template -class ComputePipelineT { +class ComputePipelineHandle { public: using ComputeShader = ComputeShader_; using Builder = ComputePipelineBuilder; - explicit ComputePipelineT(const Context& context) - : ComputePipelineT(CreatePipelineFuture( + explicit ComputePipelineHandle(const Context& context) + : ComputePipelineHandle(CreatePipelineFuture( context, Builder::MakeDefaultPipelineDescriptor(context))) {} - explicit ComputePipelineT( + explicit ComputePipelineHandle( const Context& context, std::optional compute_desc) - : ComputePipelineT(CreatePipelineFuture(context, compute_desc)) {} + : ComputePipelineHandle(CreatePipelineFuture(context, compute_desc)) {} - explicit ComputePipelineT(PipelineFuture future) + explicit ComputePipelineHandle( + PipelineFuture future) : pipeline_future_(std::move(future)) {} std::shared_ptr> WaitAndGet() { @@ -172,9 +173,9 @@ class ComputePipelineT { std::shared_ptr> pipeline_; bool did_wait_ = false; - ComputePipelineT(const ComputePipelineT&) = delete; + ComputePipelineHandle(const ComputePipelineHandle&) = delete; - ComputePipelineT& operator=(const ComputePipelineT&) = delete; + ComputePipelineHandle& operator=(const ComputePipelineHandle&) = delete; }; } // namespace impeller From 4e724aee6f13f2d6a90544507bbf6a1182d72e8e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 18 Apr 2024 14:33:01 -0700 Subject: [PATCH 3/5] added docstring and cleaned up CreateIfNeeded --- impeller/entity/contents/content_context.h | 15 ++++++++------- impeller/renderer/pipeline.h | 6 ++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index af403e66811a4..0381e6d2573e4 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -1021,9 +1021,10 @@ class ContentContext { return pipeline->WaitAndGet(); } - template - TypedPipeline* CreateIfNeeded(Variants& container, - ContentContextOptions opts) const { + template + RenderPipelineHandleT* CreateIfNeeded( + Variants& container, + ContentContextOptions opts) const { if (!IsValid()) { return nullptr; } @@ -1032,11 +1033,11 @@ class ContentContext { opts.wireframe = true; } - if (TypedPipeline* found = container.Get(opts)) { + if (RenderPipelineHandleT* found = container.Get(opts)) { return found; } - TypedPipeline* prototype = container.GetDefault(); + RenderPipelineHandleT* prototype = container.GetDefault(); // The prototype must always be initialized in the constructor. FML_CHECK(prototype != nullptr); @@ -1054,8 +1055,8 @@ class ContentContext { desc.SetLabel( SPrintF("%s V#%zu", desc.GetLabel().c_str(), variants_count)); }); - std::unique_ptr variant = - std::make_unique(std::move(variant_future)); + std::unique_ptr variant = + std::make_unique(std::move(variant_future)); container.Set(opts, std::move(variant)); return container.Get(opts); } diff --git a/impeller/renderer/pipeline.h b/impeller/renderer/pipeline.h index 29c6635b6dba7..c6b407467c67a 100644 --- a/impeller/renderer/pipeline.h +++ b/impeller/renderer/pipeline.h @@ -88,6 +88,12 @@ PipelineFuture CreatePipelineFuture( const Context& context, std::optional desc); +/// Holds a reference to a Pipeline used for rendering while also maintaining +/// the vertex shader and fragment shader types at compile-time. +/// +/// See also: +/// - impeller::ContentContext::Variants - the typical container for +/// RenderPipelineHandles. template class RenderPipelineHandle { static_assert( From 6cf086ed789751c7ea4816859e95159a63a70b69 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 18 Apr 2024 14:41:48 -0700 Subject: [PATCH 4/5] updated Variants --- impeller/entity/contents/content_context.h | 28 +++++++++++++--------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 0381e6d2573e4..b4dd873cf9395 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -853,18 +853,24 @@ class ContentContext { RuntimeEffectPipelineKey::Equal> runtime_effect_pipelines_; - template + /// Holds multiple Pipelines associated with the same PipelineHandle types. + /// + /// For example, it may have multiple + /// RenderPipelineHandle + /// instances for different blend modes. From them you can access the + /// Pipeline. + template class Variants { public: Variants() = default; void Set(const ContentContextOptions& options, - std::unique_ptr pipeline) { + std::unique_ptr pipeline) { pipelines_[options] = std::move(pipeline); } void SetDefault(const ContentContextOptions& options, - std::unique_ptr pipeline) { + std::unique_ptr pipeline) { default_options_ = options; Set(options, std::move(pipeline)); } @@ -873,23 +879,23 @@ class ContentContext { const ContentContextOptions& options, const std::initializer_list& constants = {}) { auto desc = - PipelineT::Builder::MakeDefaultPipelineDescriptor(context, constants); + PipelineHandleT::Builder::MakeDefaultPipelineDescriptor(context, constants); if (!desc.has_value()) { VALIDATION_LOG << "Failed to create default pipeline."; return; } options.ApplyToPipelineDescriptor(*desc); - SetDefault(options, std::make_unique(context, desc)); + SetDefault(options, std::make_unique(context, desc)); } - PipelineT* Get(const ContentContextOptions& options) const { + PipelineHandleT* Get(const ContentContextOptions& options) const { if (auto found = pipelines_.find(options); found != pipelines_.end()) { return found->second.get(); } return nullptr; } - PipelineT* GetDefault() const { + PipelineHandleT* GetDefault() const { if (!default_options_.has_value()) { return nullptr; } @@ -901,7 +907,7 @@ class ContentContext { private: std::optional default_options_; std::unordered_map, + std::unique_ptr, ContentContextOptions::Hash, ContentContextOptions::Equal> pipelines_; @@ -1037,13 +1043,13 @@ class ContentContext { return found; } - RenderPipelineHandleT* prototype = container.GetDefault(); + RenderPipelineHandleT* default_handle = container.GetDefault(); // The prototype must always be initialized in the constructor. - FML_CHECK(prototype != nullptr); + FML_CHECK(default_handle != nullptr); std::shared_ptr> pipeline = - prototype->WaitAndGet(); + default_handle->WaitAndGet(); if (!pipeline) { return nullptr; } From 152c33aa30d3709c861f67bd27f0cbb5089d3cfc Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 18 Apr 2024 14:47:50 -0700 Subject: [PATCH 5/5] updated comments --- impeller/entity/contents/content_context.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index b4dd873cf9395..bd4dfb7ad3d47 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -859,6 +859,13 @@ class ContentContext { /// RenderPipelineHandle /// instances for different blend modes. From them you can access the /// Pipeline. + /// + /// See also: + /// - impeller::ContentContextOptions - options from which variants are + /// created. + /// - impeller::Pipeline::CreateVariant + /// - impeller::RenderPipelineHandle<> - The type of objects this typically + /// contains. template class Variants { public: @@ -878,8 +885,8 @@ class ContentContext { void CreateDefault(const Context& context, const ContentContextOptions& options, const std::initializer_list& constants = {}) { - auto desc = - PipelineHandleT::Builder::MakeDefaultPipelineDescriptor(context, constants); + auto desc = PipelineHandleT::Builder::MakeDefaultPipelineDescriptor( + context, constants); if (!desc.has_value()) { VALIDATION_LOG << "Failed to create default pipeline."; return; @@ -1045,7 +1052,7 @@ class ContentContext { RenderPipelineHandleT* default_handle = container.GetDefault(); - // The prototype must always be initialized in the constructor. + // The default must always be initialized in the constructor. FML_CHECK(default_handle != nullptr); std::shared_ptr> pipeline =