From 3bd3b72b1b8705ac0a8f72356f3451587325c6e5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 22 Aug 2023 20:33:52 -0700 Subject: [PATCH 1/3] [impeller] combine sampler and texture maps. --- impeller/core/resource_binder.h | 10 ---- .../backend/metal/compute_pass_mtl.mm | 24 ++------ .../renderer/backend/metal/render_pass_mtl.mm | 29 +++------ .../backend/vulkan/compute_pass_vk.cc | 16 ++--- .../renderer/backend/vulkan/render_pass_vk.cc | 16 ++--- impeller/renderer/command.cc | 60 +++++-------------- impeller/renderer/command.h | 27 ++++----- impeller/renderer/compute_command.cc | 49 ++++----------- impeller/renderer/compute_command.h | 16 +---- 9 files changed, 65 insertions(+), 182 deletions(-) diff --git a/impeller/core/resource_binder.h b/impeller/core/resource_binder.h index 4bea96f868f49..a88e75649d903 100644 --- a/impeller/core/resource_binder.h +++ b/impeller/core/resource_binder.h @@ -32,16 +32,6 @@ struct ResourceBinder { const ShaderMetadata& metadata, const BufferView& view) = 0; - virtual bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - const std::shared_ptr& texture) = 0; - - virtual bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - const std::shared_ptr& sampler) = 0; - virtual bool BindResource(ShaderStage stage, const SampledImageSlot& slot, const ShaderMetadata& metadata, diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.mm b/impeller/renderer/backend/metal/compute_pass_mtl.mm index 0157ec5b38a16..68cd3f47b4082 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.mm +++ b/impeller/renderer/backend/metal/compute_pass_mtl.mm @@ -187,22 +187,13 @@ static bool Bind(ComputePassBindingsCache& pass, static bool Bind(ComputePassBindingsCache& pass, size_t bind_index, + const Sampler& sampler, const Texture& texture) { - if (!texture.IsValid()) { + if (!sampler.IsValid() || !texture.IsValid()) { return false; } pass.SetTexture(bind_index, TextureMTL::Cast(texture).GetMTLTexture()); - return true; -} - -static bool Bind(ComputePassBindingsCache& pass, - size_t bind_index, - const Sampler& sampler) { - if (!sampler.IsValid()) { - return false; - } - pass.SetSampler(bind_index, SamplerMTL::Cast(sampler).GetMTLSamplerState()); return true; } @@ -239,16 +230,13 @@ static bool Bind(ComputePassBindingsCache& pass, } } - for (const auto& texture : command.bindings.textures) { - if (!Bind(pass_bindings, texture.first, *texture.second.resource)) { - return false; - } - } - for (const auto& sampler : command.bindings.samplers) { - if (!Bind(pass_bindings, sampler.first, *sampler.second.resource)) { + for (const auto& data : command.bindings.sampled_images) { + if (!Bind(pass_bindings, data.first, *data.second.sampler.resource, + *data.second.texture.resource)) { return false; } } + // TODO(dnfield): use feature detection to support non-uniform threadgroup // sizes. // https://github.com/flutter/flutter/issues/110619 diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm index 18395bb8eea26..6be8298e3385c 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -379,8 +379,9 @@ static bool Bind(PassBindingsCache& pass, static bool Bind(PassBindingsCache& pass, ShaderStage stage, size_t bind_index, + const Sampler& sampler, const Texture& texture) { - if (!texture.IsValid()) { + if (!sampler.IsValid() || !texture.IsValid()) { return false; } @@ -395,18 +396,8 @@ static bool Bind(PassBindingsCache& pass, } return pass.SetTexture(stage, bind_index, - TextureMTL::Cast(texture).GetMTLTexture()); -} - -static bool Bind(PassBindingsCache& pass, - ShaderStage stage, - size_t bind_index, - const Sampler& sampler) { - if (!sampler.IsValid()) { - return false; - } - - return pass.SetSampler(stage, bind_index, + TextureMTL::Cast(texture).GetMTLTexture()) && + pass.SetSampler(stage, bind_index, SamplerMTL::Cast(sampler).GetMTLSamplerState()); } @@ -422,15 +413,9 @@ static bool Bind(PassBindingsCache& pass, return false; } } - for (const auto& texture : bindings.textures) { - if (!Bind(pass_bindings, stage, texture.first, - *texture.second.resource)) { - return false; - } - } - for (const auto& sampler : bindings.samplers) { - if (!Bind(pass_bindings, stage, sampler.first, - *sampler.second.resource)) { + for (const auto& data : bindings.sampled_images) { + if (!Bind(pass_bindings, stage, data.first, *data.second.sampler.resource, + *data.second.texture.resource)) { return false; } } diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 563b0845158a3..d750e0d0ecb4f 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -43,8 +43,8 @@ static bool UpdateBindingLayouts(const Bindings& bindings, barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; - for (const auto& [_, texture] : bindings.textures) { - if (!TextureVK::Cast(*texture.resource).SetLayout(barrier)) { + for (const auto& [_, data] : bindings.sampled_images) { + if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) { return false; } } @@ -88,21 +88,17 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, &writes, // &vk_desc_set // ](const Bindings& bindings) -> bool { - for (const auto& [index, sampler_handle] : bindings.samplers) { - if (bindings.textures.find(index) == bindings.textures.end()) { - return false; - } - - auto texture = bindings.textures.at(index).resource; + for (const auto& [index, data] : bindings.sampled_images) { + auto texture = data.texture.resource; const auto& texture_vk = TextureVK::Cast(*texture); - const SamplerVK& sampler = SamplerVK::Cast(*sampler_handle.resource); + const SamplerVK& sampler = SamplerVK::Cast(*data.sampler.resource); if (!encoder.Track(texture) || !encoder.Track(sampler.GetSharedSampler())) { return false; } - const SampledImageSlot& slot = bindings.sampled_images.at(index); + const SampledImageSlot& slot = data.slot; vk::DescriptorImageInfo image_info; image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index dd8b09254cd48..a981c75658853 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -283,8 +283,8 @@ static bool UpdateBindingLayouts(const Bindings& bindings, barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; - for (const auto& [_, texture] : bindings.textures) { - if (!TextureVK::Cast(*texture.resource).SetLayout(barrier)) { + for (const auto& [_, data] : bindings.sampled_images) { + if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) { return false; } } @@ -331,21 +331,17 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, &writes, // &vk_desc_set // ](const Bindings& bindings) -> bool { - for (const auto& [index, sampler_handle] : bindings.samplers) { - if (bindings.textures.find(index) == bindings.textures.end()) { - return false; - } - - auto texture = bindings.textures.at(index).resource; + for (const auto& [index, data] : bindings.sampled_images) { + auto texture = data.texture.resource; const auto& texture_vk = TextureVK::Cast(*texture); - const SamplerVK& sampler = SamplerVK::Cast(*sampler_handle.resource); + const SamplerVK& sampler = SamplerVK::Cast(*data.sampler.resource); if (!encoder.Track(texture) || !encoder.Track(sampler.GetSharedSampler())) { return false; } - const SampledImageSlot& slot = bindings.sampled_images.at(index); + const SampledImageSlot& slot = data.slot; vk::DescriptorImageInfo image_info; image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; diff --git a/impeller/renderer/command.cc b/impeller/renderer/command.cc index 068eb2fdf5578..68d835eefcac1 100644 --- a/impeller/renderer/command.cc +++ b/impeller/renderer/command.cc @@ -83,53 +83,32 @@ bool Command::DoBindResource(ShaderStage stage, bool Command::BindResource(ShaderStage stage, const SampledImageSlot& slot, const ShaderMetadata& metadata, - const std::shared_ptr& texture) { - if (!texture || !texture->IsValid()) { - return false; - } - - if (!slot.HasTexture()) { - return true; - } - - switch (stage) { - case ShaderStage::kVertex: - vertex_bindings.textures[slot.texture_index] = {&metadata, texture}; - return true; - case ShaderStage::kFragment: - fragment_bindings.textures[slot.texture_index] = {&metadata, texture}; - return true; - case ShaderStage::kCompute: - VALIDATION_LOG << "Use ComputeCommands for compute shader stages."; - case ShaderStage::kTessellationControl: - case ShaderStage::kTessellationEvaluation: - case ShaderStage::kUnknown: - return false; - } - - return false; -} - -bool Command::BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, + const std::shared_ptr& texture, const std::shared_ptr& sampler) { if (!sampler || !sampler->IsValid()) { return false; } - - if (!slot.HasSampler()) { + if (!texture || !texture->IsValid()) { + return false; + } + if (!slot.HasSampler() || !slot.HasTexture()) { return true; } switch (stage) { case ShaderStage::kVertex: - vertex_bindings.samplers[slot.sampler_index] = {&metadata, sampler}; - vertex_bindings.sampled_images[slot.sampler_index] = slot; + vertex_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{ + .slot = slot, + .texture = {&metadata, texture}, + .sampler = {&metadata, sampler}, + }; return true; case ShaderStage::kFragment: - fragment_bindings.samplers[slot.sampler_index] = {&metadata, sampler}; - fragment_bindings.sampled_images[slot.sampler_index] = slot; + vertex_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{ + .slot = slot, + .texture = {&metadata, texture}, + .sampler = {&metadata, sampler}, + }; return true; case ShaderStage::kCompute: VALIDATION_LOG << "Use ComputeCommands for compute shader stages."; @@ -142,13 +121,4 @@ bool Command::BindResource(ShaderStage stage, return false; } -bool Command::BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - const std::shared_ptr& texture, - const std::shared_ptr& sampler) { - return BindResource(stage, slot, metadata, texture) && - BindResource(stage, slot, metadata, sampler); -} - } // namespace impeller diff --git a/impeller/renderer/command.h b/impeller/renderer/command.h index a20559f3856a7..2485cbd73bb35 100644 --- a/impeller/renderer/command.h +++ b/impeller/renderer/command.h @@ -61,12 +61,17 @@ using BufferResource = Resource; using TextureResource = Resource>; using SamplerResource = Resource>; +/// @brief combines the texture, sampler and sampler slot information. +struct TextureAndSampler { + SampledImageSlot slot; + TextureResource texture; + SamplerResource sampler; +}; + struct Bindings { std::map uniforms; - std::map sampled_images; + std::map sampled_images; std::map buffers; - std::map textures; - std::map samplers; }; //------------------------------------------------------------------------------ @@ -184,18 +189,6 @@ struct Command : public ResourceBinder { const std::shared_ptr& metadata, const BufferView& view); - // |ResourceBinder| - bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - const std::shared_ptr& texture) override; - - // |ResourceBinder| - bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - const std::shared_ptr& sampler) override; - // |ResourceBinder| bool BindResource(ShaderStage stage, const SampledImageSlot& slot, @@ -205,7 +198,9 @@ struct Command : public ResourceBinder { BufferView GetVertexBuffer() const; - constexpr operator bool() const { return pipeline && pipeline->IsValid(); } + constexpr explicit operator bool() const { + return pipeline && pipeline->IsValid(); + } private: template diff --git a/impeller/renderer/compute_command.cc b/impeller/renderer/compute_command.cc index b5f166e27e24b..ad6131f05a6d5 100644 --- a/impeller/renderer/compute_command.cc +++ b/impeller/renderer/compute_command.cc @@ -32,27 +32,7 @@ bool ComputeCommand::BindResource( ShaderStage stage, const SampledImageSlot& slot, const ShaderMetadata& metadata, - const std::shared_ptr& texture) { - if (stage != ShaderStage::kCompute) { - VALIDATION_LOG << "Use Command for non-compute shader stages."; - return false; - } - if (!texture || !texture->IsValid()) { - return false; - } - - if (!slot.HasTexture()) { - return true; - } - - bindings.textures[slot.texture_index] = {&metadata, texture}; - return true; -} - -bool ComputeCommand::BindResource( - ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, + const std::shared_ptr& texture, const std::shared_ptr& sampler) { if (stage != ShaderStage::kCompute) { VALIDATION_LOG << "Use Command for non-compute shader stages."; @@ -61,27 +41,20 @@ bool ComputeCommand::BindResource( if (!sampler || !sampler->IsValid()) { return false; } - - if (!slot.HasSampler()) { + if (!texture || !texture->IsValid()) { + return false; + } + if (!slot.HasSampler() || !slot.HasTexture()) { return true; } - bindings.samplers[slot.sampler_index] = {&metadata, sampler}; - return true; -} + bindings.sampled_images[slot.sampler_index] = TextureAndSampler{ + .slot = slot, + .texture = {&metadata, texture}, + .sampler = {&metadata, sampler}, + }; -bool ComputeCommand::BindResource( - ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - const std::shared_ptr& texture, - const std::shared_ptr& sampler) { - if (stage != ShaderStage::kCompute) { - VALIDATION_LOG << "Use Command for non-compute shader stages."; - return false; - } - return BindResource(stage, slot, metadata, texture) && - BindResource(stage, slot, metadata, sampler); + return false; } } // namespace impeller diff --git a/impeller/renderer/compute_command.h b/impeller/renderer/compute_command.h index caa0cf0e60dc3..4cc917fa2a18f 100644 --- a/impeller/renderer/compute_command.h +++ b/impeller/renderer/compute_command.h @@ -64,18 +64,6 @@ struct ComputeCommand : public ResourceBinder { const ShaderMetadata& metadata, const BufferView& view) override; - // |ResourceBinder| - bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - const std::shared_ptr& texture) override; - - // |ResourceBinder| - bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - const std::shared_ptr& sampler) override; - // |ResourceBinder| bool BindResource(ShaderStage stage, const SampledImageSlot& slot, @@ -83,7 +71,9 @@ struct ComputeCommand : public ResourceBinder { const std::shared_ptr& texture, const std::shared_ptr& sampler) override; - constexpr operator bool() const { return pipeline && pipeline->IsValid(); } + constexpr explicit operator bool() const { + return pipeline && pipeline->IsValid(); + } }; } // namespace impeller From 73672dbf0881f38d4cf784838332eae547e0d4a0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 22 Aug 2023 20:47:12 -0700 Subject: [PATCH 2/3] update gles --- .../backend/gles/buffer_bindings_gles.cc | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/impeller/renderer/backend/gles/buffer_bindings_gles.cc index c72c34f807aeb..b373271cbd69a 100644 --- a/impeller/renderer/backend/gles/buffer_bindings_gles.cc +++ b/impeller/renderer/backend/gles/buffer_bindings_gles.cc @@ -305,15 +305,15 @@ bool BufferBindingsGLES::BindTextures(const ProcTableGLES& gl, const Bindings& bindings, ShaderStage stage) const { size_t active_index = 0; - for (const auto& texture : bindings.textures) { - const auto& texture_gles = TextureGLES::Cast(*texture.second.resource); - if (texture.second.GetMetadata() == nullptr) { + for (const auto& data : bindings.sampled_images) { + const auto& texture_gles = TextureGLES::Cast(*data.second.texture.resource); + if (data.second.texture.GetMetadata() == nullptr) { VALIDATION_LOG << "No metadata found for texture binding."; return false; } const auto uniform_key = - CreateUniformMemberKey(texture.second.GetMetadata()->name); + CreateUniformMemberKey(data.second.texture.GetMetadata()->name); auto uniform = uniform_locations_.find(uniform_key); if (uniform == uniform_locations_.end()) { VALIDATION_LOG << "Could not find uniform for key: " << uniform_key; @@ -341,12 +341,9 @@ bool BufferBindingsGLES::BindTextures(const ProcTableGLES& gl, /// If there is a sampler for the texture at the same index, configure the /// bound texture using that sampler. /// - auto sampler = bindings.samplers.find(texture.first); - if (sampler != bindings.samplers.end()) { - const auto& sampler_gles = SamplerGLES::Cast(*sampler->second.resource); - if (!sampler_gles.ConfigureBoundTexture(texture_gles, gl)) { - return false; - } + const auto& sampler_gles = SamplerGLES::Cast(*data.second.sampler.resource); + if (!sampler_gles.ConfigureBoundTexture(texture_gles, gl)) { + return false; } //-------------------------------------------------------------------------- From 7f553e12a6175008d3b27fd17134780397851a7e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 22 Aug 2023 20:59:30 -0700 Subject: [PATCH 3/3] bind fragment stage to fragment bindings --- impeller/renderer/command.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/command.cc b/impeller/renderer/command.cc index 68d835eefcac1..091906cea0733 100644 --- a/impeller/renderer/command.cc +++ b/impeller/renderer/command.cc @@ -104,7 +104,7 @@ bool Command::BindResource(ShaderStage stage, }; return true; case ShaderStage::kFragment: - vertex_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{ + fragment_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{ .slot = slot, .texture = {&metadata, texture}, .sampler = {&metadata, sampler},