From ff42fb26bbc51332110636f1c6d4a59168723146 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Fri, 14 Jul 2023 11:25:49 -0700 Subject: [PATCH] [Impeller] Patch the compiler to account for subpass inputs and PSO metadata. Towards https://github.com/flutter/flutter/issues/128911 Drive by fixes https://github.com/flutter/flutter/issues/123795 --- impeller/aiks/aiks_unittests.cc | 2 +- impeller/compiler/code_gen_template.h | 12 +++++++- impeller/compiler/compiler.cc | 17 +++++------ impeller/compiler/compiler_backend.cc | 5 ++-- impeller/compiler/reflector.cc | 30 +++++++++++++++++-- impeller/core/shader_types.h | 10 +++++++ .../backend/vulkan/playground_impl_vk.cc | 4 +++ .../backend/vulkan/descriptor_pool_vk.cc | 4 ++- impeller/renderer/backend/vulkan/formats_vk.h | 8 ++--- .../embedder_surface_metal_impeller.mm | 2 +- 10 files changed, 70 insertions(+), 24 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 3438056a0b6a9..3772a2b77a242 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1636,7 +1636,7 @@ TEST_P(AiksTest, ColorWheel) { auto callback = [&](AiksContext& renderer) -> std::optional { // UI state. static bool cache_the_wheel = true; - static int current_blend_index = 3; + static int current_blend_index = 14; static float dst_alpha = 1; static float src_alpha = 1; static Color color0 = Color::Red(); diff --git a/impeller/compiler/code_gen_template.h b/impeller/compiler/code_gen_template.h index 61f442d824dd4..45f7612568730 100644 --- a/impeller/compiler/code_gen_template.h +++ b/impeller/compiler/code_gen_template.h @@ -173,10 +173,11 @@ std::move({{ arg.argument_name }}){% if not loop.is_last %}, {% endif %} // =========================================================================== // Metadata for Vulkan ======================================================= // =========================================================================== - static constexpr std::array kDescriptorSetLayouts{ + static constexpr std::array kDescriptorSetLayouts{ {% for buffer in buffers %} DescriptorSetLayout{ {{buffer.binding}}, // binding = {{buffer.binding}} + {{buffer.input_attachment_index}}, // input_attachment_index = {{buffer.input_attachment_index}} {{buffer.descriptor_type}}, // descriptor_type = {{buffer.descriptor_type}} {{to_shader_stage(shader_stage)}}, // shader_stage = {{to_shader_stage(shader_stage)}} }, @@ -184,9 +185,18 @@ std::move({{ arg.argument_name }}){% if not loop.is_last %}, {% endif %} {% for sampled_image in sampled_images %} DescriptorSetLayout{ {{sampled_image.binding}}, // binding = {{sampled_image.binding}} + {{sampled_image.input_attachment_index}}, // input_attachment_index = {{sampled_image.input_attachment_index}} {{sampled_image.descriptor_type}}, // descriptor_type = {{sampled_image.descriptor_type}} {{to_shader_stage(shader_stage)}}, // shader_stage = {{to_shader_stage(shader_stage)}} }, +{% endfor %} +{% for subpass_input in subpass_inputs %} + DescriptorSetLayout{ + {{subpass_input.binding}}, // binding = {{subpass_input.binding}} + {{subpass_input.input_attachment_index}}, // input_attachment_index = {{subpass_input.input_attachment_index}} + {{subpass_input.descriptor_type}}, // descriptor_type = {{subpass_input.descriptor_type}} + {{to_shader_stage(shader_stage)}}, // shader_stage = {{to_shader_stage(shader_stage)}} + }, {% endfor %} }; diff --git a/impeller/compiler/compiler.cc b/impeller/compiler/compiler.cc index a5a2afb6d6ed0..e52c2941f558d 100644 --- a/impeller/compiler/compiler.cc +++ b/impeller/compiler/compiler.cc @@ -115,15 +115,13 @@ static CompilerBackend CreateMSLCompiler( static CompilerBackend CreateVulkanCompiler( const spirv_cross::ParsedIR& ir, const SourceOptions& source_options) { - // TODO(dnfield): It seems like what we'd want is a CompilerGLSL with - // vulkan_semantics set to true, but that has regressed some things on GLES - // somehow. In the mean time, go back to using CompilerMSL, but set the Metal - // Language version to something really high so that we don't get weird - // complaints about using Metal features while trying to build Vulkan shaders. - // https://github.com/flutter/flutter/issues/123795 - return CreateMSLCompiler( - ir, source_options, - spirv_cross::CompilerMSL::Options::make_msl_version(3, 0, 0)); + auto vk_compiler = std::make_shared(ir); + spirv_cross::CompilerGLSL::Options sl_options; + sl_options.vulkan_semantics = true; + sl_options.force_zero_initialized_variables = true; + sl_options.es = false; + vk_compiler->set_common_options(sl_options); + return CompilerBackend(vk_compiler); } static CompilerBackend CreateGLSLCompiler(const spirv_cross::ParsedIR& ir, @@ -218,6 +216,7 @@ static CompilerBackend CreateCompiler(const spirv_cross::ParsedIR& ir, break; case TargetPlatform::kSkSL: compiler = CreateSkSLCompiler(ir, source_options); + break; } if (!compiler) { return {}; diff --git a/impeller/compiler/compiler_backend.cc b/impeller/compiler/compiler_backend.cc index d6111c9f232e5..40c5390e51c7c 100644 --- a/impeller/compiler/compiler_backend.cc +++ b/impeller/compiler/compiler_backend.cc @@ -4,6 +4,8 @@ #include "impeller/compiler/compiler_backend.h" +#include + #include "impeller/base/comparable.h" namespace impeller { @@ -44,8 +46,7 @@ uint32_t CompilerBackend::GetExtendedMSLResourceBinding( if (auto compiler = GetGLSLCompiler()) { return compiler->get_decoration(id, spv::Decoration::DecorationBinding); } - const auto kOOBIndex = static_cast(-1); - return kOOBIndex; + return std::numeric_limits::max(); } const spirv_cross::Compiler* CompilerBackend::GetCompiler() const { diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc index d411422338466..65d77e79ad7f9 100644 --- a/impeller/compiler/reflector.cc +++ b/impeller/compiler/reflector.cc @@ -7,6 +7,7 @@ #include "impeller/compiler/reflector.h" #include +#include #include #include #include @@ -216,9 +217,9 @@ std::optional Reflector::GenerateTemplateArguments() const { if (auto storage_buffers_json = ReflectResources(shader_resources.storage_buffers); storage_buffers_json.has_value()) { - for (auto uniform_buffer : storage_buffers_json.value()) { - uniform_buffer["descriptor_type"] = "DescriptorType::kStorageBuffer"; - buffers.emplace_back(std::move(uniform_buffer)); + for (auto storage_buffer : storage_buffers_json.value()) { + storage_buffer["descriptor_type"] = "DescriptorType::kStorageBuffer"; + buffers.emplace_back(std::move(storage_buffer)); } } else { return std::nullopt; @@ -261,6 +262,19 @@ std::optional Reflector::GenerateTemplateArguments() const { } } + { + if (auto inputs = ReflectResources(shader_resources.subpass_inputs); + inputs.has_value()) { + auto& subpass_inputs = root["subpass_inputs"] = nlohmann::json::array_t{}; + for (auto input : inputs.value()) { + input["descriptor_type"] = "DescriptorType::kSubpassInput"; + subpass_inputs.emplace_back(std::move(input)); + } + } else { + return std::nullopt; + } + } + if (auto stage_outputs = ReflectResources(shader_resources.stage_outputs); stage_outputs.has_value()) { root["stage_outputs"] = std::move(stage_outputs.value()); @@ -431,6 +445,14 @@ std::vector Reflector::ComputeOffsets( return offsets; } +static uint32_t GetResourceDecorationIfPresent(const CompilerBackend& compiler, + spirv_cross::ID id, + spv::Decoration decoration) { + return compiler->has_decoration(id, decoration) + ? compiler->get_decoration(id, decoration) + : std::numeric_limits::max(); +} + std::optional Reflector::ReflectResource( const spirv_cross::Resource& resource, std::optional offset) const { @@ -445,6 +467,8 @@ std::optional Reflector::ReflectResource( resource.id, spv::Decoration::DecorationDescriptorSet); result["location"] = compiler_->get_decoration( resource.id, spv::Decoration::DecorationLocation); + result["input_attachment_index"] = GetResourceDecorationIfPresent( + compiler_, resource.id, spv::Decoration::DecorationInputAttachmentIndex); result["index"] = compiler_->get_decoration(resource.id, spv::Decoration::DecorationIndex); result["ext_res_0"] = compiler_.GetExtendedMSLResourceBinding( diff --git a/impeller/core/shader_types.h b/impeller/core/shader_types.h index 9ce7954998e0c..ca628f305cf89 100644 --- a/impeller/core/shader_types.h +++ b/impeller/core/shader_types.h @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include #include @@ -144,12 +145,21 @@ enum class DescriptorType { kSampledImage, kImage, kSampler, + kSubpassInput, }; struct DescriptorSetLayout { uint32_t binding; + uint32_t input_attachment_index; DescriptorType descriptor_type; ShaderStage shader_stage; + + constexpr std::optional GetInputAttachmentIndexIfValid() const { + if (std::numeric_limits::max() == input_attachment_index) { + return std::nullopt; + } + return input_attachment_index; + } }; template diff --git a/impeller/playground/backend/vulkan/playground_impl_vk.cc b/impeller/playground/backend/vulkan/playground_impl_vk.cc index 26ba03bbd2a32..43536b260fd49 100644 --- a/impeller/playground/backend/vulkan/playground_impl_vk.cc +++ b/impeller/playground/backend/vulkan/playground_impl_vk.cc @@ -13,6 +13,7 @@ #include "flutter/fml/logging.h" #include "flutter/fml/mapping.h" #include "impeller/entity/vk/entity_shaders_vk.h" +#include "impeller/entity/vk/framebuffer_blend_shaders_vk.h" #include "impeller/entity/vk/modern_shaders_vk.h" #include "impeller/fixtures/vk/fixtures_shaders_vk.h" #include "impeller/playground/imgui/vk/imgui_shaders_vk.h" @@ -33,6 +34,9 @@ ShaderLibraryMappingsForPlayground() { impeller_entity_shaders_vk_length), std::make_shared(impeller_modern_shaders_vk_data, impeller_modern_shaders_vk_length), + std::make_shared( + impeller_framebuffer_blend_shaders_vk_data, + impeller_framebuffer_blend_shaders_vk_length), std::make_shared( impeller_fixtures_shaders_vk_data, impeller_fixtures_shaders_vk_length), diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc index 59cc440efb9a9..f29ea01f40ba9 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc @@ -24,7 +24,9 @@ static vk::UniqueDescriptorPool CreatePool(const vk::Device& device, std::vector pools = { {vk::DescriptorType::eCombinedImageSampler, pool_count}, {vk::DescriptorType::eUniformBuffer, pool_count}, - {vk::DescriptorType::eStorageBuffer, pool_count}}; + {vk::DescriptorType::eStorageBuffer, pool_count}, + {vk::DescriptorType::eInputAttachment, pool_count}, + }; vk::DescriptorPoolCreateInfo pool_info; pool_info.setMaxSets(pools.size() * pool_count); diff --git a/impeller/renderer/backend/vulkan/formats_vk.h b/impeller/renderer/backend/vulkan/formats_vk.h index a865659764655..eebdcf68d1ac1 100644 --- a/impeller/renderer/backend/vulkan/formats_vk.h +++ b/impeller/renderer/backend/vulkan/formats_vk.h @@ -274,21 +274,17 @@ constexpr vk::DescriptorType ToVKDescriptorType(DescriptorType type) { switch (type) { case DescriptorType::kSampledImage: return vk::DescriptorType::eCombinedImageSampler; - break; case DescriptorType::kUniformBuffer: return vk::DescriptorType::eUniformBuffer; - break; case DescriptorType::kStorageBuffer: return vk::DescriptorType::eStorageBuffer; - break; case DescriptorType::kImage: return vk::DescriptorType::eSampledImage; - break; case DescriptorType::kSampler: return vk::DescriptorType::eSampler; - break; + case DescriptorType::kSubpassInput: + return vk::DescriptorType::eInputAttachment; } - FML_UNREACHABLE(); } diff --git a/shell/platform/embedder/embedder_surface_metal_impeller.mm b/shell/platform/embedder/embedder_surface_metal_impeller.mm index c9f463a059700..3698d4a8d6f7e 100644 --- a/shell/platform/embedder/embedder_surface_metal_impeller.mm +++ b/shell/platform/embedder/embedder_surface_metal_impeller.mm @@ -11,7 +11,7 @@ #include "flutter/fml/synchronization/sync_switch.h" #include "flutter/shell/gpu/gpu_surface_metal_delegate.h" #include "flutter/shell/gpu/gpu_surface_metal_impeller.h" -#import "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalImpeller.h" +#include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalImpeller.h" #include "impeller/entity/mtl/entity_shaders.h" #include "impeller/entity/mtl/framebuffer_blend_shaders.h" #include "impeller/entity/mtl/modern_shaders.h"