Fix vkscript buffers being added twice

src/CMakeLists.txt

@@ -180,7 +180,9 @@ if (${AMBER_ENABLE_TESTS})
   )
 
   if (${Vulkan_FOUND})
-    list(APPEND TEST_SRCS vulkan/vertex_buffer_test.cc)
+    list(APPEND TEST_SRCS
+            vulkan/vertex_buffer_test.cc
+            vulkan/pipeline_test.cc)
   endif()
 
   if (${Dawn_FOUND})

src/vulkan/engine_vulkan.cc

@@ -652,13 +652,17 @@ Result EngineVulkan::DoBuffer(const BufferCommand* cmd) {
         "Vulkan::DoBuffer exceed maxBoundDescriptorSets limit of physical "
         "device");
   }
-  auto& info = pipeline_map_[cmd->GetPipeline()];
   if (cmd->GetValues().empty()) {
     cmd->GetBuffer()->SetSizeInElements(cmd->GetBuffer()->ElementCount());
   } else {
     cmd->GetBuffer()->SetDataWithOffset(cmd->GetValues(), cmd->GetOffset());
   }
-  return info.vk_pipeline->AddBufferDescriptor(cmd);
+  if (cmd->IsPushConstant()) {
+    auto& info = pipeline_map_[cmd->GetPipeline()];
+    return info.vk_pipeline->AddPushConstantBuffer(cmd->GetBuffer(),
+                                                   cmd->GetOffset());
+  }
+  return {};
 }
 
 }  // namespace vulkan

src/vulkan/pipeline.cc

@@ -276,8 +276,6 @@ Result Pipeline::GetDescriptorSlot(uint32_t desc_set,
 Result Pipeline::AddBufferDescriptor(const BufferCommand* cmd) {
   if (cmd == nullptr)
     return Result("Pipeline::AddBufferDescriptor BufferCommand is nullptr");
-  if (cmd->IsPushConstant())
-    return AddPushConstantBuffer(cmd->GetBuffer(), cmd->GetOffset());
   if (!cmd->IsSSBO() && !cmd->IsUniform() && !cmd->IsStorageImage() &&
       !cmd->IsSampledImage() && !cmd->IsCombinedImageSampler() &&
       !cmd->IsUniformTexelBuffer() && !cmd->IsStorageTexelBuffer() &&
@@ -338,6 +336,12 @@ Result Pipeline::AddBufferDescriptor(const BufferCommand* cmd) {
           "Descriptors bound to the same binding needs to have matching "
           "descriptor types");
     }
+    // Check that the buffer is not added already.
+    const auto& buffers = desc->AsBufferBackedDescriptor()->GetAmberBuffers();
+    if (std::find(buffers.begin(), buffers.end(), cmd->GetBuffer()) !=
+        buffers.end()) {
+      return Result("Buffer has been added already");
+    }
     desc->AsBufferBackedDescriptor()->AddAmberBuffer(cmd->GetBuffer());
   }
 

src/vulkan/pipeline_test.cc

@@ -0,0 +1,53 @@
+// Copyright 2020 The Amber Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/vulkan/pipeline.h"
+
+#include "gtest/gtest.h"
+#include "src/pipeline.h"
+#include "src/vulkan/compute_pipeline.h"
+
+namespace amber {
+namespace vulkan {
+
+using VulkanPipelineTest = testing::Test;
+
+TEST_F(VulkanPipelineTest, AddBufferDescriptorAddPushConstant) {
+  amber::Pipeline amber_pipeline(PipelineType::kCompute);
+  std::vector<VkPipelineShaderStageCreateInfo> create_infos;
+  ComputePipeline pipeline(nullptr, 0, create_infos);
+
+  auto cmd = MakeUnique<BufferCommand>(BufferCommand::BufferType::kPushConstant,
+                                       &amber_pipeline);
+  // Push constant buffers should not be passed to AddBufferDescriptor().
+  Result r = pipeline.AddBufferDescriptor(cmd.get());
+  ASSERT_FALSE(r.IsSuccess());
+}
+
+TEST_F(VulkanPipelineTest, AddBufferDescriptorAddBufferTwice) {
+  amber::Pipeline amber_pipeline(PipelineType::kCompute);
+  std::vector<VkPipelineShaderStageCreateInfo> create_infos;
+  ComputePipeline pipeline(nullptr, 0, create_infos);
+
+  auto cmd = MakeUnique<BufferCommand>(BufferCommand::BufferType::kUniform,
+                                       &amber_pipeline);
+  Result r = pipeline.AddBufferDescriptor(cmd.get());
+  ASSERT_TRUE(r.IsSuccess()) << r.Error();
+  // Adding same buffer again should fail.
+  r = pipeline.AddBufferDescriptor(cmd.get());
+  ASSERT_FALSE(r.IsSuccess());
+}
+
+}  // namespace vulkan
+}  // namespace amber

tests/cases/graphics_uniform_buffer.vkscript

@@ -0,0 +1,35 @@
+# Copyright 2019 The Amber Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+[vertex shader passthrough]
+[fragment shader]
+#version 430
+
+layout(set = 0, binding = 0) uniform Uniform {
+  float uniform_value;
+};
+
+layout(location = 0) out vec4 outColor;
+
+void main() {
+    outColor = vec4(uniform_value, 0.0, 0.0, uniform_value);
+}
+
+[test]
+uniform ubo 0:0 float 0 1.0
+
+clear
+draw rect -1 -1 2 2
+
+probe rect rgba (0, 0, 250, 250) (1.0 0.0 0.0 1.0)

tests/run_tests.py

@@ -333,7 +333,7 @@ def Run(self):
       print("--test-prog-path must point to an executable")
       return 1
 
-    input_file_re = re.compile('^.+[\.]amber|vkscript')
+    input_file_re = re.compile('^.+[\.](amber|vkscript)')
     self.test_cases = []
 
     if self.args: