Differentiate between instance and device extensions. (#304)

This CL adds the necessary code to differentiate that
VK_KHR_get_physical_device_properties2 is an instance extension and not
a device extension.

12 files changed, 123 insertions, 47 deletions

diff --git a/src/amber.cc b/src/amber.cc
index ddebe93..48bfb87 100644
--- a/src/amber.cc
+++ b/src/amber.cc
@@ -73,7 +73,8 @@ amber::Result Amber::ExecuteWithShaderData(const amber::Recipe* recipe,
     return Result("Failed to create engine");
 
   Result r = engine->Initialize(opts->config, script->RequiredFeatures(),
-                                script->RequiredExtensions());
+                                script->GetRequiredInstanceExtensions(),
+                                script->GetRequiredDeviceExtensions());
   if (!r.IsSuccess())
     return r;
 
diff --git a/src/dawn/engine_dawn.cc b/src/dawn/engine_dawn.cc
index d388870..dde2e49 100644
--- a/src/dawn/engine_dawn.cc
+++ b/src/dawn/engine_dawn.cc
@@ -183,6 +183,7 @@ EngineDawn::~EngineDawn() = default;
 
 Result EngineDawn::Initialize(EngineConfig* config,
                               const std::vector<Feature>&,
+                              const std::vector<std::string>&,
                               const std::vector<std::string>&) {
   if (device_)
     return Result("Dawn:Initialize device_ already exists");
diff --git a/src/dawn/engine_dawn.h b/src/dawn/engine_dawn.h
index 47c46fe..26e2b1b 100644
--- a/src/dawn/engine_dawn.h
+++ b/src/dawn/engine_dawn.h
@@ -38,7 +38,8 @@ class EngineDawn : public Engine {
   // Initialize with given configuration data.
   Result Initialize(EngineConfig* config,
                     const std::vector<Feature>& features,
-                    const std::vector<std::string>& extensions) override;
+                    const std::vector<std::string>& instance_extensions,
+                    const std::vector<std::string>& device_extensions) override;
 
   Result Shutdown() override;
   // Record info for a pipeline.  The Dawn render pipeline will be created
diff --git a/src/engine.h b/src/engine.h
index 98e972f..c21bff7 100644
--- a/src/engine.h
+++ b/src/engine.h
@@ -84,9 +84,11 @@ class Engine {
   /// are for validation purposes only. If possible the engine should verify
   /// that the constraints in |features| and |extensions| are valid and fail
   /// otherwise.
-  virtual Result Initialize(EngineConfig* config,
-                            const std::vector<Feature>& features,
-                            const std::vector<std::string>& extensions) = 0;
+  virtual Result Initialize(
+      EngineConfig* config,
+      const std::vector<Feature>& features,
+      const std::vector<std::string>& instance_extensions,
+      const std::vector<std::string>& device_extensions) = 0;
 
   /// Shutdown the engine and cleanup any resources.
   virtual Result Shutdown() = 0;
diff --git a/src/executor_test.cc b/src/executor_test.cc
index d6e91bc..2c1126d 100644
--- a/src/executor_test.cc
+++ b/src/executor_test.cc
@@ -35,16 +35,20 @@ class EngineStub : public Engine {
   // Engine
   Result Initialize(EngineConfig*,
                     const std::vector<Feature>& features,
-                    const std::vector<std::string>& extensions) override {
+                    const std::vector<std::string>& instance_exts,
+                    const std::vector<std::string>& device_exts) override {
     features_ = features;
-    extensions_ = extensions;
+    instance_extensions_ = instance_exts;
+    device_extensions_ = device_exts;
     return {};
   }
 
   Result Shutdown() override { return {}; }
 
   const std::vector<Feature>& GetFeatures() const { return features_; }
-  const std::vector<std::string>& GetExtensions() const { return extensions_; }
+  const std::vector<std::string>& GetDeviceExtensions() const {
+    return device_extensions_;
+  }
   uint32_t GetFenceTimeoutMs() { return GetEngineData().fence_timeout_ms; }
 
   Result CreatePipeline(Pipeline*) override { return {}; }
@@ -188,7 +192,8 @@ class EngineStub : public Engine {
   bool did_buffer_command_ = false;
 
   std::vector<Feature> features_;
-  std::vector<std::string> extensions_;
+  std::vector<std::string> instance_extensions_;
+  std::vector<std::string> device_extensions_;
 
   ClearColorCommand* last_clear_color_ = nullptr;
 };
@@ -201,9 +206,11 @@ class VkScriptExecutorTest : public testing::Test {
   std::unique_ptr<Engine> MakeEngine() { return MakeUnique<EngineStub>(); }
   std::unique_ptr<Engine> MakeAndInitializeEngine(
       const std::vector<Feature>& features,
-      const std::vector<std::string>& extensions) {
+      const std::vector<std::string>& instance_extensions,
+      const std::vector<std::string>& device_extensions) {
     auto engine = MakeUnique<EngineStub>();
-    engine->Initialize(nullptr, features, extensions);
+    engine->Initialize(nullptr, features, instance_extensions,
+                       device_extensions);
     return std::move(engine);
   }
   EngineStub* ToStub(Engine* engine) {
@@ -224,7 +231,8 @@ logicOp)";
 
   auto script = parser.GetScript();
   auto engine = MakeAndInitializeEngine(script->RequiredFeatures(),
-                                        script->RequiredExtensions());
+                                        script->GetRequiredInstanceExtensions(),
+                                        script->GetRequiredDeviceExtensions());
 
   Executor ex;
   Result r = ex.Execute(engine.get(), script.get(), ShaderMap(),
@@ -236,7 +244,7 @@ logicOp)";
   EXPECT_EQ(Feature::kRobustBufferAccess, features[0]);
   EXPECT_EQ(Feature::kLogicOp, features[1]);
 
-  const auto& extensions = ToStub(engine.get())->GetExtensions();
+  const auto& extensions = ToStub(engine.get())->GetDeviceExtensions();
   ASSERT_EQ(static_cast<size_t>(0U), extensions.size());
 
   EXPECT_EQ(100U, ToStub(engine.get())->GetFenceTimeoutMs());
@@ -253,7 +261,8 @@ VK_KHR_variable_pointers)";
 
   auto script = parser.GetScript();
   auto engine = MakeAndInitializeEngine(script->RequiredFeatures(),
-                                        script->RequiredExtensions());
+                                        script->GetRequiredInstanceExtensions(),
+                                        script->GetRequiredDeviceExtensions());
 
   Executor ex;
   Result r = ex.Execute(engine.get(), script.get(), ShaderMap(),
@@ -263,7 +272,7 @@ VK_KHR_variable_pointers)";
   const auto& features = ToStub(engine.get())->GetFeatures();
   ASSERT_EQ(static_cast<size_t>(0U), features.size());
 
-  const auto& extensions = ToStub(engine.get())->GetExtensions();
+  const auto& extensions = ToStub(engine.get())->GetDeviceExtensions();
   ASSERT_EQ(2U, extensions.size());
   EXPECT_EQ("VK_KHR_storage_buffer_storage_class", extensions[0]);
   EXPECT_EQ("VK_KHR_variable_pointers", extensions[1]);
@@ -282,7 +291,8 @@ depthstencil D24_UNORM_S8_UINT)";
 
   auto script = parser.GetScript();
   auto engine = MakeAndInitializeEngine(script->RequiredFeatures(),
-                                        script->RequiredExtensions());
+                                        script->GetRequiredInstanceExtensions(),
+                                        script->GetRequiredDeviceExtensions());
 
   Executor ex;
   Result r = ex.Execute(engine.get(), script.get(), ShaderMap(),
@@ -292,7 +302,7 @@ depthstencil D24_UNORM_S8_UINT)";
   const auto& features = ToStub(engine.get())->GetFeatures();
   ASSERT_EQ(static_cast<size_t>(0U), features.size());
 
