From 75b0fe2aad5d754a822058afc05dbcb932853250 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Thu, 18 Apr 2024 12:37:52 -0700 Subject: [PATCH] [Impeller] sorted commonly used default pipelines first (flutter/engine#52231) A minor optimization that places default pipelines that are synchronized upon in the `InitializeCommonlyUsedShadersIfNeeded` first in the queue to be created. issue: https://github.com/flutter/flutter/issues/145843 sequence diagram of the synchronization: ![pipeline-seq](https://github.com/flutter/engine/assets/30870216/b70d84fb-2e8e-4d08-a404-ae1434ad03da) ## 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.cc | 78 ++++++++++--------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/engine/src/flutter/impeller/entity/contents/content_context.cc b/engine/src/flutter/impeller/entity/contents/content_context.cc index 2aab8414854..1a7a8a9b026 100644 --- a/engine/src/flutter/impeller/entity/contents/content_context.cc +++ b/engine/src/flutter/impeller/entity/contents/content_context.cc @@ -282,18 +282,48 @@ ContentContext::ContentContext( checkerboard_pipelines_.CreateDefault(*context_, options); #endif // IMPELLER_DEBUG - solid_fill_pipelines_.CreateDefault(*context_, options); + // These pipelines are created first since they are immediately used by + // InitializeCommonlyUsedShadersIfNeeded. Their order matches the order in + // InitializeCommonlyUsedShadersIfNeeded. + { + solid_fill_pipelines_.CreateDefault(*context_, options); + texture_pipelines_.CreateDefault(*context_, options); - if (context_->GetCapabilities()->SupportsSSBO()) { - linear_gradient_ssbo_fill_pipelines_.CreateDefault(*context_, options); - radial_gradient_ssbo_fill_pipelines_.CreateDefault(*context_, options); - conical_gradient_ssbo_fill_pipelines_.CreateDefault(*context_, options); - sweep_gradient_ssbo_fill_pipelines_.CreateDefault(*context_, options); - } else { - linear_gradient_fill_pipelines_.CreateDefault(*context_, options); - radial_gradient_fill_pipelines_.CreateDefault(*context_, options); - conical_gradient_fill_pipelines_.CreateDefault(*context_, options); - sweep_gradient_fill_pipelines_.CreateDefault(*context_, options); + if (context_->GetCapabilities()->SupportsSSBO()) { + linear_gradient_ssbo_fill_pipelines_.CreateDefault(*context_, options); + radial_gradient_ssbo_fill_pipelines_.CreateDefault(*context_, options); + conical_gradient_ssbo_fill_pipelines_.CreateDefault(*context_, options); + sweep_gradient_ssbo_fill_pipelines_.CreateDefault(*context_, options); + } else { + linear_gradient_fill_pipelines_.CreateDefault(*context_, options); + radial_gradient_fill_pipelines_.CreateDefault(*context_, options); + conical_gradient_fill_pipelines_.CreateDefault(*context_, options); + sweep_gradient_fill_pipelines_.CreateDefault(*context_, options); + } + + /// Setup default clip pipeline. + + auto clip_pipeline_descriptor = + ClipPipeline::Builder::MakeDefaultPipelineDescriptor(*context_); + if (!clip_pipeline_descriptor.has_value()) { + return; + } + ContentContextOptions{ + .sample_count = SampleCount::kCount4, + .color_attachment_pixel_format = + context_->GetCapabilities()->GetDefaultColorFormat()} + .ApplyToPipelineDescriptor(*clip_pipeline_descriptor); + // Disable write to all color attachments. + auto clip_color_attachments = + clip_pipeline_descriptor->GetColorAttachmentDescriptors(); + for (auto& color_attachment : clip_color_attachments) { + color_attachment.second.write_mask = ColorWriteMaskBits::kNone; + } + clip_pipeline_descriptor->SetColorAttachmentDescriptors( + std::move(clip_color_attachments)); + clip_pipelines_.SetDefault( + options, + std::make_unique(*context_, clip_pipeline_descriptor)); } if (context_->GetCapabilities()->SupportsFramebufferFetch()) { @@ -392,7 +422,6 @@ ContentContext::ContentContext( rrect_blur_pipelines_.CreateDefault(*context_, options_trianglestrip); texture_blend_pipelines_.CreateDefault(*context_, options); - texture_pipelines_.CreateDefault(*context_, options); texture_strict_src_pipelines_.CreateDefault(*context_, options); position_uv_pipelines_.CreateDefault(*context_, options); tiled_texture_pipelines_.CreateDefault(*context_, options); @@ -438,29 +467,6 @@ ContentContext::ContentContext( context_->GetPipelineLibrary()->GetPipeline(uv_pipeline_desc).Get(); } - /// Setup default clip pipeline. - - auto clip_pipeline_descriptor = - ClipPipeline::Builder::MakeDefaultPipelineDescriptor(*context_); - if (!clip_pipeline_descriptor.has_value()) { - return; - } - ContentContextOptions{ - .sample_count = SampleCount::kCount4, - .color_attachment_pixel_format = - context_->GetCapabilities()->GetDefaultColorFormat()} - .ApplyToPipelineDescriptor(*clip_pipeline_descriptor); - // Disable write to all color attachments. - auto clip_color_attachments = - clip_pipeline_descriptor->GetColorAttachmentDescriptors(); - for (auto& color_attachment : clip_color_attachments) { - color_attachment.second.write_mask = ColorWriteMaskBits::kNone; - } - clip_pipeline_descriptor->SetColorAttachmentDescriptors( - std::move(clip_color_attachments)); - clip_pipelines_.SetDefault(options, std::make_unique( - *context_, clip_pipeline_descriptor)); - is_valid_ = true; InitializeCommonlyUsedShadersIfNeeded(); } @@ -606,6 +612,8 @@ void ContentContext::InitializeCommonlyUsedShadersIfNeeded() const { .color_attachment_pixel_format = context_->GetCapabilities()->GetDefaultColorFormat()}; + // Note: When editing this, check the order the default pipelines are created. + // These should be first. for (const auto mode : {BlendMode::kSource, BlendMode::kSourceOver}) { for (const auto geometry : {PrimitiveType::kTriangle, PrimitiveType::kTriangleStrip}) {