From 41381d267cf86118aa902aa434316bd65fd7c72c Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Wed, 13 Sep 2023 10:42:43 -0700 Subject: [PATCH] Revert "[Impeller] Patch the compiler to account for subpass inputs and PSO metadata. (#45739)" This reverts commit 6f223db0fc2729e24b9c255186c2766d89a8c056. --- 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, 24 insertions(+), 70 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index b813f42856668..6af0cbc9283a9 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1667,7 +1667,7 @@ TEST_P(AiksTest, ColorWheel) { auto callback = [&](AiksContext& renderer) -> std::optional { // UI state. static bool cache_the_wheel = true; - static int current_blend_index = 14; + static int current_blend_index = 3; 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 45f7612568730..61f442d824dd4 100644 --- a/impeller/compiler/code_gen_template.h +++ b/impeller/compiler/code_gen_template.h @@ -173,11 +173,10 @@ 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)}} }, @@ -185,18 +184,9 @@ 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 e52c2941f558d..a5a2afb6d6ed0 100644 --- a/impeller/compiler/compiler.cc +++ b/impeller/compiler/compiler.cc @@ -115,13 +115,15 @@ static CompilerBackend CreateMSLCompiler( static CompilerBackend CreateVulkanCompiler( const spirv_cross::ParsedIR& ir, const SourceOptions& source_options) { - 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); + // 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)); } static CompilerBackend CreateGLSLCompiler(const spirv_cross::ParsedIR& ir, @@ -216,7 +218,6 @@ 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 40c5390e51c7c..d6111c9f232e5 100644 --- a/impeller/compiler/compiler_backend.cc +++ b/impeller/compiler/compiler_backend.cc @@ -4,8 +4,6 @@ #include "impeller/compiler/compiler_backend.h" -#include - #include "impeller/base/comparable.h" namespace impeller { @@ -46,7 +44,8 @@ uint32_t CompilerBackend::GetExtendedMSLResourceBinding( if (auto compiler = GetGLSLCompiler()) { return compiler->get_decoration(id, spv::Decoration::DecorationBinding); } - return std::numeric_limits::max(); + const auto kOOBIndex = static_cast(-1); + return kOOBIndex; } const spirv_cross::Compiler* CompilerBackend::GetCompiler() const { diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc index 65d77e79ad7f9..d411422338466 100644 --- a/impeller/compiler/reflector.cc +++ b/impeller/compiler/reflector.cc @@ -7,7 +7,6 @@ #include "impeller/compiler/reflector.h" #include -#include #include #include #include @@ -217,9 +216,9 @@ std::optional Reflector::GenerateTemplateArguments() const { if (auto storage_buffers_json = ReflectResources(shader_resources.storage_buffers); storage_buffers_json.has_value()) { - for (auto storage_buffer : storage_buffers_json.value()) { - storage_buffer["descriptor_type"] = "DescriptorType::kStorageBuffer"; - buffers.emplace_back(std::move(storage_buffer)); + for (auto uniform_buffer : storage_buffers_json.value()) { + uniform_buffer["descriptor_type"] = "DescriptorType::kStorageBuffer"; + buffers.emplace_back(std::move(uniform_buffer)); } } else { return std::nullopt; @@ -262,19 +261,6 @@ 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()); @@ -445,14 +431,6 @@ 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 { @@ -467,8 +445,6 @@ 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 ca628f305cf89..9ce7954998e0c 100644 --- a/impeller/core/shader_types.h +++ b/impeller/core/shader_types.h @@ -5,7 +5,6 @@ #pragma once #include -#include #include #include #include @@ -145,21 +144,12 @@ 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 43536b260fd49..26ba03bbd2a32 100644 --- a/impeller/playground/backend/vulkan/playground_impl_vk.cc +++ b/impeller/playground/backend/vulkan/playground_impl_vk.cc @@ -13,7 +13,6 @@ #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" @@ -34,9 +33,6 @@ 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 f29ea01f40ba9..59cc440efb9a9 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc @@ -24,9 +24,7 @@ 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::eInputAttachment, pool_count}, - }; + {vk::DescriptorType::eStorageBuffer, 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 eebdcf68d1ac1..a865659764655 100644 --- a/impeller/renderer/backend/vulkan/formats_vk.h +++ b/impeller/renderer/backend/vulkan/formats_vk.h @@ -274,17 +274,21 @@ 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; - case DescriptorType::kSubpassInput: - return vk::DescriptorType::eInputAttachment; + break; } + FML_UNREACHABLE(); } diff --git a/shell/platform/embedder/embedder_surface_metal_impeller.mm b/shell/platform/embedder/embedder_surface_metal_impeller.mm index 3698d4a8d6f7e..c9f463a059700 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" -#include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalImpeller.h" +#import "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"