From d65bd100075f7c80ea9e090fdb430e4841e79a7b Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Fri, 19 Apr 2024 09:15:08 -0700 Subject: [PATCH] [Impeller] cleaned up semantics for RenderPipelineT and added docstrings (flutter/engine#52237) I just tried to capture all thing things I found confusing about these and clean them up. We were mixing up Pipeline and RenderPipelineT in our generics arguments and RenderPipelineT wasn't clear since it was named like a template parameter but was a runtime type. Summary of edits: - RenderPipelineT -> RenderPipelineHandle - Added docstrings for RenderPipelineHandle and Variants - cleaned up generics parameters to ContentContext methods and Variants - replaced references that called the variant's default pipeline handle a "prototype" since it clashed with places that just call it the "default". ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- .../entity/contents/content_context.h | 253 ++++++++++-------- .../src/flutter/impeller/renderer/pipeline.h | 41 +-- .../flutter/impeller/scene/scene_context.h | 4 +- 3 files changed, 168 insertions(+), 130 deletions(-) diff --git a/engine/src/flutter/impeller/entity/contents/content_context.h b/engine/src/flutter/impeller/entity/contents/content_context.h index ecebcccf3bc..bd4dfb7ad3d 100644 --- a/engine/src/flutter/impeller/entity/contents/content_context.h +++ b/engine/src/flutter/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 @@ -836,18 +853,31 @@ 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. + /// + /// 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: 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)); } @@ -855,24 +885,24 @@ class ContentContext { void CreateDefault(const Context& context, const ContentContextOptions& options, const std::initializer_list& constants = {}) { - auto desc = - PipelineT::Builder::MakeDefaultPipelineDescriptor(context, constants); + auto desc = 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; } @@ -884,7 +914,7 @@ class ContentContext { private: std::optional default_options_; std::unordered_map, + std::unique_ptr, ContentContextOptions::Hash, ContentContextOptions::Equal> pipelines_; @@ -1004,9 +1034,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; } @@ -1015,17 +1046,17 @@ class ContentContext { opts.wireframe = true; } - if (TypedPipeline* found = container.Get(opts)) { + if (RenderPipelineHandleT* found = container.Get(opts)) { return found; } - TypedPipeline* prototype = container.GetDefault(); + RenderPipelineHandleT* default_handle = container.GetDefault(); - // The prototype must always be initialized in the constructor. - FML_CHECK(prototype != nullptr); + // The default must always be initialized in the constructor. + FML_CHECK(default_handle != nullptr); std::shared_ptr> pipeline = - prototype->WaitAndGet(); + default_handle->WaitAndGet(); if (!pipeline) { return nullptr; } @@ -1037,8 +1068,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/engine/src/flutter/impeller/renderer/pipeline.h b/engine/src/flutter/impeller/renderer/pipeline.h index 7bed0902d2b..c6b407467c6 100644 --- a/engine/src/flutter/impeller/renderer/pipeline.h +++ b/engine/src/flutter/impeller/renderer/pipeline.h @@ -88,8 +88,14 @@ 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 RenderPipelineT { +class RenderPipelineHandle { static_assert( ShaderStageCompatibilityChecker::Check(), "The output slots for the fragment shader don't have matches in the " @@ -100,16 +106,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,28 +138,29 @@ 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 -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 +179,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 diff --git a/engine/src/flutter/impeller/scene/scene_context.h b/engine/src/flutter/impeller/scene/scene_context.h index 4c99e998672..48ff7b8ec94 100644 --- a/engine/src/flutter/impeller/scene/scene_context.h +++ b/engine/src/flutter/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)); }