-  const auto& extensions = ToStub(engine.get())->GetExtensions();
+  const auto& extensions = ToStub(engine.get())->GetDeviceExtensions();
   ASSERT_EQ(static_cast<size_t>(0U), extensions.size());
 
   EXPECT_EQ(100U, ToStub(engine.get())->GetFenceTimeoutMs());
@@ -308,7 +318,8 @@ fence_timeout 12345)";
 
   auto script = parser.GetScript();
   auto engine = MakeAndInitializeEngine(script->RequiredFeatures(),
-                                        script->RequiredExtensions());
+                                        script->GetRequiredInstanceExtensions(),
+                                        script->GetRequiredDeviceExtensions());
 
   Executor ex;
   Result r = ex.Execute(engine.get(), script.get(), ShaderMap(),
@@ -318,7 +329,7 @@ fence_timeout 12345)";
   const auto& features = ToStub(engine.get())->GetFeatures();
   ASSERT_EQ(static_cast<size_t>(0U), features.size());
 
-  const auto& extensions = ToStub(engine.get())->GetExtensions();
+  const auto& extensions = ToStub(engine.get())->GetDeviceExtensions();
   ASSERT_EQ(static_cast<size_t>(0U), extensions.size());
 
   EXPECT_EQ(12345U, ToStub(engine.get())->GetFenceTimeoutMs());
@@ -340,7 +351,8 @@ fence_timeout 12345)";
 
   auto script = parser.GetScript();
   auto engine = MakeAndInitializeEngine(script->RequiredFeatures(),
-                                        script->RequiredExtensions());
+                                        script->GetRequiredInstanceExtensions(),
+                                        script->GetRequiredDeviceExtensions());
 
   Executor ex;
   Result r = ex.Execute(engine.get(), script.get(), ShaderMap(),
@@ -352,7 +364,7 @@ fence_timeout 12345)";
   EXPECT_EQ(Feature::kRobustBufferAccess, features[0]);
   EXPECT_EQ(Feature::kLogicOp, features[1]);
 
-  const auto& extensions = ToStub(engine.get())->GetExtensions();
+  const auto& extensions = ToStub(engine.get())->GetDeviceExtensions();
   ASSERT_EQ(2U, extensions.size());
   EXPECT_EQ("VK_KHR_storage_buffer_storage_class", extensions[0]);
   EXPECT_EQ("VK_KHR_variable_pointers", extensions[1]);
diff --git a/src/recipe.cc b/src/recipe.cc
index 973ea33..bf7182a 100644
--- a/src/recipe.cc
+++ b/src/recipe.cc
@@ -35,8 +35,14 @@ std::vector<std::string> Recipe::GetRequiredFeatures() const {
   return impl_ ? impl_->GetRequiredFeatures() : std::vector<std::string>();
 }
 
-std::vector<std::string> Recipe::GetRequiredExtensions() const {
-  return impl_ ? impl_->GetRequiredExtensions() : std::vector<std::string>();
+std::vector<std::string> Recipe::GetRequiredDeviceExtensions() const {
+  return impl_ ? impl_->GetRequiredDeviceExtensions()
+               : std::vector<std::string>();
+}
+
+std::vector<std::string> Recipe::GetRequiredInstanceExtensions() const {
+  return impl_ ? impl_->GetRequiredInstanceExtensions()
+               : std::vector<std::string>();
 }
 
 }  // namespace amber
diff --git a/src/script.cc b/src/script.cc
index ce9f3aa..5d86538 100644
--- a/src/script.cc
+++ b/src/script.cc
@@ -165,8 +165,12 @@ std::vector<std::string> Script::GetRequiredFeatures() const {
   return required_features_in_string;
 }
 
-std::vector<std::string> Script::GetRequiredExtensions() const {
-  return engine_info_.required_extensions;
+void Script::AddRequiredExtension(const std::string& ext) {
+  // Make this smarter when we have more instance extensions to match.
+  if (ext == "VK_KHR_get_physical_device_properties2")
+    engine_info_.required_instance_extensions.push_back(ext);
+  else
+    engine_info_.required_device_extensions.push_back(ext);
 }
 
 }  // namespace amber
