From 1a4abd28c5dc6917eb0ef4b9115b0a74d49b6d50 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 10 Dec 2018 11:44:03 -0500 Subject: [PATCH 01/10] Remove IndicesNode and use Buffer directly This CL removes the IndicesNode and adds the Index buffer directly to the Script object. The Executor is updated to pass all the set buffers to the engine. --- src/script.h | 16 +++++++++++++ src/vkscript/executor.cc | 9 ++------ src/vkscript/nodes.cc | 9 -------- src/vkscript/nodes.h | 14 ------------ src/vkscript/parser.cc | 5 ++++- src/vkscript/parser_test.cc | 45 ++++++++++++++++++++----------------- src/vkscript/script.cc | 4 ---- src/vkscript/script.h | 1 - 8 files changed, 46 insertions(+), 57 deletions(-) diff --git a/src/script.h b/src/script.h index 7248984cf..024ca9ce5 100644 --- a/src/script.h +++ b/src/script.h @@ -24,6 +24,7 @@ #include "amber/recipe.h" #include "amber/result.h" +#include "src/buffer.h" #include "src/command.h" #include "src/engine.h" #include "src/feature.h" @@ -62,6 +63,19 @@ class Script : public RecipeImpl { return shaders_; } + Result AddBuffer(std::unique_ptr buffer) { + if (name_to_buffer_.count(buffer->GetName()) > 0) + return Result("duplicate buffer name provided"); + + buffers_.push_back(std::move(buffer)); + name_to_buffer_[buffers_.back()->GetName()] = buffers_.back().get(); + return {}; + } + + const std::vector>& GetBuffers() const { + return buffers_; + } + void AddRequiredFeature(Feature feature) { engine_info_.required_features.push_back(feature); } @@ -98,8 +112,10 @@ class Script : public RecipeImpl { ScriptType script_type_; EngineData engine_data_; std::map name_to_shader_; + std::map name_to_buffer_; std::vector> shaders_; std::vector> commands_; + std::vector> buffers_; }; } // namespace amber diff --git a/src/vkscript/executor.cc b/src/vkscript/executor.cc index 0523b1f51..cf6de3a0a 100644 --- a/src/vkscript/executor.cc +++ b/src/vkscript/executor.cc @@ -90,13 +90,8 @@ Result Executor::Execute(Engine* engine, } } - // Process Indices nodes - for (const auto& node : script->Nodes()) { - if (!node->IsIndices()) - continue; - - r = engine->SetBuffer(BufferType::kIndex, 0, Format(), - node->AsIndices()->Indices()); + for (const auto& buf : script->GetBuffers()) { + r = engine->SetBuffer(buf->GetBufferType(), 0, Format(), buf->GetData()); if (!r.IsSuccess()) return r; } diff --git a/src/vkscript/nodes.cc b/src/vkscript/nodes.cc index 993f9897b..8bbc9c1aa 100644 --- a/src/vkscript/nodes.cc +++ b/src/vkscript/nodes.cc @@ -24,10 +24,6 @@ Node::Node(NodeType type) : node_type_(type) {} Node::~Node() = default; -IndicesNode* Node::AsIndices() { - return static_cast(this); -} - RequireNode* Node::AsRequire() { return static_cast(this); } @@ -55,11 +51,6 @@ void RequireNode::AddRequirement(Feature feature, requirements_.emplace_back(feature, std::move(format)); } -IndicesNode::IndicesNode(std::unique_ptr buffer) - : Node(NodeType::kIndices), buffer_(std::move(buffer)) {} - -IndicesNode::~IndicesNode() = default; - VertexDataNode::VertexDataNode() : Node(NodeType::kVertexData) {} VertexDataNode::~VertexDataNode() = default; diff --git a/src/vkscript/nodes.h b/src/vkscript/nodes.h index 95b9556b6..0442d6884 100644 --- a/src/vkscript/nodes.h +++ b/src/vkscript/nodes.h @@ -30,7 +30,6 @@ namespace amber { namespace vkscript { -class IndicesNode; class RequireNode; class VertexDataNode; @@ -38,11 +37,9 @@ class Node { public: virtual ~Node(); - bool IsIndices() const { return node_type_ == NodeType::kIndices; } bool IsRequire() const { return node_type_ == NodeType::kRequire; } bool IsVertexData() const { return node_type_ == NodeType::kVertexData; } - IndicesNode* AsIndices(); RequireNode* AsRequire(); VertexDataNode* AsVertexData(); @@ -81,17 +78,6 @@ class RequireNode : public Node { std::vector requirements_; }; -class IndicesNode : public Node { - public: - explicit IndicesNode(std::unique_ptr buffer); - ~IndicesNode() override; - - const std::vector& Indices() const { return buffer_->GetData(); } - - private: - std::unique_ptr buffer_; -}; - class VertexDataNode : public Node { public: struct Header { diff --git a/src/vkscript/parser.cc b/src/vkscript/parser.cc index ad4d54faf..94635e63c 100644 --- a/src/vkscript/parser.cc +++ b/src/vkscript/parser.cc @@ -297,9 +297,12 @@ Result Parser::ProcessIndicesBlock(const std::string& data) { type.SetType(DataType::kUint16); auto b = MakeUnique(BufferType::kIndex); + b->SetName("indices"); b->SetDatumType(type); b->SetData(std::move(indices)); - script_->AddIndexBuffer(std::move(b)); + Result r = script_->AddBuffer(std::move(b)); + if (!r.IsSuccess()) + return r; } return {}; diff --git a/src/vkscript/parser_test.cc b/src/vkscript/parser_test.cc index 2ecc0a898..4f2bc10d4 100644 --- a/src/vkscript/parser_test.cc +++ b/src/vkscript/parser_test.cc @@ -227,19 +227,22 @@ TEST_F(VkScriptParserTest, IndicesBlock) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - auto& nodes = ToVkScript(script.get())->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsIndices()); - - auto& indices = nodes[0]->AsIndices()->Indices(); - ASSERT_EQ(3U, indices.size()); - - EXPECT_TRUE(indices[0].IsInteger()); - EXPECT_EQ(1, indices[0].AsUint16()); - EXPECT_TRUE(indices[1].IsInteger()); - EXPECT_EQ(2, indices[1].AsUint16()); - EXPECT_TRUE(indices[2].IsInteger()); - EXPECT_EQ(3, indices[2].AsUint16()); + const auto& buffers = script->GetBuffers(); + ASSERT_EQ(1U, buffers.size()); + ASSERT_EQ(BufferType::kIndex, buffers[0]->GetBufferType()); + + auto buffer = buffers[0].get(); + EXPECT_TRUE(buffer->GetDatumType().IsUint16()); + EXPECT_EQ(3U, buffer->GetSize()); + auto& data = buffer->GetData(); + ASSERT_EQ(3U, data.size()); + + EXPECT_TRUE(data[0].IsInteger()); + EXPECT_EQ(1, data[0].AsUint16()); + EXPECT_TRUE(data[1].IsInteger()); + EXPECT_EQ(2, data[1].AsUint16()); + EXPECT_TRUE(data[2].IsInteger()); + EXPECT_EQ(3, data[2].AsUint16()); } TEST_F(VkScriptParserTest, IndicesBlockMultipleLines) { @@ -256,16 +259,16 @@ TEST_F(VkScriptParserTest, IndicesBlockMultipleLines) { Result r = parser.ProcessIndicesBlockForTesting(block); ASSERT_TRUE(r.IsSuccess()) << r.Error(); - auto amber_script = parser.GetScript(); - auto& nodes = ToVkScript(amber_script.get())->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsIndices()); + auto script = parser.GetScript(); + auto& buffers = script->GetBuffers(); + ASSERT_EQ(1U, buffers.size()); + ASSERT_EQ(buffers[0]->GetBufferType(), BufferType::kIndex); - auto& indices = nodes[0]->AsIndices()->Indices(); - ASSERT_EQ(results.size(), indices.size()); + auto& data = buffers[0]->GetData(); + ASSERT_EQ(results.size(), data.size()); for (size_t i = 0; i < results.size(); ++i) { - EXPECT_TRUE(indices[i].IsInteger()); - EXPECT_EQ(results[i], indices[i].AsUint16()); + EXPECT_TRUE(data[i].IsInteger()); + EXPECT_EQ(results[i], data[i].AsUint16()); } } diff --git a/src/vkscript/script.cc b/src/vkscript/script.cc index c087d00e6..01f7e58ab 100644 --- a/src/vkscript/script.cc +++ b/src/vkscript/script.cc @@ -36,10 +36,6 @@ void Script::AddRequireNode(std::unique_ptr node) { test_nodes_.push_back(std::move(tn)); } -void Script::AddIndexBuffer(std::unique_ptr buffer) { - test_nodes_.push_back(MakeUnique(std::move(buffer))); -} - void Script::AddVertexData(std::unique_ptr node) { std::unique_ptr tn(node.release()); test_nodes_.push_back(std::move(tn)); diff --git a/src/vkscript/script.h b/src/vkscript/script.h index 022aaa0ad..a60c77303 100644 --- a/src/vkscript/script.h +++ b/src/vkscript/script.h @@ -37,7 +37,6 @@ class Script : public amber::Script { ~Script() override; void AddRequireNode(std::unique_ptr node); - void AddIndexBuffer(std::unique_ptr b); void AddVertexData(std::unique_ptr node); const std::vector>& Nodes() const { From 22b4c94f38acab7fe5ded4e3be7353efd8905175 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 10 Dec 2018 13:39:08 -0500 Subject: [PATCH 02/10] Split Buffer into a Data and Format type buffer. This CL allows for Buffers to be described by a DatumType (like the indices buffers) or by a Format type. The VertexNodes have been removed and converted to Format type buffers. --- src/amberscript/parser.cc | 14 +-- src/amberscript/parser.h | 12 +-- src/amberscript/parser_test.cc | 155 +++++++++++++++++++-------------- src/amberscript/script_test.cc | 18 ++-- src/buffer.cc | 12 +++ src/buffer.h | 91 ++++++++++++++++--- src/buffer_test.cc | 20 ++--- src/vkscript/executor.cc | 21 ++--- src/vkscript/nodes.cc | 12 --- src/vkscript/nodes.h | 29 ------ src/vkscript/parser.cc | 21 +++-- src/vkscript/parser_test.cc | 91 +++++++++---------- src/vkscript/script.cc | 5 -- src/vkscript/script.h | 2 - 14 files changed, 274 insertions(+), 229 deletions(-) diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc index c11d3fbc2..d45c52ae0 100644 --- a/src/amberscript/parser.cc +++ b/src/amberscript/parser.cc @@ -432,7 +432,7 @@ Result Parser::ParseBuffer() { if (!r.IsSuccess()) return r; - auto buffer = MakeUnique(type); + auto buffer = MakeUnique(type); token = tokenizer_->NextToken(); if (!token->IsString()) @@ -474,7 +474,7 @@ Result Parser::ParseBuffer() { return {}; } -Result Parser::ParseBufferFramebuffer(Buffer* buffer) { +Result Parser::ParseBufferFramebuffer(DataBuffer* buffer) { auto token = tokenizer_->NextToken(); if (token->IsEOL() || token->IsEOS()) return Result("BUFFER framebuffer missing DIMS values"); @@ -506,7 +506,7 @@ Result Parser::ParseBufferFramebuffer(Buffer* buffer) { return ValidateEndOfStatement("BUFFER framebuffer command"); } -Result Parser::ParseBufferInitializer(Buffer* buffer) { +Result Parser::ParseBufferInitializer(DataBuffer* buffer) { auto token = tokenizer_->NextToken(); if (!token->IsString()) return Result("BUFFER invalid data type"); @@ -530,7 +530,7 @@ Result Parser::ParseBufferInitializer(Buffer* buffer) { return Result("unknown initializer for BUFFER"); } -Result Parser::ParseBufferInitializerSize(Buffer* buffer) { +Result Parser::ParseBufferInitializerSize(DataBuffer* buffer) { auto token = tokenizer_->NextToken(); if (token->IsEOS() || token->IsEOL()) return Result("BUFFER size missing"); @@ -552,7 +552,7 @@ Result Parser::ParseBufferInitializerSize(Buffer* buffer) { return Result("invalid BUFFER initializer provided"); } -Result Parser::ParseBufferInitializerFill(Buffer* buffer, +Result Parser::ParseBufferInitializerFill(DataBuffer* buffer, uint32_t size_in_items) { auto token = tokenizer_->NextToken(); if (token->IsEOS() || token->IsEOL()) @@ -578,7 +578,7 @@ Result Parser::ParseBufferInitializerFill(Buffer* buffer, return ValidateEndOfStatement("BUFFER fill command"); } -Result Parser::ParseBufferInitializerSeries(Buffer* buffer, +Result Parser::ParseBufferInitializerSeries(DataBuffer* buffer, uint32_t size_in_items) { auto token = tokenizer_->NextToken(); if (token->IsEOS() || token->IsEOL()) @@ -627,7 +627,7 @@ Result Parser::ParseBufferInitializerSeries(Buffer* buffer, return ValidateEndOfStatement("BUFFER series_from command"); } -Result Parser::ParseBufferInitializerData(Buffer* buffer) { +Result Parser::ParseBufferInitializerData(DataBuffer* buffer) { auto token = tokenizer_->NextToken(); if (!token->IsEOL()) return Result("extra parameters after BUFFER data command"); diff --git a/src/amberscript/parser.h b/src/amberscript/parser.h index aa86e191f..7e4b764d1 100644 --- a/src/amberscript/parser.h +++ b/src/amberscript/parser.h @@ -51,12 +51,12 @@ class Parser : public amber::Parser { Result ValidateEndOfStatement(const std::string& name); Result ParseBuffer(); - Result ParseBufferInitializer(Buffer*); - Result ParseBufferInitializerSize(Buffer*); - Result ParseBufferInitializerFill(Buffer*, uint32_t); - Result ParseBufferInitializerSeries(Buffer*, uint32_t); - Result ParseBufferInitializerData(Buffer*); - Result ParseBufferFramebuffer(Buffer*); + Result ParseBufferInitializer(DataBuffer*); + Result ParseBufferInitializerSize(DataBuffer*); + Result ParseBufferInitializerFill(DataBuffer*, uint32_t); + Result ParseBufferInitializerSeries(DataBuffer*, uint32_t); + Result ParseBufferInitializerData(DataBuffer*); + Result ParseBufferFramebuffer(DataBuffer*); Result ParseShaderBlock(); Result ParsePipelineBlock(); Result ParsePipelineAttach(Pipeline*); diff --git a/src/amberscript/parser_test.cc b/src/amberscript/parser_test.cc index 5d191392e..fd57a5d3e 100644 --- a/src/amberscript/parser_test.cc +++ b/src/amberscript/parser_test.cc @@ -957,12 +957,15 @@ END)"; ASSERT_TRUE(buffers[0] != nullptr); EXPECT_EQ("my_buffer", buffers[0]->GetName()); EXPECT_EQ(BufferType::kStorage, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsUint32()); - EXPECT_EQ(7U, buffers[0]->GetSize()); - EXPECT_EQ(7U * sizeof(uint32_t), buffers[0]->GetSizeInBytes()); + + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_TRUE(buffer->GetDatumType().IsUint32()); + EXPECT_EQ(7U, buffer->GetSize()); + EXPECT_EQ(7U * sizeof(uint32_t), buffer->GetSizeInBytes()); std::vector results = {1, 2, 3, 4, 55, 99, 1234}; - const auto& data = buffers[0]->GetData(); + const auto& data = buffer->GetData(); ASSERT_EQ(results.size(), data.size()); for (size_t i = 0; i < results.size(); ++i) { ASSERT_TRUE(data[i].IsInteger()); @@ -983,14 +986,16 @@ TEST_F(AmberScriptParserTest, BufferFill) { ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("my_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsUint8()); - EXPECT_EQ(5U, buffers[0]->GetSize()); - EXPECT_EQ(5U * sizeof(uint8_t), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("my_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kColor, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsUint8()); + EXPECT_EQ(5U, buffer->GetSize()); + EXPECT_EQ(5U * sizeof(uint8_t), buffer->GetSizeInBytes()); std::vector results = {5, 5, 5, 5, 5}; - const auto& data = buffers[0]->GetData(); + const auto& data = buffer->GetData(); ASSERT_EQ(results.size(), data.size()); for (size_t i = 0; i < results.size(); ++i) { ASSERT_TRUE(data[i].IsInteger()); @@ -1011,14 +1016,16 @@ TEST_F(AmberScriptParserTest, BufferFillFloat) { ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("my_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsFloat()); - EXPECT_EQ(5U, buffers[0]->GetSize()); - EXPECT_EQ(5U * sizeof(float), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("my_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kColor, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsFloat()); + EXPECT_EQ(5U, buffer->GetSize()); + EXPECT_EQ(5U * sizeof(float), buffer->GetSizeInBytes()); std::vector results = {5.2f, 5.2f, 5.2f, 5.2f, 5.2f}; - const auto& data = buffers[0]->GetData(); + const auto& data = buffer->GetData(); ASSERT_EQ(results.size(), data.size()); for (size_t i = 0; i < results.size(); ++i) { ASSERT_TRUE(data[i].IsFloat()); @@ -1040,14 +1047,16 @@ TEST_F(AmberScriptParserTest, BufferSeries) { ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("my_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsUint8()); - EXPECT_EQ(5U, buffers[0]->GetSize()); - EXPECT_EQ(5U * sizeof(uint8_t), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("my_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kColor, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsUint8()); + EXPECT_EQ(5U, buffer->GetSize()); + EXPECT_EQ(5U * sizeof(uint8_t), buffer->GetSizeInBytes()); std::vector results = {2, 3, 4, 5, 6}; - const auto& data = buffers[0]->GetData(); + const auto& data = buffer->GetData(); ASSERT_EQ(results.size(), data.size()); for (size_t i = 0; i < results.size(); ++i) { ASSERT_TRUE(data[i].IsInteger()); @@ -1070,14 +1079,16 @@ TEST_F(AmberScriptParserTest, BufferSeriesFloat) { ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("my_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsFloat()); - EXPECT_EQ(5U, buffers[0]->GetSize()); - EXPECT_EQ(5U * sizeof(float), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("my_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kColor, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsFloat()); + EXPECT_EQ(5U, buffer->GetSize()); + EXPECT_EQ(5U * sizeof(float), buffer->GetSizeInBytes()); std::vector results = {2.2f, 3.3f, 4.4f, 5.5f, 6.6f}; - const auto& data = buffers[0]->GetData(); + const auto& data = buffer->GetData(); ASSERT_EQ(results.size(), data.size()); for (size_t i = 0; i < results.size(); ++i) { ASSERT_TRUE(data[i].IsFloat()); @@ -1098,12 +1109,14 @@ TEST_F(AmberScriptParserTest, BufferFramebuffer) { ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("my_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kFramebuffer, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsUint32()); - EXPECT_EQ(4U, buffers[0]->GetDatumType().ColumnCount()); - EXPECT_EQ(800U * 600U, buffers[0]->GetSize()); - EXPECT_EQ(800U * 600U * 4U * sizeof(uint32_t), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("my_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kFramebuffer, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsUint32()); + EXPECT_EQ(4U, buffer->GetDatumType().ColumnCount()); + EXPECT_EQ(800U * 600U, buffer->GetSize()); + EXPECT_EQ(800U * 600U * 4U * sizeof(uint32_t), buffer->GetSizeInBytes()); } TEST_F(AmberScriptParserTest, BufferMultipleBuffers) { @@ -1124,14 +1137,16 @@ END)"; ASSERT_EQ(2U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("color_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsUint8()); - EXPECT_EQ(5U, buffers[0]->GetSize()); - EXPECT_EQ(5U * sizeof(uint8_t), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("color_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kColor, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsUint8()); + EXPECT_EQ(5U, buffer->GetSize()); + EXPECT_EQ(5U * sizeof(uint8_t), buffer->GetSizeInBytes()); std::vector results0 = {5, 5, 5, 5, 5}; - const auto& data0 = buffers[0]->GetData(); + const auto& data0 = buffer->GetData(); ASSERT_EQ(results0.size(), data0.size()); for (size_t i = 0; i < results0.size(); ++i) { ASSERT_TRUE(data0[i].IsInteger()); @@ -1139,14 +1154,16 @@ END)"; } ASSERT_TRUE(buffers[1] != nullptr); - EXPECT_EQ("storage_buffer", buffers[1]->GetName()); - EXPECT_EQ(BufferType::kStorage, buffers[1]->GetBufferType()); - EXPECT_TRUE(buffers[1]->GetDatumType().IsUint32()); - EXPECT_EQ(7U, buffers[1]->GetSize()); - EXPECT_EQ(7U * sizeof(uint32_t), buffers[1]->GetSizeInBytes()); + ASSERT_TRUE(buffers[1]->IsDataBuffer()); + buffer = buffers[1]->AsDataBuffer(); + EXPECT_EQ("storage_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kStorage, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsUint32()); + EXPECT_EQ(7U, buffer->GetSize()); + EXPECT_EQ(7U * sizeof(uint32_t), buffer->GetSizeInBytes()); std::vector results1 = {1, 2, 3, 4, 55, 99, 1234}; - const auto& data1 = buffers[1]->GetData(); + const auto& data1 = buffer->GetData(); ASSERT_EQ(results1.size(), data1.size()); for (size_t i = 0; i < results1.size(); ++i) { ASSERT_TRUE(data1[i].IsInteger()); @@ -1168,14 +1185,16 @@ BUFFER index my_index_buffer DATA_TYPE vec2 SIZE 5 FILL 2)"; ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("my_index_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kIndex, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsInt32()); - EXPECT_EQ(5U, buffers[0]->GetSize()); - EXPECT_EQ(5U * 2 * sizeof(int32_t), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("my_index_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kIndex, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsInt32()); + EXPECT_EQ(5U, buffer->GetSize()); + EXPECT_EQ(5U * 2 * sizeof(int32_t), buffer->GetSizeInBytes()); std::vector results0 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2}; - const auto& data0 = buffers[0]->GetData(); + const auto& data0 = buffer->GetData(); ASSERT_EQ(results0.size(), data0.size()); for (size_t i = 0; i < results0.size(); ++i) { ASSERT_TRUE(data0[i].IsInteger()); @@ -1203,14 +1222,16 @@ END ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("my_index_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kIndex, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsInt32()); - EXPECT_EQ(4U, buffers[0]->GetSize()); - EXPECT_EQ(4U * 2 * sizeof(int32_t), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("my_index_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kIndex, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsInt32()); + EXPECT_EQ(4U, buffer->GetSize()); + EXPECT_EQ(4U * 2 * sizeof(int32_t), buffer->GetSizeInBytes()); std::vector results0 = {2, 3, 4, 5, 6, 7, 8, 9}; - const auto& data0 = buffers[0]->GetData(); + const auto& data0 = buffer->GetData(); ASSERT_EQ(results0.size(), data0.size()); for (size_t i = 0; i < results0.size(); ++i) { ASSERT_TRUE(data0[i].IsInteger()); @@ -1238,14 +1259,16 @@ END ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - EXPECT_EQ("my_index_buffer", buffers[0]->GetName()); - EXPECT_EQ(BufferType::kIndex, buffers[0]->GetBufferType()); - EXPECT_TRUE(buffers[0]->GetDatumType().IsUint32()); - EXPECT_EQ(4U, buffers[0]->GetSize()); - EXPECT_EQ(4U * sizeof(uint32_t), buffers[0]->GetSizeInBytes()); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + EXPECT_EQ("my_index_buffer", buffer->GetName()); + EXPECT_EQ(BufferType::kIndex, buffer->GetBufferType()); + EXPECT_TRUE(buffer->GetDatumType().IsUint32()); + EXPECT_EQ(4U, buffer->GetSize()); + EXPECT_EQ(4U * sizeof(uint32_t), buffer->GetSizeInBytes()); std::vector results0 = {4278190080, 16711680, 65280, 255}; - const auto& data0 = buffers[0]->GetData(); + const auto& data0 = buffer->GetData(); ASSERT_EQ(results0.size(), data0.size()); for (size_t i = 0; i < results0.size(); ++i) { ASSERT_TRUE(data0[i].IsInteger()); @@ -1648,7 +1671,9 @@ TEST_P(AmberScriptParserBufferDataTypeTest, BufferTypes) { ASSERT_EQ(1U, buffers.size()); ASSERT_TRUE(buffers[0] != nullptr); - auto& datum = buffers[0]->GetDatumType(); + ASSERT_TRUE(buffers[0]->IsDataBuffer()); + auto* buffer = buffers[0]->AsDataBuffer(); + auto& datum = buffer->GetDatumType(); EXPECT_EQ(test_data.type, datum.GetType()); EXPECT_EQ(test_data.row_count, datum.RowCount()); EXPECT_EQ(test_data.column_count, datum.ColumnCount()); diff --git a/src/amberscript/script_test.cc b/src/amberscript/script_test.cc index 6110f887a..2a2bd5ed1 100644 --- a/src/amberscript/script_test.cc +++ b/src/amberscript/script_test.cc @@ -171,8 +171,8 @@ TEST_F(ScriptTest, GetPipelines) { EXPECT_EQ(ptr2, pipelines[1].get()); } -TEST_F(ScriptTest, AddBuffer) { - auto buffer = MakeUnique(BufferType::kStorage); +TEST_F(ScriptTest, AddDataBuffer) { + auto buffer = MakeUnique(BufferType::kStorage); buffer->SetName("my_buffer"); Script s; @@ -180,15 +180,15 @@ TEST_F(ScriptTest, AddBuffer) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); } -TEST_F(ScriptTest, AddDuplicateBuffer) { - auto buffer1 = MakeUnique(BufferType::kStorage); +TEST_F(ScriptTest, AddDuplicateDataBuffer) { + auto buffer1 = MakeUnique(BufferType::kStorage); buffer1->SetName("my_buffer"); Script s; Result r = s.AddBuffer(std::move(buffer1)); ASSERT_TRUE(r.IsSuccess()) << r.Error(); - auto buffer2 = MakeUnique(BufferType::kUniform); + auto buffer2 = MakeUnique(BufferType::kUniform); buffer2->SetName("my_buffer"); r = s.AddBuffer(std::move(buffer2)); @@ -196,8 +196,8 @@ TEST_F(ScriptTest, AddDuplicateBuffer) { EXPECT_EQ("duplicate buffer name provided", r.Error()); } -TEST_F(ScriptTest, GetBuffer) { - auto buffer = MakeUnique(BufferType::kStorage); +TEST_F(ScriptTest, GetDataBuffer) { + auto buffer = MakeUnique(BufferType::kStorage); buffer->SetName("my_buffer"); const auto* ptr = buffer.get(); @@ -221,7 +221,7 @@ TEST_F(ScriptTest, GetBuffersEmpty) { } TEST_F(ScriptTest, GetBuffers) { - auto buffer1 = MakeUnique(BufferType::kStorage); + auto buffer1 = MakeUnique(BufferType::kStorage); buffer1->SetName("my_buffer1"); const auto* ptr1 = buffer1.get(); @@ -230,7 +230,7 @@ TEST_F(ScriptTest, GetBuffers) { Result r = s.AddBuffer(std::move(buffer1)); ASSERT_TRUE(r.IsSuccess()) << r.Error(); - auto buffer2 = MakeUnique(BufferType::kUniform); + auto buffer2 = MakeUnique(BufferType::kUniform); buffer2->SetName("my_buffer2"); const auto* ptr2 = buffer2.get(); diff --git a/src/buffer.cc b/src/buffer.cc index 08e3ddd8d..93649afb2 100644 --- a/src/buffer.cc +++ b/src/buffer.cc @@ -20,4 +20,16 @@ Buffer::Buffer(BufferType type) : buffer_type_(type) {} Buffer::~Buffer() = default; +DataBuffer* Buffer::AsDataBuffer() { return static_cast(this); } + +FormatBuffer* Buffer::AsFormatBuffer() { return static_cast(this); } + +DataBuffer::DataBuffer(BufferType type) : Buffer(type) {} + +DataBuffer::~DataBuffer() = default; + +FormatBuffer::FormatBuffer(BufferType type) : Buffer(type) {} + +FormatBuffer::~FormatBuffer() = default; + } // namespace amber diff --git a/src/buffer.h b/src/buffer.h index edc1eba6c..8dbca832e 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -22,54 +22,121 @@ #include "src/buffer_data.h" #include "src/datum_type.h" +#include "src/format.h" #include "src/value.h" namespace amber { +class DataBuffer; +class FormatBuffer; + /// A buffer stores data. The buffer maybe provided from the input script, or /// maybe created as needed. A buffer must have a unique name. class Buffer { public: - /// Create a buffer of |type|. - explicit Buffer(BufferType type); - ~Buffer(); + virtual ~Buffer(); + + /// Returns |true| if this is a buffer described by a |DatumType|. + virtual bool IsDataBuffer() const { return false; } + /// Returns |true| if this is a buffer described by a |Format|. + virtual bool IsFormatBuffer() const { return false; } + + /// Converts the buffer to a |DataBuffer|. Note, |IsDataBuffer| must be true + /// for this method to be used. + DataBuffer* AsDataBuffer(); + /// Converts the buffer to a |FormatBuffer|. Note, |IsFormatBuffer| must be + /// true for this method to be used. + FormatBuffer* AsFormatBuffer(); /// Returns the BufferType of this buffer. BufferType GetBufferType() const { return buffer_type_; } + /// Set the location binding value for the buffer. + void SetLocation(uint8_t loc) { location_ = loc; } + /// Get the location binding value for the buffer. + uint8_t GetLocation() const { return location_; } + /// Sets the buffer |name|. void SetName(const std::string& name) { name_ = name; } /// Returns the name of the buffer. std::string GetName() const { return name_; } - /// Sets the DatumType of the buffer to |type|. - void SetDatumType(const DatumType& type) { datum_type_ = type; } - /// Returns the DatumType describing the buffer data. - const DatumType& GetDatumType() const { return datum_type_; } - /// Sets the buffer to |size| items. void SetSize(size_t size) { size_ = size; } /// Returns the number of items in the buffer. size_t GetSize() const { return size_; } /// Returns the number of bytes needed for the data in the buffer. - size_t GetSizeInBytes() const { return size_ * datum_type_.SizeInBytes(); } + virtual size_t GetSizeInBytes() const = 0; /// Sets the data into the buffer. The size will also be updated to be the /// size of the data provided. - void SetData(std::vector&& data) { - size_ = data.size() / datum_type_.ColumnCount() / datum_type_.RowCount(); + virtual void SetData(std::vector&& data) { data_ = std::move(data); } /// Returns the vector of Values stored in the buffer. const std::vector& GetData() const { return data_; } + protected: + /// Create a buffer of |type|. + explicit Buffer(BufferType type); + private: BufferType buffer_type_; - DatumType datum_type_; std::vector data_; std::string name_; size_t size_ = 0; + uint8_t location_ = 0; +}; + +/// A buffer class where the data is described by a |DatumType| object. +class DataBuffer : public Buffer { + public: + explicit DataBuffer(BufferType type); + ~DataBuffer() override; + + bool IsDataBuffer() const override { return true; } + size_t GetSizeInBytes() const override { + return GetSize() * datum_type_.SizeInBytes(); + } + void SetData(std::vector&& data) override { + SetSize(data.size() / datum_type_.ColumnCount() / datum_type_.RowCount()); + Buffer::SetData(std::move(data)); + } + + /// Sets the DatumType of the buffer to |type|. + void SetDatumType(const DatumType& type) { datum_type_ = type; } + /// Returns the DatumType describing the buffer data. + const DatumType& GetDatumType() const { return datum_type_; } + + private: + DatumType datum_type_; +}; + +/// A buffer class where the data is described by a |format| object. +class FormatBuffer : public Buffer { + public: + explicit FormatBuffer(BufferType type); + ~FormatBuffer() override; + + bool IsFormatBuffer() const override { return true; } + size_t GetSizeInBytes() const override { + return GetSize() * format_->GetByteSize(); + } + void SetData(std::vector&& data) override { + SetSize(data.size()); + Buffer::SetData(std::move(data)); + } + + /// Sets the Format of the buffer to |format|. + void SetFormat(std::unique_ptr format) { + format_ = std::move(format); + } + /// Returns the Format describing the buffer data. + const Format& GetFormat() const { return *(format_.get()); } + + private: + std::unique_ptr format_; }; } // namespace amber diff --git a/src/buffer_test.cc b/src/buffer_test.cc index dedcabafe..6bebdd6c7 100644 --- a/src/buffer_test.cc +++ b/src/buffer_test.cc @@ -21,31 +21,31 @@ namespace amber { using BufferTest = testing::Test; -TEST_F(BufferTest, BufferEmptyByDefault) { - Buffer b(BufferType::kColor); +TEST_F(BufferTest, DataBufferEmptyByDefault) { + DataBuffer b(BufferType::kColor); EXPECT_EQ(static_cast(0U), b.GetSize()); EXPECT_EQ(static_cast(0U), b.GetSizeInBytes()); } -TEST_F(BufferTest, BufferSize) { +TEST_F(BufferTest, DataBufferSize) { DatumType type; type.SetType(DataType::kInt16); - Buffer b(BufferType::kColor); + DataBuffer b(BufferType::kColor); b.SetDatumType(type); b.SetSize(10); EXPECT_EQ(10, b.GetSize()); EXPECT_EQ(2 * 10, b.GetSizeInBytes()); } -TEST_F(BufferTest, BufferSizeFromData) { +TEST_F(BufferTest, DataBufferSizeFromData) { DatumType type; type.SetType(DataType::kInt16); std::vector values; values.resize(5); - Buffer b(BufferType::kColor); + DataBuffer b(BufferType::kColor); b.SetDatumType(type); b.SetData(std::move(values)); @@ -53,14 +53,14 @@ TEST_F(BufferTest, BufferSizeFromData) { EXPECT_EQ(2 * 5, b.GetSizeInBytes()); } -TEST_F(BufferTest, BufferSizeFromDataOverrideSize) { +TEST_F(BufferTest, DataBufferSizeFromDataOverrideSize) { DatumType type; type.SetType(DataType::kInt16); std::vector values; values.resize(5); - Buffer b(BufferType::kColor); + DataBuffer b(BufferType::kColor); b.SetDatumType(type); b.SetSize(20); b.SetData(std::move(values)); @@ -69,13 +69,13 @@ TEST_F(BufferTest, BufferSizeFromDataOverrideSize) { EXPECT_EQ(2 * 5, b.GetSizeInBytes()); } -TEST_F(BufferTest, BufferSizeMatrix) { +TEST_F(BufferTest, DataBufferSizeMatrix) { DatumType type; type.SetType(DataType::kInt16); type.SetRowCount(2); type.SetColumnCount(3); - Buffer b(BufferType::kColor); + DataBuffer b(BufferType::kColor); b.SetDatumType(type); b.SetSize(10); EXPECT_EQ(10, b.GetSize()); diff --git a/src/vkscript/executor.cc b/src/vkscript/executor.cc index cf6de3a0a..c3385e67d 100644 --- a/src/vkscript/executor.cc +++ b/src/vkscript/executor.cc @@ -75,23 +75,12 @@ Result Executor::Execute(Engine* engine, if (!r.IsSuccess()) return r; - // Process VertexData nodes - for (const auto& node : script->Nodes()) { - if (!node->IsVertexData()) - continue; - - const auto data = node->AsVertexData(); - for (size_t i = 0; i < data->SegmentCount(); ++i) { - const auto& header = data->GetHeader(i); - r = engine->SetBuffer(BufferType::kVertex, header.location, - *(header.format), data->GetSegment(i)); - if (!r.IsSuccess()) - return r; - } - } - for (const auto& buf : script->GetBuffers()) { - r = engine->SetBuffer(buf->GetBufferType(), 0, Format(), buf->GetData()); + r = engine->SetBuffer( + buf->GetBufferType(), + buf->GetLocation(), + buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(), + buf->GetData()); if (!r.IsSuccess()) return r; } diff --git a/src/vkscript/nodes.cc b/src/vkscript/nodes.cc index 8bbc9c1aa..4ca2decca 100644 --- a/src/vkscript/nodes.cc +++ b/src/vkscript/nodes.cc @@ -28,10 +28,6 @@ RequireNode* Node::AsRequire() { return static_cast(this); } -VertexDataNode* Node::AsVertexData() { - return static_cast(this); -} - RequireNode::RequireNode() : Node(NodeType::kRequire) {} RequireNode::~RequireNode() = default; @@ -51,13 +47,5 @@ void RequireNode::AddRequirement(Feature feature, requirements_.emplace_back(feature, std::move(format)); } -VertexDataNode::VertexDataNode() : Node(NodeType::kVertexData) {} - -VertexDataNode::~VertexDataNode() = default; - -void VertexDataNode::SetSegment(Header&& header, std::vector&& data) { - data_.push_back({std::move(header), std::move(data)}); -} - } // namespace vkscript } // namespace amber diff --git a/src/vkscript/nodes.h b/src/vkscript/nodes.h index 0442d6884..be8c5ee2f 100644 --- a/src/vkscript/nodes.h +++ b/src/vkscript/nodes.h @@ -31,17 +31,14 @@ namespace amber { namespace vkscript { class RequireNode; -class VertexDataNode; class Node { public: virtual ~Node(); bool IsRequire() const { return node_type_ == NodeType::kRequire; } - bool IsVertexData() const { return node_type_ == NodeType::kVertexData; } RequireNode* AsRequire(); - VertexDataNode* AsVertexData(); protected: explicit Node(NodeType type); @@ -78,32 +75,6 @@ class RequireNode : public Node { std::vector requirements_; }; -class VertexDataNode : public Node { - public: - struct Header { - uint8_t location; - std::unique_ptr format; - }; - - VertexDataNode(); - ~VertexDataNode() override; - - void SetSegment(Header&& header, std::vector&& data); - size_t SegmentCount() const { return data_.size(); } - - const Header& GetHeader(size_t idx) const { return data_[idx].header; } - const std::vector& GetSegment(size_t idx) const { - return data_[idx].buffer; - } - - private: - struct NodeData { - Header header; - std::vector buffer; - }; - std::vector data_; -}; - } // namespace vkscript } // namespace amber diff --git a/src/vkscript/parser.cc b/src/vkscript/parser.cc index 94635e63c..01f4691a7 100644 --- a/src/vkscript/parser.cc +++ b/src/vkscript/parser.cc @@ -296,7 +296,7 @@ Result Parser::ProcessIndicesBlock(const std::string& data) { DatumType type; type.SetType(DataType::kUint16); - auto b = MakeUnique(BufferType::kIndex); + auto b = MakeUnique(BufferType::kIndex); b->SetName("indices"); b->SetDatumType(type); b->SetData(std::move(indices)); @@ -321,7 +321,11 @@ Result Parser::ProcessVertexDataBlock(const std::string& data) { return {}; // Process the header line. - std::vector headers; + struct Header { + uint8_t location; + std::unique_ptr format; + }; + std::vector
headers; while (!token->IsEOL() && !token->IsEOS()) { // Because of the way the tokenizer works we'll see a number then a string // the string will start with a slash which we have to remove. @@ -397,11 +401,14 @@ Result Parser::ProcessVertexDataBlock(const std::string& data) { } } - auto node = MakeUnique(); - for (size_t i = 0; i < headers.size(); ++i) - node->SetSegment(std::move(headers[i]), std::move(values[i])); - - script_->AddVertexData(std::move(node)); + for (size_t i = 0; i < headers.size(); ++i) { + auto buffer = MakeUnique(BufferType::kVertex); + buffer->SetName("Vertices" + std::to_string(i)); + buffer->SetFormat(std::move(headers[i].format)); + buffer->SetLocation(headers[i].location); + buffer->SetData(std::move(values[i])); + script_->AddBuffer(std::move(buffer)); + } return {}; } diff --git a/src/vkscript/parser_test.cc b/src/vkscript/parser_test.cc index 4f2bc10d4..4dc0ad491 100644 --- a/src/vkscript/parser_test.cc +++ b/src/vkscript/parser_test.cc @@ -231,7 +231,10 @@ TEST_F(VkScriptParserTest, IndicesBlock) { ASSERT_EQ(1U, buffers.size()); ASSERT_EQ(BufferType::kIndex, buffers[0]->GetBufferType()); - auto buffer = buffers[0].get(); + auto buffer_ptr = buffers[0].get(); + ASSERT_TRUE(buffer_ptr->IsDataBuffer()); + + auto buffer = buffer_ptr->AsDataBuffer(); EXPECT_TRUE(buffer->GetDatumType().IsUint16()); EXPECT_EQ(3U, buffer->GetSize()); auto& data = buffer->GetData(); @@ -309,24 +312,21 @@ TEST_F(VkScriptParserTest, VertexDataHeaderFormatString) { Result r = parser.ProcessVertexDataBlockForTesting(block); ASSERT_TRUE(r.IsSuccess()) << r.Error(); - auto amber_script = parser.GetScript(); - auto& nodes = ToVkScript(amber_script.get())->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsVertexData()); - - auto* data = nodes[0]->AsVertexData(); - ASSERT_EQ(2U, data->SegmentCount()); + auto script = parser.GetScript(); + const auto& buffers = script->GetBuffers(); + ASSERT_EQ(2U, buffers.size()); - auto& header_0 = data->GetHeader(0); - EXPECT_EQ(static_cast(0U), header_0.location); - EXPECT_EQ(FormatType::kR32G32_SFLOAT, header_0.format->GetFormatType()); - EXPECT_TRUE(data->GetSegment(0).empty()); + ASSERT_EQ(BufferType::kVertex, buffers[0]->GetBufferType()); + EXPECT_EQ(static_cast(0U), buffers[0]->GetLocation()); + EXPECT_EQ(FormatType::kR32G32_SFLOAT, + buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); + EXPECT_TRUE(buffers[0]->GetData().empty()); - auto& header_1 = data->GetHeader(1); - EXPECT_EQ(1U, header_1.location); + ASSERT_EQ(BufferType::kVertex, buffers[1]->GetBufferType()); + EXPECT_EQ(1U, buffers[1]->GetLocation()); EXPECT_EQ(FormatType::kA8B8G8R8_UNORM_PACK32, - header_1.format->GetFormatType()); - EXPECT_TRUE(data->GetSegment(1).empty()); + buffers[1]->AsFormatBuffer()->GetFormat().GetFormatType()); + EXPECT_TRUE(buffers[1]->GetData().empty()); } TEST_F(VkScriptParserTest, VertexDataHeaderGlslString) { @@ -336,34 +336,30 @@ TEST_F(VkScriptParserTest, VertexDataHeaderGlslString) { Result r = parser.ProcessVertexDataBlockForTesting(block); ASSERT_TRUE(r.IsSuccess()) << r.Error(); - auto amber_script = parser.GetScript(); - auto& nodes = ToVkScript(amber_script.get())->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsVertexData()); - - auto* data = nodes[0]->AsVertexData(); - ASSERT_EQ(2U, data->SegmentCount()); - - auto& header_0 = data->GetHeader(0); - EXPECT_EQ(static_cast(0U), header_0.location); - EXPECT_EQ(FormatType::kR32G32_SFLOAT, header_0.format->GetFormatType()); - EXPECT_TRUE(data->GetSegment(0).empty()); + auto script = parser.GetScript(); + const auto& buffers = script->GetBuffers(); + ASSERT_EQ(2U, buffers.size()); - auto& comps1 = header_0.format->GetComponents(); + ASSERT_EQ(BufferType::kVertex, buffers[0]->GetBufferType()); + EXPECT_EQ(static_cast(0U), buffers[0]->GetLocation()); + EXPECT_EQ(FormatType::kR32G32_SFLOAT, + buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); + auto& comps1 = buffers[0]->AsFormatBuffer()->GetFormat().GetComponents(); ASSERT_EQ(2U, comps1.size()); EXPECT_EQ(FormatMode::kSFloat, comps1[0].mode); EXPECT_EQ(FormatMode::kSFloat, comps1[1].mode); + EXPECT_TRUE(buffers[0]->GetData().empty()); - auto& header_1 = data->GetHeader(1); - EXPECT_EQ(1U, header_1.location); - EXPECT_EQ(FormatType::kR32G32B32_SINT, header_1.format->GetFormatType()); - EXPECT_TRUE(data->GetSegment(1).empty()); - - auto& comps2 = header_1.format->GetComponents(); + ASSERT_EQ(BufferType::kVertex, buffers[1]->GetBufferType()); + EXPECT_EQ(1U, buffers[1]->GetLocation()); + EXPECT_EQ(FormatType::kR32G32B32_SINT, + buffers[1]->AsFormatBuffer()->GetFormat().GetFormatType()); + auto& comps2 = buffers[1]->AsFormatBuffer()->GetFormat().GetComponents(); ASSERT_EQ(3U, comps2.size()); EXPECT_EQ(FormatMode::kSInt, comps2[0].mode); EXPECT_EQ(FormatMode::kSInt, comps2[1].mode); EXPECT_EQ(FormatMode::kSInt, comps2[2].mode); + EXPECT_TRUE(buffers[1]->GetData().empty()); } TEST_F(VkScriptParserTest, TestBlock) { @@ -410,23 +406,23 @@ TEST_F(VkScriptParserTest, VertexDataRows) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - auto& nodes = ToVkScript(script.get())->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsVertexData()); + const auto& buffers = script->GetBuffers(); + ASSERT_EQ(2U, buffers.size()); - auto* data = nodes[0]->AsVertexData(); - ASSERT_EQ(2U, data->SegmentCount()); + ASSERT_EQ(BufferType::kVertex, buffers[0]->GetBufferType()); std::vector seg_0 = {-1.f, -1.f, 0.25f, 0.25f, -1.f, 0.25f}; - const auto& values_0 = data->GetSegment(0); + const auto& values_0 = buffers[0]->GetData(); ASSERT_EQ(seg_0.size(), values_0.size()); for (size_t i = 0; i < seg_0.size(); ++i) { ASSERT_TRUE(values_0[i].IsFloat()); EXPECT_FLOAT_EQ(seg_0[i], values_0[i].AsFloat()); } + ASSERT_EQ(BufferType::kVertex, buffers[1]->GetBufferType()); + std::vector seg_1 = {255, 0, 0, 255, 0, 255}; - const auto& values_1 = data->GetSegment(1); + const auto& values_1 = buffers[1]->GetData(); ASSERT_EQ(seg_1.size(), values_1.size()); for (size_t i = 0; i < seg_1.size(); ++i) { ASSERT_TRUE(values_1[i].IsInteger()); @@ -472,15 +468,12 @@ TEST_F(VkScriptParserTest, VertexDataRowsWithHex) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - auto& nodes = ToVkScript(script.get())->Nodes(); - ASSERT_EQ(1U, nodes.size()); - ASSERT_TRUE(nodes[0]->IsVertexData()); - - auto* data = nodes[0]->AsVertexData(); - ASSERT_EQ(1U, data->SegmentCount()); + const auto& buffers = script->GetBuffers(); + ASSERT_EQ(1U, buffers.size()); + ASSERT_EQ(BufferType::kVertex, buffers[0]->GetBufferType()); std::vector seg_0 = {0xff0000ff, 0xffff0000}; - const auto& values_0 = data->GetSegment(0); + const auto& values_0 = buffers[0]->GetData(); ASSERT_EQ(seg_0.size(), values_0.size()); for (size_t i = 0; i < seg_0.size(); ++i) { diff --git a/src/vkscript/script.cc b/src/vkscript/script.cc index 01f7e58ab..722ea79bd 100644 --- a/src/vkscript/script.cc +++ b/src/vkscript/script.cc @@ -36,10 +36,5 @@ void Script::AddRequireNode(std::unique_ptr node) { test_nodes_.push_back(std::move(tn)); } -void Script::AddVertexData(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 a60c77303..fd3e3a983 100644 --- a/src/vkscript/script.h +++ b/src/vkscript/script.h @@ -29,7 +29,6 @@ namespace vkscript { class Node; class RequireNode; -class VertexDataNode; class Script : public amber::Script { public: @@ -37,7 +36,6 @@ class Script : public amber::Script { ~Script() override; void AddRequireNode(std::unique_ptr node); - void AddVertexData(std::unique_ptr node); const std::vector>& Nodes() const { return test_nodes_; From 3b5a95b51996b1da09396e0a735bff2fe1be6288 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 10 Dec 2018 16:57:35 -0500 Subject: [PATCH 03/10] stuff --- src/CMakeLists.txt | 1 - src/dawn/engine_dawn.cc | 4 -- src/dawn/engine_dawn.h | 1 - src/engine.h | 5 --- src/vkscript/executor.cc | 20 ++------- src/vkscript/nodes.cc | 51 ----------------------- src/vkscript/nodes.h | 81 ------------------------------------- src/vkscript/parser.cc | 12 +++++- src/vkscript/script.cc | 6 --- src/vkscript/script.h | 12 ------ src/vulkan/engine_vulkan.cc | 18 --------- src/vulkan/engine_vulkan.h | 1 - 12 files changed, 13 insertions(+), 199 deletions(-) delete mode 100644 src/vkscript/nodes.cc delete mode 100644 src/vkscript/nodes.h 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 c3385e67d..381eaff6c 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()) { @@ -75,6 +60,7 @@ Result Executor::Execute(Engine* engine, if (!r.IsSuccess()) return r; + // Process Buffers for (const auto& buf : script->GetBuffers()) { r = engine->SetBuffer( buf->GetBufferType(), 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..59aaeb2f7 100644 --- a/src/vkscript/parser.cc +++ b/src/vkscript/parser.cc @@ -239,7 +239,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 + pipeline_->AddImageAttachment(std::move(framebuffer)); } else if (feature == Feature::kDepthStencil) { token = tokenizer.NextToken(); if (!token->IsString()) @@ -250,7 +254,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); + pipeline_->AddDepthStencilAttachment(std::move(depthbuffer)); } else if (feature == Feature::kFenceTimeout) { token = tokenizer.NextToken(); if (!token->IsInteger()) 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..9421f2a2d 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(); 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, From 75d8f88ead22143429aeef35b4abe096fa82a384 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 10 Dec 2018 16:58:57 -0500 Subject: [PATCH 04/10] Formatting fixes --- src/buffer.cc | 4 +++- src/buffer.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/buffer.cc b/src/buffer.cc index 93649afb2..b4e392ff4 100644 --- a/src/buffer.cc +++ b/src/buffer.cc @@ -22,7 +22,9 @@ Buffer::~Buffer() = default; DataBuffer* Buffer::AsDataBuffer() { return static_cast(this); } -FormatBuffer* Buffer::AsFormatBuffer() { return static_cast(this); } +FormatBuffer* Buffer::AsFormatBuffer() { + return static_cast(this); +} DataBuffer::DataBuffer(BufferType type) : Buffer(type) {} diff --git a/src/buffer.h b/src/buffer.h index 8dbca832e..a313fcea4 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -16,6 +16,7 @@ #define SRC_BUFFER_H_ #include +#include #include #include #include From a36ab3bbd7f2280faa6a22cfaa46f7eeb1936ad4 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 10 Dec 2018 17:07:22 -0500 Subject: [PATCH 05/10] formatting --- src/buffer.cc | 4 +++- src/buffer.h | 4 +--- src/vkscript/executor.cc | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/buffer.cc b/src/buffer.cc index b4e392ff4..e4736da5e 100644 --- a/src/buffer.cc +++ b/src/buffer.cc @@ -20,7 +20,9 @@ Buffer::Buffer(BufferType type) : buffer_type_(type) {} Buffer::~Buffer() = default; -DataBuffer* Buffer::AsDataBuffer() { return static_cast(this); } +DataBuffer* Buffer::AsDataBuffer() { + return static_cast(this); +} FormatBuffer* Buffer::AsFormatBuffer() { return static_cast(this); diff --git a/src/buffer.h b/src/buffer.h index a313fcea4..771ff0c97 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -72,9 +72,7 @@ class Buffer { /// Sets the data into the buffer. The size will also be updated to be the /// size of the data provided. - virtual void SetData(std::vector&& data) { - data_ = std::move(data); - } + virtual void SetData(std::vector&& data) { data_ = std::move(data); } /// Returns the vector of Values stored in the buffer. const std::vector& GetData() const { return data_; } diff --git a/src/vkscript/executor.cc b/src/vkscript/executor.cc index c3385e67d..cfa6de51a 100644 --- a/src/vkscript/executor.cc +++ b/src/vkscript/executor.cc @@ -77,10 +77,10 @@ Result Executor::Execute(Engine* engine, for (const auto& buf : script->GetBuffers()) { r = engine->SetBuffer( - buf->GetBufferType(), - buf->GetLocation(), - buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(), - buf->GetData()); + buf->GetBufferType(), + buf->GetLocation(), + buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(), + buf->GetData()); if (!r.IsSuccess()) return r; } From 70136a73c367344bfb8ecdcf954e2dd9291a69f6 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 10 Dec 2018 17:17:01 -0500 Subject: [PATCH 06/10] fix tests --- src/vkscript/executor.cc | 4 +- src/vkscript/executor_test.cc | 59 ++++-------------------- src/vkscript/parser.cc | 7 --- src/vkscript/parser_test.cc | 87 +++++++++++------------------------ 4 files changed, 37 insertions(+), 120 deletions(-) diff --git a/src/vkscript/executor.cc b/src/vkscript/executor.cc index d85bb1cca..320930069 100644 --- a/src/vkscript/executor.cc +++ b/src/vkscript/executor.cc @@ -60,7 +60,7 @@ Result Executor::Execute(Engine* engine, 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::kImage && + if (buf->GetBufferType() != BufferType::kColor && buf->GetBufferType() != BufferType::kDepth) { continue; } @@ -83,7 +83,7 @@ Result Executor::Execute(Engine* engine, 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::kImage || + if (buf->GetBufferType() == BufferType::kColor || buf->GetBufferType() == BufferType::kDepth) { continue; } 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/parser.cc b/src/vkscript/parser.cc index df932d922..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()) { @@ -273,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..0f4c0cad8 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) { From da2179d896a16035e6fe0205da454564b733321c Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 11 Dec 2018 09:11:02 -0500 Subject: [PATCH 07/10] Format --- src/vkscript/executor.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vkscript/executor.cc b/src/vkscript/executor.cc index cfa6de51a..f9ee42295 100644 --- a/src/vkscript/executor.cc +++ b/src/vkscript/executor.cc @@ -77,8 +77,7 @@ Result Executor::Execute(Engine* engine, for (const auto& buf : script->GetBuffers()) { r = engine->SetBuffer( - buf->GetBufferType(), - buf->GetLocation(), + buf->GetBufferType(), buf->GetLocation(), buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(), buf->GetData()); if (!r.IsSuccess()) From c24ef07906a1addbde7e004dd837ecc6ea4a2b85 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 11 Dec 2018 10:57:31 -0500 Subject: [PATCH 08/10] Fixes --- Android.mk | 1 - src/vkscript/executor.cc | 8 ++++---- src/vkscript/parser_test.cc | 8 ++++---- 3 files changed, 8 insertions(+), 9 deletions(-) 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/vkscript/executor.cc b/src/vkscript/executor.cc index 7efcc4d0b..ee1631f4c 100644 --- a/src/vkscript/executor.cc +++ b/src/vkscript/executor.cc @@ -66,10 +66,10 @@ Result Executor::Execute(Engine* engine, } Result r = engine->SetBuffer( - buf->GetBufferType(), - buf->GetLocation(), - buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(), - buf->GetData()); + buf->GetBufferType(), + buf->GetLocation(), + buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(), + buf->GetData()); if (!r.IsSuccess()) return r; } diff --git a/src/vkscript/parser_test.cc b/src/vkscript/parser_test.cc index 0f4c0cad8..2a57fe8a1 100644 --- a/src/vkscript/parser_test.cc +++ b/src/vkscript/parser_test.cc @@ -136,7 +136,7 @@ TEST_F(VkScriptParserTest, RequireBlockFramebuffer) { EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); EXPECT_TRUE(buffers[0]->IsFormatBuffer()); EXPECT_EQ(FormatType::kR32G32B32A32_SFLOAT, - buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); + buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); } TEST_F(VkScriptParserTest, RequireBlockDepthStencil) { @@ -152,7 +152,7 @@ TEST_F(VkScriptParserTest, RequireBlockDepthStencil) { EXPECT_EQ(BufferType::kDepth, buffers[0]->GetBufferType()); EXPECT_TRUE(buffers[0]->IsFormatBuffer()); EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, - buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); + buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); } TEST_F(VkScriptParserTest, RequireBlockMultipleLines) { @@ -175,12 +175,12 @@ inheritedQueries # line comment EXPECT_EQ(BufferType::kDepth, buffers[0]->GetBufferType()); EXPECT_TRUE(buffers[0]->IsFormatBuffer()); EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, - buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); + buffers[0]->AsFormatBuffer()->GetFormat().GetFormatType()); EXPECT_EQ(BufferType::kColor, buffers[1]->GetBufferType()); EXPECT_TRUE(buffers[1]->IsFormatBuffer()); EXPECT_EQ(FormatType::kR32G32B32A32_SFLOAT, - buffers[1]->AsFormatBuffer()->GetFormat().GetFormatType()); + buffers[1]->AsFormatBuffer()->GetFormat().GetFormatType()); auto& feats = script->RequiredFeatures(); EXPECT_EQ(Feature::kSparseResidency4Samples, feats[0]); From c666e8c08dd78e211152d0566700a5cdb1312eec Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 11 Dec 2018 11:12:12 -0500 Subject: [PATCH 09/10] fix formatting --- src/vkscript/executor.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vkscript/executor.cc b/src/vkscript/executor.cc index ee1631f4c..1bed1565d 100644 --- a/src/vkscript/executor.cc +++ b/src/vkscript/executor.cc @@ -66,8 +66,7 @@ Result Executor::Execute(Engine* engine, } Result r = engine->SetBuffer( - buf->GetBufferType(), - buf->GetLocation(), + buf->GetBufferType(), buf->GetLocation(), buf->IsFormatBuffer() ? buf->AsFormatBuffer()->GetFormat() : Format(), buf->GetData()); if (!r.IsSuccess()) From 77d2574986d1b01951b3a9691e3fe3991fc59f2c Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 11 Dec 2018 13:28:55 -0500 Subject: [PATCH 10/10] Fixup vulkan compile issue --- src/vulkan/engine_vulkan.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index 3df9dd4f7..7fc7660a9 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -173,8 +173,8 @@ Result EngineVulkan::SetBuffer(BufferType type, const std::vector& values) { // Handle image and depth attachments special as they come in before // the pipeline is created. - if (type == BufferType::kImage || type == BufferType::kDepth) { - if (type == BufferType::kImage) + 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());