Remove Requirement nodes and use buffers for frame and depth buffers

Android.mk

@@ -30,7 +30,6 @@ LOCAL_SRC_FILES:= \
     src/vkscript/datum_type_parser.cc \
     src/vkscript/executor.cc \
     src/vkscript/format_parser.cc \
-    src/vkscript/nodes.cc \
     src/vkscript/parser.cc \
     src/vkscript/script.cc \
     src/vkscript/section_parser.cc

src/CMakeLists.txt

@@ -40,7 +40,6 @@ set(AMBER_SOURCES
     vkscript/datum_type_parser.cc
     vkscript/executor.cc
     vkscript/format_parser.cc
-    vkscript/nodes.cc
     vkscript/parser.cc
     vkscript/script.cc
     vkscript/section_parser.cc

src/dawn/engine_dawn.cc

@@ -236,10 +236,6 @@ Result EngineDawn::CreatePipeline(PipelineType type) {
   return {};
 }
 
-Result EngineDawn::AddRequirement(Feature, const Format*) {
-  return Result("Dawn:AddRequirement not implemented");
-}
-
 Result EngineDawn::SetShader(ShaderType type,
                              const std::vector<uint32_t>& code) {
   ::dawn::ShaderModuleDescriptor descriptor;

src/dawn/engine_dawn.h

@@ -50,7 +50,6 @@ class EngineDawn : public Engine {
   // pipeline requires a compute shader.  A graphics pipeline requires a vertex
   // and a fragment shader.
   Result CreatePipeline(PipelineType) override;
-  Result AddRequirement(Feature feature, const Format*) override;
   Result SetShader(ShaderType type, const std::vector<uint32_t>& data) override;
   Result SetBuffer(BufferType type,
                    uint8_t location,

src/engine.h

@@ -95,11 +95,6 @@ class Engine {
   // Shutdown the engine and cleanup any resources.
   virtual Result Shutdown() = 0;
 
-  // Enable |feature|. If the feature requires a pixel format it will be
-  // provided in |format|, otherwise |format| is a nullptr. If the feature
-  // requires a uint32 value it will be set in the |uint32_t|.
-  virtual Result AddRequirement(Feature feature, const Format* format) = 0;
-
   // Create graphics pipeline.
   virtual Result CreatePipeline(PipelineType type) = 0;
 

src/vkscript/executor.cc

@@ -19,7 +19,6 @@
 
 #include "src/engine.h"
 #include "src/shader_compiler.h"
-#include "src/vkscript/nodes.h"
 #include "src/vkscript/script.h"
 
 namespace amber {
@@ -30,27 +29,13 @@ Executor::Executor() : amber::Executor() {}
 Executor::~Executor() = default;
 
 Result Executor::Execute(Engine* engine,
-                         const amber::Script* src_script,
+                         const amber::Script* script,
                          const ShaderMap& shader_map) {
-  if (!src_script->IsVkScript())
+  if (!script->IsVkScript())
     return Result("VkScript Executor called with non-vkscript source");
 
-  const Script* script = ToVkScript(src_script);
   engine->SetEngineData(script->GetEngineData());
 
-  // Process Requirement nodes
-  for (const auto& node : script->Nodes()) {
-    if (!node->IsRequire())
-      continue;
-
-    for (const auto& require : node->AsRequire()->Requirements()) {
-      Result r =
-          engine->AddRequirement(require.GetFeature(), require.GetFormat());
-      if (!r.IsSuccess())
-        return r;
-    }
-  }
-
   // Process Shader nodes
   PipelineType pipeline_type = PipelineType::kGraphics;
   for (const auto& shader : script->GetShaders()) {
@@ -70,12 +55,38 @@ Result Executor::Execute(Engine* engine,
       pipeline_type = PipelineType::kCompute;
   }
 
+  // Handle Image and Depth buffers early so they are available when we call
+  // the CreatePipeline method.
+  for (const auto& buf : script->GetBuffers()) {
+    // Image and depth are handled earler. They will be moved to the pipeline
+    // object when it exists.
+    if (buf->GetBufferType() != BufferType::kColor &&
+        buf->GetBufferType() != BufferType::kDepth) {
+      continue;
+    }
+
+    Result r = engine->SetBuffer(
+        buf->GetBufferType(), buf->GetLocation(),
+        buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(),
+        buf->GetData());
+    if (!r.IsSuccess())
+      return r;
+  }
+
   // TODO(jaebaek): Support multiple pipelines.
   Result r = engine->CreatePipeline(pipeline_type);
   if (!r.IsSuccess())
     return r;
 
+  // Process Buffers
   for (const auto& buf : script->GetBuffers()) {
+    // Image and depth are handled earler. They will be moved to the pipeline
+    // object when it exists.
+    if (buf->GetBufferType() == BufferType::kColor ||
+        buf->GetBufferType() == BufferType::kDepth) {
+      continue;
+    }
+
     r = engine->SetBuffer(
         buf->GetBufferType(), buf->GetLocation(),
         buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(),

src/vkscript/executor_test.cc

@@ -48,28 +48,6 @@ class EngineStub : public Engine {
 
   Result Shutdown() override { return {}; }
 
-  void FailRequirements() { fail_requirements_ = true; }
-  Result AddRequirement(Feature feature, const Format* fmt) override {
-    if (fail_requirements_)
-      return Result("requirements failed");
-
-    if (feature == Feature::kFramebuffer) {
-      if (fmt != nullptr)
-        color_frame_format_ = fmt->GetFormatType();
-      return {};
-    }
-
-    if (feature == Feature::kDepthStencil) {
-      if (fmt != nullptr)
-        depth_frame_format_ = fmt->GetFormatType();
-      return {};
-    }
-
-    return Result(
-        "Vulkan::AddRequirement features and extensions must be handled by "
-        "Initialize()");
-  }
-
   const std::vector<Feature>& GetFeatures() const { return features_; }
   const std::vector<std::string>& GetExtensions() const { return extensions_; }
   FormatType GetColorFrameFormat() const { return color_frame_format_; }
@@ -101,6 +79,14 @@ class EngineStub : public Engine {
                    uint8_t location,
                    const Format& format,
                    const std::vector<Value>& data) override {
+    if (type == BufferType::kColor || type == BufferType::kDepth) {
+      if (type == BufferType::kColor)
+        color_frame_format_ = format.GetFormatType();
+      else if (type == BufferType::kDepth)
+        depth_frame_format_ = format.GetFormatType();
+      return {};
+    }
+
     ++buffer_call_count_;
     buffer_types_.push_back(type);
     buffer_locations_.push_back(location);
@@ -224,7 +210,6 @@ class EngineStub : public Engine {
   }
 
  private:
-  bool fail_requirements_ = false;
   bool fail_shader_command_ = false;
   bool fail_clear_command_ = false;
   bool fail_clear_color_command_ = false;
@@ -283,24 +268,6 @@ class VkScriptExecutorTest : public testing::Test {
 
 }  // namespace
 
-TEST_F(VkScriptExecutorTest, ExecuteRequirementsFailed) {
-  std::string input = R"(
-[require]
-framebuffer R32G32B32A32_SINT)";
-
-  Parser parser;
-  ASSERT_TRUE(parser.Parse(input).IsSuccess());
-
-  auto engine = MakeEngine();
-  ToStub(engine.get())->FailRequirements();
-  auto script = parser.GetScript();
-
-  Executor ex;
-  Result r = ex.Execute(engine.get(), ToVkScript(script.get()), ShaderMap());
-  ASSERT_FALSE(r.IsSuccess());
-  EXPECT_EQ("requirements failed", r.Error());
-}
-
 TEST_F(VkScriptExecutorTest, ExecutesRequiredFeatures) {
   std::string input = R"(
 [require]
@@ -468,16 +435,6 @@ fence_timeout 12345)";
   EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, depth_frame_format);
 }
 
-TEST_F(VkScriptExecutorTest, EngineAddRequirementFailed) {
-  auto engine = MakeEngine();
-  Result r = engine->AddRequirement(Feature::kRobustBufferAccess, nullptr);
-  ASSERT_FALSE(r.IsSuccess());
-  EXPECT_EQ(
-      "Vulkan::AddRequirement features and extensions must be handled by "
-      "Initialize()",
-      r.Error());
-}
-
 #if AMBER_ENABLE_SHADERC
 TEST_F(VkScriptExecutorTest, ExecutesShaders) {
   std::string input = R"(