[impeller] combine sampler and texture maps. (flutter#44990)

Simplify command encoding and reduce binding size by placing all texture/sampler data in a single map instead of 3. We don't currently (nor do we plan to) support separate textures and samplers. The vulkan backend is particularly bad, because there are 3 map lookups to pull all of the texture and sampler data out of the bindings. Also see [go/impeller-vulkan-cmd-recording-performance](http://goto.google.com/impeller-vulkan-cmd-recording-performance)
izouxv · Aug 23, 2023 · 5a4bbd0 · 5a4bbd0
1 parent f9264f0
commit 5a4bbd0
Show file tree

Hide file tree

Showing 10 changed files with 72 additions and 192 deletions.
diff --git 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<const Texture>& texture) = 0;
-
-  virtual bool BindResource(ShaderStage stage,
-                            const SampledImageSlot& slot,
-                            const ShaderMetadata& metadata,
-                            const std::shared_ptr<const Sampler>& sampler) = 0;
-
   virtual bool BindResource(ShaderStage stage,
                             const SampledImageSlot& slot,
                             const ShaderMetadata& metadata,

diff --git 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;
     }
 
     //--------------------------------------------------------------------------

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -83,53 +83,32 @@ bool Command::DoBindResource(ShaderStage stage,
 bool Command::BindResource(ShaderStage stage,
                            const SampledImageSlot& slot,
                            const ShaderMetadata& metadata,
-                           const std::shared_ptr<const Texture>& 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<const Texture>& texture,
                            const std::shared_ptr<const Sampler>& 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;
+      fragment_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<const Texture>& texture,
-                           const std::shared_ptr<const Sampler>& 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
@@ -61,12 +61,17 @@ using BufferResource = Resource<BufferView>;
 using TextureResource = Resource<std::shared_ptr<const Texture>>;
 using SamplerResource = Resource<std::shared_ptr<const Sampler>>;
 
+/// @brief combines the texture, sampler and sampler slot information.
+struct TextureAndSampler {
+  SampledImageSlot slot;
+  TextureResource texture;
+  SamplerResource sampler;
+};
+
 struct Bindings {
   std::map<size_t, ShaderUniformSlot> uniforms;
-  std::map<size_t, SampledImageSlot> sampled_images;
+  std::map<size_t, TextureAndSampler> sampled_images;
   std::map<size_t, BufferResource> buffers;
-  std::map<size_t, TextureResource> textures;
-  std::map<size_t, SamplerResource> samplers;
 };
 
 //------------------------------------------------------------------------------
@@ -184,18 +189,6 @@ struct Command : public ResourceBinder {
                     const std::shared_ptr<const ShaderMetadata>& metadata,
                     const BufferView& view);
 
-  // |ResourceBinder|
-  bool BindResource(ShaderStage stage,
-                    const SampledImageSlot& slot,
-                    const ShaderMetadata& metadata,
-                    const std::shared_ptr<const Texture>& texture) override;
-
-  // |ResourceBinder|
-  bool BindResource(ShaderStage stage,
-                    const SampledImageSlot& slot,
-                    const ShaderMetadata& metadata,
-                    const std::shared_ptr<const Sampler>& 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 <class T>