diff --git a/Android.mk b/Android.mk index 2be059a1e..3c4dfb1d8 100644 --- a/Android.mk +++ b/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 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6ef29107c..230c8db5e 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/dawn/engine_dawn.cc b/src/dawn/engine_dawn.cc index 56c0e88f1..2a6ba5b1c 100644 --- a/src/dawn/engine_dawn.cc +++ b/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& code) { ::dawn::ShaderModuleDescriptor descriptor; diff --git a/src/dawn/engine_dawn.h b/src/dawn/engine_dawn.h index 965ba4c06..664841513 100644 --- a/src/dawn/engine_dawn.h +++ b/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& data) override; Result SetBuffer(BufferType type, uint8_t location, diff --git a/src/engine.h b/src/engine.h index 64c8f03d8..4ee149f54 100644 --- a/src/engine.h +++ b/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; diff --git a/src/vkscript/executor.cc b/src/vkscript/executor.cc index f9ee42295..1bed1565d 100644 --- a/src/vkscript/executor.cc +++ b/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(), diff --git a/src/vkscript/executor_test.cc b/src/vkscript/executor_test.cc index 5c696536e..8d5177205 100644 --- a/src/vkscript/executor_test.cc +++ b/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& GetFeatures() const { return features_; } const std::vector& 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& 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"( diff --git a/src/vkscript/nodes.cc b/src/vkscript/nodes.cc deleted file mode 100644 index 4ca2decca..000000000 --- a/src/vkscript/nodes.cc +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2018 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/vkscript/nodes.h" - -#include -#include - -namespace amber { -namespace vkscript { - -Node::Node(NodeType type) : node_type_(type) {} - -Node::~Node() = default; - -RequireNode* Node::AsRequire() { - return static_cast(this); -} - -RequireNode::RequireNode() : Node(NodeType::kRequire) {} - -RequireNode::~RequireNode() = default; - -RequireNode::Requirement::Requirement(Feature feature) : feature_(feature) {} - -RequireNode::Requirement::Requirement(Feature feature, - std::unique_ptr format) - : feature_(feature), format_(std::move(format)) {} - -RequireNode::Requirement::Requirement(Requirement&&) = default; - -RequireNode::Requirement::~Requirement() = default; - -void RequireNode::AddRequirement(Feature feature, - std::unique_ptr format) { - requirements_.emplace_back(feature, std::move(format)); -} - -} // namespace vkscript -} // namespace amber diff --git a/src/vkscript/nodes.h b/src/vkscript/nodes.h deleted file mode 100644 index be8c5ee2f..000000000 --- a/src/vkscript/nodes.h +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2018 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. - -#ifndef SRC_VKSCRIPT_NODES_H_ -#define SRC_VKSCRIPT_NODES_H_ - -#include -#include -#include -#include - -#include "src/buffer.h" -#include "src/command.h" -#include "src/feature.h" -#include "src/format.h" -#include "src/tokenizer.h" -#include "src/vkscript/section_parser.h" - -namespace amber { -namespace vkscript { - -class RequireNode; - -class Node { - public: - virtual ~Node(); - - bool IsRequire() const { return node_type_ == NodeType::kRequire; } - - RequireNode* AsRequire(); - - protected: - explicit Node(NodeType type); - - private: - NodeType node_type_; -}; - -class RequireNode : public Node { - public: - class Requirement { - public: - explicit Requirement(Feature feature); - Requirement(Feature feature, std::unique_ptr format); - Requirement(Requirement&&); - ~Requirement(); - - Feature GetFeature() const { return feature_; } - const Format* GetFormat() const { return format_.get(); } - - private: - Feature feature_; - std::unique_ptr format_; - }; - - RequireNode(); - ~RequireNode() override; - - void AddRequirement(Feature feature, std::unique_ptr format); - - const std::vector& Requirements() const { return requirements_; } - - private: - std::vector requirements_; -}; - -} // namespace vkscript -} // namespace amber - -#endif // SRC_VKSCRIPT_NODES_H_ diff --git a/src/vkscript/parser.cc b/src/vkscript/parser.cc index 01f4691a7..b375eeddc 100644 --- a/src/vkscript/parser.cc +++ b/src/vkscript/parser.cc @@ -26,7 +26,6 @@ #include "src/tokenizer.h" #include "src/vkscript/command_parser.h" #include "src/vkscript/format_parser.h" -#include "src/vkscript/nodes.h" namespace amber { namespace vkscript { @@ -209,8 +208,6 @@ Result Parser::ProcessShaderBlock(const SectionParser::Section& section) { } Result Parser::ProcessRequireBlock(const std::string& data) { - auto node = MakeUnique(); - Tokenizer tokenizer(data); for (auto token = tokenizer.NextToken(); !token->IsEOS(); token = tokenizer.NextToken()) { @@ -239,7 +236,11 @@ Result Parser::ProcessRequireBlock(const std::string& data) { if (fmt == nullptr) return Result("Failed to parse framebuffer format"); - node->AddRequirement(feature, std::move(fmt)); + auto framebuffer = MakeUnique(BufferType::kColor); + framebuffer->SetName("framebuffer"); + framebuffer->SetFormat(std::move(fmt)); + framebuffer->SetLocation(0); // Only one image attachment in vkscript + script_->AddBuffer(std::move(framebuffer)); } else if (feature == Feature::kDepthStencil) { token = tokenizer.NextToken(); if (!token->IsString()) @@ -250,7 +251,11 @@ Result Parser::ProcessRequireBlock(const std::string& data) { if (fmt == nullptr) return Result("Failed to parse depthstencil format"); - node->AddRequirement(feature, std::move(fmt)); + auto depthbuffer = MakeUnique(BufferType::kDepth); + depthbuffer->SetName("depth_stencil_buffer"); + depthbuffer->SetFormat(std::move(fmt)); + depthbuffer->SetLocation(0); + script_->AddBuffer(std::move(depthbuffer)); } else if (feature == Feature::kFenceTimeout) { token = tokenizer.NextToken(); if (!token->IsInteger()) @@ -265,10 +270,6 @@ Result Parser::ProcessRequireBlock(const std::string& data) { if (!token->IsEOS() && !token->IsEOL()) return Result("Failed to parser requirements block: invalid token"); } - - if (!node->Requirements().empty()) - script_->AddRequireNode(std::move(node)); - return {}; } diff --git a/src/vkscript/parser_test.cc b/src/vkscript/parser_test.cc index 4dc0ad491..2a57fe8a1 100644 --- a/src/vkscript/parser_test.cc +++ b/src/vkscript/parser_test.cc @@ -17,7 +17,6 @@ #include "gtest/gtest.h" #include "src/feature.h" #include "src/format.h" -#include "src/vkscript/nodes.h" #include "src/vkscript/parser.h" namespace amber { @@ -25,18 +24,6 @@ namespace vkscript { using VkScriptParserTest = testing::Test; -TEST_F(VkScriptParserTest, EmptyRequireBlock) { - std::string block = ""; - - Parser parser; - Result r = parser.ProcessRequireBlockForTesting(block); - ASSERT_TRUE(r.IsSuccess()); - - auto script = parser.GetScript(); - ASSERT_TRUE(script->IsVkScript()); - EXPECT_TRUE(ToVkScript(script.get())->Nodes().empty()); -} - TEST_F(VkScriptParserTest, RequireBlockNoArgumentFeatures) { struct { const char* name; @@ -143,20 +130,13 @@ TEST_F(VkScriptParserTest, RequireBlockFramebuffer) { Result r = parser.ProcessRequireBlockForTesting(block); ASSERT_TRUE(r.IsSuccess()); - auto amber_script = parser.GetScript(); - auto script = ToVkScript(amber_script.get()); - EXPECT_EQ(1U, script->Nodes().size()); - - auto& nodes = script->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsRequire()); - - auto* req = nodes[0]->AsRequire(); - ASSERT_EQ(1U, req->Requirements().size()); - EXPECT_EQ(Feature::kFramebuffer, req->Requirements()[0].GetFeature()); - - auto format = req->Requirements()[0].GetFormat(); - EXPECT_EQ(FormatType::kR32G32B32A32_SFLOAT, format->GetFormatType()); + auto script = parser.GetScript(); + const auto& buffers = script->GetBuffers(); + ASSERT_EQ(1U, buffers.size()); + EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); + EXPECT_TRUE(buffers[0]->IsFormatBuffer()); + EXPECT_EQ(FormatType::kR32G32B32A32_SFLOAT, + buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); } TEST_F(VkScriptParserTest, RequireBlockDepthStencil) { @@ -166,20 +146,13 @@ TEST_F(VkScriptParserTest, RequireBlockDepthStencil) { Result r = parser.ProcessRequireBlockForTesting(block); ASSERT_TRUE(r.IsSuccess()) << r.Error(); - auto amber_script = parser.GetScript(); - auto script = ToVkScript(amber_script.get()); - EXPECT_EQ(1U, script->Nodes().size()); - - auto& nodes = script->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsRequire()); - - auto* req = nodes[0]->AsRequire(); - ASSERT_EQ(1U, req->Requirements().size()); - EXPECT_EQ(Feature::kDepthStencil, req->Requirements()[0].GetFeature()); - - auto format = req->Requirements()[0].GetFormat(); - EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, format->GetFormatType()); + auto script = parser.GetScript(); + const auto& buffers = script->GetBuffers(); + ASSERT_EQ(1U, buffers.size()); + EXPECT_EQ(BufferType::kDepth, buffers[0]->GetBufferType()); + EXPECT_TRUE(buffers[0]->IsFormatBuffer()); + EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, + buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); } TEST_F(VkScriptParserTest, RequireBlockMultipleLines) { @@ -196,23 +169,18 @@ inheritedQueries # line comment Result r = parser.ProcessRequireBlockForTesting(block); ASSERT_TRUE(r.IsSuccess()) << r.Error(); - auto amber_script = parser.GetScript(); - auto script = ToVkScript(amber_script.get()); - EXPECT_EQ(1U, script->Nodes().size()); - - auto& nodes = script->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsRequire()); - - auto* req = nodes[0]->AsRequire(); - ASSERT_EQ(2U, req->Requirements().size()); - EXPECT_EQ(Feature::kDepthStencil, req->Requirements()[0].GetFeature()); - auto format = req->Requirements()[0].GetFormat(); - EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, format->GetFormatType()); + auto script = parser.GetScript(); + const auto& buffers = script->GetBuffers(); + ASSERT_EQ(2U, buffers.size()); + EXPECT_EQ(BufferType::kDepth, buffers[0]->GetBufferType()); + EXPECT_TRUE(buffers[0]->IsFormatBuffer()); + EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, + buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); - EXPECT_EQ(Feature::kFramebuffer, req->Requirements()[1].GetFeature()); - format = req->Requirements()[1].GetFormat(); - EXPECT_EQ(FormatType::kR32G32B32A32_SFLOAT, format->GetFormatType()); + EXPECT_EQ(BufferType::kColor, buffers[1]->GetBufferType()); + EXPECT_TRUE(buffers[1]->IsFormatBuffer()); + EXPECT_EQ(FormatType::kR32G32B32A32_SFLOAT, + buffers[1]->AsFormatBuffer()->GetFormat().GetFormatType()); auto& feats = script->RequiredFeatures(); EXPECT_EQ(Feature::kSparseResidency4Samples, feats[0]); @@ -300,9 +268,8 @@ TEST_F(VkScriptParserTest, VertexDataEmpty) { Result r = parser.ProcessVertexDataBlockForTesting(block); ASSERT_TRUE(r.IsSuccess()); - auto amber_script = parser.GetScript(); - auto& nodes = ToVkScript(amber_script.get())->Nodes(); - EXPECT_TRUE(nodes.empty()); + auto script = parser.GetScript(); + EXPECT_TRUE(script->GetBuffers().empty()); } TEST_F(VkScriptParserTest, VertexDataHeaderFormatString) { diff --git a/src/vkscript/script.cc b/src/vkscript/script.cc index 722ea79bd..c18599500 100644 --- a/src/vkscript/script.cc +++ b/src/vkscript/script.cc @@ -17,7 +17,6 @@ #include #include "src/make_unique.h" -#include "src/vkscript/nodes.h" namespace amber { @@ -31,10 +30,5 @@ Script::Script() : amber::Script(ScriptType::kVkScript) {} Script::~Script() = default; -void Script::AddRequireNode(std::unique_ptr node) { - std::unique_ptr tn(node.release()); - test_nodes_.push_back(std::move(tn)); -} - } // namespace vkscript } // namespace amber diff --git a/src/vkscript/script.h b/src/vkscript/script.h index fd3e3a983..bf88a6cea 100644 --- a/src/vkscript/script.h +++ b/src/vkscript/script.h @@ -27,22 +27,10 @@ namespace amber { namespace vkscript { -class Node; -class RequireNode; - class Script : public amber::Script { public: Script(); ~Script() override; - - void AddRequireNode(std::unique_ptr node); - - const std::vector>& Nodes() const { - return test_nodes_; - } - - private: - std::vector> test_nodes_; }; } // namespace vkscript diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index b517000f7..7fc7660a9 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -108,24 +108,6 @@ Result EngineVulkan::Shutdown() { return {}; } -Result EngineVulkan::AddRequirement(Feature feature, const Format* fmt) { - if (feature == Feature::kFramebuffer) { - if (fmt != nullptr) - color_frame_format_ = ToVkFormat(fmt->GetFormatType()); - return {}; - } - - if (feature == Feature::kDepthStencil) { - if (fmt != nullptr) - depth_frame_format_ = ToVkFormat(fmt->GetFormatType()); - return {}; - } - - return Result( - "Vulkan::AddRequirement features and extensions must be handled by " - "Initialize()"); -} - Result EngineVulkan::CreatePipeline(PipelineType type) { const auto& engine_data = GetEngineData(); @@ -189,6 +171,17 @@ Result EngineVulkan::SetBuffer(BufferType type, uint8_t location, const Format& format, const std::vector& values) { + // Handle image and depth attachments special as they come in before + // the pipeline is created. + if (type == BufferType::kColor || type == BufferType::kDepth) { + if (type == BufferType::kColor) + color_frame_format_ = ToVkFormat(format.GetFormatType()); + else if (type == BufferType::kDepth) + depth_frame_format_ = ToVkFormat(format.GetFormatType()); + + return {}; + } + if (!pipeline_) return Result("Vulkan::SetBuffer no Pipeline exists"); diff --git a/src/vulkan/engine_vulkan.h b/src/vulkan/engine_vulkan.h index aa06c0c60..56e7386a8 100644 --- a/src/vulkan/engine_vulkan.h +++ b/src/vulkan/engine_vulkan.h @@ -43,7 +43,6 @@ class EngineVulkan : public Engine { const std::vector& features, const std::vector& extensions) override; Result Shutdown() override; - Result AddRequirement(Feature feature, const Format*) override; Result CreatePipeline(PipelineType type) override; Result SetShader(ShaderType type, const std::vector& data) override; Result SetBuffer(BufferType type,