Added combined image sampler support

docs/amber_script.md

@@ -270,6 +270,11 @@ contain image attachment content, depth/stencil content, uniform buffers, etc.
   # be a `FORMAT` buffer.
   BIND BUFFER {buffer_name} AS sampled_image
 
+  # Attach |buffer_name| as a combined image image sampler. The provided
+  # buffer must be a `FORMAT` buffer. A sampler |sampler_name| must also
+  # be specified.
+  BIND BUFFER {buffer_name} AS combined_image_sampler SAMPLER {sampler_name}
+
   # Bind the sampler at the given descriptor set and binding.
   BIND SAMPLER {sampler_name} DESCRIPTOR_SET _id_ BINDING _id_
 

src/amberscript/parser.cc

@@ -676,6 +676,8 @@ Result Parser::ToBufferType(const std::string& name, BufferType* type) {
     *type = BufferType::kStorageImage;
   else if (name == "sampled_image")
     *type = BufferType::kSampledImage;
+  else if (name == "combined_image_sampler")
+    *type = BufferType::kCombinedImageSampler;
   else
     return Result("unknown buffer_type: " + name);
 
@@ -733,6 +735,22 @@ Result Parser::ParsePipelineBind(Pipeline* pipeline) {
         buffer->SetBufferType(BufferType::kStorageImage);
       } else if (token->AsString() == "sampled_image") {
         buffer->SetBufferType(BufferType::kSampledImage);
+      } else if (token->AsString() == "combined_image_sampler") {
+        buffer->SetBufferType(BufferType::kCombinedImageSampler);
+
+        token = tokenizer_->NextToken();
+        if (!token->IsString() || token->AsString() != "SAMPLER")
+          return Result("expecting SAMPLER for combined image sampler");
+
+        token = tokenizer_->NextToken();
+        if (!token->IsString())
+          return Result("missing sampler name in BIND command");
+
+        auto* sampler = script_->GetSampler(token->AsString());
+        if (!sampler)
+          return Result("unknown sampler: " + token->AsString());
+
+        buffer->SetSampler(sampler);
       } else {
         BufferType type = BufferType::kColor;
         Result r = ToBufferType(token->AsString(), &type);
@@ -750,7 +768,8 @@ Result Parser::ParsePipelineBind(Pipeline* pipeline) {
         buffer->GetBufferType() == BufferType::kStorage ||
         buffer->GetBufferType() == BufferType::kUniform ||
         buffer->GetBufferType() == BufferType::kStorageImage ||
-        buffer->GetBufferType() == BufferType::kSampledImage) {
+        buffer->GetBufferType() == BufferType::kSampledImage ||
+        buffer->GetBufferType() == BufferType::kCombinedImageSampler) {
       // If AS was parsed above consume the next token.
       if (buffer->GetBufferType() != BufferType::kUnknown)
         token = tokenizer_->NextToken();

src/amberscript/parser_bind_test.cc

@@ -1574,6 +1574,171 @@ END)";
   EXPECT_EQ("13: missing BINDING for BIND command", r.Error());
 }
 
+TEST_F(AmberScriptParserTest, BindBufferCombinedImageSampler) {
+  std::string in = R"(
+SHADER vertex vert_shader PASSTHROUGH
+SHADER fragment frag_shader GLSL
+# GLSL Shader
+END
+
+BUFFER texture FORMAT R8G8B8A8_UNORM
+BUFFER framebuffer FORMAT R8G8B8A8_UNORM
+SAMPLER sampler
+
+PIPELINE graphics pipeline
+  ATTACH vert_shader
+  ATTACH frag_shader
+  BIND BUFFER texture AS combined_image_sampler SAMPLER sampler DESCRIPTOR_SET 0 BINDING 0
+  BIND BUFFER framebuffer AS color LOCATION 0
+END)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_TRUE(r.IsSuccess()) << r.Error();
+
+  auto script = parser.GetScript();
+  const auto& pipelines = script->GetPipelines();
+  ASSERT_EQ(1U, pipelines.size());
+
+  const auto* pipeline = pipelines[0].get();
+  const auto& bufs = pipeline->GetBuffers();
+  ASSERT_EQ(1U, bufs.size());
+  EXPECT_EQ(BufferType::kCombinedImageSampler, bufs[0].buffer->GetBufferType());
+}
+
+TEST_F(AmberScriptParserTest,
+       BindBufferCombinedImageSamplerMissingDescriptorSetValue) {
+  std::string in = R"(
+SHADER vertex vert_shader PASSTHROUGH
+SHADER fragment frag_shader GLSL
+# GLSL Shader
+END
+
+BUFFER texture FORMAT R8G8B8A8_UNORM
+SAMPLER sampler
+
+PIPELINE graphics pipeline
+  ATTACH vert_shader
+  ATTACH frag_shader
+  BIND BUFFER texture AS combined_image_sampler SAMPLER sampler DESCRIPTOR_SET BINDING 0
+END)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_FALSE(r.IsSuccess());
+  EXPECT_EQ("13: invalid value for DESCRIPTOR_SET in BIND command", r.Error());
+}
+
+TEST_F(AmberScriptParserTest,
+       BindBufferCombinedImageSamplerMissingDescriptorSet) {
+  std::string in = R"(
+SHADER vertex vert_shader PASSTHROUGH
+SHADER fragment frag_shader GLSL
+# GLSL Shader
+END
+
+BUFFER texture FORMAT R8G8B8A8_UNORM
+SAMPLER sampler
+
+PIPELINE graphics pipeline
+  ATTACH vert_shader
+  ATTACH frag_shader
+  BIND BUFFER texture AS combined_image_sampler SAMPLER sampler BINDING 0
+END)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_FALSE(r.IsSuccess());
+  EXPECT_EQ("13: missing DESCRIPTOR_SET or KERNEL for BIND command", r.Error());
+}
+
+TEST_F(AmberScriptParserTest,
+       BindBufferCombinedImageSamplerMissingBindingValue) {
+  std::string in = R"(
+SHADER vertex vert_shader PASSTHROUGH
+SHADER fragment frag_shader GLSL
+# GLSL Shader
+END
+
+BUFFER texture FORMAT R8G8B8A8_UNORM
+SAMPLER sampler
+
+PIPELINE graphics pipeline
+  ATTACH vert_shader
+  ATTACH frag_shader
+  BIND BUFFER texture AS combined_image_sampler SAMPLER sampler DESCRIPTOR_SET 0 BINDING
+END)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_FALSE(r.IsSuccess());
+  EXPECT_EQ("14: invalid value for BINDING in BIND command", r.Error());
+}
+
+TEST_F(AmberScriptParserTest, BindBufferCombinedImageSamplerMissingBinding) {
+  std::string in = R"(
+SHADER vertex vert_shader PASSTHROUGH
+SHADER fragment frag_shader GLSL
+# GLSL Shader
+END
+
+BUFFER texture FORMAT R8G8B8A8_UNORM
+SAMPLER sampler
+
+PIPELINE graphics pipeline
+  ATTACH vert_shader
+  ATTACH frag_shader
+  BIND BUFFER texture AS combined_image_sampler SAMPLER sampler DESCRIPTOR_SET 0
+END)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_FALSE(r.IsSuccess());
+  EXPECT_EQ("14: missing BINDING for BIND command", r.Error());
+}
+
+TEST_F(AmberScriptParserTest, BindBufferCombinedImageSamplerMissingSampler) {
+  std::string in = R"(
+SHADER vertex vert_shader PASSTHROUGH
+SHADER fragment frag_shader GLSL
+# GLSL Shader
+END
+
+BUFFER texture FORMAT R8G8B8A8_UNORM
+
+PIPELINE graphics pipeline
+  ATTACH vert_shader
+  ATTACH frag_shader
+  BIND BUFFER texture AS combined_image_sampler DESCRIPTOR_SET 0 BINDING 0
+END)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_FALSE(r.IsSuccess());
+  EXPECT_EQ("12: expecting SAMPLER for combined image sampler", r.Error());
+}
+
+TEST_F(AmberScriptParserTest, BindBufferCombinedImageSamplerUnknownSampler) {
+  std::string in = R"(
+SHADER vertex vert_shader PASSTHROUGH
+SHADER fragment frag_shader GLSL
+# GLSL Shader
+END
+
+BUFFER texture FORMAT R8G8B8A8_UNORM
+
+PIPELINE graphics pipeline
+  ATTACH vert_shader
+  ATTACH frag_shader
+  BIND BUFFER texture AS combined_image_sampler SAMPLER foo DESCRIPTOR_SET 0 BINDING 0
+END)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_FALSE(r.IsSuccess());
+  EXPECT_EQ("12: unknown sampler: foo", r.Error());
+}
+
 TEST_F(AmberScriptParserTest, BindSampler) {
   std::string in = R"(
 SHADER vertex vert_shader PASSTHROUGH

src/buffer.h

@@ -27,6 +27,8 @@
 
 namespace amber {
 
+class Sampler;
+
 /// Types of buffers which can be created.
 enum class BufferType : int8_t {
   /// Unknown buffer type
@@ -39,6 +41,8 @@ enum class BufferType : int8_t {
   kIndex,
   /// A sampled image.
   kSampledImage,
+  /// A combined image sampler.
+  kCombinedImageSampler,
   /// A storage buffer.
   kStorage,
   /// A uniform buffer.
@@ -75,6 +79,11 @@ class Buffer {
   /// Returns the Format describing the buffer data.
   Format* GetFormat() const { return format_; }
 
+  /// Sets the sampler used with buffer of combined image sampler type.
+  void SetSampler(Sampler* sampler) { sampler_ = sampler; }
+  /// Returns the sampler of combined image sampler buffer.
+  Sampler* GetSampler() const { return sampler_; }
+
   void SetFormatIsDefault(bool val) { format_is_default_ = val; }
   bool FormatIsDefault() const { return format_is_default_; }
 
@@ -230,6 +239,7 @@ class Buffer {
   bool format_is_default_ = false;
   std::vector<uint8_t> bytes_;
   Format* format_ = nullptr;
+  Sampler* sampler_ = nullptr;
 };
 
 }  // namespace amber

src/command.h

@@ -440,7 +440,8 @@ class BufferCommand : public BindableResourceCommand {
     kUniform,
     kPushConstant,
     kStorageImage,
-    kSampledImage
+    kSampledImage,
+    kCombinedImageSampler
   };
 
   BufferCommand(BufferType type, Pipeline* pipeline);
@@ -454,6 +455,9 @@ class BufferCommand : public BindableResourceCommand {
   bool IsSampledImage() const {
     return buffer_type_ == BufferType::kSampledImage;
   }
+  bool IsCombinedImageSampler() const {
+    return buffer_type_ == BufferType::kCombinedImageSampler;
+  }
   bool IsPushConstant() const {
     return buffer_type_ == BufferType::kPushConstant;
   }

src/vulkan/device.cc

@@ -480,6 +480,7 @@ bool Device::IsFormatSupportedByPhysicalDevice(const Format& format,
       is_buffer_type_image = true;
       break;
     case BufferType::kSampledImage:
+    case BufferType::kCombinedImageSampler:
       flag = VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT;
       is_buffer_type_image = true;
       break;

src/vulkan/engine_vulkan.cc

@@ -234,6 +234,8 @@ Result EngineVulkan::CreatePipeline(amber::Pipeline* pipeline) {
       type = BufferCommand::BufferType::kStorageImage;
     } else if (buf_info.type == BufferType::kSampledImage) {
       type = BufferCommand::BufferType::kSampledImage;
+    } else if (buf_info.type == BufferType::kCombinedImageSampler) {
+      type = BufferCommand::BufferType::kCombinedImageSampler;
     } else if (buf_info.type == BufferType::kUniform) {
       type = BufferCommand::BufferType::kUniform;
     } else if (buf_info.type != BufferType::kStorage) {

src/vulkan/image_descriptor.cc

@@ -15,6 +15,7 @@
 #include "src/vulkan/image_descriptor.h"
 #include "src/vulkan/device.h"
 #include "src/vulkan/resource.h"
+#include "src/vulkan/sampler_descriptor.h"
 
 namespace amber {
 namespace vulkan {
@@ -23,10 +24,17 @@ ImageDescriptor::ImageDescriptor(Buffer* buffer,
                                  DescriptorType type,
                                  Device* device,
                                  uint32_t desc_set,
-                                 uint32_t binding)
-    : BufferBackedDescriptor(buffer, type, device, desc_set, binding) {}
-
-ImageDescriptor::~ImageDescriptor() = default;
+                                 uint32_t binding,
+                                 Sampler* sampler)
+    : BufferBackedDescriptor(buffer, type, device, desc_set, binding),
+      amber_sampler_(sampler) {}
+
+ImageDescriptor::~ImageDescriptor() {
+  if (sampler_ != VK_NULL_HANDLE) {
+    device_->GetPtrs()->vkDestroySampler(device_->GetVkDevice(), sampler_,
+                                         nullptr);
+  }
+}
 
 void ImageDescriptor::RecordCopyDataToResourceIfNeeded(CommandBuffer* command) {
   transfer_image_->ImageBarrier(command, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
@@ -78,6 +86,16 @@ Result ImageDescriptor::CreateResourceIfNeeded() {
   if (!r.IsSuccess())
     return r;
 
+  if (amber_sampler_) {
+    VkSamplerCreateInfo sampler_info = GetSamplerCreateInfo(amber_sampler_);
+
+    if (device_->GetPtrs()->vkCreateSampler(device_->GetVkDevice(),
+                                            &sampler_info, nullptr,
+                                            &sampler_) != VK_SUCCESS) {
+      return Result("Vulkan::Calling vkCreateSampler Fail");
+    }
+  }
+
   is_descriptor_set_update_needed_ = true;
   return {};
 }
@@ -109,8 +127,7 @@ void ImageDescriptor::UpdateDescriptorSetIfNeeded(
     layout = VK_IMAGE_LAYOUT_GENERAL;
 
   VkDescriptorImageInfo image_info = {
-      VK_NULL_HANDLE,  // TODO(asuonpaa): Add sampler here later if used
-      transfer_image_->GetVkImageView(), layout};
+      sampler_, transfer_image_->GetVkImageView(), layout};
 
   VkWriteDescriptorSet write = VkWriteDescriptorSet();
   write.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;

src/vulkan/image_descriptor.h

@@ -30,7 +30,8 @@ class ImageDescriptor : public BufferBackedDescriptor {
                   DescriptorType type,
                   Device* device,
                   uint32_t desc_set,
-                  uint32_t binding);
+                  uint32_t binding,
+                  Sampler* sampler = nullptr);
   ~ImageDescriptor() override;
 
   void UpdateDescriptorSetIfNeeded(VkDescriptorSet descriptor_set) override;
@@ -44,6 +45,8 @@ class ImageDescriptor : public BufferBackedDescriptor {
 
  private:
   std::unique_ptr<TransferImage> transfer_image_;
+  Sampler* amber_sampler_;
+  VkSampler sampler_ = VK_NULL_HANDLE;
 };
 
 }  // namespace vulkan

src/vulkan/pipeline.cc

@@ -279,7 +279,7 @@ Result Pipeline::AddBufferDescriptor(const BufferCommand* cmd) {
   if (cmd->IsPushConstant())
     return AddPushConstantBuffer(cmd->GetBuffer(), cmd->GetOffset());
   if (!cmd->IsSSBO() && !cmd->IsUniform() && !cmd->IsStorageImage() &&
-      !cmd->IsSampledImage())
+      !cmd->IsSampledImage() && !cmd->IsCombinedImageSampler())
     return Result("Pipeline::AddBufferDescriptor not supported buffer type");
 
   Descriptor* desc;
@@ -301,6 +301,12 @@ Result Pipeline::AddBufferDescriptor(const BufferCommand* cmd) {
           cmd->GetBuffer(), DescriptorType::kSampledImage, device_,
           cmd->GetDescriptorSet(), cmd->GetBinding());
       descriptors.push_back(std::move(image_desc));
+    } else if (cmd->IsCombinedImageSampler()) {
+      auto image_desc = MakeUnique<ImageDescriptor>(
+          cmd->GetBuffer(), DescriptorType::kCombinedImageSampler, device_,
+          cmd->GetDescriptorSet(), cmd->GetBinding(),
+          cmd->GetBuffer()->GetSampler());
+      descriptors.push_back(std::move(image_desc));
     } else {
       auto desc_type = cmd->IsSSBO() ? DescriptorType::kStorageBuffer
                                      : DescriptorType::kUniformBuffer;