diff --git a/src/script.h b/src/script.h
index ccf7f8f..6940471 100644
--- a/src/script.h
+++ b/src/script.h
@@ -45,8 +45,15 @@ class Script : public RecipeImpl {
   /// Returns required features in the given recipe.
   std::vector<std::string> GetRequiredFeatures() const override;
 
-  /// Returns required extensions in the given recipe.
-  std::vector<std::string> GetRequiredExtensions() const override;
+  /// Returns required device extensions in the given recipe.
+  std::vector<std::string> GetRequiredDeviceExtensions() const override {
+    return engine_info_.required_device_extensions;
+  }
+
+  /// Returns required instance extensions in the given recipe.
+  std::vector<std::string> GetRequiredInstanceExtensions() const override {
+    return engine_info_.required_instance_extensions;
+  }
 
   /// Adds |pipeline| to the list of known pipelines. The |pipeline| must have
   /// a unique name over all pipelines in the script.
@@ -126,14 +133,7 @@ class Script : public RecipeImpl {
   }
 
   /// Adds |ext| to the list of extensions that must be supported by the engine.
-  void AddRequiredExtension(const std::string& ext) {
-    engine_info_.required_extensions.push_back(ext);
-  }
-
-  /// Retrieves a list of extensions required for this script.
-  const std::vector<std::string>& RequiredExtensions() const {
-    return engine_info_.required_extensions;
-  }
+  void AddRequiredExtension(const std::string& ext);
 
   /// Retrieves the engine configuration data for this script.
   EngineData& GetEngineData() { return engine_data_; }
@@ -158,7 +158,8 @@ class Script : public RecipeImpl {
  private:
   struct {
     std::vector<Feature> required_features;
-    std::vector<std::string> required_extensions;
+    std::vector<std::string> required_device_extensions;
+    std::vector<std::string> required_instance_extensions;
   } engine_info_;
 
   EngineData engine_data_;
diff --git a/src/script_test.cc b/src/script_test.cc
index f54057a..07388f5 100644
--- a/src/script_test.cc
+++ b/src/script_test.cc
@@ -288,4 +288,22 @@ TEST_F(ScriptTest, GetBuffers) {
   EXPECT_EQ(ptr2, buffers[1].get());
 }
 
+TEST_F(ScriptTest, IdentifiesDeviceExtensions) {
+  Script s;
+  s.AddRequiredExtension("VK_KHR_16bit_storage");
+  EXPECT_TRUE(s.GetRequiredInstanceExtensions().empty());
+  ASSERT_EQ(1U, s.GetRequiredDeviceExtensions().size());
+  EXPECT_EQ("VK_KHR_16bit_storage", s.GetRequiredDeviceExtensions()[0]);
+}
+
+TEST_F(ScriptTest,
+       IdentifesInstanceExt_VK_KHR_get_physical_device_properties2) {
+  Script s;
+  s.AddRequiredExtension("VK_KHR_get_physical_device_properties2");
+  EXPECT_TRUE(s.GetRequiredDeviceExtensions().empty());
+  ASSERT_EQ(1U, s.GetRequiredInstanceExtensions().size());
+  EXPECT_EQ("VK_KHR_get_physical_device_properties2",
+            s.GetRequiredInstanceExtensions()[0]);
+}
+
 }  // namespace amber
diff --git a/src/vkscript/parser_test.cc b/src/vkscript/parser_test.cc
index f2f47be..80df2aa 100644
--- a/src/vkscript/parser_test.cc
+++ b/src/vkscript/parser_test.cc
@@ -113,17 +113,22 @@ TEST_F(VkScriptParserTest, RequireBlockNoArgumentFeatures) {
 TEST_F(VkScriptParserTest, RequireBlockExtensions) {
   std::string block = R"([require]
 VK_KHR_storage_buffer_storage_class
-VK_KHR_variable_pointers)";
+VK_KHR_variable_pointers
+VK_KHR_get_physical_device_properties2)";
 
   Parser parser;
   Result r = parser.Parse(block);
   ASSERT_TRUE(r.IsSuccess()) << r.Error();
 
   auto script = parser.GetScript();
-  auto& exts = script->RequiredExtensions();
-  ASSERT_EQ(2U, exts.size());
-  EXPECT_EQ("VK_KHR_storage_buffer_storage_class", exts[0]);
-  EXPECT_EQ("VK_KHR_variable_pointers", exts[1]);
+  auto device_exts = script->GetRequiredDeviceExtensions();
+  ASSERT_EQ(2U, device_exts.size());
+  EXPECT_EQ("VK_KHR_storage_buffer_storage_class", device_exts[0]);
+  EXPECT_EQ("VK_KHR_variable_pointers", device_exts[1]);
+
+  auto inst_exts = script->GetRequiredInstanceExtensions();
+  ASSERT_EQ(1U, inst_exts.size());
+  EXPECT_EQ("VK_KHR_get_physical_device_properties2", inst_exts[0]);
 }
 
 TEST_F(VkScriptParserTest, RequireBlockFramebuffer) {
diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc
index feba173..f27e22d 100644
--- a/src/vulkan/engine_vulkan.cc
+++ b/src/vulkan/engine_vulkan.cc
@@ -16,6 +16,7 @@
 
 #include <algorithm>
 #include <cassert>
+#include <set>
 
 #include "amber/amber_vulkan.h"
 #include "src/make_unique.h"
@@ -55,15 +56,32 @@ VkShaderStageFlagBits ToVkShaderStage(ShaderType type) {
   return VK_SHADER_STAGE_FRAGMENT_BIT;
 }
 
+bool AreAllExtensionsSupported(
+    const std::vector<std::string>& available_extensions,
+    const std::vector<std::string>& required_extensions) {
+  if (required_extensions.empty())
+    return true;
+
+  std::set<std::string> required_extension_set(required_extensions.begin(),
+                                               required_extensions.end());
+  for (const auto& extension : available_extensions) {
+    required_extension_set.erase(extension);
+  }
+
+  return required_extension_set.empty();
+}
+
 }  // namespace
 
 EngineVulkan::EngineVulkan() : Engine() {}
 
 EngineVulkan::~EngineVulkan() = default;
 
