From efbb8b34c154caa3f3f3efa7dc9187825edbcd11 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 24 May 2023 13:28:22 -0700 Subject: [PATCH 01/15] Feature detection for subgroup support in Vulkan --- impeller/playground/compute_playground_test.h | 9 +++++++++ .../backend/vulkan/capabilities_vk.cc | 19 +++++++++++++++++-- .../renderer/backend/vulkan/capabilities_vk.h | 1 + impeller/renderer/compute_pass.cc | 3 ++- .../renderer/compute_subgroup_unittests.cc | 9 +++++++++ impeller/renderer/compute_unittests.cc | 9 +++++++++ 6 files changed, 47 insertions(+), 3 deletions(-) diff --git a/impeller/playground/compute_playground_test.h b/impeller/playground/compute_playground_test.h index 6beacd7d92415..5c87ed191b1db 100644 --- a/impeller/playground/compute_playground_test.h +++ b/impeller/playground/compute_playground_test.h @@ -55,6 +55,15 @@ class ComputePlaygroundTest FML_DISALLOW_COPY_AND_ASSIGN(ComputePlaygroundTest); }; +// TODO(dnfield): Remove this macro as Vulkan support reaches parith with Metal. +#define INSTANTIATE_COMPUTE_SUITE_WITH_VULKAN(playground) \ + INSTANTIATE_TEST_SUITE_P( \ + Compute, playground, \ + ::testing::Values(PlaygroundBackend::kMetal, \ + PlaygroundBackend::kVulkan), \ + [](const ::testing::TestParamInfo& \ + info) { return PlaygroundBackendToString(info.param); }); + #define INSTANTIATE_COMPUTE_SUITE(playground) \ INSTANTIATE_TEST_SUITE_P( \ Compute, playground, ::testing::Values(PlaygroundBackend::kMetal), \ diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index ff4410f15935a..57103da7fa0e6 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -295,6 +295,19 @@ bool CapabilitiesVK::SetDevice(const vk::PhysicalDevice& device) { device_properties_ = device.getProperties(); + auto physical_properties_2 = + device.getProperties2(); + + // Currently shaders only want access to arithmetic subgroup features. + // If that changes this needs to get updated, and so does Metal (which right + // now assumes it from compile time flags based on the MSL target version). + + supports_compute_subgroups_ = + !!(physical_properties_2.get() + .supportedOperations & + vk::SubgroupFeatureFlagBits::eArithmetic); + return true; } @@ -330,12 +343,14 @@ bool CapabilitiesVK::SupportsFramebufferFetch() const { // |Capabilities| bool CapabilitiesVK::SupportsCompute() const { - return false; + // Vulkan 1.1 requires support for compute. + return true; } // |Capabilities| bool CapabilitiesVK::SupportsComputeSubgroups() const { - return false; + // Set after in |SetDevice|. + return supports_compute_subgroups_; } // |Capabilities| diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index 006ed6f0e0889..e7ce12aee21d5 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -91,6 +91,7 @@ class CapabilitiesVK final : public Capabilities, PixelFormat color_format_ = PixelFormat::kUnknown; PixelFormat depth_stencil_format_ = PixelFormat::kUnknown; vk::PhysicalDeviceProperties device_properties_; + bool supports_compute_subgroups_ = false; bool is_valid_ = false; bool HasExtension(const std::string& ext) const; diff --git a/impeller/renderer/compute_pass.cc b/impeller/renderer/compute_pass.cc index 3efda50d4c757..9107ab93e09df 100644 --- a/impeller/renderer/compute_pass.cc +++ b/impeller/renderer/compute_pass.cc @@ -38,7 +38,8 @@ void ComputePass::SetThreadGroupSize(const ISize& size) { bool ComputePass::AddCommand(ComputeCommand command) { if (!command) { - VALIDATION_LOG << "Attempted to add an invalid command to the render pass."; + VALIDATION_LOG + << "Attempted to add an invalid command to the compute pass."; return false; } diff --git a/impeller/renderer/compute_subgroup_unittests.cc b/impeller/renderer/compute_subgroup_unittests.cc index 0850b884fb81d..66a89c396554d 100644 --- a/impeller/renderer/compute_subgroup_unittests.cc +++ b/impeller/renderer/compute_subgroup_unittests.cc @@ -39,6 +39,15 @@ namespace testing { using ComputeSubgroupTest = ComputePlaygroundTest; INSTANTIATE_COMPUTE_SUITE(ComputeSubgroupTest); +using ComputeMetalAndVulkanSubgroupTest = ComputePlaygroundTest; +INSTANTIATE_COMPUTE_SUITE_WITH_VULKAN(ComputeMetalAndVulkanSubgroupTest); + +TEST_P(ComputeMetalAndVulkanSubgroupTest, CapabilitiesSuportSubgroups) { + auto context = GetContext(); + ASSERT_TRUE(context); + ASSERT_TRUE(context->GetCapabilities()->SupportsComputeSubgroups()); +} + TEST_P(ComputeSubgroupTest, PathPlayground) { // Renders stroked SVG paths in an interactive playground. using SS = StrokeComputeShader; diff --git a/impeller/renderer/compute_unittests.cc b/impeller/renderer/compute_unittests.cc index c96171f8d204e..4f01d0f7fdf66 100644 --- a/impeller/renderer/compute_unittests.cc +++ b/impeller/renderer/compute_unittests.cc @@ -25,6 +25,15 @@ namespace testing { using ComputeTest = ComputePlaygroundTest; INSTANTIATE_COMPUTE_SUITE(ComputeTest); +using ComputeMetalAndVulkanSubgroupTest = ComputePlaygroundTest; +INSTANTIATE_COMPUTE_SUITE_WITH_VULKAN(ComputeMetalAndVulkanSubgroupTest); + +TEST_P(ComputeMetalAndVulkanSubgroupTest, CapabilitiesReportSupport) { + auto context = GetContext(); + ASSERT_TRUE(context); + ASSERT_TRUE(context->GetCapabilities()->SupportsCompute()); +} + TEST_P(ComputeTest, CanCreateComputePass) { using CS = SampleComputeShader; auto context = GetContext(); From eab8f2c329cc1ee7e60032b9cceda3cd45a3be54 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 24 May 2023 13:29:57 -0700 Subject: [PATCH 02/15] fix english --- impeller/renderer/backend/vulkan/capabilities_vk.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index 57103da7fa0e6..e917bed3ae3b8 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -349,7 +349,7 @@ bool CapabilitiesVK::SupportsCompute() const { // |Capabilities| bool CapabilitiesVK::SupportsComputeSubgroups() const { - // Set after in |SetDevice|. + // Set by |SetDevice|. return supports_compute_subgroups_; } From 37eaae7f202832e0ff33ac5a361a885b18f555ed Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 May 2023 21:37:08 -0700 Subject: [PATCH 03/15] start pass --- .../renderer/backend/metal/compute_pass_mtl.h | 2 +- .../backend/metal/compute_pass_mtl.mm | 6 +-- .../backend/vulkan/compute_pass_vk.cc | 36 ++++++++++++++++++ .../renderer/backend/vulkan/compute_pass_vk.h | 37 +++++++++++++++++++ impeller/renderer/compute_pass.h | 11 +++--- 5 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 impeller/renderer/backend/vulkan/compute_pass_vk.cc create mode 100644 impeller/renderer/backend/vulkan/compute_pass_vk.h diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.h b/impeller/renderer/backend/metal/compute_pass_mtl.h index df395c3434bb2..2da2285030737 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.h +++ b/impeller/renderer/backend/metal/compute_pass_mtl.h @@ -30,7 +30,7 @@ class ComputePassMTL final : public ComputePass { bool IsValid() const override; // |ComputePass| - void OnSetLabel(std::string label) override; + void OnSetLabel(const std::string& label) override; // |ComputePass| bool OnEncodeCommands(const Context& context, diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.mm b/impeller/renderer/backend/metal/compute_pass_mtl.mm index 5e461a8d0e764..864b7d7d0de0e 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.mm +++ b/impeller/renderer/backend/metal/compute_pass_mtl.mm @@ -40,11 +40,11 @@ return is_valid_; } -void ComputePassMTL::OnSetLabel(std::string label) { +void ComputePassMTL::OnSetLabel(const std::string& label) { if (label.empty()) { return; } - label_ = std::move(label); + label_ = label; } bool ComputePassMTL::OnEncodeCommands(const Context& context, @@ -55,7 +55,7 @@ return false; } - FML_DCHECK(!grid_size_.IsEmpty() && !thread_group_size_.IsEmpty()); + FML_DCHECK(!grid_size.IsEmpty() && !thread_group_size.IsEmpty()); // TODO(dnfield): Support non-serial dispatch type on higher iOS versions. auto compute_command_encoder = [buffer_ computeCommandEncoder]; diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc new file mode 100644 index 0000000000000..79bdbf1f7bc7b --- /dev/null +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -0,0 +1,36 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/renderer/backend/vulkan/compute_pass_vk.h" + +namespace impeller { + +ComputePassVK::ComputePassVK(std::weak_ptr context) + : ComputePass(std::move(context)) { + is_valid_ = true; +} + +ComputePassVK::~ComputePassVK() = default; + +bool ComputePassVK::IsValid() const { + return is_valid_; +} + +void ComputePassVK::OnSetLabel(const std::string& label) { + if (label.empty()) { + return; + } + label_ = label; +} + +bool ComputePassVK::OnEncodeCommands(const Context& context, + const ISize& grid_size, + const ISize& thread_group_size) const { + TRACE_EVENT0("impeller", "ComputePassVK::EncodeCommands"); + if (!IsValid()) { + return; + } +} + +} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.h b/impeller/renderer/backend/vulkan/compute_pass_vk.h new file mode 100644 index 0000000000000..d337eb2b237cb --- /dev/null +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.h @@ -0,0 +1,37 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include "flutter/fml/macros.h" +#include "impeller/renderer/compute_pass.h" + +namespace impeller { + +class ComputePassVK final : public ComputePass { + public: + // |ComputePass| + ~ComputePassVK() override; + + private: + friend class CommandBufferVK; + + std::string label_; + bool is_valid_ = false; + + ComputePassVK(std::weak_ptr context); + + // |ComputePass| + bool IsValid() const override; + + // |ComputePass| + void OnSetLabel(const std::string& label) override; + + // |ComputePass| + void OnEncodeCommands(const Context& context, + const ISize& grid_size, + const ISize& thread_group_size) const override; +}; + +} // namespace impeller \ No newline at end of file diff --git a/impeller/renderer/compute_pass.h b/impeller/renderer/compute_pass.h index 8d4af547d3e67..b12cb55fcace1 100644 --- a/impeller/renderer/compute_pass.h +++ b/impeller/renderer/compute_pass.h @@ -28,7 +28,7 @@ class ComputePass { virtual bool IsValid() const = 0; - void SetLabel(std::string label); + void SetLabel(const std::string& label); void SetGridSize(const ISize& size); @@ -59,20 +59,21 @@ class ComputePass { protected: const std::weak_ptr context_; - ISize grid_size_ = ISize(32, 32); - ISize thread_group_size_ = ISize(32, 32); - std::shared_ptr transients_buffer_; std::vector commands_; explicit ComputePass(std::weak_ptr context); - virtual void OnSetLabel(std::string label) = 0; + virtual void OnSetLabel(const std::string& label) = 0; virtual bool OnEncodeCommands(const Context& context, const ISize& grid_size, const ISize& thread_group_size) const = 0; private: + std::shared_ptr transients_buffer_; + ISize grid_size_ = ISize(32, 32); + ISize thread_group_size_ = ISize(32, 32); + FML_DISALLOW_COPY_AND_ASSIGN(ComputePass); }; From f07bfd998a0bc0e6be798a4313f9d90222066a53 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 26 May 2023 14:38:51 -0700 Subject: [PATCH 04/15] more --- .../backend/vulkan/compute_pass_vk.cc | 58 ++++++++++++++++++- .../renderer/backend/vulkan/compute_pass_vk.h | 4 +- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 79bdbf1f7bc7b..fd7b4cbb20695 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -6,8 +6,9 @@ namespace impeller { -ComputePassVK::ComputePassVK(std::weak_ptr context) - : ComputePass(std::move(context)) { +ComputePassVK::ComputePassVK(std::weak_ptr context, + std::weak_ptr encoder) + : ComputePass(std::move(context)) encoder_(std::move(encoder)) { is_valid_ = true; } @@ -31,6 +32,59 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, if (!IsValid()) { return; } + + FML_DCHECK(!grid_size.IsEmpty() && !thread_group_size.IsEmpty()); + + const auto& vk_context = ContextVK::Cast(context); + auto encoder = encoder_.lock(); + if (!encoder) { + VALIDATION_LOG << "Command encoder died before commands could be encoded"; + return false; + } + + fml::ScopedCleanupClosure pop_marker( + [&encoder]() { encoder->PopDebugGroup(); }); + if (!label_.empty()) { + encoder->PushDebugGroup(label_.c_str()); + } else { + pop_marker.Release(); + } + + auto cmd_buffer = encoder->GetCommandBuffer(); + + if (!UpdateBindingLayouts(commands_, cmd_buffer)) { + return false; + } + + auto compute_pass = CreateVKComputePass(vk_context); + if (!compute_pass) { + VALIDATION_LOG << "Could not create computepass."; + return false; + } + + if (!encoder->Track(compute_pass)) { + return false; + } + + { + TRACE_EVENT0("impeller", "EncodeComputePassCommands"); + cmd_buffer.beginRenderPass(pass_info, vk::SubpassContents::eInline); + + fml::ScopedCleanupClosure end_render_pass( + [cmd_buffer]() { cmd_buffer.endRenderPass(); }); + + for (const auto& command : commands_) { + if (!command.pipeline) { + continue; + } + + if (!EncodeCommand(context, command, *encoder, target_size)) { + return false; + } + } + } + + return true; } } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.h b/impeller/renderer/backend/vulkan/compute_pass_vk.h index d337eb2b237cb..a1ac1add93de8 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.h +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.h @@ -17,10 +17,12 @@ class ComputePassVK final : public ComputePass { private: friend class CommandBufferVK; + std::weak_ptr encoder_; std::string label_; bool is_valid_ = false; - ComputePassVK(std::weak_ptr context); + ComputePassVK(std::weak_ptr context, + std::weak_ptr encoder); // |ComputePass| bool IsValid() const override; From 21684619bf83aa0d4ea2c2a01b758cab16e1261d Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 26 May 2023 21:08:10 -0700 Subject: [PATCH 05/15] KEEP GOING --- impeller/renderer/backend/vulkan/BUILD.gn | 2 + .../backend/vulkan/compute_pass_vk.cc | 155 ++++++++++++++++-- .../renderer/backend/vulkan/compute_pass_vk.h | 3 +- .../backend/vulkan/compute_pipeline_vk.cc | 54 ++++++ .../backend/vulkan/compute_pipeline_vk.h | 56 +++++++ impeller/renderer/compute_pass.cc | 4 +- 6 files changed, 254 insertions(+), 20 deletions(-) create mode 100644 impeller/renderer/backend/vulkan/compute_pipeline_vk.cc create mode 100644 impeller/renderer/backend/vulkan/compute_pipeline_vk.h diff --git a/impeller/renderer/backend/vulkan/BUILD.gn b/impeller/renderer/backend/vulkan/BUILD.gn index 6c0403008be0a..aa1cfc1833802 100644 --- a/impeller/renderer/backend/vulkan/BUILD.gn +++ b/impeller/renderer/backend/vulkan/BUILD.gn @@ -34,6 +34,8 @@ impeller_component("vulkan") { "command_encoder_vk.h", "command_pool_vk.cc", "command_pool_vk.h", + # "compute_pass_vk.cc", + # "compute_pass_vk.h", "context_vk.cc", "context_vk.h", "debug_report_vk.cc", diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index fd7b4cbb20695..0bd52c774a7c5 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -4,11 +4,16 @@ #include "impeller/renderer/backend/vulkan/compute_pass_vk.h" +#include "flutter/fml/trace_event.h" +#include "impeller/renderer/backend/vulkan/compute_pipeline_vk.h" +#include "impeller/renderer/backend/vulkan/sampler_vk.h" +#include "impeller/renderer/backend/vulkan/texture_vk.h" + namespace impeller { ComputePassVK::ComputePassVK(std::weak_ptr context, std::weak_ptr encoder) - : ComputePass(std::move(context)) encoder_(std::move(encoder)) { + : ComputePass(std::move(context)), encoder_(std::move(encoder)) { is_valid_ = true; } @@ -25,12 +30,129 @@ void ComputePassVK::OnSetLabel(const std::string& label) { label_ = label; } +static bool AllocateAndBindDescriptorSets(const ContextVK& context, + const ComputeCommand& command, + CommandEncoderVK& encoder, + const PipelineVK& pipeline) { + auto desc_set = + encoder.AllocateDescriptorSet(pipeline.GetDescriptorSetLayout()); + if (!desc_set) { + return false; + } + + auto& allocator = *context.GetResourceAllocator(); + + std::unordered_map buffers; + std::unordered_map images; + std::vector writes; + + auto bind_images = [&encoder, // + &images, // + &writes, // + &desc_set // + ](const Bindings& bindings) -> bool { + for (const auto& [index, sampler_handle] : bindings.samplers) { + if (bindings.textures.find(index) == bindings.textures.end()) { + return false; + } + + auto texture = bindings.textures.at(index).resource; + const auto& texture_vk = TextureVK::Cast(*texture); + const SamplerVK& sampler = SamplerVK::Cast(*sampler_handle.resource); + + if (!encoder.Track(texture) || + !encoder.Track(sampler.GetSharedSampler())) { + return false; + } + + const SampledImageSlot& slot = bindings.sampled_images.at(index); + + vk::DescriptorImageInfo image_info; + image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; + image_info.sampler = sampler.GetSampler(); + image_info.imageView = texture_vk.GetImageView(); + + vk::WriteDescriptorSet write_set; + write_set.dstSet = desc_set.value(); + write_set.dstBinding = slot.binding; + write_set.descriptorCount = 1u; + write_set.descriptorType = vk::DescriptorType::eCombinedImageSampler; + write_set.pImageInfo = &(images[slot.binding] = image_info); + + writes.push_back(write_set); + } + + return true; + }; + + auto bind_buffers = [&allocator, // + &encoder, // + &buffers, // + &writes, // + &desc_set // + ](const Bindings& bindings) -> bool { + for (const auto& [buffer_index, view] : bindings.buffers) { + const auto& buffer_view = view.resource.buffer; + + auto device_buffer = buffer_view->GetDeviceBuffer(allocator); + if (!device_buffer) { + VALIDATION_LOG << "Failed to get device buffer for vertex binding"; + return false; + } + + auto buffer = DeviceBufferVK::Cast(*device_buffer).GetBuffer(); + if (!buffer) { + return false; + } + + // Reserved index used for per-vertex data. + if (buffer_index == VertexDescriptor::kReservedVertexBufferIndex) { + continue; + } + + if (!encoder.Track(device_buffer)) { + return false; + } + + uint32_t offset = view.resource.range.offset; + + vk::DescriptorBufferInfo buffer_info; + buffer_info.buffer = buffer; + buffer_info.offset = offset; + buffer_info.range = view.resource.range.length; + + const ShaderUniformSlot& uniform = bindings.uniforms.at(buffer_index); + + vk::WriteDescriptorSet write_set; + write_set.dstSet = desc_set.value(); + write_set.dstBinding = uniform.binding; + write_set.descriptorCount = 1u; + write_set.descriptorType = vk::DescriptorType::eUniformBuffer; + write_set.pBufferInfo = &(buffers[uniform.binding] = buffer_info); + + writes.push_back(write_set); + } + return true; + }; + + context.GetDevice().updateDescriptorSets(writes, {}); + + encoder.GetCommandBuffer().bindDescriptorSets( + vk::PipelineBindPoint::eCompute, // bind point + pipeline.GetPipelineLayout(), // layout + 0, // first set + {vk::DescriptorSet{*desc_set}}, // sets + nullptr // offsets + ); + return true; +} + bool ComputePassVK::OnEncodeCommands(const Context& context, const ISize& grid_size, const ISize& thread_group_size) const { TRACE_EVENT0("impeller", "ComputePassVK::EncodeCommands"); if (!IsValid()) { - return; + return false; } FML_DCHECK(!grid_size.IsEmpty() && !thread_group_size.IsEmpty()); @@ -56,29 +178,28 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, return false; } - auto compute_pass = CreateVKComputePass(vk_context); - if (!compute_pass) { - VALIDATION_LOG << "Could not create computepass."; - return false; - } - - if (!encoder->Track(compute_pass)) { - return false; - } - { TRACE_EVENT0("impeller", "EncodeComputePassCommands"); - cmd_buffer.beginRenderPass(pass_info, vk::SubpassContents::eInline); - - fml::ScopedCleanupClosure end_render_pass( - [cmd_buffer]() { cmd_buffer.endRenderPass(); }); for (const auto& command : commands_) { if (!command.pipeline) { continue; } - if (!EncodeCommand(context, command, *encoder, target_size)) { + const auto& pipeline_vk = ComputePipelineVK::Cast(*command.pipeline); + + cmd_buffer.bindPipeline(vk::PipelineBindPoint::eCompute), pipeline_vk.GetPipeline()); + if (!AllocateAndBindDescriptorSets(vk_context, // + command, // + encoder, // + pipeline_vk // + )) { + return false; + } + + cmd_buffer.dispatch(grid_size.x, grid_size.y, 0); + vk::Result result = cmd_buffer.end(); + if (result != vk::Result::Success) { return false; } } diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.h b/impeller/renderer/backend/vulkan/compute_pass_vk.h index a1ac1add93de8..83c1c6f056adb 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.h +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.h @@ -5,6 +5,7 @@ #pragma once #include "flutter/fml/macros.h" +#include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/compute_pass.h" namespace impeller { @@ -31,7 +32,7 @@ class ComputePassVK final : public ComputePass { void OnSetLabel(const std::string& label) override; // |ComputePass| - void OnEncodeCommands(const Context& context, + bool OnEncodeCommands(const Context& context, const ISize& grid_size, const ISize& thread_group_size) const override; }; diff --git a/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc b/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc new file mode 100644 index 0000000000000..ac2bed1a1bf27 --- /dev/null +++ b/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc @@ -0,0 +1,54 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/renderer/backend/vulkan/compute_pipeline_vk.h" + +namespace impeller { + +ComputePipelineVK::ComputePipelineVK( + std::weak_ptr device_holder, + std::weak_ptr library, + const PipelineDescriptor& desc, + vk::UniquePipeline pipeline, + vk::UniquePipelineLayout layout, + vk::UniqueDescriptorSetLayout descriptor_set_layout) + : Pipeline(std::move(library), desc), + device_holder_(device_holder), + pipeline_(std::move(pipeline)), + layout_(std::move(layout)), + descriptor_set_layout_(std::move(descriptor_set_layout)) { + is_valid_ = pipeline_ && layout_ && descriptor_set_layout_; +} + +ComputePipelineVK::~ComputePipelineVK() { + std::shared_ptr device_holder = device_holder_.lock(); + if (device_holder) { + descriptor_set_layout_.reset(); + layout_.reset(); + pipeline_.reset(); + } else { + descriptor_set_layout_.release(); + layout_.release(); + pipeline_.release(); + } +} + +bool ComputePipelineVK::IsValid() const { + return is_valid_; +} + +const vk::Pipeline& ComputePipelineVK::GetPipeline() const { + return *pipeline_; +} + +const vk::PipelineLayout& ComputePipelineVK::GetPipelineLayout() const { + return *layout_; +} + +const vk::DescriptorSetLayout& ComputePipelineVK::GetDescriptorSetLayout() + const { + return *descriptor_set_layout_; +} + +} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/compute_pipeline_vk.h b/impeller/renderer/backend/vulkan/compute_pipeline_vk.h new file mode 100644 index 0000000000000..bbda6ef480de7 --- /dev/null +++ b/impeller/renderer/backend/vulkan/compute_pipeline_vk.h @@ -0,0 +1,56 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include + +#include "flutter/fml/macros.h" +#include "impeller/base/backend_cast.h" +#include "impeller/renderer/backend/vulkan/device_holder.h" +#include "impeller/renderer/backend/vulkan/vk.h" +#include "impeller/renderer/pipeline.h" + +namespace impeller { + +class ComputePipelineVK final + : public Pipeline, + public BackendCast> { + public: + ComputePipelineVK(std::weak_ptr device_holder, + std::weak_ptr library, + const PipelineDescriptor& desc, + vk::UniquePipeline pipeline, + vk::UniquePipelineLayout layout, + vk::UniqueDescriptorSetLayout descriptor_set_layout); + + // |Pipeline| + ~ComputePipelineVK() override; + + const vk::Pipeline& GetPipeline() const; + + const vk::PipelineLayout& GetPipelineLayout() const; + + const vk::DescriptorSetLayout& GetDescriptorSetLayout() const; + + private: + friend class PipelineLibraryVK; + + std::weak_ptr device_holder_; + vk::UniquePipeline pipeline_; + vk::UniquePipelineLayout layout_; + vk::UniqueDescriptorSetLayout descriptor_set_layout_; + bool is_valid_ = false; + + // |Pipeline| + bool IsValid() const override; + + std::unique_ptr CreatePipeline( + const ComputePipelineDescriptor& desc); + + FML_DISALLOW_COPY_AND_ASSIGN(PipelineVK); +}; + +} // namespace impeller diff --git a/impeller/renderer/compute_pass.cc b/impeller/renderer/compute_pass.cc index 9107ab93e09df..95478f964acf9 100644 --- a/impeller/renderer/compute_pass.cc +++ b/impeller/renderer/compute_pass.cc @@ -20,12 +20,12 @@ HostBuffer& ComputePass::GetTransientsBuffer() { return *transients_buffer_; } -void ComputePass::SetLabel(std::string label) { +void ComputePass::SetLabel(const std::string& label) { if (label.empty()) { return; } transients_buffer_->SetLabel(SPrintF("%s Transients", label.c_str())); - OnSetLabel(std::move(label)); + OnSetLabel(label); } void ComputePass::SetGridSize(const ISize& size) { From 9301a829ddd78ad2789b5cb5f9987325d7711c2e Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 31 May 2023 12:58:13 -0700 Subject: [PATCH 06/15] a little more --- .../backend/vulkan/playground_impl_vk.cc | 3 + impeller/playground/compute_playground_test.h | 9 +- impeller/renderer/backend/vulkan/BUILD.gn | 6 +- .../backend/vulkan/command_buffer_vk.cc | 20 ++- .../backend/vulkan/compute_pass_vk.cc | 77 +++++++++-- .../backend/vulkan/compute_pipeline_vk.cc | 2 +- .../backend/vulkan/compute_pipeline_vk.h | 7 +- .../backend/vulkan/pipeline_cache_vk.cc | 17 +++ .../backend/vulkan/pipeline_cache_vk.h | 2 + .../backend/vulkan/pipeline_library_vk.cc | 130 +++++++++++++++++- .../backend/vulkan/pipeline_library_vk.h | 7 + .../renderer/backend/vulkan/pipeline_vk.h | 2 - impeller/renderer/compute_command.cc | 2 +- impeller/renderer/compute_pipeline_builder.h | 8 ++ .../renderer/compute_pipeline_descriptor.cc | 15 ++ .../renderer/compute_pipeline_descriptor.h | 12 ++ .../renderer/compute_subgroup_unittests.cc | 5 +- impeller/renderer/compute_unittests.cc | 5 +- impeller/renderer/prefix_sum_test.comp | 5 +- 19 files changed, 289 insertions(+), 45 deletions(-) diff --git a/impeller/playground/backend/vulkan/playground_impl_vk.cc b/impeller/playground/backend/vulkan/playground_impl_vk.cc index 84a2d50700228..f92192c4a9ba4 100644 --- a/impeller/playground/backend/vulkan/playground_impl_vk.cc +++ b/impeller/playground/backend/vulkan/playground_impl_vk.cc @@ -20,6 +20,7 @@ #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/surface_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" +#include "impeller/renderer/vk/compute_shaders_vk.h" #include "impeller/scene/shaders/vk/scene_shaders_vk.h" namespace impeller { @@ -38,6 +39,8 @@ ShaderLibraryMappingsForPlayground() { impeller_imgui_shaders_vk_length), std::make_shared(impeller_scene_shaders_vk_data, impeller_scene_shaders_vk_length), + std::make_shared( + impeller_compute_shaders_vk_data, impeller_compute_shaders_vk_length), }; } diff --git a/impeller/playground/compute_playground_test.h b/impeller/playground/compute_playground_test.h index 5c87ed191b1db..9abd4829cdf57 100644 --- a/impeller/playground/compute_playground_test.h +++ b/impeller/playground/compute_playground_test.h @@ -55,8 +55,7 @@ class ComputePlaygroundTest FML_DISALLOW_COPY_AND_ASSIGN(ComputePlaygroundTest); }; -// TODO(dnfield): Remove this macro as Vulkan support reaches parith with Metal. -#define INSTANTIATE_COMPUTE_SUITE_WITH_VULKAN(playground) \ +#define INSTANTIATE_COMPUTE_SUITE(playground) \ INSTANTIATE_TEST_SUITE_P( \ Compute, playground, \ ::testing::Values(PlaygroundBackend::kMetal, \ @@ -64,10 +63,4 @@ class ComputePlaygroundTest [](const ::testing::TestParamInfo& \ info) { return PlaygroundBackendToString(info.param); }); -#define INSTANTIATE_COMPUTE_SUITE(playground) \ - INSTANTIATE_TEST_SUITE_P( \ - Compute, playground, ::testing::Values(PlaygroundBackend::kMetal), \ - [](const ::testing::TestParamInfo& \ - info) { return PlaygroundBackendToString(info.param); }); - } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/BUILD.gn b/impeller/renderer/backend/vulkan/BUILD.gn index aa1cfc1833802..595454ded36bd 100644 --- a/impeller/renderer/backend/vulkan/BUILD.gn +++ b/impeller/renderer/backend/vulkan/BUILD.gn @@ -34,8 +34,10 @@ impeller_component("vulkan") { "command_encoder_vk.h", "command_pool_vk.cc", "command_pool_vk.h", - # "compute_pass_vk.cc", - # "compute_pass_vk.h", + "compute_pass_vk.cc", + "compute_pass_vk.h", + "compute_pipeline_vk.cc", + "compute_pipeline_vk.h", "context_vk.cc", "context_vk.h", "debug_report_vk.cc", diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index 37d6e7886e32b..c1168c651eac8 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -11,6 +11,7 @@ #include "impeller/base/validation.h" #include "impeller/renderer/backend/vulkan/blit_pass_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" +#include "impeller/renderer/backend/vulkan/compute_pass_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/render_pass_vk.h" @@ -65,7 +66,7 @@ std::shared_ptr CommandBufferVK::OnCreateRenderPass( } auto pass = std::shared_ptr(new RenderPassVK(context, // target, // - encoder_ //) + encoder_ // )); if (!pass->IsValid()) { return nullptr; @@ -85,9 +86,20 @@ std::shared_ptr CommandBufferVK::OnCreateBlitPass() const { } std::shared_ptr CommandBufferVK::OnCreateComputePass() const { - // TODO(110622): Wire up compute passes in Vulkan. - IMPELLER_UNIMPLEMENTED; - return nullptr; + if (!IsValid()) { + return nullptr; + } + auto context = context_.lock(); + if (!context) { + return nullptr; + } + auto pass = std::shared_ptr(new ComputePassVK(context, // + encoder_ // + )); + if (!pass->IsValid()) { + return nullptr; + } + return pass; } } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 0bd52c774a7c5..7da9b4d99dcca 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -30,10 +30,46 @@ void ComputePassVK::OnSetLabel(const std::string& label) { label_ = label; } +static bool UpdateBindingLayouts(const Bindings& bindings, + const vk::CommandBuffer& buffer) { + LayoutTransition transition; + transition.cmd_buffer = buffer; + transition.src_access = vk::AccessFlagBits::eColorAttachmentWrite | + vk::AccessFlagBits::eTransferWrite; + transition.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | + vk::PipelineStageFlagBits::eTransfer; + transition.dst_access = vk::AccessFlagBits::eShaderRead; + transition.dst_stage = vk::PipelineStageFlagBits::eFragmentShader; + + transition.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; + + for (const auto& [_, texture] : bindings.textures) { + if (!TextureVK::Cast(*texture.resource).SetLayout(transition)) { + return false; + } + } + return true; +} + +static bool UpdateBindingLayouts(const ComputeCommand& command, + const vk::CommandBuffer& buffer) { + return UpdateBindingLayouts(command.bindings, buffer); +} + +static bool UpdateBindingLayouts(const std::vector& commands, + const vk::CommandBuffer& buffer) { + for (const auto& command : commands) { + if (!UpdateBindingLayouts(command, buffer)) { + return false; + } + } + return true; +} + static bool AllocateAndBindDescriptorSets(const ContextVK& context, const ComputeCommand& command, CommandEncoderVK& encoder, - const PipelineVK& pipeline) { + const ComputePipelineVK& pipeline) { auto desc_set = encoder.AllocateDescriptorSet(pipeline.GetDescriptorSetLayout()); if (!desc_set) { @@ -105,11 +141,6 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, return false; } - // Reserved index used for per-vertex data. - if (buffer_index == VertexDescriptor::kReservedVertexBufferIndex) { - continue; - } - if (!encoder.Track(device_buffer)) { return false; } @@ -135,6 +166,10 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, return true; }; + if (!bind_buffers(command.bindings) || !bind_images(command.bindings)) { + return false; + } + context.GetDevice().updateDescriptorSets(writes, {}); encoder.GetCommandBuffer().bindDescriptorSets( @@ -160,7 +195,7 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, const auto& vk_context = ContextVK::Cast(context); auto encoder = encoder_.lock(); if (!encoder) { - VALIDATION_LOG << "Command encoder died before commands could be encoded"; + VALIDATION_LOG << "Command encoder died before commands could be encoded."; return false; } @@ -175,12 +210,31 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, auto cmd_buffer = encoder->GetCommandBuffer(); if (!UpdateBindingLayouts(commands_, cmd_buffer)) { + VALIDATION_LOG << "Could not update binding layouts for compute pass."; return false; } { TRACE_EVENT0("impeller", "EncodeComputePassCommands"); + vk::CommandBufferBeginInfo pass_info; + pass_info.sType = vk::StructureType::eCommandBufferBeginInfo; + + auto begin_result = cmd_buffer.begin(pass_info); + if (begin_result != vk::Result::eSuccess) { + VALIDATION_LOG << "Failed to begin compute pass: " + << vk::to_string(begin_result) << "."; + return false; + } + + fml::ScopedCleanupClosure end_compute_pass([cmd_buffer]() { + auto end_result = cmd_buffer.end(); + if (end_result != vk::Result::eSuccess) { + VALIDATION_LOG << "Failed to end compute pass: " + << vk::to_string(end_result) << "."; + } + }); + for (const auto& command : commands_) { if (!command.pipeline) { continue; @@ -188,18 +242,19 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, const auto& pipeline_vk = ComputePipelineVK::Cast(*command.pipeline); - cmd_buffer.bindPipeline(vk::PipelineBindPoint::eCompute), pipeline_vk.GetPipeline()); + cmd_buffer.bindPipeline(vk::PipelineBindPoint::eCompute, + pipeline_vk.GetPipeline()); if (!AllocateAndBindDescriptorSets(vk_context, // command, // - encoder, // + *encoder, // pipeline_vk // )) { return false; } - cmd_buffer.dispatch(grid_size.x, grid_size.y, 0); + cmd_buffer.dispatch(grid_size.width, grid_size.height, 1); vk::Result result = cmd_buffer.end(); - if (result != vk::Result::Success) { + if (result != vk::Result::eSuccess) { return false; } } diff --git a/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc b/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc index ac2bed1a1bf27..84187030d654d 100644 --- a/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc @@ -9,7 +9,7 @@ namespace impeller { ComputePipelineVK::ComputePipelineVK( std::weak_ptr device_holder, std::weak_ptr library, - const PipelineDescriptor& desc, + const ComputePipelineDescriptor& desc, vk::UniquePipeline pipeline, vk::UniquePipelineLayout layout, vk::UniqueDescriptorSetLayout descriptor_set_layout) diff --git a/impeller/renderer/backend/vulkan/compute_pipeline_vk.h b/impeller/renderer/backend/vulkan/compute_pipeline_vk.h index bbda6ef480de7..3d1b7622bc741 100644 --- a/impeller/renderer/backend/vulkan/compute_pipeline_vk.h +++ b/impeller/renderer/backend/vulkan/compute_pipeline_vk.h @@ -21,7 +21,7 @@ class ComputePipelineVK final public: ComputePipelineVK(std::weak_ptr device_holder, std::weak_ptr library, - const PipelineDescriptor& desc, + const ComputePipelineDescriptor& desc, vk::UniquePipeline pipeline, vk::UniquePipelineLayout layout, vk::UniqueDescriptorSetLayout descriptor_set_layout); @@ -47,10 +47,7 @@ class ComputePipelineVK final // |Pipeline| bool IsValid() const override; - std::unique_ptr CreatePipeline( - const ComputePipelineDescriptor& desc); - - FML_DISALLOW_COPY_AND_ASSIGN(PipelineVK); + FML_DISALLOW_COPY_AND_ASSIGN(ComputePipelineVK); }; } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc b/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc index 01d984e606089..8c40f6a75d309 100644 --- a/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc @@ -136,6 +136,23 @@ vk::UniquePipeline PipelineCacheVK::CreatePipeline( return std::move(pipeline); } +vk::UniquePipeline PipelineCacheVK::CreatePipeline( + const vk::ComputePipelineCreateInfo& info) { + std::shared_ptr strong_device = device_holder_.lock(); + if (!strong_device) { + return {}; + } + + Lock lock(cache_mutex_); + auto [result, pipeline] = + strong_device->GetDevice().createComputePipelineUnique(*cache_, info); + if (result != vk::Result::eSuccess) { + VALIDATION_LOG << "Could not create compute pipeline: " + << vk::to_string(result); + } + return std::move(pipeline); +} + std::shared_ptr PipelineCacheVK::CopyPipelineCacheData() const { std::shared_ptr strong_device = device_holder_.lock(); if (!strong_device) { diff --git a/impeller/renderer/backend/vulkan/pipeline_cache_vk.h b/impeller/renderer/backend/vulkan/pipeline_cache_vk.h index c0c71bcfd6d30..fdcb8a90b0df3 100644 --- a/impeller/renderer/backend/vulkan/pipeline_cache_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_cache_vk.h @@ -29,6 +29,8 @@ class PipelineCacheVK { vk::UniquePipeline CreatePipeline(const vk::GraphicsPipelineCreateInfo& info); + vk::UniquePipeline CreatePipeline(const vk::ComputePipelineCreateInfo& info); + void PersistCacheToDisk() const; private: diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index f68369a4fb0b9..c2e26c74bf254 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -343,6 +343,94 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( ); } +std::unique_ptr PipelineLibraryVK::CreateComputePipeline( + const ComputePipelineDescriptor& desc) { + TRACE_EVENT0("flutter", __FUNCTION__); + vk::ComputePipelineCreateInfo pipeline_info; + + //---------------------------------------------------------------------------- + /// Shader Stage + /// + const auto entrypoint = desc.GetStageEntrypoint(); + if (!entrypoint) { + VALIDATION_LOG << "Compute shader is missing an entrypoint."; + return nullptr; + } + + vk::PipelineShaderStageCreateInfo info; + info.setStage(vk::ShaderStageFlagBits::eCompute); + info.setPName("main"); + info.setModule(ShaderFunctionVK::Cast(entrypoint.get())->GetModule()); + pipeline_info.setStage(info); + + std::shared_ptr strong_device = device_holder_.lock(); + if (!strong_device) { + return nullptr; + } + + //---------------------------------------------------------------------------- + /// Pipeline Layout a.k.a the descriptor sets and uniforms. + /// + std::vector desc_bindings; + + for (auto layout : desc.GetDescriptorSetLayouts()) { + auto vk_desc_layout = ToVKDescriptorSetLayoutBinding(layout); + desc_bindings.push_back(vk_desc_layout); + } + + vk::DescriptorSetLayoutCreateInfo descs_layout_info; + descs_layout_info.setBindings(desc_bindings); + + auto [descs_result, descs_layout] = + strong_device->GetDevice().createDescriptorSetLayoutUnique( + descs_layout_info); + if (descs_result != vk::Result::eSuccess) { + VALIDATION_LOG << "unable to create uniform descriptors"; + return nullptr; + } + + ContextVK::SetDebugName(strong_device->GetDevice(), descs_layout.get(), + "Descriptor Set Layout " + desc.GetLabel()); + + //---------------------------------------------------------------------------- + /// Create the pipeline layout. + /// + vk::PipelineLayoutCreateInfo pipeline_layout_info; + pipeline_layout_info.setSetLayouts(descs_layout.get()); + auto pipeline_layout = strong_device->GetDevice().createPipelineLayoutUnique( + pipeline_layout_info); + if (pipeline_layout.result != vk::Result::eSuccess) { + VALIDATION_LOG << "Could not create pipeline layout for pipeline " + << desc.GetLabel() << ": " + << vk::to_string(pipeline_layout.result); + return nullptr; + } + pipeline_info.setLayout(pipeline_layout.value.get()); + + //---------------------------------------------------------------------------- + /// Finally, all done with the setup info. Create the pipeline itself. + /// + auto pipeline = pso_cache_->CreatePipeline(pipeline_info); + if (!pipeline) { + VALIDATION_LOG << "Could not create graphics pipeline: " << desc.GetLabel(); + return nullptr; + } + + ContextVK::SetDebugName(strong_device->GetDevice(), *pipeline_layout.value, + "Pipeline Layout " + desc.GetLabel()); + ContextVK::SetDebugName(strong_device->GetDevice(), *pipeline, + "Pipeline " + desc.GetLabel()); + + return std::make_unique( + device_holder_, + weak_from_this(), // + desc, // + std::move(pipeline), // + std::move(pipeline_layout.value), // + std::move(descs_layout) // + ); +} + // |PipelineLibrary| PipelineFuture PipelineLibraryVK::GetPipeline( PipelineDescriptor descriptor) { @@ -390,10 +478,48 @@ PipelineFuture PipelineLibraryVK::GetPipeline( // |PipelineLibrary| PipelineFuture PipelineLibraryVK::GetPipeline( ComputePipelineDescriptor descriptor) { + Lock lock(compute_pipelines_mutex_); + if (auto found = compute_pipelines_.find(descriptor); + found != compute_pipelines_.end()) { + return found->second; + } + + if (!IsValid()) { + return { + descriptor, + RealizedFuture>>( + nullptr)}; + } + auto promise = std::make_shared< std::promise>>>(); - promise->set_value(nullptr); - return {descriptor, promise->get_future()}; + auto pipeline_future = PipelineFuture{ + descriptor, promise->get_future()}; + compute_pipelines_[descriptor] = pipeline_future; + + auto weak_this = weak_from_this(); + + worker_task_runner_->PostTask([descriptor, weak_this, promise]() { + auto self = weak_this.lock(); + if (!self) { + promise->set_value(nullptr); + VALIDATION_LOG << "Pipeline library was collected before the pipeline " + "could be created."; + return; + } + + auto pipeline = + PipelineLibraryVK::Cast(*self).CreateComputePipeline(descriptor); + if (!pipeline) { + promise->set_value(nullptr); + VALIDATION_LOG << "Could not create pipeline: " << descriptor.GetLabel(); + return; + } + + promise->set_value(std::move(pipeline)); + }); + + return pipeline_future; } // |PipelineLibrary| diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.h b/impeller/renderer/backend/vulkan/pipeline_library_vk.h index 1302323735a3a..538a2b9834c62 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.h @@ -12,6 +12,7 @@ #include "flutter/fml/unique_fd.h" #include "impeller/base/backend_cast.h" #include "impeller/base/thread.h" +#include "impeller/renderer/backend/vulkan/compute_pipeline_vk.h" #include "impeller/renderer/backend/vulkan/pipeline_cache_vk.h" #include "impeller/renderer/backend/vulkan/pipeline_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" @@ -39,6 +40,9 @@ class PipelineLibraryVK final std::shared_ptr worker_task_runner_; Mutex pipelines_mutex_; PipelineMap pipelines_ IPLR_GUARDED_BY(pipelines_mutex_); + Mutex compute_pipelines_mutex_; + ComputePipelineMap compute_pipelines_ + IPLR_GUARDED_BY(compute_pipelines_mutex_); std::atomic_size_t frames_acquired_ = 0u; bool is_valid_ = false; @@ -65,6 +69,9 @@ class PipelineLibraryVK final std::unique_ptr CreatePipeline(const PipelineDescriptor& desc); + std::unique_ptr CreateComputePipeline( + const ComputePipelineDescriptor& desc); + void PersistPipelineCacheToDisk(); FML_DISALLOW_COPY_AND_ASSIGN(PipelineLibraryVK); diff --git a/impeller/renderer/backend/vulkan/pipeline_vk.h b/impeller/renderer/backend/vulkan/pipeline_vk.h index c7f41703304dc..6b9734b93a980 100644 --- a/impeller/renderer/backend/vulkan/pipeline_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_vk.h @@ -50,8 +50,6 @@ class PipelineVK final // |Pipeline| bool IsValid() const override; - std::unique_ptr CreatePipeline(const PipelineDescriptor& desc); - FML_DISALLOW_COPY_AND_ASSIGN(PipelineVK); }; diff --git a/impeller/renderer/compute_command.cc b/impeller/renderer/compute_command.cc index 41ce7586ff221..b5f166e27e24b 100644 --- a/impeller/renderer/compute_command.cc +++ b/impeller/renderer/compute_command.cc @@ -8,7 +8,6 @@ #include "impeller/base/validation.h" #include "impeller/core/formats.h" -#include "impeller/renderer/vertex_descriptor.h" namespace impeller { @@ -24,6 +23,7 @@ bool ComputeCommand::BindResource(ShaderStage stage, return false; } + bindings.uniforms[slot.ext_res_0] = slot; bindings.buffers[slot.ext_res_0] = {&metadata, view}; return true; } diff --git a/impeller/renderer/compute_pipeline_builder.h b/impeller/renderer/compute_pipeline_builder.h index aec09e607180f..f4fe1b8b9763f 100644 --- a/impeller/renderer/compute_pipeline_builder.h +++ b/impeller/renderer/compute_pipeline_builder.h @@ -69,6 +69,14 @@ struct ComputePipelineBuilder { return false; } + if (!desc.RegisterDescriptorSetLayouts( + ComputeShader::kDescriptorSetLayouts)) { + VALIDATION_LOG << "Could not configure compute descriptor set layout " + "for pipeline named '" + << ComputeShader::kLabel << "'."; + return false; + } + desc.SetStageEntrypoint(std::move(compute_function)); } return true; diff --git a/impeller/renderer/compute_pipeline_descriptor.cc b/impeller/renderer/compute_pipeline_descriptor.cc index 4563784591510..efb9cda26cfc0 100644 --- a/impeller/renderer/compute_pipeline_descriptor.cc +++ b/impeller/renderer/compute_pipeline_descriptor.cc @@ -63,4 +63,19 @@ const std::string& ComputePipelineDescriptor::GetLabel() const { return label_; } +bool ComputePipelineDescriptor::RegisterDescriptorSetLayouts( + const DescriptorSetLayout desc_set_layout[], + size_t count) { + descriptor_set_layouts_.reserve(descriptor_set_layouts_.size() + count); + for (size_t i = 0; i < count; i++) { + descriptor_set_layouts_.emplace_back(desc_set_layout[i]); + } + return true; +} + +const std::vector& +ComputePipelineDescriptor::GetDescriptorSetLayouts() const { + return descriptor_set_layouts_; +} + } // namespace impeller diff --git a/impeller/renderer/compute_pipeline_descriptor.h b/impeller/renderer/compute_pipeline_descriptor.h index 9886d2691ddf8..8a38f12d4f260 100644 --- a/impeller/renderer/compute_pipeline_descriptor.h +++ b/impeller/renderer/compute_pipeline_descriptor.h @@ -48,9 +48,21 @@ class ComputePipelineDescriptor final // Comparable bool IsEqual(const ComputePipelineDescriptor& other) const override; + template + bool RegisterDescriptorSetLayouts( + const std::array& inputs) { + return RegisterDescriptorSetLayouts(inputs.data(), inputs.size()); + } + + bool RegisterDescriptorSetLayouts(const DescriptorSetLayout desc_set_layout[], + size_t count); + + const std::vector& GetDescriptorSetLayouts() const; + private: std::string label_; std::shared_ptr entrypoint_; + std::vector descriptor_set_layouts_; }; } // namespace impeller diff --git a/impeller/renderer/compute_subgroup_unittests.cc b/impeller/renderer/compute_subgroup_unittests.cc index 66a89c396554d..39e5fac2587a2 100644 --- a/impeller/renderer/compute_subgroup_unittests.cc +++ b/impeller/renderer/compute_subgroup_unittests.cc @@ -39,10 +39,7 @@ namespace testing { using ComputeSubgroupTest = ComputePlaygroundTest; INSTANTIATE_COMPUTE_SUITE(ComputeSubgroupTest); -using ComputeMetalAndVulkanSubgroupTest = ComputePlaygroundTest; -INSTANTIATE_COMPUTE_SUITE_WITH_VULKAN(ComputeMetalAndVulkanSubgroupTest); - -TEST_P(ComputeMetalAndVulkanSubgroupTest, CapabilitiesSuportSubgroups) { +TEST_P(ComputeSubgroupTest, CapabilitiesSuportSubgroups) { auto context = GetContext(); ASSERT_TRUE(context); ASSERT_TRUE(context->GetCapabilities()->SupportsComputeSubgroups()); diff --git a/impeller/renderer/compute_unittests.cc b/impeller/renderer/compute_unittests.cc index 4f01d0f7fdf66..2ebb92c96563b 100644 --- a/impeller/renderer/compute_unittests.cc +++ b/impeller/renderer/compute_unittests.cc @@ -25,10 +25,7 @@ namespace testing { using ComputeTest = ComputePlaygroundTest; INSTANTIATE_COMPUTE_SUITE(ComputeTest); -using ComputeMetalAndVulkanSubgroupTest = ComputePlaygroundTest; -INSTANTIATE_COMPUTE_SUITE_WITH_VULKAN(ComputeMetalAndVulkanSubgroupTest); - -TEST_P(ComputeMetalAndVulkanSubgroupTest, CapabilitiesReportSupport) { +TEST_P(ComputeTest, CapabilitiesReportSupport) { auto context = GetContext(); ASSERT_TRUE(context); ASSERT_TRUE(context->GetCapabilities()->SupportsCompute()); diff --git a/impeller/renderer/prefix_sum_test.comp b/impeller/renderer/prefix_sum_test.comp index e964d551cf6fe..74523cbd15359 100644 --- a/impeller/renderer/prefix_sum_test.comp +++ b/impeller/renderer/prefix_sum_test.comp @@ -31,7 +31,10 @@ void main() { value = input_data.data[ident]; } - memory[ident] = value; + if (ident < BLOCK_SIZE) { + memory[ident] = value; + } + barrier(); ExclusivePrefixSum(ident, memory, BLOCK_SIZE); From c88dd19a8832de0423ef1b79f20b2ed544181866 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 11:15:16 -0700 Subject: [PATCH 07/15] Fix command buffer usage, SSBOs --- .../renderer/backend/vulkan/allocator_vk.cc | 1 + .../backend/vulkan/compute_pass_vk.cc | 82 ++++++++----------- impeller/renderer/backend/vulkan/formats_vk.h | 30 ++++--- .../renderer/backend/vulkan/render_pass_vk.cc | 49 +++++++---- 4 files changed, 83 insertions(+), 79 deletions(-) diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 66cd2de602545..23df6c1688140 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -356,6 +356,7 @@ std::shared_ptr AllocatorVK::OnCreateBuffer( buffer_info.usage = vk::BufferUsageFlagBits::eVertexBuffer | vk::BufferUsageFlagBits::eIndexBuffer | vk::BufferUsageFlagBits::eUniformBuffer | + vk::BufferUsageFlagBits::eStorageBuffer | vk::BufferUsageFlagBits::eTransferSrc | vk::BufferUsageFlagBits::eTransferDst; buffer_info.size = desc.size; diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 7da9b4d99dcca..7356a12db03e2 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -34,12 +34,10 @@ static bool UpdateBindingLayouts(const Bindings& bindings, const vk::CommandBuffer& buffer) { LayoutTransition transition; transition.cmd_buffer = buffer; - transition.src_access = vk::AccessFlagBits::eColorAttachmentWrite | - vk::AccessFlagBits::eTransferWrite; - transition.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | - vk::PipelineStageFlagBits::eTransfer; + transition.src_access = vk::AccessFlagBits::eTransferWrite; + transition.src_stage = vk::PipelineStageFlagBits::eTransfer; transition.dst_access = vk::AccessFlagBits::eShaderRead; - transition.dst_stage = vk::PipelineStageFlagBits::eFragmentShader; + transition.dst_stage = vk::PipelineStageFlagBits::eComputeShader; transition.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; @@ -70,9 +68,10 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, const ComputeCommand& command, CommandEncoderVK& encoder, const ComputePipelineVK& pipeline) { - auto desc_set = + auto desc_set = pipeline.GetDescriptor().GetDescriptorSetLayouts(); + auto vk_desc_set = encoder.AllocateDescriptorSet(pipeline.GetDescriptorSetLayout()); - if (!desc_set) { + if (!vk_desc_set) { return false; } @@ -82,10 +81,10 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, std::unordered_map images; std::vector writes; - auto bind_images = [&encoder, // - &images, // - &writes, // - &desc_set // + auto bind_images = [&encoder, // + &images, // + &writes, // + &vk_desc_set // ](const Bindings& bindings) -> bool { for (const auto& [index, sampler_handle] : bindings.samplers) { if (bindings.textures.find(index) == bindings.textures.end()) { @@ -109,7 +108,7 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, image_info.imageView = texture_vk.GetImageView(); vk::WriteDescriptorSet write_set; - write_set.dstSet = desc_set.value(); + write_set.dstSet = vk_desc_set.value(); write_set.dstBinding = slot.binding; write_set.descriptorCount = 1u; write_set.descriptorType = vk::DescriptorType::eCombinedImageSampler; @@ -121,11 +120,12 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, return true; }; - auto bind_buffers = [&allocator, // - &encoder, // - &buffers, // - &writes, // - &desc_set // + auto bind_buffers = [&allocator, // + &encoder, // + &buffers, // + &writes, // + &desc_set, // + &vk_desc_set // ](const Bindings& bindings) -> bool { for (const auto& [buffer_index, view] : bindings.buffers) { const auto& buffer_view = view.resource.buffer; @@ -153,12 +153,22 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, buffer_info.range = view.resource.range.length; const ShaderUniformSlot& uniform = bindings.uniforms.at(buffer_index); + auto layout_it = std::find_if(desc_set.begin(), desc_set.end(), + [&uniform](DescriptorSetLayout& layout) { + return layout.binding == uniform.binding; + }); + if (layout_it == desc_set.end()) { + VALIDATION_LOG << "Failed to get descriptor set layout for binding " + << uniform.binding; + return false; + } + auto layout = *layout_it; vk::WriteDescriptorSet write_set; - write_set.dstSet = desc_set.value(); + write_set.dstSet = vk_desc_set.value(); write_set.dstBinding = uniform.binding; write_set.descriptorCount = 1u; - write_set.descriptorType = vk::DescriptorType::eUniformBuffer; + write_set.descriptorType = ToVKDescriptorType(layout.descriptor_type); write_set.pBufferInfo = &(buffers[uniform.binding] = buffer_info); writes.push_back(write_set); @@ -173,11 +183,11 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, context.GetDevice().updateDescriptorSets(writes, {}); encoder.GetCommandBuffer().bindDescriptorSets( - vk::PipelineBindPoint::eCompute, // bind point - pipeline.GetPipelineLayout(), // layout - 0, // first set - {vk::DescriptorSet{*desc_set}}, // sets - nullptr // offsets + vk::PipelineBindPoint::eCompute, // bind point + pipeline.GetPipelineLayout(), // layout + 0, // first set + {vk::DescriptorSet{*vk_desc_set}}, // sets + nullptr // offsets ); return true; } @@ -206,7 +216,6 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, } else { pop_marker.Release(); } - auto cmd_buffer = encoder->GetCommandBuffer(); if (!UpdateBindingLayouts(commands_, cmd_buffer)) { @@ -217,24 +226,6 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, { TRACE_EVENT0("impeller", "EncodeComputePassCommands"); - vk::CommandBufferBeginInfo pass_info; - pass_info.sType = vk::StructureType::eCommandBufferBeginInfo; - - auto begin_result = cmd_buffer.begin(pass_info); - if (begin_result != vk::Result::eSuccess) { - VALIDATION_LOG << "Failed to begin compute pass: " - << vk::to_string(begin_result) << "."; - return false; - } - - fml::ScopedCleanupClosure end_compute_pass([cmd_buffer]() { - auto end_result = cmd_buffer.end(); - if (end_result != vk::Result::eSuccess) { - VALIDATION_LOG << "Failed to end compute pass: " - << vk::to_string(end_result) << "."; - } - }); - for (const auto& command : commands_) { if (!command.pipeline) { continue; @@ -251,12 +242,7 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, )) { return false; } - cmd_buffer.dispatch(grid_size.width, grid_size.height, 1); - vk::Result result = cmd_buffer.end(); - if (result != vk::Result::eSuccess) { - return false; - } } } diff --git a/impeller/renderer/backend/vulkan/formats_vk.h b/impeller/renderer/backend/vulkan/formats_vk.h index c40440351133b..da2aa616eea19 100644 --- a/impeller/renderer/backend/vulkan/formats_vk.h +++ b/impeller/renderer/backend/vulkan/formats_vk.h @@ -266,30 +266,34 @@ constexpr vk::ShaderStageFlags ToVkShaderStage(ShaderStage stage) { FML_UNREACHABLE(); } -constexpr vk::DescriptorSetLayoutBinding ToVKDescriptorSetLayoutBinding( - const DescriptorSetLayout& layout) { - vk::DescriptorSetLayoutBinding binding; - binding.binding = layout.binding; - binding.descriptorCount = 1u; - vk::DescriptorType desc_type = vk::DescriptorType(); - switch (layout.descriptor_type) { +constexpr vk::DescriptorType ToVKDescriptorType(DescriptorType type) { + switch (type) { case DescriptorType::kSampledImage: - desc_type = vk::DescriptorType::eCombinedImageSampler; + return vk::DescriptorType::eCombinedImageSampler; break; case DescriptorType::kUniformBuffer: - desc_type = vk::DescriptorType::eUniformBuffer; + return vk::DescriptorType::eUniformBuffer; break; case DescriptorType::kStorageBuffer: - desc_type = vk::DescriptorType::eStorageBuffer; + return vk::DescriptorType::eStorageBuffer; break; case DescriptorType::kImage: - desc_type = vk::DescriptorType::eSampledImage; + return vk::DescriptorType::eSampledImage; break; case DescriptorType::kSampler: - desc_type = vk::DescriptorType::eSampler; + return vk::DescriptorType::eSampler; break; } - binding.descriptorType = desc_type; + + FML_UNREACHABLE(); +} + +constexpr vk::DescriptorSetLayoutBinding ToVKDescriptorSetLayoutBinding( + const DescriptorSetLayout& layout) { + vk::DescriptorSetLayoutBinding binding; + binding.binding = layout.binding; + binding.descriptorCount = 1u; + binding.descriptorType = ToVKDescriptorType(layout.descriptor_type); binding.stageFlags = ToVkShaderStage(layout.shader_stage); return binding; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 40c8da7fe8420..9806fb675a460 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -284,8 +284,10 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, CommandEncoderVK& encoder, const PipelineVK& pipeline) { auto desc_set = + pipeline.GetDescriptor().GetVertexDescriptor()->GetDescriptorSetLayouts(); + auto vk_desc_set = encoder.AllocateDescriptorSet(pipeline.GetDescriptorSetLayout()); - if (!desc_set) { + if (!vk_desc_set) { return false; } @@ -295,10 +297,10 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, std::unordered_map images; std::vector writes; - auto bind_images = [&encoder, // - &images, // - &writes, // - &desc_set // + auto bind_images = [&encoder, // + &images, // + &writes, // + &vk_desc_set // ](const Bindings& bindings) -> bool { for (const auto& [index, sampler_handle] : bindings.samplers) { if (bindings.textures.find(index) == bindings.textures.end()) { @@ -322,7 +324,7 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, image_info.imageView = texture_vk.GetImageView(); vk::WriteDescriptorSet write_set; - write_set.dstSet = desc_set.value(); + write_set.dstSet = vk_desc_set.value(); write_set.dstBinding = slot.binding; write_set.descriptorCount = 1u; write_set.descriptorType = vk::DescriptorType::eCombinedImageSampler; @@ -334,11 +336,12 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, return true; }; - auto bind_buffers = [&allocator, // - &encoder, // - &buffers, // - &writes, // - &desc_set // + auto bind_buffers = [&allocator, // + &encoder, // + &buffers, // + &writes, // + &desc_set, // + &vk_desc_set // ](const Bindings& bindings) -> bool { for (const auto& [buffer_index, view] : bindings.buffers) { const auto& buffer_view = view.resource.buffer; @@ -371,12 +374,22 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, buffer_info.range = view.resource.range.length; const ShaderUniformSlot& uniform = bindings.uniforms.at(buffer_index); + auto layout_it = std::find_if(desc_set.begin(), desc_set.end(), + [&uniform](DescriptorSetLayout& layout) { + return layout.binding == uniform.binding; + }); + if (layout_it == desc_set.end()) { + VALIDATION_LOG << "Failed to get descriptor set layout for binding " + << uniform.binding; + return false; + } + auto layout = *layout_it; vk::WriteDescriptorSet write_set; - write_set.dstSet = desc_set.value(); + write_set.dstSet = vk_desc_set.value(); write_set.dstBinding = uniform.binding; write_set.descriptorCount = 1u; - write_set.descriptorType = vk::DescriptorType::eUniformBuffer; + write_set.descriptorType = ToVKDescriptorType(layout.descriptor_type); write_set.pBufferInfo = &(buffers[uniform.binding] = buffer_info); writes.push_back(write_set); @@ -393,11 +406,11 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, context.GetDevice().updateDescriptorSets(writes, {}); encoder.GetCommandBuffer().bindDescriptorSets( - vk::PipelineBindPoint::eGraphics, // bind point - pipeline.GetPipelineLayout(), // layout - 0, // first set - {vk::DescriptorSet{*desc_set}}, // sets - nullptr // offsets + vk::PipelineBindPoint::eGraphics, // bind point + pipeline.GetPipelineLayout(), // layout + 0, // first set + {vk::DescriptorSet{*vk_desc_set}}, // sets + nullptr // offsets ); return true; } From 14f407620f47e03dfea5d603d7be5fa8c2dfcfbd Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 12:45:01 -0700 Subject: [PATCH 08/15] Fix completion callbacks for Vulkan --- .../backend/vulkan/command_buffer_vk.cc | 13 ++++++---- .../backend/vulkan/command_encoder_vk.cc | 26 +++++++++++++++---- .../backend/vulkan/command_encoder_vk.h | 5 +++- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index c1168c651eac8..725276bd7f864 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -48,12 +48,15 @@ const std::shared_ptr& CommandBufferVK::GetEncoder() const { } bool CommandBufferVK::OnSubmitCommands(CompletionCallback callback) { - const auto submit = encoder_->Submit(); - if (callback) { - callback(submit ? CommandBuffer::Status::kCompleted - : CommandBuffer::Status::kError); + if (!callback) { + return encoder_->Submit(); } - return submit; + return encoder_->Submit([callback](bool submit) { + if (!submit) { + callback(CommandBuffer::Status::kError); + } + callback(CommandBuffer::Status::kCompleted); + }); } void CommandBufferVK::OnWaitUntilScheduled() {} diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index 7b234b470c1dc..c4fa759047a37 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -124,13 +124,23 @@ bool CommandEncoderVK::IsValid() const { return is_valid_; } -bool CommandEncoderVK::Submit() { +bool CommandEncoderVK::Submit(SubmitCallback callback) { + // Make sure to call callback with `false` if anything returns early. + bool fail_callback = !!callback; if (!IsValid()) { + if (fail_callback) { + callback(false); + } return false; } // Success or failure, you only get to submit once. - fml::ScopedCleanupClosure reset([&]() { Reset(); }); + fml::ScopedCleanupClosure reset([&]() { + if (fail_callback) { + callback(false); + } + Reset(); + }); InsertDebugMarker("QueueSubmit"); @@ -155,10 +165,16 @@ bool CommandEncoderVK::Submit() { return false; } + // Submit will proceed, call callback with true when it is done and do not + // call when `reset` is collected. + fail_callback = false; + return fence_waiter_->AddFence( - std::move(fence), [tracked_objects = std::move(tracked_objects_)] { - // Nothing to do, we just drop the tracked - // objects on the floor. + std::move(fence), + [callback, tracked_objects = std::move(tracked_objects_)] { + if (callback) { + callback(true); + } }); } diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.h b/impeller/renderer/backend/vulkan/command_encoder_vk.h index d160385472702..e9abada8a746d 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.h +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.h @@ -4,6 +4,7 @@ #pragma once +#include #include #include @@ -32,11 +33,13 @@ class FenceWaiterVK; class CommandEncoderVK { public: + using SubmitCallback = std::function; + ~CommandEncoderVK(); bool IsValid() const; - bool Submit(); + bool Submit(SubmitCallback callback = {}); bool Track(std::shared_ptr object); From 86152114c66d20c3d847261c4207566e10b373bf Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 13:19:23 -0700 Subject: [PATCH 09/15] Chinmay review, license/malioc --- ci/licenses_golden/licenses_flutter | 8 ++++++++ .../backend/vulkan/command_buffer_vk.cc | 8 +++----- impeller/tools/malioc.json | 19 ++++++++++--------- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 362ae744bd67f..b8bcb56bdd30d 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1496,6 +1496,10 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk.cc ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/compute_pass_vk.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/compute_pass_vk.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/compute_pipeline_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/context_vk.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/context_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/debug_report_vk.cc + ../../../flutter/LICENSE @@ -4158,6 +4162,10 @@ FILE: ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk.h +FILE: ../../../flutter/impeller/renderer/backend/vulkan/compute_pass_vk.cc +FILE: ../../../flutter/impeller/renderer/backend/vulkan/compute_pass_vk.h +FILE: ../../../flutter/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc +FILE: ../../../flutter/impeller/renderer/backend/vulkan/compute_pipeline_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/context_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/context_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/debug_report_vk.cc diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index 725276bd7f864..b66a091abfbc9 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -51,11 +51,9 @@ bool CommandBufferVK::OnSubmitCommands(CompletionCallback callback) { if (!callback) { return encoder_->Submit(); } - return encoder_->Submit([callback](bool submit) { - if (!submit) { - callback(CommandBuffer::Status::kError); - } - callback(CommandBuffer::Status::kCompleted); + return encoder_->Submit([callback](bool submitted) { + callback(submitted ? CommandBuffer::Status::kCompleted + : CommandBuffer::Status::kError); }); } diff --git a/impeller/tools/malioc.json b/impeller/tools/malioc.json index 4d232a02bf887..68afc86f992ed 100644 --- a/impeller/tools/malioc.json +++ b/impeller/tools/malioc.json @@ -13569,9 +13569,9 @@ "load_store" ], "longest_path_cycles": [ - 2.549999952316284, + 2.65625, 0.0, - 2.549999952316284, + 2.65625, 1.0, 72.0, 0.0 @@ -13585,23 +13585,24 @@ "texture" ], "shortest_path_bound_pipelines": [ - "load_store" + "arith_total", + "arith_cvt" ], "shortest_path_cycles": [ - 0.949999988079071, + 0.9375, + 0.0, + 0.9375, 0.0, - 0.949999988079071, 0.0, - 1.0, 0.0 ], "total_bound_pipelines": [ "load_store" ], "total_cycles": [ - 2.549999952316284, + 2.65625, 0.0, - 2.549999952316284, + 2.65625, 1.0, 72.0, 0.0 @@ -13611,7 +13612,7 @@ "stack_spill_bytes": 0, "thread_occupancy": 100, "uniform_registers_used": 8, - "work_registers_used": 21 + "work_registers_used": 17 } } } From b2b37ec72b81c5233fb7e46ece394b3865544f50 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 14:10:03 -0700 Subject: [PATCH 10/15] limits --- .../backend/vulkan/compute_pass_vk.cc | 19 ++++++++++++++++++- .../backend/vulkan/compute_pipeline_vk.cc | 2 +- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 7356a12db03e2..ce0722787e143 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -242,7 +242,24 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, )) { return false; } - cmd_buffer.dispatch(grid_size.width, grid_size.height, 1); + + // TOOD(dnfield): This should be moved to caps. But for now keeping this + // in parallel with Metal. + auto device_properties = vk_context.GetPhysicalDevice().getProperties(); + + auto max_wg_size = device_properties.limits.maxComputeWorkGroupSize; + + auto width = grid_size.width; + auto height = grid_size.height; + + while (width > max_wg_size[0]) { + width = std::max(1LL, width / 2); + } + while (height > max_wg_size[1]) { + height = std::max(1LL, height / 2); + } + + cmd_buffer.dispatch(width, height, 1); } } diff --git a/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc b/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc index 84187030d654d..af48c2691ddcc 100644 --- a/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc @@ -14,7 +14,7 @@ ComputePipelineVK::ComputePipelineVK( vk::UniquePipelineLayout layout, vk::UniqueDescriptorSetLayout descriptor_set_layout) : Pipeline(std::move(library), desc), - device_holder_(device_holder), + device_holder_(std::move(device_holder)), pipeline_(std::move(pipeline)), layout_(std::move(layout)), descriptor_set_layout_(std::move(descriptor_set_layout)) { From becb8283e8205db404fb3a7a6c6e5e655038c32d Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 14:16:19 -0700 Subject: [PATCH 11/15] typo --- impeller/renderer/backend/vulkan/compute_pass_vk.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index ce0722787e143..a594fe0cdc7ce 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -253,10 +253,10 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, auto height = grid_size.height; while (width > max_wg_size[0]) { - width = std::max(1LL, width / 2); + width = std::max(1L, width / 2); } while (height > max_wg_size[1]) { - height = std::max(1LL, height / 2); + height = std::max(1L, height / 2); } cmd_buffer.dispatch(width, height, 1); From 45aeb3bc5416ebfe919cd5d11a081625b4702847 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 14:41:58 -0700 Subject: [PATCH 12/15] ... --- impeller/renderer/backend/vulkan/compute_pass_vk.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index a594fe0cdc7ce..d60d05ae5d912 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -249,14 +249,14 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, auto max_wg_size = device_properties.limits.maxComputeWorkGroupSize; - auto width = grid_size.width; - auto height = grid_size.height; + int64_t width = grid_size.width; + int64_t height = grid_size.height; while (width > max_wg_size[0]) { - width = std::max(1L, width / 2); + width = std::max(1LL, width / 2); } while (height > max_wg_size[1]) { - height = std::max(1L, height / 2); + height = std::max(1LL, height / 2); } cmd_buffer.dispatch(width, height, 1); From 53220acca0908dbf9d558fe025ccf8c54c7f4cff Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 15:11:00 -0700 Subject: [PATCH 13/15] fix type clash on different arch --- impeller/renderer/backend/vulkan/compute_pass_vk.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index d60d05ae5d912..bb1651c83d332 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -253,10 +253,10 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, int64_t height = grid_size.height; while (width > max_wg_size[0]) { - width = std::max(1LL, width / 2); + width = std::max(static_cast(1), width / 2); } while (height > max_wg_size[1]) { - height = std::max(1LL, height / 2); + height = std::max(static_cast(1), height / 2); } cmd_buffer.dispatch(width, height, 1); From 454ed44ec90f78fe7862780f93659b308b6b713d Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 15:18:15 -0700 Subject: [PATCH 14/15] fix size --- impeller/renderer/prefix_sum_test.comp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/prefix_sum_test.comp b/impeller/renderer/prefix_sum_test.comp index 74523cbd15359..0f8bff231ffa5 100644 --- a/impeller/renderer/prefix_sum_test.comp +++ b/impeller/renderer/prefix_sum_test.comp @@ -2,7 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -layout(local_size_x = 512, local_size_y = 1) in; +// TODO(dnfield): This should not need to be so small, +// https://github.com/flutter/flutter/issues/119357 +layout(local_size_x = 256, local_size_y = 1) in; layout(std430) buffer; #include From d26abf2c8e978bfd06e789f1b0292416bdebfd31 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 1 Jun 2023 15:50:32 -0700 Subject: [PATCH 15/15] more shaders --- impeller/renderer/path_polyline.comp | 2 +- impeller/renderer/stroke.comp | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/impeller/renderer/path_polyline.comp b/impeller/renderer/path_polyline.comp index 73850a80b0786..0804c8f12d827 100644 --- a/impeller/renderer/path_polyline.comp +++ b/impeller/renderer/path_polyline.comp @@ -3,7 +3,7 @@ // found in the LICENSE file. #extension GL_KHR_shader_subgroup_arithmetic : enable -layout(local_size_x = 512, local_size_y = 1) in; +layout(local_size_x = 256, local_size_y = 1) in; layout(std430) buffer; #include diff --git a/impeller/renderer/stroke.comp b/impeller/renderer/stroke.comp index 0090c0af7336d..5d9cf55fafa4b 100644 --- a/impeller/renderer/stroke.comp +++ b/impeller/renderer/stroke.comp @@ -3,11 +3,7 @@ // found in the LICENSE file. #extension GL_KHR_shader_subgroup_arithmetic : enable -#define N_ROWS 8 -#define LG_WG_SIZE 9 -#define WG_SIZE (1 << LG_WG_SIZE) - -layout(local_size_x = WG_SIZE, local_size_y = 1) in; +layout(local_size_x = 256, local_size_y = 1) in; layout(std430) buffer; layout(binding = 0) buffer Polyline {