From 181e64ca5a6c5de76606d673b584016b064ac0aa Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 5 Dec 2023 22:47:32 -0800 Subject: [PATCH 1/5] [Impeller] simplify binding logic further. --- impeller/compiler/code_gen_template.h | 7 ++--- impeller/core/resource_binder.h | 6 +--- impeller/core/shader_types.h | 28 +------------------ .../entity/contents/checkerboard_contents.cc | 1 + .../border_mask_blur_filter_contents.cc | 1 + .../filters/color_matrix_filter_contents.cc | 1 + ...rectional_gaussian_blur_filter_contents.cc | 1 + .../filters/gaussian_blur_filter_contents.cc | 1 + .../filters/linear_to_srgb_filter_contents.cc | 1 + .../filters/morphology_filter_contents.cc | 1 + .../filters/srgb_to_linear_filter_contents.cc | 1 + .../filters/yuv_to_rgb_filter_contents.cc | 1 + .../contents/runtime_effect_contents.cc | 5 ++-- .../contents/test/contents_test_helpers.h | 20 +++++++++---- impeller/entity/contents/texture_contents.cc | 2 +- impeller/entity/geometry/geometry.h | 1 + .../backend/gles/buffer_bindings_gles.cc | 12 ++++---- .../backend/metal/compute_pass_mtl.mm | 8 +++--- .../renderer/backend/metal/render_pass_mtl.mm | 8 +++--- .../backend/vulkan/binding_helpers_vk.cc | 7 +++-- .../backend/vulkan/compute_pass_vk.cc | 2 +- .../renderer/backend/vulkan/render_pass_vk.cc | 2 +- impeller/renderer/command.cc | 21 ++++++-------- impeller/renderer/command.h | 12 +++----- impeller/renderer/compute_command.cc | 13 ++++----- impeller/renderer/compute_command.h | 10 +------ lib/gpu/render_pass.cc | 5 ++-- 27 files changed, 73 insertions(+), 105 deletions(-) diff --git a/impeller/compiler/code_gen_template.h b/impeller/compiler/code_gen_template.h index 1e73aee4a885a..4945155a420e5 100644 --- a/impeller/compiler/code_gen_template.h +++ b/impeller/compiler/code_gen_template.h @@ -118,12 +118,11 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader { // =========================================================================== {% for sampled_image in sampled_images %} - static constexpr auto kResource{{camel_case(sampled_image.name)}} = SampledImageSlot { // {{sampled_image.name}} + static constexpr auto kResource{{camel_case(sampled_image.name)}} = ShaderUniformSlot { // {{sampled_image.name}} "{{sampled_image.name}}", // name - {{sampled_image.ext_res_0}}u, // texture - {{sampled_image.ext_res_1}}u, // sampler - {{sampled_image.binding}}u, // binding + {{sampled_image.ext_res_0}}u, // ext_res_0 {{sampled_image.set}}u, // set + {{sampled_image.binding}}u, // binding }; static ShaderMetadata kMetadata{{camel_case(sampled_image.name)}}; {% endfor %} diff --git a/impeller/core/resource_binder.h b/impeller/core/resource_binder.h index 18b9d4c250121..eb9054c1ff69a 100644 --- a/impeller/core/resource_binder.h +++ b/impeller/core/resource_binder.h @@ -4,12 +4,8 @@ #pragma once -#include #include -#include -#include -#include "flutter/fml/logging.h" #include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" #include "impeller/core/sampler.h" @@ -32,7 +28,7 @@ struct ResourceBinder { BufferView view) = 0; virtual bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, + const ShaderUniformSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) = 0; diff --git a/impeller/core/shader_types.h b/impeller/core/shader_types.h index 425792d1ac7c0..3003f2f7569f2 100644 --- a/impeller/core/shader_types.h +++ b/impeller/core/shader_types.h @@ -73,7 +73,7 @@ struct ShaderMetadata { std::vector members; }; -/// @brief Metadata required to bind a buffer. +/// @brief Metadata required to bind a buffer or texture/sampler. /// /// OpenGL binding requires the usage of the separate shader metadata struct. struct ShaderUniformSlot { @@ -131,32 +131,6 @@ struct ShaderStageBufferLayout { } }; -/// @brief Metadata required to bind a texture and sampler. -/// -/// OpenGL binding requires the usage of the separate shader metadata struct. -struct SampledImageSlot { - /// @brief The name of the texture slot. - const char* name; - - /// @brief This value is `ext_res_0`, the Metal binding value for the texture. - size_t texture_index; - - /// @brief This value is `ext_res_1`, the Metal binding value for the sampler. - /// - /// Since only combined texture and samplers are used, this index is unused. - size_t sampler_index; - - /// @brief The Vulkan binding value for a combined texture and sampler. - size_t binding; - - /// @brief The Vulkan descriptor set index. - size_t set; - - constexpr bool HasTexture() const { return texture_index < 32u; } - - constexpr bool HasSampler() const { return sampler_index < 32u; } -}; - enum class DescriptorType { kUniformBuffer, kStorageBuffer, diff --git a/impeller/entity/contents/checkerboard_contents.cc b/impeller/entity/contents/checkerboard_contents.cc index debb65da9cbda..f58bb38850891 100644 --- a/impeller/entity/contents/checkerboard_contents.cc +++ b/impeller/entity/contents/checkerboard_contents.cc @@ -10,6 +10,7 @@ #include "impeller/entity/contents/content_context.h" #include "impeller/renderer/command.h" #include "impeller/renderer/render_pass.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/filters/border_mask_blur_filter_contents.cc b/impeller/entity/contents/filters/border_mask_blur_filter_contents.cc index da21b9134940a..5e41befaa7039 100644 --- a/impeller/entity/contents/filters/border_mask_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/border_mask_blur_filter_contents.cc @@ -9,6 +9,7 @@ #include "impeller/entity/contents/contents.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/filters/color_matrix_filter_contents.cc b/impeller/entity/contents/filters/color_matrix_filter_contents.cc index 2c159c803aac9..2ce6526169f3e 100644 --- a/impeller/entity/contents/filters/color_matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/color_matrix_filter_contents.cc @@ -14,6 +14,7 @@ #include "impeller/geometry/vector.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.cc index cbda25181664f..d9965ded5bd46 100644 --- a/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.cc @@ -20,6 +20,7 @@ #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index dd1d0256a921d..61ac31f3d3162 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -10,6 +10,7 @@ #include "impeller/renderer/command.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/filters/linear_to_srgb_filter_contents.cc b/impeller/entity/contents/filters/linear_to_srgb_filter_contents.cc index 64f13cdddf6a3..2bad955bf0416 100644 --- a/impeller/entity/contents/filters/linear_to_srgb_filter_contents.cc +++ b/impeller/entity/contents/filters/linear_to_srgb_filter_contents.cc @@ -11,6 +11,7 @@ #include "impeller/geometry/vector.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/filters/morphology_filter_contents.cc b/impeller/entity/contents/filters/morphology_filter_contents.cc index 2aa8290830cd5..086dc000e8666 100644 --- a/impeller/entity/contents/filters/morphology_filter_contents.cc +++ b/impeller/entity/contents/filters/morphology_filter_contents.cc @@ -11,6 +11,7 @@ #include "impeller/entity/contents/contents.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/filters/srgb_to_linear_filter_contents.cc b/impeller/entity/contents/filters/srgb_to_linear_filter_contents.cc index 38c6a2fe2842f..4488b5f21b5f7 100644 --- a/impeller/entity/contents/filters/srgb_to_linear_filter_contents.cc +++ b/impeller/entity/contents/filters/srgb_to_linear_filter_contents.cc @@ -11,6 +11,7 @@ #include "impeller/geometry/vector.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/filters/yuv_to_rgb_filter_contents.cc b/impeller/entity/contents/filters/yuv_to_rgb_filter_contents.cc index 21ddf7ee82bf2..99c5ac545ee4b 100644 --- a/impeller/entity/contents/filters/yuv_to_rgb_filter_contents.cc +++ b/impeller/entity/contents/filters/yuv_to_rgb_filter_contents.cc @@ -10,6 +10,7 @@ #include "impeller/geometry/matrix.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc index 91c18a3b7e0ce..9c4e163d95c38 100644 --- a/impeller/entity/contents/runtime_effect_contents.cc +++ b/impeller/entity/contents/runtime_effect_contents.cc @@ -238,10 +238,9 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, auto sampler = context->GetSamplerLibrary()->GetSampler(input.sampler_descriptor); - SampledImageSlot image_slot; + ShaderUniformSlot image_slot; image_slot.name = uniform.name.c_str(); - image_slot.texture_index = uniform.location - minimum_sampler_index; - image_slot.sampler_index = uniform.location - minimum_sampler_index; + image_slot.ext_res_0 = uniform.location - minimum_sampler_index; cmd.BindResource(ShaderStage::kFragment, image_slot, metadata, input.texture, sampler); diff --git a/impeller/entity/contents/test/contents_test_helpers.h b/impeller/entity/contents/test/contents_test_helpers.h index 6d3803ef9969d..e65b5636ef8d5 100644 --- a/impeller/entity/contents/test/contents_test_helpers.h +++ b/impeller/entity/contents/test/contents_test_helpers.h @@ -11,26 +11,34 @@ namespace impeller { /// @brief Retrieve the [VertInfo] struct data from the provided [command]. template typename T::VertInfo* GetVertInfo(const Command& command) { - auto resource = command.vertex_bindings.buffers.find(0u); + auto resource = std::find_if(command.vertex_bindings.buffers.begin(), + command.vertex_bindings.buffers.end(), + [](const BufferAndUniformSlot& data) { + return data.slot.ext_res_0 == 0u; + }); if (resource == command.vertex_bindings.buffers.end()) { return nullptr; } - auto data = (resource->second.view.resource.contents + - resource->second.view.resource.range.offset); + auto data = + (resource->view.resource.contents + resource->view.resource.range.offset); return reinterpret_cast(data); } /// @brief Retrieve the [FragInfo] struct data from the provided [command]. template typename T::FragInfo* GetFragInfo(const Command& command) { - auto resource = command.fragment_bindings.buffers.find(0u); + auto resource = std::find_if(command.fragment_bindings.buffers.begin(), + command.fragment_bindings.buffers.end(), + [](const BufferAndUniformSlot& data) { + return data.slot.ext_res_0 == 0u; + }); if (resource == command.fragment_bindings.buffers.end()) { return nullptr; } - auto data = (resource->second.view.resource.contents + - resource->second.view.resource.range.offset); + auto data = + (resource->view.resource.contents + resource->view.resource.range.offset); return reinterpret_cast(data); } diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index 9b9614aaf04f0..3ffd92f6b7a40 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -15,9 +15,9 @@ #include "impeller/entity/texture_fill.vert.h" #include "impeller/entity/texture_fill_external.frag.h" #include "impeller/geometry/constants.h" -#include "impeller/geometry/path_builder.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/entity/geometry/geometry.h b/impeller/entity/geometry/geometry.h index f41503551e0f6..de2f0e40c3271 100644 --- a/impeller/entity/geometry/geometry.h +++ b/impeller/entity/geometry/geometry.h @@ -10,6 +10,7 @@ #include "impeller/entity/entity.h" #include "impeller/entity/texture_fill.vert.h" #include "impeller/renderer/render_pass.h" +#include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/impeller/renderer/backend/gles/buffer_bindings_gles.cc index c96c4a5a04ac9..696ab62b42b35 100644 --- a/impeller/renderer/backend/gles/buffer_bindings_gles.cc +++ b/impeller/renderer/backend/gles/buffer_bindings_gles.cc @@ -145,12 +145,12 @@ bool BufferBindingsGLES::BindUniformData(const ProcTableGLES& gl, const Bindings& vertex_bindings, const Bindings& fragment_bindings) { for (const auto& buffer : vertex_bindings.buffers) { - if (!BindUniformBuffer(gl, transients_allocator, buffer.second.view)) { + if (!BindUniformBuffer(gl, transients_allocator, buffer.view)) { return false; } } for (const auto& buffer : fragment_bindings.buffers) { - if (!BindUniformBuffer(gl, transients_allocator, buffer.second.view)) { + if (!BindUniformBuffer(gl, transients_allocator, buffer.view)) { return false; } } @@ -345,13 +345,13 @@ std::optional BufferBindingsGLES::BindTextures( size_t unit_start_index) { size_t active_index = unit_start_index; for (const auto& data : bindings.sampled_images) { - const auto& texture_gles = TextureGLES::Cast(*data.second.texture.resource); - if (data.second.texture.GetMetadata() == nullptr) { + const auto& texture_gles = TextureGLES::Cast(*data.texture.resource); + if (data.texture.GetMetadata() == nullptr) { VALIDATION_LOG << "No metadata found for texture binding."; return std::nullopt; } - auto location = ComputeTextureLocation(data.second.texture.GetMetadata()); + auto location = ComputeTextureLocation(data.texture.GetMetadata()); if (location == -1) { return std::nullopt; } @@ -377,7 +377,7 @@ std::optional BufferBindingsGLES::BindTextures( /// If there is a sampler for the texture at the same index, configure the /// bound texture using that sampler. /// - const auto& sampler_gles = SamplerGLES::Cast(*data.second.sampler); + const auto& sampler_gles = SamplerGLES::Cast(*data.sampler); if (!sampler_gles.ConfigureBoundTexture(texture_gles, gl)) { return std::nullopt; } diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.mm b/impeller/renderer/backend/metal/compute_pass_mtl.mm index a55ec4fe07f60..a13eee599dbb0 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.mm +++ b/impeller/renderer/backend/metal/compute_pass_mtl.mm @@ -224,15 +224,15 @@ static bool Bind(ComputePassBindingsCache& pass, .GetMTLComputePipelineState()); for (const auto& buffer : command.bindings.buffers) { - if (!Bind(pass_bindings, *allocator, buffer.first, - buffer.second.view.resource)) { + if (!Bind(pass_bindings, *allocator, buffer.slot.ext_res_0, + buffer.view.resource)) { return false; } } for (const auto& data : command.bindings.sampled_images) { - if (!Bind(pass_bindings, data.first, *data.second.sampler, - *data.second.texture.resource)) { + if (!Bind(pass_bindings, data.slot.ext_res_0, *data.sampler, + *data.texture.resource)) { return false; } } diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm index f4fc37f0dfaf7..8fe42989f6f3c 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -409,14 +409,14 @@ static bool Bind(PassBindingsCache& pass, const Bindings& bindings, ShaderStage stage) -> bool { for (const auto& buffer : bindings.buffers) { - if (!Bind(pass_bindings, *allocator, stage, buffer.first, - buffer.second.view.resource)) { + if (!Bind(pass_bindings, *allocator, stage, buffer.slot.ext_res_0, + buffer.view.resource)) { return false; } } for (const auto& data : bindings.sampled_images) { - if (!Bind(pass_bindings, stage, data.first, *data.second.sampler, - *data.second.texture.resource)) { + if (!Bind(pass_bindings, stage, data.slot.ext_res_0, *data.sampler, + *data.texture.resource)) { return false; } } diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index d029a76f6d137..10ee695a189a7 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -4,6 +4,7 @@ #include "impeller/renderer/backend/vulkan/binding_helpers_vk.h" #include "fml/status.h" +#include "impeller/core/shader_types.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/backend/vulkan/command_pool_vk.h" @@ -28,7 +29,7 @@ static bool BindImages(const Bindings& bindings, vk::DescriptorSet& vk_desc_set, std::vector& images, std::vector& writes) { - for (const auto& [index, data] : bindings.sampled_images) { + for (const auto& data : bindings.sampled_images) { auto texture = data.texture.resource; const auto& texture_vk = TextureVK::Cast(*texture); const SamplerVK& sampler = SamplerVK::Cast(*data.sampler); @@ -38,7 +39,7 @@ static bool BindImages(const Bindings& bindings, return false; } - const SampledImageSlot& slot = data.slot; + const ShaderUniformSlot& slot = data.slot; vk::DescriptorImageInfo image_info; image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; @@ -66,7 +67,7 @@ static bool BindBuffers(const Bindings& bindings, const std::vector& desc_set, std::vector& buffers, std::vector& writes) { - for (const auto& [buffer_index, data] : bindings.buffers) { + for (const auto& data : bindings.buffers) { const auto& buffer_view = data.view.resource.buffer; auto device_buffer = buffer_view->GetDeviceBuffer(allocator); diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 43d1ce34f7590..56bc71cd782a0 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -43,7 +43,7 @@ static bool UpdateBindingLayouts(const Bindings& bindings, barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; - for (const auto& [_, data] : bindings.sampled_images) { + for (const auto& data : bindings.sampled_images) { if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) { return false; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index e5d313748881f..9b624155f9b13 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -315,7 +315,7 @@ static bool UpdateBindingLayouts(const Bindings& bindings, barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; - for (const auto& [_, data] : bindings.sampled_images) { + for (const auto& data : bindings.sampled_images) { if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) { return false; } diff --git a/impeller/renderer/command.cc b/impeller/renderer/command.cc index ed7fc0bc4960f..c40350d54a24b 100644 --- a/impeller/renderer/command.cc +++ b/impeller/renderer/command.cc @@ -49,12 +49,12 @@ bool Command::DoBindResource(ShaderStage stage, switch (stage) { case ShaderStage::kVertex: - vertex_bindings.buffers[slot.ext_res_0] = { - .slot = slot, .view = BufferResource(metadata, std::move(view))}; + vertex_bindings.buffers.emplace_back(BufferAndUniformSlot{ + .slot = slot, .view = BufferResource(metadata, std::move(view))}); return true; case ShaderStage::kFragment: - fragment_bindings.buffers[slot.ext_res_0] = { - .slot = slot, .view = BufferResource(metadata, std::move(view))}; + fragment_bindings.buffers.emplace_back(BufferAndUniformSlot{ + .slot = slot, .view = BufferResource(metadata, std::move(view))}); return true; case ShaderStage::kCompute: VALIDATION_LOG << "Use ComputeCommands for compute shader stages."; @@ -66,7 +66,7 @@ bool Command::DoBindResource(ShaderStage stage, } bool Command::BindResource(ShaderStage stage, - const SampledImageSlot& slot, + const ShaderUniformSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) { @@ -76,24 +76,21 @@ bool Command::BindResource(ShaderStage stage, if (!texture || !texture->IsValid()) { return false; } - if (!slot.HasSampler() || !slot.HasTexture()) { - return true; - } switch (stage) { case ShaderStage::kVertex: - vertex_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{ + vertex_bindings.sampled_images.emplace_back(TextureAndSampler{ .slot = slot, .texture = {&metadata, std::move(texture)}, .sampler = std::move(sampler), - }; + }); return true; case ShaderStage::kFragment: - fragment_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{ + fragment_bindings.sampled_images.emplace_back(TextureAndSampler{ .slot = slot, .texture = {&metadata, std::move(texture)}, .sampler = std::move(sampler), - }; + }); return true; case ShaderStage::kCompute: VALIDATION_LOG << "Use ComputeCommands for compute shader stages."; diff --git a/impeller/renderer/command.h b/impeller/renderer/command.h index d64215d134a43..8f041d5086871 100644 --- a/impeller/renderer/command.h +++ b/impeller/renderer/command.h @@ -10,8 +10,6 @@ #include #include -#include "flutter/fml/logging.h" -#include "flutter/fml/macros.h" #include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" #include "impeller/core/resource_binder.h" @@ -21,8 +19,6 @@ #include "impeller/core/vertex_buffer.h" #include "impeller/geometry/rect.h" #include "impeller/renderer/pipeline.h" -#include "impeller/renderer/vertex_buffer_builder.h" -#include "impeller/tessellator/tessellator.h" namespace impeller { @@ -63,7 +59,7 @@ using TextureResource = Resource>; /// @brief combines the texture, sampler and sampler slot information. struct TextureAndSampler { - SampledImageSlot slot; + ShaderUniformSlot slot; TextureResource texture; std::shared_ptr sampler; }; @@ -75,8 +71,8 @@ struct BufferAndUniformSlot { }; struct Bindings { - std::map sampled_images; - std::map buffers; + std::vector sampled_images; + std::vector buffers; }; //------------------------------------------------------------------------------ @@ -177,7 +173,7 @@ struct Command : public ResourceBinder { // |ResourceBinder| bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, + const ShaderUniformSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) override; diff --git a/impeller/renderer/compute_command.cc b/impeller/renderer/compute_command.cc index 78b9fb99ec004..62f2491b4f456 100644 --- a/impeller/renderer/compute_command.cc +++ b/impeller/renderer/compute_command.cc @@ -23,13 +23,13 @@ bool ComputeCommand::BindResource(ShaderStage stage, return false; } - bindings.buffers[slot.ext_res_0] = {.slot = slot, - .view = {&metadata, std::move(view)}}; + bindings.buffers.emplace_back( + BufferAndUniformSlot{.slot = slot, .view = {&metadata, std::move(view)}}); return true; } bool ComputeCommand::BindResource(ShaderStage stage, - const SampledImageSlot& slot, + const ShaderUniformSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) { @@ -43,15 +43,12 @@ bool ComputeCommand::BindResource(ShaderStage stage, if (!texture || !texture->IsValid()) { return false; } - if (!slot.HasSampler() || !slot.HasTexture()) { - return true; - } - bindings.sampled_images[slot.sampler_index] = TextureAndSampler{ + bindings.sampled_images.emplace_back(TextureAndSampler{ .slot = slot, .texture = {&metadata, std::move(texture)}, .sampler = std::move(sampler), - }; + }); return false; } diff --git a/impeller/renderer/compute_command.h b/impeller/renderer/compute_command.h index f537b37e62c20..511590ab31690 100644 --- a/impeller/renderer/compute_command.h +++ b/impeller/renderer/compute_command.h @@ -4,26 +4,18 @@ #pragma once -#include #include -#include #include -#include "flutter/fml/logging.h" -#include "flutter/fml/macros.h" #include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" #include "impeller/core/resource_binder.h" #include "impeller/core/sampler.h" #include "impeller/core/shader_types.h" #include "impeller/core/texture.h" -#include "impeller/core/vertex_buffer.h" -#include "impeller/geometry/rect.h" #include "impeller/renderer/command.h" #include "impeller/renderer/compute_pipeline_descriptor.h" #include "impeller/renderer/pipeline.h" -#include "impeller/renderer/vertex_buffer_builder.h" -#include "impeller/tessellator/tessellator.h" namespace impeller { @@ -66,7 +58,7 @@ struct ComputeCommand : public ResourceBinder { // |ResourceBinder| bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, + const ShaderUniformSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) override; diff --git a/lib/gpu/render_pass.cc b/lib/gpu/render_pass.cc index 24e01f2ad2f12..4be78f80dc6d9 100644 --- a/lib/gpu/render_pass.cc +++ b/lib/gpu/render_pass.cc @@ -384,9 +384,8 @@ bool InternalFlutterGpu_RenderPass_BindTexture( auto sampler = wrapper->GetContext().lock()->GetSamplerLibrary()->GetSampler( sampler_desc); - impeller::SampledImageSlot image_slot; - image_slot.texture_index = slot_id; - image_slot.sampler_index = slot_id; + impeller::ShaderUniformSlot image_slot; + image_slot.ext_res_0 = slot_id; return command.BindResource(flutter::gpu::ToImpellerShaderStage(stage), image_slot, metadata, texture->GetTexture(), sampler); From 3d2c44d5513c02cbcd9af80fc612e05d32c1c563 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 6 Dec 2023 09:58:03 -0800 Subject: [PATCH 2/5] Add back sampled image slot. --- impeller/compiler/code_gen_template.h | 2 +- impeller/core/resource_binder.h | 2 +- impeller/core/shader_types.h | 19 ++++++++++++++++++- .../contents/runtime_effect_contents.cc | 4 ++-- .../backend/metal/compute_pass_mtl.mm | 9 +++++---- .../renderer/backend/metal/render_pass_mtl.mm | 7 ++++--- .../backend/vulkan/binding_helpers_vk.cc | 6 +++--- .../backend/vulkan/compute_pass_vk.cc | 5 +++-- impeller/renderer/command.cc | 2 +- impeller/renderer/command.h | 4 ++-- impeller/renderer/compute_command.cc | 2 +- impeller/renderer/compute_command.h | 2 +- lib/gpu/render_pass.cc | 4 ++-- 13 files changed, 44 insertions(+), 24 deletions(-) diff --git a/impeller/compiler/code_gen_template.h b/impeller/compiler/code_gen_template.h index 4945155a420e5..7927c71d2952c 100644 --- a/impeller/compiler/code_gen_template.h +++ b/impeller/compiler/code_gen_template.h @@ -118,7 +118,7 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader { // =========================================================================== {% for sampled_image in sampled_images %} - static constexpr auto kResource{{camel_case(sampled_image.name)}} = ShaderUniformSlot { // {{sampled_image.name}} + static constexpr auto kResource{{camel_case(sampled_image.name)}} = SampledImageSlot { // {{sampled_image.name}} "{{sampled_image.name}}", // name {{sampled_image.ext_res_0}}u, // ext_res_0 {{sampled_image.set}}u, // set diff --git a/impeller/core/resource_binder.h b/impeller/core/resource_binder.h index eb9054c1ff69a..a3bbb532f9522 100644 --- a/impeller/core/resource_binder.h +++ b/impeller/core/resource_binder.h @@ -28,7 +28,7 @@ struct ResourceBinder { BufferView view) = 0; virtual bool BindResource(ShaderStage stage, - const ShaderUniformSlot& slot, + const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) = 0; diff --git a/impeller/core/shader_types.h b/impeller/core/shader_types.h index 3003f2f7569f2..83fd120baf7ba 100644 --- a/impeller/core/shader_types.h +++ b/impeller/core/shader_types.h @@ -73,7 +73,7 @@ struct ShaderMetadata { std::vector members; }; -/// @brief Metadata required to bind a buffer or texture/sampler. +/// @brief Metadata required to bind a buffer. /// /// OpenGL binding requires the usage of the separate shader metadata struct. struct ShaderUniformSlot { @@ -90,6 +90,23 @@ struct ShaderUniformSlot { size_t binding; }; +/// @brief Metadata required to bind a combined texture and sampler. +/// +/// OpenGL binding requires the usage of the separate shader metadata struct. +struct SampledImageSlot { + /// @brief The name of the uniform slot. + const char* name; + + /// @brief `ext_res_0` is the Metal binding value. + size_t texture_index; + + /// @brief The Vulkan descriptor set index. + size_t set; + + /// @brief The Vulkan binding value. + size_t binding; +}; + struct ShaderStageIOSlot { const char* name; size_t location; diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc index 9c4e163d95c38..82ad3508ff274 100644 --- a/impeller/entity/contents/runtime_effect_contents.cc +++ b/impeller/entity/contents/runtime_effect_contents.cc @@ -238,9 +238,9 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, auto sampler = context->GetSamplerLibrary()->GetSampler(input.sampler_descriptor); - ShaderUniformSlot image_slot; + SampledImageSlot image_slot; image_slot.name = uniform.name.c_str(); - image_slot.ext_res_0 = uniform.location - minimum_sampler_index; + image_slot.texture_index = uniform.location - minimum_sampler_index; cmd.BindResource(ShaderStage::kFragment, image_slot, metadata, input.texture, sampler); diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.mm b/impeller/renderer/backend/metal/compute_pass_mtl.mm index a13eee599dbb0..0cbce56c48c31 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.mm +++ b/impeller/renderer/backend/metal/compute_pass_mtl.mm @@ -21,6 +21,7 @@ #include "impeller/renderer/backend/metal/formats_mtl.h" #include "impeller/renderer/backend/metal/sampler_mtl.h" #include "impeller/renderer/backend/metal/texture_mtl.h" +#include "impeller/renderer/command.h" #include "impeller/renderer/compute_command.h" namespace impeller { @@ -209,7 +210,7 @@ static bool Bind(ComputePassBindingsCache& pass, ComputePassBindingsCache pass_bindings(encoder); fml::closure pop_debug_marker = [encoder]() { [encoder popDebugGroup]; }; - for (const auto& command : commands_) { + for (const ComputeCommand& command : commands_) { #ifdef IMPELLER_DEBUG fml::ScopedCleanupClosure auto_pop_debug_marker(pop_debug_marker); if (!command.label.empty()) { @@ -223,15 +224,15 @@ static bool Bind(ComputePassBindingsCache& pass, ComputePipelineMTL::Cast(*command.pipeline) .GetMTLComputePipelineState()); - for (const auto& buffer : command.bindings.buffers) { + for (const BufferAndUniformSlot& buffer : command.bindings.buffers) { if (!Bind(pass_bindings, *allocator, buffer.slot.ext_res_0, buffer.view.resource)) { return false; } } - for (const auto& data : command.bindings.sampled_images) { - if (!Bind(pass_bindings, data.slot.ext_res_0, *data.sampler, + for (const TextureAndSampler& data : command.bindings.sampled_images) { + if (!Bind(pass_bindings, data.slot.texture_index, *data.sampler, *data.texture.resource)) { return false; } diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm index 8fe42989f6f3c..d313ca2b0f953 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -18,6 +18,7 @@ #include "impeller/renderer/backend/metal/pipeline_mtl.h" #include "impeller/renderer/backend/metal/sampler_mtl.h" #include "impeller/renderer/backend/metal/texture_mtl.h" +#include "impeller/renderer/command.h" #include "impeller/renderer/vertex_descriptor.h" namespace impeller { @@ -408,14 +409,14 @@ static bool Bind(PassBindingsCache& pass, auto bind_stage_resources = [&allocator, &pass_bindings]( const Bindings& bindings, ShaderStage stage) -> bool { - for (const auto& buffer : bindings.buffers) { + for (const BufferAndUniformSlot& buffer : bindings.buffers) { if (!Bind(pass_bindings, *allocator, stage, buffer.slot.ext_res_0, buffer.view.resource)) { return false; } } - for (const auto& data : bindings.sampled_images) { - if (!Bind(pass_bindings, stage, data.slot.ext_res_0, *data.sampler, + for (const TextureAndSampler& data : bindings.sampled_images) { + if (!Bind(pass_bindings, stage, data.slot.texture_index, *data.sampler, *data.texture.resource)) { return false; } diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index 10ee695a189a7..676980689ce65 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -29,7 +29,7 @@ static bool BindImages(const Bindings& bindings, vk::DescriptorSet& vk_desc_set, std::vector& images, std::vector& writes) { - for (const auto& data : bindings.sampled_images) { + for (const TextureAndSampler& data : bindings.sampled_images) { auto texture = data.texture.resource; const auto& texture_vk = TextureVK::Cast(*texture); const SamplerVK& sampler = SamplerVK::Cast(*data.sampler); @@ -39,7 +39,7 @@ static bool BindImages(const Bindings& bindings, return false; } - const ShaderUniformSlot& slot = data.slot; + const SampledImageSlot& slot = data.slot; vk::DescriptorImageInfo image_info; image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; @@ -67,7 +67,7 @@ static bool BindBuffers(const Bindings& bindings, const std::vector& desc_set, std::vector& buffers, std::vector& writes) { - for (const auto& data : bindings.buffers) { + for (const BufferAndUniformSlot& data : bindings.buffers) { const auto& buffer_view = data.view.resource.buffer; auto device_buffer = buffer_view->GetDeviceBuffer(allocator); diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 56bc71cd782a0..fffc615e5d04b 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -9,6 +9,7 @@ #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/compute_pipeline_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" +#include "impeller/renderer/command.h" namespace impeller { @@ -43,7 +44,7 @@ static bool UpdateBindingLayouts(const Bindings& bindings, barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; - for (const auto& data : bindings.sampled_images) { + for (const TextureAndSampler& data : bindings.sampled_images) { if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) { return false; } @@ -58,7 +59,7 @@ static bool UpdateBindingLayouts(const ComputeCommand& command, static bool UpdateBindingLayouts(const std::vector& commands, const vk::CommandBuffer& buffer) { - for (const auto& command : commands) { + for (const ComputeCommand& command : commands) { if (!UpdateBindingLayouts(command, buffer)) { return false; } diff --git a/impeller/renderer/command.cc b/impeller/renderer/command.cc index c40350d54a24b..40dfd8c09d518 100644 --- a/impeller/renderer/command.cc +++ b/impeller/renderer/command.cc @@ -66,7 +66,7 @@ bool Command::DoBindResource(ShaderStage stage, } bool Command::BindResource(ShaderStage stage, - const ShaderUniformSlot& slot, + const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) { diff --git a/impeller/renderer/command.h b/impeller/renderer/command.h index 8f041d5086871..6351a4599df9f 100644 --- a/impeller/renderer/command.h +++ b/impeller/renderer/command.h @@ -59,7 +59,7 @@ using TextureResource = Resource>; /// @brief combines the texture, sampler and sampler slot information. struct TextureAndSampler { - ShaderUniformSlot slot; + SampledImageSlot slot; TextureResource texture; std::shared_ptr sampler; }; @@ -173,7 +173,7 @@ struct Command : public ResourceBinder { // |ResourceBinder| bool BindResource(ShaderStage stage, - const ShaderUniformSlot& slot, + const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) override; diff --git a/impeller/renderer/compute_command.cc b/impeller/renderer/compute_command.cc index 62f2491b4f456..fbbfe3b58666f 100644 --- a/impeller/renderer/compute_command.cc +++ b/impeller/renderer/compute_command.cc @@ -29,7 +29,7 @@ bool ComputeCommand::BindResource(ShaderStage stage, } bool ComputeCommand::BindResource(ShaderStage stage, - const ShaderUniformSlot& slot, + const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) { diff --git a/impeller/renderer/compute_command.h b/impeller/renderer/compute_command.h index 511590ab31690..1a2d51503698c 100644 --- a/impeller/renderer/compute_command.h +++ b/impeller/renderer/compute_command.h @@ -58,7 +58,7 @@ struct ComputeCommand : public ResourceBinder { // |ResourceBinder| bool BindResource(ShaderStage stage, - const ShaderUniformSlot& slot, + const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) override; diff --git a/lib/gpu/render_pass.cc b/lib/gpu/render_pass.cc index 4be78f80dc6d9..98ea3160cbcc5 100644 --- a/lib/gpu/render_pass.cc +++ b/lib/gpu/render_pass.cc @@ -384,8 +384,8 @@ bool InternalFlutterGpu_RenderPass_BindTexture( auto sampler = wrapper->GetContext().lock()->GetSamplerLibrary()->GetSampler( sampler_desc); - impeller::ShaderUniformSlot image_slot; - image_slot.ext_res_0 = slot_id; + impeller::SampledImageSlot image_slot; + image_slot.texture_index = slot_id; return command.BindResource(flutter::gpu::ToImpellerShaderStage(stage), image_slot, metadata, texture->GetTexture(), sampler); From 1f7dfdc4675f4e5de40df9001c4f9a6b08207f79 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 6 Dec 2023 09:59:33 -0800 Subject: [PATCH 3/5] missed one. --- impeller/renderer/backend/vulkan/render_pass_vk.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 9b624155f9b13..615ab1e27617f 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 "impeller/renderer/command.h" #include "vulkan/vulkan_enums.hpp" #include "vulkan/vulkan_handles.hpp" #include "vulkan/vulkan_to_string.hpp" @@ -315,7 +316,7 @@ static bool UpdateBindingLayouts(const Bindings& bindings, barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; - for (const auto& data : bindings.sampled_images) { + for (const TextureAndSampler& data : bindings.sampled_images) { if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) { return false; } @@ -331,7 +332,7 @@ static bool UpdateBindingLayouts(const Command& command, static bool UpdateBindingLayouts(const std::vector& commands, const vk::CommandBuffer& buffer) { - for (const auto& command : commands) { + for (const Command& command : commands) { if (!UpdateBindingLayouts(command, buffer)) { return false; } From 298c2b5964e6f897d72d53bb3c21cc80fd4f403b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 6 Dec 2023 10:06:23 -0800 Subject: [PATCH 4/5] revert template changes. --- impeller/compiler/code_gen_template.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/impeller/compiler/code_gen_template.h b/impeller/compiler/code_gen_template.h index 7927c71d2952c..1e73aee4a885a 100644 --- a/impeller/compiler/code_gen_template.h +++ b/impeller/compiler/code_gen_template.h @@ -120,9 +120,10 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader { static constexpr auto kResource{{camel_case(sampled_image.name)}} = SampledImageSlot { // {{sampled_image.name}} "{{sampled_image.name}}", // name - {{sampled_image.ext_res_0}}u, // ext_res_0 - {{sampled_image.set}}u, // set + {{sampled_image.ext_res_0}}u, // texture + {{sampled_image.ext_res_1}}u, // sampler {{sampled_image.binding}}u, // binding + {{sampled_image.set}}u, // set }; static ShaderMetadata kMetadata{{camel_case(sampled_image.name)}}; {% endfor %} From 4e5a81418bdaa3b4d63c0e8fb450e8f8b874db8b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 6 Dec 2023 10:17:35 -0800 Subject: [PATCH 5/5] ++ --- impeller/compiler/code_gen_template.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/impeller/compiler/code_gen_template.h b/impeller/compiler/code_gen_template.h index 1e73aee4a885a..7927c71d2952c 100644 --- a/impeller/compiler/code_gen_template.h +++ b/impeller/compiler/code_gen_template.h @@ -120,10 +120,9 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader { static constexpr auto kResource{{camel_case(sampled_image.name)}} = SampledImageSlot { // {{sampled_image.name}} "{{sampled_image.name}}", // name - {{sampled_image.ext_res_0}}u, // texture - {{sampled_image.ext_res_1}}u, // sampler - {{sampled_image.binding}}u, // binding + {{sampled_image.ext_res_0}}u, // ext_res_0 {{sampled_image.set}}u, // set + {{sampled_image.binding}}u, // binding }; static ShaderMetadata kMetadata{{camel_case(sampled_image.name)}}; {% endfor %}