From 9180bfb20df5ba9b77e2ef5ab4e17cceeebac91b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 23 Nov 2023 23:36:58 -0800 Subject: [PATCH 01/18] [Impeller] Vulkan framebuffer fetch --- impeller/compiler/reflector.cc | 7 +++ impeller/entity/contents/content_context.cc | 44 ++++++++++++------ impeller/entity/contents/content_context.h | 4 +- .../contents/framebuffer_blend_contents.cc | 1 + .../backend/vulkan/playground_impl_vk.cc | 4 ++ .../backend/vulkan/binding_helpers_vk.cc | 34 +++++++++++--- .../backend/vulkan/binding_helpers_vk.h | 3 +- .../backend/vulkan/capabilities_vk.cc | 14 +++++- .../renderer/backend/vulkan/capabilities_vk.h | 4 ++ .../backend/vulkan/command_encoder_vk.cc | 3 +- .../backend/vulkan/command_encoder_vk.h | 1 + .../backend/vulkan/descriptor_pool_vk.cc | 15 ++++-- .../backend/vulkan/descriptor_pool_vk.h | 1 + .../backend/vulkan/pipeline_library_vk.cc | 46 +++++++++++++++++-- .../backend/vulkan/pipeline_library_vk.h | 1 + .../renderer/backend/vulkan/pipeline_vk.h | 1 - .../renderer/backend/vulkan/render_pass_vk.cc | 25 ++++++++-- .../renderer/backend/vulkan/render_pass_vk.h | 3 +- impeller/renderer/command.h | 2 + impeller/renderer/pipeline.h | 1 - impeller/renderer/pipeline_descriptor.cc | 4 +- impeller/renderer/pipeline_descriptor.h | 5 ++ .../android_context_vulkan_impeller.cc | 16 ++++--- 23 files changed, 192 insertions(+), 47 deletions(-) diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc index d411422338466..8d88fced2b27c 100644 --- a/impeller/compiler/reflector.cc +++ b/impeller/compiler/reflector.cc @@ -199,6 +199,13 @@ std::optional Reflector::GenerateTemplateArguments() const { } const auto shader_resources = compiler_->get_shader_resources(); + // Subpass inputs. + { + auto& subpass_inputs = root["subpass_inputs"] = nlohmann::json::array_t{}; + for (const auto& subpass_input : shader_resources.subpass_inputs) { + // + } + } // Uniform and storage buffers. { diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 9a2e66039b5f8..df3af455c07f7 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -221,49 +221,63 @@ ContentContext::ContentContext( if (context_->GetCapabilities()->SupportsFramebufferFetch()) { framebuffer_blend_color_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kColor), supports_decal}); + {static_cast(BlendSelectValues::kColor), supports_decal}, + true); framebuffer_blend_colorburn_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kColorBurn), supports_decal}); + {static_cast(BlendSelectValues::kColorBurn), supports_decal}, + true); framebuffer_blend_colordodge_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kColorDodge), supports_decal}); + {static_cast(BlendSelectValues::kColorDodge), supports_decal}, + true); framebuffer_blend_darken_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kDarken), supports_decal}); + {static_cast(BlendSelectValues::kDarken), supports_decal}, + true); framebuffer_blend_difference_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kDifference), supports_decal}); + {static_cast(BlendSelectValues::kDifference), supports_decal}, + true); framebuffer_blend_exclusion_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kExclusion), supports_decal}); + {static_cast(BlendSelectValues::kExclusion), supports_decal}, + true); framebuffer_blend_hardlight_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kHardLight), supports_decal}); + {static_cast(BlendSelectValues::kHardLight), supports_decal}, + true); framebuffer_blend_hue_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kHue), supports_decal}); + {static_cast(BlendSelectValues::kHue), supports_decal}, true); framebuffer_blend_lighten_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kLighten), supports_decal}); + {static_cast(BlendSelectValues::kLighten), supports_decal}, + true); framebuffer_blend_luminosity_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kLuminosity), supports_decal}); + {static_cast(BlendSelectValues::kLuminosity), supports_decal}, + true); framebuffer_blend_multiply_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kMultiply), supports_decal}); + {static_cast(BlendSelectValues::kMultiply), supports_decal}, + true); framebuffer_blend_overlay_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kOverlay), supports_decal}); + {static_cast(BlendSelectValues::kOverlay), supports_decal}, + true); framebuffer_blend_saturation_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kSaturation), supports_decal}); + {static_cast(BlendSelectValues::kSaturation), supports_decal}, + true); framebuffer_blend_screen_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kScreen), supports_decal}); + {static_cast(BlendSelectValues::kScreen), supports_decal}, + true); framebuffer_blend_softlight_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kSoftLight), supports_decal}); + {static_cast(BlendSelectValues::kSoftLight), supports_decal}, + true); } blend_color_pipelines_.CreateDefault( diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index e262850bb3935..4f9f93a3815d5 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -713,13 +713,15 @@ class ContentContext { void CreateDefault(const Context& context, const ContentContextOptions& options, - const std::initializer_list& constants = {}) { + const std::initializer_list& constants = {}, + bool has_subpass_input = false) { auto desc = PipelineT::Builder::MakeDefaultPipelineDescriptor(context, constants); if (!desc.has_value()) { VALIDATION_LOG << "Failed to create default pipeline."; return; } + desc->SetHasSubpassDependency(has_subpass_input); options.ApplyToPipelineDescriptor(*desc); SetDefault(options, std::make_unique(context, desc)); } diff --git a/impeller/entity/contents/framebuffer_blend_contents.cc b/impeller/entity/contents/framebuffer_blend_contents.cc index 7edc478076499..d9b7a2124aa9f 100644 --- a/impeller/entity/contents/framebuffer_blend_contents.cc +++ b/impeller/entity/contents/framebuffer_blend_contents.cc @@ -146,6 +146,7 @@ bool FramebufferBlendContents::Render(const ContentContext& renderer, frag_info.src_input_alpha = src_snapshot->opacity; FS::BindFragInfo(cmd, host_buffer.EmplaceUniform(frag_info)); + cmd.bind_framebuffer = true; return pass.AddCommand(std::move(cmd)); } diff --git a/impeller/playground/backend/vulkan/playground_impl_vk.cc b/impeller/playground/backend/vulkan/playground_impl_vk.cc index 675558f6e53d8..e74541fcfdaf8 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/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index 9acc128ee3201..ebaf99edca4fa 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -14,6 +14,7 @@ #include "impeller/renderer/backend/vulkan/vk.h" #include "impeller/renderer/command.h" #include "impeller/renderer/compute_command.h" +#include "vulkan/vulkan_core.h" namespace impeller { @@ -117,7 +118,8 @@ static bool BindBuffers(const Bindings& bindings, fml::StatusOr> AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, - const std::vector& commands) { + const std::vector& commands, + const vk::ImageView& image_view) { if (commands.empty()) { return std::vector{}; } @@ -127,6 +129,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( // to allocate a correctly sized descriptor pool. size_t buffer_count = 0; size_t samplers_count = 0; + size_t subpass_count = 0; std::vector layouts; layouts.reserve(commands.size()); @@ -134,12 +137,13 @@ fml::StatusOr> AllocateAndBindDescriptorSets( buffer_count += command.vertex_bindings.buffers.size(); buffer_count += command.fragment_bindings.buffers.size(); samplers_count += command.fragment_bindings.sampled_images.size(); + subpass_count += command.bind_framebuffer ? 1 : 0; layouts.emplace_back( PipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout()); } - auto descriptor_result = - encoder->AllocateDescriptorSets(buffer_count, samplers_count, layouts); + auto descriptor_result = encoder->AllocateDescriptorSets( + buffer_count, samplers_count, subpass_count, layouts); if (!descriptor_result.ok()) { return descriptor_result.status(); } @@ -153,9 +157,9 @@ fml::StatusOr> AllocateAndBindDescriptorSets( std::vector images; std::vector buffers; std::vector writes; - images.reserve(samplers_count); + images.reserve(samplers_count + subpass_count); buffers.reserve(buffer_count); - writes.reserve(samplers_count + buffer_count); + writes.reserve(samplers_count + buffer_count + subpass_count); auto& allocator = *context.GetResourceAllocator(); auto desc_index = 0u; @@ -173,6 +177,24 @@ fml::StatusOr> AllocateAndBindDescriptorSets( return fml::Status(fml::StatusCode::kUnknown, "Failed to bind texture or buffer."); } + + if (command.bind_framebuffer) { + vk::DescriptorImageInfo image_info; + image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; + image_info.sampler = VK_NULL_HANDLE; + image_info.imageView = image_view; + images.push_back(image_info); + + vk::WriteDescriptorSet write_set; + write_set.dstSet = descriptor_sets[desc_index]; + // MAGIC! see description in pipeline_library_vk.cc + write_set.dstBinding = 64u; + write_set.descriptorCount = 1u; + write_set.descriptorType = vk::DescriptorType::eInputAttachment; + write_set.pImageInfo = &images.back(); + + writes.push_back(write_set); + } desc_index += 1; } @@ -203,7 +225,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( ComputePipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout()); } auto descriptor_result = - encoder->AllocateDescriptorSets(buffer_count, samplers_count, layouts); + encoder->AllocateDescriptorSets(buffer_count, samplers_count, 0, layouts); if (!descriptor_result.ok()) { return descriptor_result.status(); } diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.h b/impeller/renderer/backend/vulkan/binding_helpers_vk.h index 588e2a7cbacaa..ba2cc035046e0 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.h +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.h @@ -16,7 +16,8 @@ namespace impeller { fml::StatusOr> AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, - const std::vector& commands); + const std::vector& commands, + const vk::ImageView& image_view); fml::StatusOr> AllocateAndBindDescriptorSets( const ContextVK& context, diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index 8af5a31f0b3ca..add5bfa0f9522 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -9,6 +9,7 @@ #include "impeller/base/validation.h" #include "impeller/core/formats.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "vulkan/vulkan_core.h" namespace impeller { @@ -156,6 +157,10 @@ static const char* GetDeviceExtensionName(OptionalDeviceExtensionVK ext) { switch (ext) { case OptionalDeviceExtensionVK::kEXTPipelineCreationFeedback: return VK_EXT_PIPELINE_CREATION_FEEDBACK_EXTENSION_NAME; + case OptionalDeviceExtensionVK::kARMRasterizationOrderAttachmentAccess: + return VK_ARM_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; + case OptionalDeviceExtensionVK::kEXTRasterizationOrderAttachmentAccess: + return VK_EXT_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; case OptionalDeviceExtensionVK::kLast: return "Unknown"; } @@ -401,6 +406,13 @@ bool CapabilitiesVK::SetPhysicalDevice(const vk::PhysicalDevice& device) { }); } + supports_framebuffer_fetch_ = + optional_device_extensions_.find( + OptionalDeviceExtensionVK::kARMRasterizationOrderAttachmentAccess) != + optional_device_extensions_.end(); + FML_LOG(ERROR) << "supports framebuffer fetch: " + << supports_framebuffer_fetch_; + return true; } @@ -431,7 +443,7 @@ bool CapabilitiesVK::SupportsTextureToTextureBlits() const { // |Capabilities| bool CapabilitiesVK::SupportsFramebufferFetch() const { - return false; + return supports_framebuffer_fetch_; } // |Capabilities| diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index f7ffd2232fc7c..df81e7373704c 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -22,6 +22,8 @@ class ContextVK; enum class OptionalDeviceExtensionVK : uint32_t { // https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_pipeline_creation_feedback.html kEXTPipelineCreationFeedback, + kARMRasterizationOrderAttachmentAccess, + kEXTRasterizationOrderAttachmentAccess, kLast, }; @@ -110,6 +112,8 @@ class CapabilitiesVK final : public Capabilities, vk::PhysicalDeviceProperties device_properties_; bool supports_compute_subgroups_ = false; bool supports_device_transient_textures_ = false; + bool supports_framebuffer_fetch_ = false; + ; bool is_valid_ = false; bool HasExtension(const std::string& ext) const; diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index 52e4c6c74a7b1..07eb45372a07e 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -302,13 +302,14 @@ fml::StatusOr> CommandEncoderVK::AllocateDescriptorSets( uint32_t buffer_count, uint32_t sampler_count, + uint32_t subpass_count, const std::vector& layouts) { if (!IsValid()) { return fml::Status(fml::StatusCode::kUnknown, "command encoder invalid"); } return tracked_objects_->GetDescriptorPool().AllocateDescriptorSets( - buffer_count, sampler_count, layouts); + buffer_count, sampler_count, subpass_count, layouts); } void CommandEncoderVK::PushDebugGroup(const char* label) const { diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.h b/impeller/renderer/backend/vulkan/command_encoder_vk.h index ea8eb08328a6a..4a1d0a7afdc47 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.h +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.h @@ -85,6 +85,7 @@ class CommandEncoderVK { fml::StatusOr> AllocateDescriptorSets( uint32_t buffer_count, uint32_t sampler_count, + uint32_t subpass_count, const std::vector& layouts); private: diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc index ee82614bfd6e7..f79f7273fb4d8 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc @@ -6,6 +6,7 @@ #include "flutter/fml/trace_event.h" #include "impeller/base/validation.h" +#include "vulkan/vulkan_structs.hpp" namespace impeller { @@ -19,7 +20,8 @@ DescriptorPoolVK::~DescriptorPoolVK() = default; static vk::UniqueDescriptorPool CreatePool(const vk::Device& device, uint32_t image_count, - uint32_t buffer_count) { + uint32_t buffer_count, + uint32_t subpass_count) { TRACE_EVENT0("impeller", "CreateDescriptorPool"); std::vector pools = {}; if (image_count > 0) { @@ -32,8 +34,12 @@ static vk::UniqueDescriptorPool CreatePool(const vk::Device& device, pools.emplace_back(vk::DescriptorPoolSize{ vk::DescriptorType::eStorageBuffer, buffer_count}); } + if (subpass_count > 0) { + pools.emplace_back(vk::DescriptorPoolSize{ + vk::DescriptorType::eInputAttachment, subpass_count}); + } vk::DescriptorPoolCreateInfo pool_info; - pool_info.setMaxSets(image_count + buffer_count); + pool_info.setMaxSets(image_count + buffer_count + subpass_count); pool_info.setPoolSizes(pools); auto [result, pool] = device.createDescriptorPoolUnique(pool_info); if (result != vk::Result::eSuccess) { @@ -46,14 +52,15 @@ fml::StatusOr> DescriptorPoolVK::AllocateDescriptorSets( uint32_t buffer_count, uint32_t sampler_count, + uint32_t subpass_count, const std::vector& layouts) { std::shared_ptr strong_device = device_holder_.lock(); if (!strong_device) { return fml::Status(fml::StatusCode::kUnknown, "No device"); } - auto new_pool = - CreatePool(strong_device->GetDevice(), sampler_count, buffer_count); + auto new_pool = CreatePool(strong_device->GetDevice(), sampler_count, + buffer_count, subpass_count); if (!new_pool) { return fml::Status(fml::StatusCode::kUnknown, "Failed to create descriptor pool"); diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h index d5e71a10969f6..8cc3352d729bb 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h @@ -33,6 +33,7 @@ class DescriptorPoolVK { fml::StatusOr> AllocateDescriptorSets( uint32_t buffer_count, uint32_t sampler_count, + uint32_t subpass_count, const std::vector& layouts); private: diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index 8fa14a2e3b35f..fa184afeeb6e5 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -19,6 +19,8 @@ #include "impeller/renderer/backend/vulkan/pipeline_vk.h" #include "impeller/renderer/backend/vulkan/shader_function_vk.h" #include "impeller/renderer/backend/vulkan/vertex_descriptor_vk.h" +#include "vulkan/vulkan_core.h" +#include "vulkan/vulkan_enums.hpp" namespace impeller { @@ -28,6 +30,7 @@ PipelineLibraryVK::PipelineLibraryVK( fml::UniqueFD cache_directory, std::shared_ptr worker_task_runner) : device_holder_(device_holder), + supports_framebuffer_fetch_(caps->SupportsFramebufferFetch()), pso_cache_(std::make_shared(std::move(caps), device_holder, std::move(cache_directory))), @@ -78,10 +81,12 @@ static vk::AttachmentDescription CreatePlaceholderAttachmentDescription( /// static vk::UniqueRenderPass CreateCompatRenderPassForPipeline( const vk::Device& device, - const PipelineDescriptor& desc) { + const PipelineDescriptor& desc, + bool supports_framebuffer_fetch) { std::vector attachments; std::vector color_refs; + std::vector subpass_color_ref; vk::AttachmentReference depth_stencil_ref = kUnusedAttachmentReference; color_refs.resize(desc.GetMaxColorAttacmentBindIndex() + 1, @@ -96,6 +101,8 @@ static vk::UniqueRenderPass CreateCompatRenderPassForPipeline( attachments.emplace_back( CreatePlaceholderAttachmentDescription(color.format, sample_count)); } + subpass_color_ref.push_back(vk::AttachmentReference{ + static_cast(0), vk::ImageLayout::eColorAttachmentOptimal}); if (auto depth = desc.GetDepthStencilAttachmentDescriptor(); depth.has_value()) { @@ -115,6 +122,18 @@ static vk::UniqueRenderPass CreateCompatRenderPassForPipeline( vk::SubpassDescription subpass_desc; subpass_desc.pipelineBindPoint = vk::PipelineBindPoint::eGraphics; + + std::vector subpass_dependencies; + + // If the device supports framebuffer fetch, compatibility pipelines are + // always created with the self reference and rasterization order flag. This + // ensures that all compiled pipelines are compatible with a render pass that + // contains a framebuffer fetch shader (advanced blends). + if (supports_framebuffer_fetch) { + subpass_desc.setFlags(vk::SubpassDescriptionFlagBits:: + eRasterizationOrderAttachmentColorAccessARM); + subpass_desc.setInputAttachments(subpass_color_ref); + } subpass_desc.setColorAttachments(color_refs); subpass_desc.setPDepthStencilAttachment(&depth_stencil_ref); @@ -122,6 +141,7 @@ static vk::UniqueRenderPass CreateCompatRenderPassForPipeline( render_pass_desc.setAttachments(attachments); render_pass_desc.setPSubpasses(&subpass_desc); render_pass_desc.setSubpassCount(1u); + render_pass_desc.setDependencies(subpass_dependencies); auto [result, pass] = device.createRenderPassUnique(render_pass_desc); if (result != vk::Result::eSuccess) { @@ -368,8 +388,8 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( return nullptr; } - auto render_pass = - CreateCompatRenderPassForPipeline(strong_device->GetDevice(), desc); + auto render_pass = CreateCompatRenderPassForPipeline( + strong_device->GetDevice(), desc, supports_framebuffer_fetch_); if (render_pass) { pipeline_info.setBasePipelineHandle(VK_NULL_HANDLE); pipeline_info.setSubpass(0); @@ -418,6 +438,26 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( auto vk_desc_layout = ToVKDescriptorSetLayoutBinding(layout); desc_bindings.push_back(vk_desc_layout); } + // Add subpass dependency binding. + if (desc.GetHasSubpassDependency()) { + vk::DescriptorSetLayoutBinding binding; + // _record scratch, freeze frame_ + // + // I bet you're wondering where I got this number? Well its one less than + // the smallest fragment binding listed in the generated header for the + // framebuffer fetch shader. In order to look this up for real, we need to + // add support for this data to impellerc, but more importantly, we need to + // resolve trying to share a generated header across all platforms. Only the + // header generated from a shader compiled with Vulkan semantics will have + // this binding, but today its unspecified if we use the GLES or Metal + // version based on host platform. To that end, it would be safer to leave + // this as a hardcoded value until such a time as the bindings are fixed. + binding.binding = 64u; + binding.descriptorCount = 1u; + binding.descriptorType = vk::DescriptorType::eInputAttachment; + binding.stageFlags = vk::ShaderStageFlagBits::eFragment; + desc_bindings.push_back(binding); + } vk::DescriptorSetLayoutCreateInfo descs_layout_info; descs_layout_info.setBindings(desc_bindings); diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.h b/impeller/renderer/backend/vulkan/pipeline_library_vk.h index 9a5fce5976e9b..923dad397a731 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.h @@ -35,6 +35,7 @@ class PipelineLibraryVK final friend ContextVK; std::weak_ptr device_holder_; + bool supports_framebuffer_fetch_ = false; std::shared_ptr pso_cache_; std::shared_ptr worker_task_runner_; Mutex pipelines_mutex_; diff --git a/impeller/renderer/backend/vulkan/pipeline_vk.h b/impeller/renderer/backend/vulkan/pipeline_vk.h index 5d92afaf6c25b..0b6b3b6e036e4 100644 --- a/impeller/renderer/backend/vulkan/pipeline_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_vk.h @@ -6,7 +6,6 @@ #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" diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 32ecf0534e67b..8c4b80c9c0635 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -21,6 +21,7 @@ #include "impeller/renderer/backend/vulkan/pipeline_vk.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" +#include "vulkan/vulkan_enums.hpp" #include "vulkan/vulkan_handles.hpp" #include "vulkan/vulkan_to_string.hpp" @@ -96,7 +97,8 @@ static void SetTextureLayout( SharedHandleVK RenderPassVK::CreateVKRenderPass( const ContextVK& context, - const std::shared_ptr& command_buffer) const { + const std::shared_ptr& command_buffer, + bool supports_framebuffer_fetch) const { std::vector attachments; std::vector color_refs; @@ -161,6 +163,16 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( subpass_desc.setResolveAttachments(resolve_refs); subpass_desc.setPDepthStencilAttachment(&depth_stencil_ref); + std::vector subpass_dependencies; + std::vector subpass_color_ref; + subpass_color_ref.push_back(vk::AttachmentReference{ + static_cast(0), vk::ImageLayout::eColorAttachmentOptimal}); + if (supports_framebuffer_fetch) { + subpass_desc.setFlags(vk::SubpassDescriptionFlagBits:: + eRasterizationOrderAttachmentColorAccessARM); + subpass_desc.setInputAttachments(subpass_color_ref); + } + vk::RenderPassCreateInfo render_pass_desc; render_pass_desc.setAttachments(attachments); render_pass_desc.setPSubpasses(&subpass_desc); @@ -245,7 +257,6 @@ SharedHandleVK RenderPassVK::CreateVKFramebuffer( const auto target_size = render_target_.GetRenderTargetSize(); fb_info.width = target_size.width; fb_info.height = target_size.height; - fb_info.layers = 1u; std::vector attachments; @@ -495,7 +506,9 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { const auto& target_size = render_target_.GetRenderTargetSize(); - auto render_pass = CreateVKRenderPass(vk_context, command_buffer); + auto render_pass = CreateVKRenderPass( + vk_context, command_buffer, + vk_context.GetCapabilities()->SupportsFramebufferFetch()); if (!render_pass) { VALIDATION_LOG << "Could not create renderpass."; return false; @@ -521,8 +534,10 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { static_cast(target_size.height); pass_info.setClearValues(clear_values); - auto desc_sets_result = - AllocateAndBindDescriptorSets(vk_context, encoder, commands_); + auto color_image_view = + TextureVK::Cast(*render_target_.GetRenderTargetTexture()).GetImageView(); + auto desc_sets_result = AllocateAndBindDescriptorSets( + vk_context, encoder, commands_, color_image_view); if (!desc_sets_result.ok()) { return false; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 3411b9628fbae..b7164d9d6c1f0 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -43,7 +43,8 @@ class RenderPassVK final : public RenderPass { SharedHandleVK CreateVKRenderPass( const ContextVK& context, - const std::shared_ptr& command_buffer) const; + const std::shared_ptr& command_buffer, + bool has_subpass_dependency) const; SharedHandleVK CreateVKFramebuffer( const ContextVK& context, diff --git a/impeller/renderer/command.h b/impeller/renderer/command.h index dd101d4f66118..18c32165f523d 100644 --- a/impeller/renderer/command.h +++ b/impeller/renderer/command.h @@ -135,6 +135,8 @@ struct Command : public ResourceBinder { /// IndexType index_type = IndexType::kUnknown; + bool bind_framebuffer = false; + #ifdef IMPELLER_DEBUG //---------------------------------------------------------------------------- /// The debugging label to use for the command. diff --git a/impeller/renderer/pipeline.h b/impeller/renderer/pipeline.h index 4426a8984406d..1c761c0a6ff03 100644 --- a/impeller/renderer/pipeline.h +++ b/impeller/renderer/pipeline.h @@ -7,7 +7,6 @@ #include #include "compute_pipeline_descriptor.h" -#include "flutter/fml/macros.h" #include "impeller/renderer/compute_pipeline_builder.h" #include "impeller/renderer/compute_pipeline_descriptor.h" #include "impeller/renderer/context.h" diff --git a/impeller/renderer/pipeline_descriptor.cc b/impeller/renderer/pipeline_descriptor.cc index 22ee4c30c4945..39669c1ede949 100644 --- a/impeller/renderer/pipeline_descriptor.cc +++ b/impeller/renderer/pipeline_descriptor.cc @@ -45,6 +45,7 @@ std::size_t PipelineDescriptor::GetHash() const { fml::HashCombineSeed(seed, cull_mode_); fml::HashCombineSeed(seed, primitive_type_); fml::HashCombineSeed(seed, polygon_mode_); + fml::HashCombineSeed(seed, has_subpass_dependency_); return seed; } @@ -65,7 +66,8 @@ bool PipelineDescriptor::IsEqual(const PipelineDescriptor& other) const { cull_mode_ == other.cull_mode_ && primitive_type_ == other.primitive_type_ && polygon_mode_ == other.polygon_mode_ && - specialization_constants_ == other.specialization_constants_; + specialization_constants_ == other.specialization_constants_ && + has_subpass_dependency_ == other.has_subpass_dependency_; } PipelineDescriptor& PipelineDescriptor::SetLabel(std::string label) { diff --git a/impeller/renderer/pipeline_descriptor.h b/impeller/renderer/pipeline_descriptor.h index 1045ea8fbd3fd..8ea81df1247e4 100644 --- a/impeller/renderer/pipeline_descriptor.h +++ b/impeller/renderer/pipeline_descriptor.h @@ -128,6 +128,10 @@ class PipelineDescriptor final : public Comparable { const std::vector& GetSpecializationConstants() const; + void SetHasSubpassDependency(bool value) { has_subpass_dependency_ = value; } + + bool GetHasSubpassDependency() const { return has_subpass_dependency_; } + private: std::string label_; SampleCount sample_count_ = SampleCount::kCount1; @@ -146,6 +150,7 @@ class PipelineDescriptor final : public Comparable { back_stencil_attachment_descriptor_; PrimitiveType primitive_type_ = PrimitiveType::kTriangle; PolygonMode polygon_mode_ = PolygonMode::kFill; + bool has_subpass_dependency_ = false; std::vector specialization_constants_; }; diff --git a/shell/platform/android/android_context_vulkan_impeller.cc b/shell/platform/android/android_context_vulkan_impeller.cc index d9521c35b2b89..c40f997924e07 100644 --- a/shell/platform/android/android_context_vulkan_impeller.cc +++ b/shell/platform/android/android_context_vulkan_impeller.cc @@ -6,6 +6,7 @@ #include "flutter/fml/paths.h" #include "flutter/impeller/entity/vk/entity_shaders_vk.h" +#include "flutter/impeller/entity/vk/framebuffer_blend_shaders_vk.h" #include "flutter/impeller/entity/vk/modern_shaders_vk.h" #include "flutter/impeller/renderer/backend/vulkan/context_vk.h" @@ -19,14 +20,17 @@ static std::shared_ptr CreateImpellerContext( const fml::RefPtr& proc_table, bool enable_vulkan_validation) { std::vector> shader_mappings = { - std::make_shared(impeller_entity_shaders_vk_data, - impeller_entity_shaders_vk_length), + std::make_shared(impeller_entity_shaders_vk_data, + impeller_entity_shaders_vk_length), + std::make_shared( + impeller_framebuffer_blend_shaders_vk_data, + impeller_framebuffer_blend_shaders_vk_length), #if IMPELLER_ENABLE_3D - std::make_shared(impeller_scene_shaders_vk_data, - impeller_scene_shaders_vk_length), + std::make_shared(impeller_scene_shaders_vk_data, + impeller_scene_shaders_vk_length), #endif - std::make_shared(impeller_modern_shaders_vk_data, - impeller_modern_shaders_vk_length), + std::make_shared(impeller_modern_shaders_vk_data, + impeller_modern_shaders_vk_length), }; PFN_vkGetInstanceProcAddr instance_proc_addr = From a9fd1449d7424fee0f59333b0e98f29b403067c3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 28 Nov 2023 13:40:59 -0800 Subject: [PATCH 02/18] adjustments --- impeller/compiler/reflector.cc | 7 ------- impeller/renderer/backend/vulkan/capabilities_vk.cc | 6 +----- impeller/renderer/backend/vulkan/capabilities_vk.h | 2 -- impeller/renderer/backend/vulkan/pipeline_library_vk.cc | 3 +-- 4 files changed, 2 insertions(+), 16 deletions(-) diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc index 8d88fced2b27c..d411422338466 100644 --- a/impeller/compiler/reflector.cc +++ b/impeller/compiler/reflector.cc @@ -199,13 +199,6 @@ std::optional Reflector::GenerateTemplateArguments() const { } const auto shader_resources = compiler_->get_shader_resources(); - // Subpass inputs. - { - auto& subpass_inputs = root["subpass_inputs"] = nlohmann::json::array_t{}; - for (const auto& subpass_input : shader_resources.subpass_inputs) { - // - } - } // Uniform and storage buffers. { diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index add5bfa0f9522..f7eb19047cceb 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -157,8 +157,6 @@ static const char* GetDeviceExtensionName(OptionalDeviceExtensionVK ext) { switch (ext) { case OptionalDeviceExtensionVK::kEXTPipelineCreationFeedback: return VK_EXT_PIPELINE_CREATION_FEEDBACK_EXTENSION_NAME; - case OptionalDeviceExtensionVK::kARMRasterizationOrderAttachmentAccess: - return VK_ARM_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; case OptionalDeviceExtensionVK::kEXTRasterizationOrderAttachmentAccess: return VK_EXT_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; case OptionalDeviceExtensionVK::kLast: @@ -408,10 +406,8 @@ bool CapabilitiesVK::SetPhysicalDevice(const vk::PhysicalDevice& device) { supports_framebuffer_fetch_ = optional_device_extensions_.find( - OptionalDeviceExtensionVK::kARMRasterizationOrderAttachmentAccess) != + OptionalDeviceExtensionVK::kEXTRasterizationOrderAttachmentAccess) != optional_device_extensions_.end(); - FML_LOG(ERROR) << "supports framebuffer fetch: " - << supports_framebuffer_fetch_; return true; } diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index df81e7373704c..1577ca64f0a7a 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -10,7 +10,6 @@ #include #include -#include "flutter/fml/macros.h" #include "impeller/base/backend_cast.h" #include "impeller/renderer/backend/vulkan/vk.h" #include "impeller/renderer/capabilities.h" @@ -22,7 +21,6 @@ class ContextVK; enum class OptionalDeviceExtensionVK : uint32_t { // https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_pipeline_creation_feedback.html kEXTPipelineCreationFeedback, - kARMRasterizationOrderAttachmentAccess, kEXTRasterizationOrderAttachmentAccess, kLast, }; diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index fa184afeeb6e5..5e693ed33653c 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -123,12 +123,11 @@ static vk::UniqueRenderPass CreateCompatRenderPassForPipeline( vk::SubpassDescription subpass_desc; subpass_desc.pipelineBindPoint = vk::PipelineBindPoint::eGraphics; - std::vector subpass_dependencies; - // If the device supports framebuffer fetch, compatibility pipelines are // always created with the self reference and rasterization order flag. This // ensures that all compiled pipelines are compatible with a render pass that // contains a framebuffer fetch shader (advanced blends). + std::vector subpass_dependencies; if (supports_framebuffer_fetch) { subpass_desc.setFlags(vk::SubpassDescriptionFlagBits:: eRasterizationOrderAttachmentColorAccessARM); From 6921c1bc89e8b5ad10d52b14800842a5f62495dc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 28 Nov 2023 14:04:36 -0800 Subject: [PATCH 03/18] still ARM extension on ... ARM --- .../renderer/backend/vulkan/capabilities_vk.cc | 17 +++++++++++++---- .../renderer/backend/vulkan/capabilities_vk.h | 1 + 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index f7eb19047cceb..ca32ad25fbf22 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -157,6 +157,8 @@ static const char* GetDeviceExtensionName(OptionalDeviceExtensionVK ext) { switch (ext) { case OptionalDeviceExtensionVK::kEXTPipelineCreationFeedback: return VK_EXT_PIPELINE_CREATION_FEEDBACK_EXTENSION_NAME; + case OptionalDeviceExtensionVK::kARMRasterizationOrderAttachmentAccess: + return VK_ARM_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; case OptionalDeviceExtensionVK::kEXTRasterizationOrderAttachmentAccess: return VK_EXT_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; case OptionalDeviceExtensionVK::kLast: @@ -404,10 +406,17 @@ bool CapabilitiesVK::SetPhysicalDevice(const vk::PhysicalDevice& device) { }); } - supports_framebuffer_fetch_ = - optional_device_extensions_.find( - OptionalDeviceExtensionVK::kEXTRasterizationOrderAttachmentAccess) != - optional_device_extensions_.end(); + { + supports_framebuffer_fetch_ = + optional_device_extensions_.find( + OptionalDeviceExtensionVK:: + kARMRasterizationOrderAttachmentAccess) != + optional_device_extensions_.end() || + optional_device_extensions_.find( + OptionalDeviceExtensionVK:: + kEXTRasterizationOrderAttachmentAccess) != + optional_device_extensions_.end(); + } return true; } diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index 1577ca64f0a7a..99e44289b0c50 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -21,6 +21,7 @@ class ContextVK; enum class OptionalDeviceExtensionVK : uint32_t { // https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_pipeline_creation_feedback.html kEXTPipelineCreationFeedback, + kARMRasterizationOrderAttachmentAccess, kEXTRasterizationOrderAttachmentAccess, kLast, }; From b6cb2cbaf5edbefeee6bceddd44a541a72ad2efd Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 29 Nov 2023 08:31:20 -0800 Subject: [PATCH 04/18] remove command flag. --- impeller/entity/contents/framebuffer_blend_contents.cc | 1 - impeller/renderer/backend/vulkan/binding_helpers_vk.cc | 4 ++-- .../renderer/backend/vulkan/descriptor_pool_vk_unittests.cc | 6 +++--- impeller/renderer/command.h | 2 -- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/impeller/entity/contents/framebuffer_blend_contents.cc b/impeller/entity/contents/framebuffer_blend_contents.cc index d9b7a2124aa9f..7edc478076499 100644 --- a/impeller/entity/contents/framebuffer_blend_contents.cc +++ b/impeller/entity/contents/framebuffer_blend_contents.cc @@ -146,7 +146,6 @@ bool FramebufferBlendContents::Render(const ContentContext& renderer, frag_info.src_input_alpha = src_snapshot->opacity; FS::BindFragInfo(cmd, host_buffer.EmplaceUniform(frag_info)); - cmd.bind_framebuffer = true; return pass.AddCommand(std::move(cmd)); } diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index ebaf99edca4fa..ba0a34d37ce3c 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -137,7 +137,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( buffer_count += command.vertex_bindings.buffers.size(); buffer_count += command.fragment_bindings.buffers.size(); samplers_count += command.fragment_bindings.sampled_images.size(); - subpass_count += command.bind_framebuffer ? 1 : 0; + subpass_count += command.pipeline->GetDescriptor().GetHasSubpassDependency() ? 1 : 0; layouts.emplace_back( PipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout()); @@ -178,7 +178,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( "Failed to bind texture or buffer."); } - if (command.bind_framebuffer) { + if (command.pipeline->GetDescriptor().GetHasSubpassDependency()) { vk::DescriptorImageInfo image_info; image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; image_info.sampler = VK_NULL_HANDLE; diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc index c7f3cc3ee6b5c..8749f52af4daa 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc @@ -66,7 +66,7 @@ TEST(DescriptorPoolRecyclerVKTest, ReclaimMakesDescriptorPoolAvailable) { { // Fetch a pool (which will be created). auto pool = DescriptorPoolVK(context); - pool.AllocateDescriptorSets(1024, 1024, {}); + pool.AllocateDescriptorSets(1024, 1024, 1024, {}); } // There is a chance that the first death rattle item below is destroyed in @@ -106,7 +106,7 @@ TEST(DescriptorPoolRecyclerVKTest, ReclaimDropsDescriptorPoolIfSizeIsExceeded) { std::vector> pools; for (auto i = 0u; i < 33; i++) { auto pool = std::make_unique(context); - pool->AllocateDescriptorSets(1024, 1024, {}); + pool->AllocateDescriptorSets(1024, 1024, 1024, {}); pools.push_back(std::move(pool)); } } @@ -135,7 +135,7 @@ TEST(DescriptorPoolRecyclerVKTest, ReclaimDropsDescriptorPoolIfSizeIsExceeded) { std::vector> pools; for (auto i = 0u; i < 33; i++) { auto pool = std::make_unique(context); - pool->AllocateDescriptorSets(1024, 1024, {}); + pool->AllocateDescriptorSets(1024, 1024, 1024, {}); pools.push_back(std::move(pool)); } } diff --git a/impeller/renderer/command.h b/impeller/renderer/command.h index 18c32165f523d..dd101d4f66118 100644 --- a/impeller/renderer/command.h +++ b/impeller/renderer/command.h @@ -135,8 +135,6 @@ struct Command : public ResourceBinder { /// IndexType index_type = IndexType::kUnknown; - bool bind_framebuffer = false; - #ifdef IMPELLER_DEBUG //---------------------------------------------------------------------------- /// The debugging label to use for the command. From 18694977d5b180b56fc94e033580df521522facd Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 29 Nov 2023 08:31:41 -0800 Subject: [PATCH 05/18] ++ --- impeller/renderer/backend/vulkan/binding_helpers_vk.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index ba0a34d37ce3c..6586a6cd1e453 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -137,7 +137,8 @@ fml::StatusOr> AllocateAndBindDescriptorSets( buffer_count += command.vertex_bindings.buffers.size(); buffer_count += command.fragment_bindings.buffers.size(); samplers_count += command.fragment_bindings.sampled_images.size(); - subpass_count += command.pipeline->GetDescriptor().GetHasSubpassDependency() ? 1 : 0; + subpass_count += + command.pipeline->GetDescriptor().GetHasSubpassDependency() ? 1 : 0; layouts.emplace_back( PipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout()); From b428f5fa95102ab084d19747631a1474d5b257d7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 29 Nov 2023 12:47:51 -0800 Subject: [PATCH 06/18] generate binding in template --- impeller/compiler/code_gen_template.h | 9 ++++++++- impeller/compiler/reflector.cc | 15 ++++++++++++++ impeller/core/shader_types.h | 1 + impeller/renderer/backend/vulkan/formats_vk.h | 3 +++ .../backend/vulkan/pipeline_library_vk.cc | 20 ------------------- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/impeller/compiler/code_gen_template.h b/impeller/compiler/code_gen_template.h index 61f442d824dd4..1e73aee4a885a 100644 --- a/impeller/compiler/code_gen_template.h +++ b/impeller/compiler/code_gen_template.h @@ -173,7 +173,14 @@ 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 subpass_input in subpass_inputs %} + DescriptorSetLayout{ + {{subpass_input.binding}}, // binding = {{subpass_input.binding}} + {{subpass_input.descriptor_type}}, // descriptor_type = {{subpass_input.descriptor_type}} + {{to_shader_stage(shader_stage)}}, // shader_stage = {{to_shader_stage(shader_stage)}} + }, +{% endfor %} {% for buffer in buffers %} DescriptorSetLayout{ {{buffer.binding}}, // binding = {{buffer.binding}} diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc index d411422338466..873f21d485587 100644 --- a/impeller/compiler/reflector.cc +++ b/impeller/compiler/reflector.cc @@ -200,6 +200,21 @@ std::optional Reflector::GenerateTemplateArguments() const { const auto shader_resources = compiler_->get_shader_resources(); + // Subpass Inputs. + { + auto& subpass_inputs = root["subpass_inputs"] = nlohmann::json::array_t{}; + if (auto subpass_inputs_json = + ReflectResources(shader_resources.subpass_inputs); + subpass_inputs_json.has_value()) { + for (auto subpass_input : subpass_inputs_json.value()) { + subpass_input["descriptor_type"] = "DescriptorType::kInputAttachment"; + subpass_inputs.emplace_back(std::move(subpass_input)); + } + } else { + return std::nullopt; + } + } + // Uniform and storage buffers. { auto& buffers = root["buffers"] = nlohmann::json::array_t{}; diff --git a/impeller/core/shader_types.h b/impeller/core/shader_types.h index 9e549a0c2f65b..ae7471a95b170 100644 --- a/impeller/core/shader_types.h +++ b/impeller/core/shader_types.h @@ -145,6 +145,7 @@ enum class DescriptorType { kSampledImage, kImage, kSampler, + kInputAttachment, }; struct DescriptorSetLayout { diff --git a/impeller/renderer/backend/vulkan/formats_vk.h b/impeller/renderer/backend/vulkan/formats_vk.h index aa70f228634e3..d4dc48594daf5 100644 --- a/impeller/renderer/backend/vulkan/formats_vk.h +++ b/impeller/renderer/backend/vulkan/formats_vk.h @@ -11,6 +11,7 @@ #include "impeller/core/formats.h" #include "impeller/core/shader_types.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "vulkan/vulkan_enums.hpp" namespace impeller { @@ -289,6 +290,8 @@ constexpr vk::DescriptorType ToVKDescriptorType(DescriptorType type) { case DescriptorType::kSampler: return vk::DescriptorType::eSampler; break; + case DescriptorType::kInputAttachment: + return vk::DescriptorType::eInputAttachment; } FML_UNREACHABLE(); diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index 5e693ed33653c..45c6bad467679 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -437,26 +437,6 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( auto vk_desc_layout = ToVKDescriptorSetLayoutBinding(layout); desc_bindings.push_back(vk_desc_layout); } - // Add subpass dependency binding. - if (desc.GetHasSubpassDependency()) { - vk::DescriptorSetLayoutBinding binding; - // _record scratch, freeze frame_ - // - // I bet you're wondering where I got this number? Well its one less than - // the smallest fragment binding listed in the generated header for the - // framebuffer fetch shader. In order to look this up for real, we need to - // add support for this data to impellerc, but more importantly, we need to - // resolve trying to share a generated header across all platforms. Only the - // header generated from a shader compiled with Vulkan semantics will have - // this binding, but today its unspecified if we use the GLES or Metal - // version based on host platform. To that end, it would be safer to leave - // this as a hardcoded value until such a time as the bindings are fixed. - binding.binding = 64u; - binding.descriptorCount = 1u; - binding.descriptorType = vk::DescriptorType::eInputAttachment; - binding.stageFlags = vk::ShaderStageFlagBits::eFragment; - desc_bindings.push_back(binding); - } vk::DescriptorSetLayoutCreateInfo descs_layout_info; descs_layout_info.setBindings(desc_bindings); From 808d2574aa4998ef36c1a1ed8977124183996493 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 29 Nov 2023 13:11:55 -0800 Subject: [PATCH 07/18] Cleanups to layout and barriers. --- .../renderer/backend/vulkan/allocator_vk.cc | 20 +++++++++++++------ .../renderer/backend/vulkan/allocator_vk.h | 1 + .../backend/vulkan/binding_helpers_vk.cc | 6 +++--- .../backend/vulkan/binding_helpers_vk.h | 3 ++- .../renderer/backend/vulkan/capabilities_vk.h | 1 - .../renderer/backend/vulkan/render_pass_vk.cc | 18 ++++++++--------- .../backend/vulkan/swapchain_impl_vk.cc | 9 ++++++++- 7 files changed, 36 insertions(+), 22 deletions(-) diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 1e8fffe995d81..8189e83b32068 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -12,6 +12,7 @@ #include "impeller/renderer/backend/vulkan/device_buffer_vk.h" #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" +#include "vulkan/vulkan_enums.hpp" namespace impeller { @@ -148,6 +149,7 @@ AllocatorVK::AllocatorVK(std::weak_ptr context, allocator_.reset(allocator); supports_memoryless_textures_ = capabilities.SupportsDeviceTransientTextures(); + supports_framebuffer_fetch_ = capabilities.SupportsFramebufferFetch(); is_valid_ = true; } @@ -167,7 +169,8 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( PixelFormat format, TextureUsageMask usage, StorageMode mode, - bool supports_memoryless_textures) { + bool supports_memoryless_textures, + bool supports_framebuffer_fetch) { vk::ImageUsageFlags vk_usage; switch (mode) { @@ -186,6 +189,9 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( vk_usage |= vk::ImageUsageFlagBits::eDepthStencilAttachment; } else { vk_usage |= vk::ImageUsageFlagBits::eColorAttachment; + if (supports_framebuffer_fetch) { + vk_usage |= vk::ImageUsageFlagBits::eInputAttachment; + } } } @@ -263,7 +269,8 @@ class AllocatedTextureSourceVK final : public TextureSourceVK { const TextureDescriptor& desc, VmaAllocator allocator, vk::Device device, - bool supports_memoryless_textures) + bool supports_memoryless_textures, + bool supports_framebuffer_fetch) : TextureSourceVK(desc), resource_(std::move(resource_manager)) { FML_DCHECK(desc.format != PixelFormat::kUnknown); TRACE_EVENT0("impeller", "CreateDeviceTexture"); @@ -281,9 +288,9 @@ class AllocatedTextureSourceVK final : public TextureSourceVK { image_info.arrayLayers = ToArrayLayerCount(desc.type); image_info.tiling = vk::ImageTiling::eOptimal; image_info.initialLayout = vk::ImageLayout::eUndefined; - image_info.usage = - ToVKImageUsageFlags(desc.format, desc.usage, desc.storage_mode, - supports_memoryless_textures); + image_info.usage = ToVKImageUsageFlags( + desc.format, desc.usage, desc.storage_mode, + supports_memoryless_textures, supports_framebuffer_fetch); image_info.sharingMode = vk::SharingMode::eExclusive; VmaAllocationCreateInfo alloc_nfo = {}; @@ -412,7 +419,8 @@ std::shared_ptr AllocatorVK::OnCreateTexture( desc, // allocator_.get(), // device_holder->GetDevice(), // - supports_memoryless_textures_ // + supports_memoryless_textures_, // + supports_framebuffer_fetch_ // ); if (!source->IsValid()) { return nullptr; diff --git a/impeller/renderer/backend/vulkan/allocator_vk.h b/impeller/renderer/backend/vulkan/allocator_vk.h index a22369462d8b7..1977b79115358 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.h +++ b/impeller/renderer/backend/vulkan/allocator_vk.h @@ -33,6 +33,7 @@ class AllocatorVK final : public Allocator { ISize max_texture_size_; bool is_valid_ = false; bool supports_memoryless_textures_ = false; + bool supports_framebuffer_fetch_ = false; // TODO(jonahwilliams): figure out why CI can't create these buffer pools. bool created_buffer_pool_ = true; uint32_t frame_count_ = 0; diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index 6586a6cd1e453..c076979de7910 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -119,7 +119,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, const std::vector& commands, - const vk::ImageView& image_view) { + const TextureVK& input_attachment) { if (commands.empty()) { return std::vector{}; } @@ -181,9 +181,9 @@ fml::StatusOr> AllocateAndBindDescriptorSets( if (command.pipeline->GetDescriptor().GetHasSubpassDependency()) { vk::DescriptorImageInfo image_info; - image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; + image_info.imageLayout = vk::ImageLayout::eGeneral; image_info.sampler = VK_NULL_HANDLE; - image_info.imageView = image_view; + image_info.imageView = input_attachment.GetImageView(); images.push_back(image_info); vk::WriteDescriptorSet write_set; diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.h b/impeller/renderer/backend/vulkan/binding_helpers_vk.h index ba2cc035046e0..14d3145a8d26b 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.h +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.h @@ -8,6 +8,7 @@ #include "fml/status_or.h" #include "impeller/renderer/backend/vulkan/context_vk.h" +#include "impeller/renderer/backend/vulkan/texture_vk.h" #include "impeller/renderer/command.h" #include "impeller/renderer/compute_command.h" @@ -17,7 +18,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, const std::vector& commands, - const vk::ImageView& image_view); + const TextureVK& input_attachment); fml::StatusOr> AllocateAndBindDescriptorSets( const ContextVK& context, diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index 99e44289b0c50..eb9eb08a6f7a0 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -112,7 +112,6 @@ class CapabilitiesVK final : public Capabilities, bool supports_compute_subgroups_ = false; bool supports_device_transient_textures_ = false; bool supports_framebuffer_fetch_ = false; - ; bool is_valid_ = false; bool HasExtension(const std::string& ext) const; diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 8c4b80c9c0635..464353a6d5196 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -51,11 +51,8 @@ static vk::AttachmentDescription CreateAttachmentDescription( store_action = StoreAction::kStore; } - if (current_layout != vk::ImageLayout::ePresentSrcKHR && - current_layout != vk::ImageLayout::eUndefined) { - // Note: This should incur a barrier. - current_layout = vk::ImageLayout::eGeneral; - } + // Always transition to general. + current_layout = vk::ImageLayout::eGeneral; return CreateAttachmentDescription(desc.format, // desc.sample_count, // @@ -127,8 +124,9 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( SetTextureLayout(color, attachments.back(), command_buffer, &Attachment::texture); if (color.resolve_texture) { - resolve_refs[bind_point] = vk::AttachmentReference{ - static_cast(attachments.size()), vk::ImageLayout::eGeneral}; + resolve_refs[bind_point] = + vk::AttachmentReference{static_cast(attachments.size()), + vk::ImageLayout::eColorAttachmentOptimal}; attachments.emplace_back( CreateAttachmentDescription(color, &Attachment::resolve_texture)); SetTextureLayout(color, attachments.back(), command_buffer, @@ -534,10 +532,10 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { static_cast(target_size.height); pass_info.setClearValues(clear_values); - auto color_image_view = - TextureVK::Cast(*render_target_.GetRenderTargetTexture()).GetImageView(); + const auto& color_image_vk = + TextureVK::Cast(*render_target_.GetRenderTargetTexture()); auto desc_sets_result = AllocateAndBindDescriptorSets( - vk_context, encoder, commands_, color_image_view); + vk_context, encoder, commands_, color_image_vk); if (!desc_sets_result.ok()) { return false; } diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index ac31b6eea5be8..356e6cd0c346f 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -219,7 +219,14 @@ SwapchainImplVK::SwapchainImplVK( // Swapchain images are primarily used as color attachments (via resolve) or // blit targets. swapchain_info.imageUsage = vk::ImageUsageFlagBits::eColorAttachment | - vk::ImageUsageFlagBits::eTransferDst; + vk::ImageUsageFlagBits::eTransferDst | + vk::ImageUsageFlagBits::eInputAttachment; + + // With framebuffer fetch the engine may readback from the onscreen + // attachment. + if (context->GetCapabilities()->SupportsFramebufferFetch()) { + swapchain_info.imageUsage |= vk::ImageUsageFlagBits::eInputAttachment; + } swapchain_info.preTransform = vk::SurfaceTransformFlagBitsKHR::eIdentity; swapchain_info.compositeAlpha = composite.value(); // If we set the clipped value to true, Vulkan expects we will never read back From 4e79b47cdd4844750b46d90afaed78d6f3432bc3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 29 Nov 2023 13:21:34 -0800 Subject: [PATCH 08/18] enums instead of bool :/ --- impeller/entity/contents/content_context.cc | 31 ++++++++++--------- impeller/entity/contents/content_context.h | 5 +-- .../backend/vulkan/binding_helpers_vk.cc | 4 +-- .../backend/vulkan/swapchain_impl_vk.cc | 2 +- impeller/renderer/pipeline_descriptor.cc | 4 +-- impeller/renderer/pipeline_descriptor.h | 18 +++++++++-- 6 files changed, 39 insertions(+), 25 deletions(-) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 2134d0b1836fa..25b7afdeedb50 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -222,62 +222,63 @@ ContentContext::ContentContext( framebuffer_blend_color_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kColor), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_colorburn_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kColorBurn), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_colordodge_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kColorDodge), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_darken_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kDarken), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_difference_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kDifference), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_exclusion_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kExclusion), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_hardlight_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kHardLight), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_hue_pipelines_.CreateDefault( *context_, options_trianglestrip, - {static_cast(BlendSelectValues::kHue), supports_decal}, true); + {static_cast(BlendSelectValues::kHue), supports_decal}, + UseSubpassInput::kYes); framebuffer_blend_lighten_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kLighten), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_luminosity_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kLuminosity), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_multiply_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kMultiply), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_overlay_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kOverlay), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_saturation_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kSaturation), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_screen_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kScreen), supports_decal}, - true); + UseSubpassInput::kYes); framebuffer_blend_softlight_pipelines_.CreateDefault( *context_, options_trianglestrip, {static_cast(BlendSelectValues::kSoftLight), supports_decal}, - true); + UseSubpassInput::kYes); } blend_color_pipelines_.CreateDefault( diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 4f9f93a3815d5..baae36c77e1ef 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -18,6 +18,7 @@ #include "impeller/entity/entity.h" #include "impeller/renderer/capabilities.h" #include "impeller/renderer/pipeline.h" +#include "impeller/renderer/pipeline_descriptor.h" #include "impeller/renderer/render_target.h" #include "impeller/typographer/typographer_context.h" @@ -714,14 +715,14 @@ class ContentContext { void CreateDefault(const Context& context, const ContentContextOptions& options, const std::initializer_list& constants = {}, - bool has_subpass_input = false) { + UseSubpassInput subpass_input = UseSubpassInput::kNo) { auto desc = PipelineT::Builder::MakeDefaultPipelineDescriptor(context, constants); if (!desc.has_value()) { VALIDATION_LOG << "Failed to create default pipeline."; return; } - desc->SetHasSubpassDependency(has_subpass_input); + desc->SetUseSubpassInput(subpass_input); options.ApplyToPipelineDescriptor(*desc); SetDefault(options, std::make_unique(context, desc)); } diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index c076979de7910..7441e7b58302a 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -138,7 +138,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( buffer_count += command.fragment_bindings.buffers.size(); samplers_count += command.fragment_bindings.sampled_images.size(); subpass_count += - command.pipeline->GetDescriptor().GetHasSubpassDependency() ? 1 : 0; + command.pipeline->GetDescriptor().UsesSubpassInput() ? 1 : 0; layouts.emplace_back( PipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout()); @@ -179,7 +179,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( "Failed to bind texture or buffer."); } - if (command.pipeline->GetDescriptor().GetHasSubpassDependency()) { + if (command.pipeline->GetDescriptor().UsesSubpassInput()) { vk::DescriptorImageInfo image_info; image_info.imageLayout = vk::ImageLayout::eGeneral; image_info.sampler = VK_NULL_HANDLE; diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 356e6cd0c346f..863128a7743b1 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -225,7 +225,7 @@ SwapchainImplVK::SwapchainImplVK( // With framebuffer fetch the engine may readback from the onscreen // attachment. if (context->GetCapabilities()->SupportsFramebufferFetch()) { - swapchain_info.imageUsage |= vk::ImageUsageFlagBits::eInputAttachment; + swapchain_info.imageUsage |= vk::ImageUsageFlagBits::eInputAttachment; } swapchain_info.preTransform = vk::SurfaceTransformFlagBitsKHR::eIdentity; swapchain_info.compositeAlpha = composite.value(); diff --git a/impeller/renderer/pipeline_descriptor.cc b/impeller/renderer/pipeline_descriptor.cc index 39669c1ede949..f82c1e9dbaaa3 100644 --- a/impeller/renderer/pipeline_descriptor.cc +++ b/impeller/renderer/pipeline_descriptor.cc @@ -45,7 +45,7 @@ std::size_t PipelineDescriptor::GetHash() const { fml::HashCombineSeed(seed, cull_mode_); fml::HashCombineSeed(seed, primitive_type_); fml::HashCombineSeed(seed, polygon_mode_); - fml::HashCombineSeed(seed, has_subpass_dependency_); + fml::HashCombineSeed(seed, use_subpass_input_); return seed; } @@ -67,7 +67,7 @@ bool PipelineDescriptor::IsEqual(const PipelineDescriptor& other) const { primitive_type_ == other.primitive_type_ && polygon_mode_ == other.polygon_mode_ && specialization_constants_ == other.specialization_constants_ && - has_subpass_dependency_ == other.has_subpass_dependency_; + use_subpass_input_ == other.use_subpass_input_; } PipelineDescriptor& PipelineDescriptor::SetLabel(std::string label) { diff --git a/impeller/renderer/pipeline_descriptor.h b/impeller/renderer/pipeline_descriptor.h index 8ea81df1247e4..c71b192b023e0 100644 --- a/impeller/renderer/pipeline_descriptor.h +++ b/impeller/renderer/pipeline_descriptor.h @@ -20,6 +20,11 @@ class VertexDescriptor; template class Pipeline; +enum class UseSubpassInput { + kYes, + kNo, +}; + class PipelineDescriptor final : public Comparable { public: PipelineDescriptor(); @@ -128,9 +133,16 @@ class PipelineDescriptor final : public Comparable { const std::vector& GetSpecializationConstants() const; - void SetHasSubpassDependency(bool value) { has_subpass_dependency_ = value; } + void SetUseSubpassInput(UseSubpassInput value) { use_subpass_input_ = value; } - bool GetHasSubpassDependency() const { return has_subpass_dependency_; } + bool UsesSubpassInput() const { + switch (use_subpass_input_) { + case UseSubpassInput::kYes: + return true; + case UseSubpassInput::kNo: + return false; + } + } private: std::string label_; @@ -150,7 +162,7 @@ class PipelineDescriptor final : public Comparable { back_stencil_attachment_descriptor_; PrimitiveType primitive_type_ = PrimitiveType::kTriangle; PolygonMode polygon_mode_ = PolygonMode::kFill; - bool has_subpass_dependency_ = false; + UseSubpassInput use_subpass_input_ = UseSubpassInput::kNo; std::vector specialization_constants_; }; From a1153d9960b903c068d422fa05d51023ec8cd92a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 29 Nov 2023 17:58:25 -0800 Subject: [PATCH 09/18] subpassInputMS --- .../shaders/blending/framebuffer_blend.frag | 10 +++++++++ .../renderer/backend/vulkan/allocator_vk.cc | 6 +++--- .../backend/vulkan/capabilities_vk.cc | 21 ++++++++++++------- .../renderer/backend/vulkan/render_pass_vk.cc | 9 ++++---- .../backend/vulkan/swapchain_impl_vk.cc | 8 +------ 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/impeller/entity/shaders/blending/framebuffer_blend.frag b/impeller/entity/shaders/blending/framebuffer_blend.frag index e01316924ec8a..20ac7a59bc5d8 100644 --- a/impeller/entity/shaders/blending/framebuffer_blend.frag +++ b/impeller/entity/shaders/blending/framebuffer_blend.frag @@ -13,6 +13,7 @@ layout(constant_id = 0) const int blend_type = 0; layout(constant_id = 1) const int supports_decal = 1; +#ifdef IMPELLER_TARGET_OPENGLES layout(set = 0, binding = 0, input_attachment_index = 0) uniform subpassInput uSub; @@ -20,6 +21,15 @@ layout(set = 0, vec4 ReadDestination() { return subpassLoad(uSub); } +#else +layout(set = 0, + binding = 0, + input_attachment_index = 0) uniform subpassInputMS uSub; + +vec4 ReadDestination() { + return subpassLoad(uSub, gl_SampleID); +} +#endif uniform sampler2D texture_sampler_src; diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 8189e83b32068..2b585d56ea7c3 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -189,9 +189,9 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( vk_usage |= vk::ImageUsageFlagBits::eDepthStencilAttachment; } else { vk_usage |= vk::ImageUsageFlagBits::eColorAttachment; - if (supports_framebuffer_fetch) { - vk_usage |= vk::ImageUsageFlagBits::eInputAttachment; - } + } + if (mode == StorageMode::kDeviceTransient && supports_framebuffer_fetch) { + vk_usage |= vk::ImageUsageFlagBits::eInputAttachment; } } diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index ca32ad25fbf22..229d966729ff7 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -318,6 +318,9 @@ CapabilitiesVK::GetEnabledDeviceFeatures( // necessarily a big deal if we don't have this feature. required.fillModeNonSolid = device_features.fillModeNonSolid; + // This is required for multisample framebuffer fetch. + required.sampleRateShading = device_features.sampleRateShading; + return required; } @@ -408,14 +411,16 @@ bool CapabilitiesVK::SetPhysicalDevice(const vk::PhysicalDevice& device) { { supports_framebuffer_fetch_ = - optional_device_extensions_.find( - OptionalDeviceExtensionVK:: - kARMRasterizationOrderAttachmentAccess) != - optional_device_extensions_.end() || - optional_device_extensions_.find( - OptionalDeviceExtensionVK:: - kEXTRasterizationOrderAttachmentAccess) != - optional_device_extensions_.end(); + (optional_device_extensions_.find( + OptionalDeviceExtensionVK:: + kARMRasterizationOrderAttachmentAccess) != + optional_device_extensions_.end() || + optional_device_extensions_.find( + OptionalDeviceExtensionVK:: + kEXTRasterizationOrderAttachmentAccess) != + optional_device_extensions_.end()) && + device.getFeatures().sampleRateShading; + FML_LOG(ERROR) << supports_framebuffer_fetch_; } return true; diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 464353a6d5196..4d2b4162b0b58 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -116,9 +116,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( kUnusedAttachmentReference); for (const auto& [bind_point, color] : render_target_.GetColorAttachments()) { - color_refs[bind_point] = - vk::AttachmentReference{static_cast(attachments.size()), - vk::ImageLayout::eColorAttachmentOptimal}; + color_refs[bind_point] = vk::AttachmentReference{ + static_cast(attachments.size()), vk::ImageLayout::eGeneral}; attachments.emplace_back( CreateAttachmentDescription(color, &Attachment::texture)); SetTextureLayout(color, attachments.back(), command_buffer, @@ -532,8 +531,8 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { static_cast(target_size.height); pass_info.setClearValues(clear_values); - const auto& color_image_vk = - TextureVK::Cast(*render_target_.GetRenderTargetTexture()); + const auto& color_image_vk = TextureVK::Cast( + *render_target_.GetColorAttachments().find(0u)->second.texture); auto desc_sets_result = AllocateAndBindDescriptorSets( vk_context, encoder, commands_, color_image_vk); if (!desc_sets_result.ok()) { diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 863128a7743b1..f4f5b8860b354 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -219,14 +219,8 @@ SwapchainImplVK::SwapchainImplVK( // Swapchain images are primarily used as color attachments (via resolve) or // blit targets. swapchain_info.imageUsage = vk::ImageUsageFlagBits::eColorAttachment | - vk::ImageUsageFlagBits::eTransferDst | - vk::ImageUsageFlagBits::eInputAttachment; + vk::ImageUsageFlagBits::eTransferDst; - // With framebuffer fetch the engine may readback from the onscreen - // attachment. - if (context->GetCapabilities()->SupportsFramebufferFetch()) { - swapchain_info.imageUsage |= vk::ImageUsageFlagBits::eInputAttachment; - } swapchain_info.preTransform = vk::SurfaceTransformFlagBitsKHR::eIdentity; swapchain_info.compositeAlpha = composite.value(); // If we set the clipped value to true, Vulkan expects we will never read back From 76c433667e7cbc721d76f2387b5e941f28b17063 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 29 Nov 2023 19:22:20 -0800 Subject: [PATCH 10/18] more tweaks --- .../entity/shaders/blending/framebuffer_blend.frag | 2 +- .../playground/backend/vulkan/playground_impl_vk.cc | 3 +++ impeller/renderer/backend/vulkan/capabilities_vk.cc | 7 +------ impeller/renderer/backend/vulkan/render_pass_vk.cc | 10 ++++++---- impeller/renderer/backend/vulkan/swapchain_impl_vk.cc | 1 - 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/impeller/entity/shaders/blending/framebuffer_blend.frag b/impeller/entity/shaders/blending/framebuffer_blend.frag index 20ac7a59bc5d8..caf7b242354be 100644 --- a/impeller/entity/shaders/blending/framebuffer_blend.frag +++ b/impeller/entity/shaders/blending/framebuffer_blend.frag @@ -27,7 +27,7 @@ layout(set = 0, input_attachment_index = 0) uniform subpassInputMS uSub; vec4 ReadDestination() { - return subpassLoad(uSub, gl_SampleID); + return subpassLoad(uSub, 1); } #endif diff --git a/impeller/playground/backend/vulkan/playground_impl_vk.cc b/impeller/playground/backend/vulkan/playground_impl_vk.cc index e74541fcfdaf8..9186d626af2d2 100644 --- a/impeller/playground/backend/vulkan/playground_impl_vk.cc +++ b/impeller/playground/backend/vulkan/playground_impl_vk.cc @@ -37,6 +37,9 @@ ShaderLibraryMappingsForPlayground() { std::make_shared( impeller_framebuffer_blend_shaders_vk_data, impeller_framebuffer_blend_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/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index 229d966729ff7..4d83152795af7 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -318,9 +318,6 @@ CapabilitiesVK::GetEnabledDeviceFeatures( // necessarily a big deal if we don't have this feature. required.fillModeNonSolid = device_features.fillModeNonSolid; - // This is required for multisample framebuffer fetch. - required.sampleRateShading = device_features.sampleRateShading; - return required; } @@ -418,9 +415,7 @@ bool CapabilitiesVK::SetPhysicalDevice(const vk::PhysicalDevice& device) { optional_device_extensions_.find( OptionalDeviceExtensionVK:: kEXTRasterizationOrderAttachmentAccess) != - optional_device_extensions_.end()) && - device.getFeatures().sampleRateShading; - FML_LOG(ERROR) << supports_framebuffer_fetch_; + optional_device_extensions_.end()); } return true; diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 4d2b4162b0b58..45cf5c9b88a0c 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -51,8 +51,11 @@ static vk::AttachmentDescription CreateAttachmentDescription( store_action = StoreAction::kStore; } - // Always transition to general. + // if (current_layout != vk::ImageLayout::ePresentSrcKHR && + // current_layout != vk::ImageLayout::eUndefined) { + // // Note: This should incur a barrier. current_layout = vk::ImageLayout::eGeneral; + // } return CreateAttachmentDescription(desc.format, // desc.sample_count, // @@ -123,9 +126,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( SetTextureLayout(color, attachments.back(), command_buffer, &Attachment::texture); if (color.resolve_texture) { - resolve_refs[bind_point] = - vk::AttachmentReference{static_cast(attachments.size()), - vk::ImageLayout::eColorAttachmentOptimal}; + resolve_refs[bind_point] = vk::AttachmentReference{ + static_cast(attachments.size()), vk::ImageLayout::eGeneral}; attachments.emplace_back( CreateAttachmentDescription(color, &Attachment::resolve_texture)); SetTextureLayout(color, attachments.back(), command_buffer, diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index f4f5b8860b354..ac31b6eea5be8 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -220,7 +220,6 @@ SwapchainImplVK::SwapchainImplVK( // blit targets. swapchain_info.imageUsage = vk::ImageUsageFlagBits::eColorAttachment | vk::ImageUsageFlagBits::eTransferDst; - swapchain_info.preTransform = vk::SurfaceTransformFlagBitsKHR::eIdentity; swapchain_info.compositeAlpha = composite.value(); // If we set the clipped value to true, Vulkan expects we will never read back From d5bff60013baf76105b31528aefc866f2a315edd Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 29 Nov 2023 19:53:50 -0800 Subject: [PATCH 11/18] ++ --- .../entity/shaders/blending/framebuffer_blend.frag | 4 +++- impeller/renderer/backend/vulkan/render_pass_vk.cc | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/impeller/entity/shaders/blending/framebuffer_blend.frag b/impeller/entity/shaders/blending/framebuffer_blend.frag index caf7b242354be..fa0eb62ca979e 100644 --- a/impeller/entity/shaders/blending/framebuffer_blend.frag +++ b/impeller/entity/shaders/blending/framebuffer_blend.frag @@ -27,7 +27,9 @@ layout(set = 0, input_attachment_index = 0) uniform subpassInputMS uSub; vec4 ReadDestination() { - return subpassLoad(uSub, 1); + return (subpassLoad(uSub, 0) + subpassLoad(uSub, 1) + subpassLoad(uSub, 2) + + subpassLoad(uSub, 3)) / + vec4(4.0); } #endif diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 45cf5c9b88a0c..14ee2b32864e9 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -51,11 +51,11 @@ static vk::AttachmentDescription CreateAttachmentDescription( store_action = StoreAction::kStore; } - // if (current_layout != vk::ImageLayout::ePresentSrcKHR && - // current_layout != vk::ImageLayout::eUndefined) { - // // Note: This should incur a barrier. - current_layout = vk::ImageLayout::eGeneral; - // } + if (current_layout != vk::ImageLayout::ePresentSrcKHR && + current_layout != vk::ImageLayout::eUndefined) { + // Note: This should incur a barrier. + current_layout = vk::ImageLayout::eGeneral; + } return CreateAttachmentDescription(desc.format, // desc.sample_count, // From cef358123c357aa0442a4ce9bcae3055ad812774 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 30 Nov 2023 08:02:48 -0800 Subject: [PATCH 12/18] include shaders. --- impeller/playground/backend/vulkan/playground_impl_vk.cc | 3 --- shell/testing/tester_main.cc | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/impeller/playground/backend/vulkan/playground_impl_vk.cc b/impeller/playground/backend/vulkan/playground_impl_vk.cc index 9186d626af2d2..e74541fcfdaf8 100644 --- a/impeller/playground/backend/vulkan/playground_impl_vk.cc +++ b/impeller/playground/backend/vulkan/playground_impl_vk.cc @@ -37,9 +37,6 @@ ShaderLibraryMappingsForPlayground() { std::make_shared( impeller_framebuffer_blend_shaders_vk_data, impeller_framebuffer_blend_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/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index d7027ed096fe2..dff3338d8bae7 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -37,6 +37,7 @@ #include "flutter/vulkan/procs/vulkan_proc_table.h" // nogncheck #include "flutter/vulkan/swiftshader_path.h" // nogncheck #include "impeller/entity/vk/entity_shaders_vk.h" // nogncheck +#include "impeller/entity/vk/framebuffer_blend_shaders_vk.h" // nogncheck #include "impeller/entity/vk/modern_shaders_vk.h" // nogncheck #include "impeller/renderer/backend/vulkan/context_vk.h" // nogncheck #include "impeller/renderer/backend/vulkan/surface_context_vk.h" // nogncheck @@ -53,6 +54,9 @@ static std::vector> ShaderLibraryMappings() { 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), #if IMPELLER_ENABLE_3D std::make_shared(impeller_scene_shaders_vk_data, impeller_scene_shaders_vk_length), From 371fbdd311e69c6e62231cdd77688679dd3361e7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 30 Nov 2023 12:42:27 -0800 Subject: [PATCH 13/18] WIP --- .../renderer/backend/vulkan/allocator_vk.cc | 2 +- impeller/renderer/backend/vulkan/formats_vk.h | 9 +++++++-- .../renderer/backend/vulkan/render_pass_vk.cc | 20 ++++++++++++------- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 2b585d56ea7c3..a5dd9b7f14b00 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -190,7 +190,7 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( } else { vk_usage |= vk::ImageUsageFlagBits::eColorAttachment; } - if (mode == StorageMode::kDeviceTransient && supports_framebuffer_fetch) { + if (supports_framebuffer_fetch) { vk_usage |= vk::ImageUsageFlagBits::eInputAttachment; } } diff --git a/impeller/renderer/backend/vulkan/formats_vk.h b/impeller/renderer/backend/vulkan/formats_vk.h index d4dc48594daf5..7185d580517e0 100644 --- a/impeller/renderer/backend/vulkan/formats_vk.h +++ b/impeller/renderer/backend/vulkan/formats_vk.h @@ -438,7 +438,8 @@ constexpr vk::AttachmentDescription CreateAttachmentDescription( SampleCount sample_count, LoadAction load_action, StoreAction store_action, - vk::ImageLayout current_layout) { + vk::ImageLayout current_layout, + bool supports_framebuffer_fetch) { vk::AttachmentDescription vk_attachment; vk_attachment.format = ToVKImageFormat(format); @@ -477,7 +478,11 @@ constexpr vk::AttachmentDescription CreateAttachmentDescription( switch (kind) { case AttachmentKind::kColor: vk_attachment.initialLayout = current_layout; - vk_attachment.finalLayout = vk::ImageLayout::eColorAttachmentOptimal; + if (supports_framebuffer_fetch) { + vk_attachment.finalLayout = vk::ImageLayout::eGeneral; + } else { + vk_attachment.finalLayout = vk::ImageLayout::eColorAttachmentOptimal; + } break; case AttachmentKind::kDepth: case AttachmentKind::kStencil: diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 14ee2b32864e9..d02adabab3eeb 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -29,7 +29,8 @@ namespace impeller { static vk::AttachmentDescription CreateAttachmentDescription( const Attachment& attachment, - const std::shared_ptr Attachment::*texture_ptr) { + const std::shared_ptr Attachment::*texture_ptr, + bool supports_framebuffer_fetch) { const auto& texture = attachment.*texture_ptr; if (!texture) { return {}; @@ -51,9 +52,9 @@ static vk::AttachmentDescription CreateAttachmentDescription( store_action = StoreAction::kStore; } - if (current_layout != vk::ImageLayout::ePresentSrcKHR && - current_layout != vk::ImageLayout::eUndefined) { - // Note: This should incur a barrier. + // Always insert a barrier to transition to color attachment optimal. + if (supports_framebuffer_fetch) { + } else { current_layout = vk::ImageLayout::eGeneral; } @@ -61,7 +62,8 @@ static vk::AttachmentDescription CreateAttachmentDescription( desc.sample_count, // load_action, // store_action, // - current_layout // + current_layout, + supports_framebuffer_fetch // ); } @@ -120,14 +122,18 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( for (const auto& [bind_point, color] : render_target_.GetColorAttachments()) { color_refs[bind_point] = vk::AttachmentReference{ - static_cast(attachments.size()), vk::ImageLayout::eGeneral}; + static_cast(attachments.size()), + supports_framebuffer_fetch ? vk::ImageLayout::eColorAttachmentOptimal + : vk::ImageLayout::eGeneral}; attachments.emplace_back( CreateAttachmentDescription(color, &Attachment::texture)); SetTextureLayout(color, attachments.back(), command_buffer, &Attachment::texture); if (color.resolve_texture) { resolve_refs[bind_point] = vk::AttachmentReference{ - static_cast(attachments.size()), vk::ImageLayout::eGeneral}; + static_cast(attachments.size()), + supports_framebuffer_fetch ? vk::ImageLayout::eColorAttachmentOptimal + : vk::ImageLayout::eGeneral}; attachments.emplace_back( CreateAttachmentDescription(color, &Attachment::resolve_texture)); SetTextureLayout(color, attachments.back(), command_buffer, From 55d111d9a374e052f22554310416c971e8f27566 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 30 Nov 2023 18:41:59 -0800 Subject: [PATCH 14/18] testing adjustments. --- impeller/entity/entity_unittests.cc | 35 +++++++++++++++++++ .../backend/vulkan/pipeline_library_vk.cc | 11 +++--- .../renderer/backend/vulkan/render_pass_vk.cc | 31 ++++++++-------- 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 60aae37ddf693..8d1da931bd8ba 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -10,6 +10,7 @@ #include "fml/logging.h" #include "gtest/gtest.h" +#include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" #include "impeller/entity/contents/atlas_contents.h" #include "impeller/entity/contents/clip_contents.h" @@ -41,6 +42,7 @@ #include "impeller/playground/playground.h" #include "impeller/playground/widgets.h" #include "impeller/renderer/command.h" +#include "impeller/renderer/pipeline_descriptor.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/vertex_buffer_builder.h" #include "impeller/typographer/backends/skia/text_frame_skia.h" @@ -2527,6 +2529,39 @@ TEST_P(EntityTest, DecalSpecializationAppliedToMorphologyFilter) { expected_constants); } +TEST_P(EntityTest, FramebufferFetchPipelinesDeclareUsage) { + auto content_context = + ContentContext(GetContext(), TypographerContextSkia::Make()); + if (!content_context.GetDeviceCapabilities().SupportsFramebufferFetch()) { + GTEST_SKIP() << "Framebuffer fetch not supported."; + } + + ContentContextOptions options; + options.color_attachment_pixel_format = PixelFormat::kR8G8B8A8UNormInt; + auto color_burn = + content_context.GetFramebufferBlendColorBurnPipeline(options); + + EXPECT_TRUE(color_burn->GetDescriptor().UsesSubpassInput()); +} + +TEST_P(EntityTest, PipelineDescriptorEqAndHash) { + auto desc_1 = std::make_shared(); + auto desc_2 = std::make_shared(); + + EXPECT_TRUE(desc_1->IsEqual(*desc_2)); + EXPECT_EQ(desc_1->GetHash(), desc_2->GetHash()); + + desc_1->SetUseSubpassInput(UseSubpassInput::kYes); + + EXPECT_FALSE(desc_1->IsEqual(*desc_2)); + EXPECT_NE(desc_1->GetHash(), desc_2->GetHash()); + + desc_2->SetUseSubpassInput(UseSubpassInput::kYes); + + EXPECT_TRUE(desc_1->IsEqual(*desc_2)); + EXPECT_EQ(desc_1->GetHash(), desc_2->GetHash()); +} + } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index 45c6bad467679..a2fd58d31a6ca 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -63,11 +63,12 @@ static vk::AttachmentDescription CreatePlaceholderAttachmentDescription( SampleCount sample_count) { // Load store ops are immaterial for pass compatibility. The right ops will be // picked up when the pass associated with framebuffer. - return CreateAttachmentDescription(format, // - sample_count, // - LoadAction::kDontCare, // - StoreAction::kDontCare, // - vk::ImageLayout::eUndefined // + return CreateAttachmentDescription(format, // + sample_count, // + LoadAction::kDontCare, // + StoreAction::kDontCare, // + vk::ImageLayout::eUndefined, // + false // ); } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index d02adabab3eeb..c57cfa2bdbb4e 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -53,11 +53,7 @@ static vk::AttachmentDescription CreateAttachmentDescription( } // Always insert a barrier to transition to color attachment optimal. - if (supports_framebuffer_fetch) { - } else { - current_layout = vk::ImageLayout::eGeneral; - } - + current_layout = vk::ImageLayout::eGeneral; return CreateAttachmentDescription(desc.format, // desc.sample_count, // load_action, // @@ -123,19 +119,20 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( for (const auto& [bind_point, color] : render_target_.GetColorAttachments()) { color_refs[bind_point] = vk::AttachmentReference{ static_cast(attachments.size()), - supports_framebuffer_fetch ? vk::ImageLayout::eColorAttachmentOptimal - : vk::ImageLayout::eGeneral}; - attachments.emplace_back( - CreateAttachmentDescription(color, &Attachment::texture)); + supports_framebuffer_fetch ? vk::ImageLayout::eGeneral + : vk::ImageLayout::eColorAttachmentOptimal}; + attachments.emplace_back(CreateAttachmentDescription( + color, &Attachment::texture, supports_framebuffer_fetch)); SetTextureLayout(color, attachments.back(), command_buffer, &Attachment::texture); if (color.resolve_texture) { resolve_refs[bind_point] = vk::AttachmentReference{ static_cast(attachments.size()), - supports_framebuffer_fetch ? vk::ImageLayout::eColorAttachmentOptimal - : vk::ImageLayout::eGeneral}; - attachments.emplace_back( - CreateAttachmentDescription(color, &Attachment::resolve_texture)); + supports_framebuffer_fetch + ? vk::ImageLayout::eGeneral + : vk::ImageLayout::eColorAttachmentOptimal}; + attachments.emplace_back(CreateAttachmentDescription( + color, &Attachment::resolve_texture, supports_framebuffer_fetch)); SetTextureLayout(color, attachments.back(), command_buffer, &Attachment::resolve_texture); } @@ -145,8 +142,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( depth_stencil_ref = vk::AttachmentReference{ static_cast(attachments.size()), vk::ImageLayout::eDepthStencilAttachmentOptimal}; - attachments.emplace_back( - CreateAttachmentDescription(depth.value(), &Attachment::texture)); + attachments.emplace_back(CreateAttachmentDescription( + depth.value(), &Attachment::texture, supports_framebuffer_fetch)); SetTextureLayout(depth.value(), attachments.back(), command_buffer, &Attachment::texture); } @@ -156,8 +153,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( depth_stencil_ref = vk::AttachmentReference{ static_cast(attachments.size()), vk::ImageLayout::eDepthStencilAttachmentOptimal}; - attachments.emplace_back( - CreateAttachmentDescription(stencil.value(), &Attachment::texture)); + attachments.emplace_back(CreateAttachmentDescription( + stencil.value(), &Attachment::texture, supports_framebuffer_fetch)); SetTextureLayout(stencil.value(), attachments.back(), command_buffer, &Attachment::texture); } From f5949b88dece04b25ff00584d80d0b969c2ee7c4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 30 Nov 2023 19:48:14 -0800 Subject: [PATCH 15/18] ++ --- .../shaders/blending/framebuffer_blend.frag | 21 +++++++++++-------- .../shaders/blending/framebuffer_blend.vert | 3 +++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/impeller/entity/shaders/blending/framebuffer_blend.frag b/impeller/entity/shaders/blending/framebuffer_blend.frag index fa0eb62ca979e..eff3afd390a19 100644 --- a/impeller/entity/shaders/blending/framebuffer_blend.frag +++ b/impeller/entity/shaders/blending/framebuffer_blend.frag @@ -10,18 +10,13 @@ #include #include "blend_select.glsl" +// Warning: if any of the constant values or layouts are changed in this +// file, then the hard-coded constant value in +// impeller/renderer/backend/vulkan/binding_helpers_vk.cc layout(constant_id = 0) const int blend_type = 0; layout(constant_id = 1) const int supports_decal = 1; -#ifdef IMPELLER_TARGET_OPENGLES -layout(set = 0, - binding = 0, - input_attachment_index = 0) uniform subpassInput uSub; - -vec4 ReadDestination() { - return subpassLoad(uSub); -} -#else +#ifdef IMPELLER_TARGET_VULKAN layout(set = 0, binding = 0, input_attachment_index = 0) uniform subpassInputMS uSub; @@ -32,6 +27,14 @@ vec4 ReadDestination() { vec4(4.0); } #endif +layout(set = 0, + binding = 0, + input_attachment_index = 0) uniform subpassInput uSub; + +vec4 ReadDestination() { + return subpassLoad(uSub); +} +#else uniform sampler2D texture_sampler_src; diff --git a/impeller/entity/shaders/blending/framebuffer_blend.vert b/impeller/entity/shaders/blending/framebuffer_blend.vert index 234d1e0c0b3e6..362362e4e3f8f 100644 --- a/impeller/entity/shaders/blending/framebuffer_blend.vert +++ b/impeller/entity/shaders/blending/framebuffer_blend.vert @@ -5,6 +5,9 @@ #include #include +// Warning: if any of the constant values or layouts are changed in this +// file, then the hard-coded constant value in +// impeller/renderer/backend/vulkan/binding_helpers_vk.cc uniform FrameInfo { mat4 mvp; float src_y_coord_scale; From 8dfb9420767b49602660268eb23fc3a0cfeff1bc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 30 Nov 2023 19:55:24 -0800 Subject: [PATCH 16/18] warning --- impeller/entity/shaders/blending/framebuffer_blend.frag | 4 ++-- impeller/renderer/backend/vulkan/binding_helpers_vk.cc | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/impeller/entity/shaders/blending/framebuffer_blend.frag b/impeller/entity/shaders/blending/framebuffer_blend.frag index eff3afd390a19..aea38baf37afa 100644 --- a/impeller/entity/shaders/blending/framebuffer_blend.frag +++ b/impeller/entity/shaders/blending/framebuffer_blend.frag @@ -26,7 +26,7 @@ vec4 ReadDestination() { subpassLoad(uSub, 3)) / vec4(4.0); } -#endif +#else layout(set = 0, binding = 0, input_attachment_index = 0) uniform subpassInput uSub; @@ -34,7 +34,7 @@ layout(set = 0, vec4 ReadDestination() { return subpassLoad(uSub); } -#else +#endif // IMPELLER_TARGET_VULKAN uniform sampler2D texture_sampler_src; diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index 7441e7b58302a..27713666d002f 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -11,13 +11,17 @@ #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/sampler_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" -#include "impeller/renderer/backend/vulkan/vk.h" #include "impeller/renderer/command.h" #include "impeller/renderer/compute_command.h" #include "vulkan/vulkan_core.h" namespace impeller { +// Warning: if any of the constant values or layouts are changed in the +// framebuffer fetch shader, then this input binding may need to be +// manually changed. +static constexpr size_t kMagicSubpassInputBinding = 64; + static bool BindImages(const Bindings& bindings, Allocator& allocator, const std::shared_ptr& encoder, @@ -188,8 +192,7 @@ fml::StatusOr> AllocateAndBindDescriptorSets( vk::WriteDescriptorSet write_set; write_set.dstSet = descriptor_sets[desc_index]; - // MAGIC! see description in pipeline_library_vk.cc - write_set.dstBinding = 64u; + write_set.dstBinding = kMagicSubpassInputBinding; write_set.descriptorCount = 1u; write_set.descriptorType = vk::DescriptorType::eInputAttachment; write_set.pImageInfo = &images.back(); From 4501eb3dee65ca82da636aa8ad07f9c7221925df Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Dec 2023 14:25:35 -0800 Subject: [PATCH 17/18] add test --- impeller/entity/entity_unittests.cc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index ed3c35afcf2f4..9d7f725cbb5d2 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -15,6 +15,7 @@ #include "impeller/entity/contents/atlas_contents.h" #include "impeller/entity/contents/clip_contents.h" #include "impeller/entity/contents/conical_gradient_contents.h" +#include "impeller/entity/contents/content_context.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/contents/filters/color_filter_contents.h" #include "impeller/entity/contents/filters/filter_contents.h" @@ -2562,6 +2563,30 @@ TEST_P(EntityTest, PipelineDescriptorEqAndHash) { EXPECT_EQ(desc_1->GetHash(), desc_2->GetHash()); } +#ifdef FML_OS_LINUX +TEST_P(EntityTest, FramebufferFetchVulkanBindingOffsetIsTheSame) { + // Using framebuffer fetch on Vulkan requires that we maintain a subpass input + // binding that we don't have a good route for configuring with the current + // metadata approach. This test verifies that the binding value doesn't change + // from the expected constant. + // See also: + // * impeller/renderer/backend/vulkan/binding_helpers_vk.cc + // * impeller/entity/shaders/blending/framebuffer_blend.frag + // This test only works on Linux because macOS hosts incorrectly populate the + // Vulkan descriptor sets based on the MSL compiler settings. + + bool expected_layout = false; + for (const DescriptorSetLayout& layout : FramebufferBlendColorBurnPipeline:: + FragmentShader::kDescriptorSetLayouts) { + if (layout.binding == 64 && + layout.descriptor_type == DescriptorType::kInputAttachment) { + expected_layout = true; + } + } + EXPECT_TRUE(expected_layout); +} +#endif + } // namespace testing } // namespace impeller From b9ef69b3dc513ae586db8dfaa68ce185b17519eb Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Dec 2023 15:56:32 -0800 Subject: [PATCH 18/18] ++ --- impeller/renderer/backend/vulkan/render_pass_vk.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 82293dd03be2d..e5d313748881f 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -53,7 +53,12 @@ static vk::AttachmentDescription CreateAttachmentDescription( } // Always insert a barrier to transition to color attachment optimal. - current_layout = vk::ImageLayout::eGeneral; + if (current_layout != vk::ImageLayout::ePresentSrcKHR && + current_layout != vk::ImageLayout::eUndefined) { + // Note: This should incur a barrier. + current_layout = vk::ImageLayout::eGeneral; + } + return CreateAttachmentDescription(desc.format, // desc.sample_count, // load_action, //