-Result EngineVulkan::Initialize(EngineConfig* config,
-                                const std::vector<Feature>& features,
-                                const std::vector<std::string>& extensions) {
+Result EngineVulkan::Initialize(
+    EngineConfig* config,
+    const std::vector<Feature>& features,
+    const std::vector<std::string>& instance_extensions,
+    const std::vector<std::string>& device_extensions) {
   if (device_)
     return Result("Vulkan::Initialize device_ already exists");
 
@@ -77,13 +95,19 @@ Result EngineVulkan::Initialize(EngineConfig* config,
   if (vk_config->queue == VK_NULL_HANDLE)
     return Result("Vulkan::Initialize queue handle is null.");
 
+  // Validate instance extensions
+  if (!AreAllExtensionsSupported(vk_config->available_instance_extensions,
+                                 instance_extensions)) {
+    return Result("Vulkan::Initialize not all instance extensions supported");
+  }
+
   device_ = MakeUnique<Device>(
       vk_config->instance, vk_config->physical_device,
-      vk_config->available_features, vk_config->available_extensions,
+      vk_config->available_features, vk_config->available_device_extensions,
       vk_config->queue_family_index, vk_config->device, vk_config->queue);
 
   Result r = device_->Initialize(vk_config->vkGetInstanceProcAddr, features,
-                                 extensions);
+                                 device_extensions);
   if (!r.IsSuccess())
     return r;
 
diff --git a/src/vulkan/engine_vulkan.h b/src/vulkan/engine_vulkan.h
index 36b550b..46468ad 100644
--- a/src/vulkan/engine_vulkan.h
+++ b/src/vulkan/engine_vulkan.h
@@ -40,7 +40,8 @@ class EngineVulkan : public Engine {
   // Engine
   Result Initialize(EngineConfig* config,
                     const std::vector<Feature>& features,
-                    const std::vector<std::string>& extensions) override;
+                    const std::vector<std::string>& instance_extensions,
+                    const std::vector<std::string>& device_extensions) override;
   Result Shutdown() override;
   Result CreatePipeline(amber::Pipeline* type) override;