Added combined image sampler support

Android.mk

@@ -64,6 +64,7 @@ LOCAL_SRC_FILES:= \
     src/vulkan/pipeline.cc \
     src/vulkan/push_constant.cc \
     src/vulkan/resource.cc \
+    src/vulkan/sampler.cc \
     src/vulkan/sampler_descriptor.cc \
     src/vulkan/transfer_buffer.cc \
     src/vulkan/transfer_image.cc \

docs/amber_script.md

@@ -262,14 +262,16 @@ contain image attachment content, depth/stencil content, uniform buffers, etc.
   BIND BUFFER {buffer_name} AS {buffer_type} DESCRIPTOR_SET _id_ \
        BINDING _id_
 
-  # Attach |buffer_name| as a storage image. The provided buffer must
-  # be a `FORMAT` buffer.
+  # Attach |buffer_name| as a storage image.
   BIND BUFFER {buffer_name} AS storage_image
 
-  # Attach |buffer_name| as a sampled image. The provided buffer must
-  # be a `FORMAT` buffer.
+  # Attach |buffer_name| as a sampled image.
   BIND BUFFER {buffer_name} AS sampled_image
 
+  # Attach |buffer_name| as a combined image sampler. 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/CMakeLists.txt

@@ -28,6 +28,7 @@ set(VULKAN_ENGINE_SOURCES
     pipeline.cc
     push_constant.cc
     resource.cc
+    sampler.cc
     sampler_descriptor.cc
     transfer_buffer.cc
     transfer_image.cc

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

@@ -24,7 +24,8 @@ ImageDescriptor::ImageDescriptor(Buffer* buffer,
                                  Device* device,
                                  uint32_t desc_set,
                                  uint32_t binding)
-    : BufferBackedDescriptor(buffer, type, device, desc_set, binding) {}
+    : BufferBackedDescriptor(buffer, type, device, desc_set, binding),
+      vulkan_sampler_(device) {}
 
 ImageDescriptor::~ImageDescriptor() = default;
 
@@ -78,6 +79,12 @@ Result ImageDescriptor::CreateResourceIfNeeded() {
   if (!r.IsSuccess())
     return r;
 
+  if (amber_sampler_) {
+    r = vulkan_sampler_.CreateSampler(amber_sampler_);
+    if (!r.IsSuccess())
+      return r;
+  }
+
   is_descriptor_set_update_needed_ = true;
   return {};
 }
@@ -108,9 +115,9 @@ void ImageDescriptor::UpdateDescriptorSetIfNeeded(
   if (type_ == DescriptorType::kStorageImage)
     layout = VK_IMAGE_LAYOUT_GENERAL;
 
-  VkDescriptorImageInfo image_info = {
-      VK_NULL_HANDLE,  // TODO(asuonpaa): Add sampler here later if used
-      transfer_image_->GetVkImageView(), layout};
+  VkDescriptorImageInfo image_info = {vulkan_sampler_.GetVkSampler(),
+                                      transfer_image_->GetVkImageView(),
+                                      layout};
 
   VkWriteDescriptorSet write = VkWriteDescriptorSet();
   write.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;

src/vulkan/image_descriptor.h

@@ -19,6 +19,7 @@
 #include <vector>
 
 #include "src/vulkan/buffer_backed_descriptor.h"
+#include "src/vulkan/sampler.h"
 #include "src/vulkan/transfer_image.h"
 
 namespace amber {
@@ -38,12 +39,15 @@ class ImageDescriptor : public BufferBackedDescriptor {
   Result CreateResourceIfNeeded() override;
   Result RecordCopyDataToHost(CommandBuffer* command) override;
   Result MoveResourceToBufferOutput() override;
+  void SetAmberSampler(amber::Sampler* sampler) { amber_sampler_ = sampler; }
 
  protected:
   Resource* GetResource() override { return transfer_image_.get(); }
 
  private:
   std::unique_ptr<TransferImage> transfer_image_;
+  amber::Sampler* amber_sampler_ = nullptr;
+  amber::vulkan::Sampler vulkan_sampler_;
 };
 
 }  // namespace vulkan