From 08ebea824879a73b9b5967f29f1d1b03d5a0abba Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Wed, 27 Nov 2019 15:49:49 -0500 Subject: Remove BufferType from the Buffer class. (#728) The BufferType is a product of the pair. A given buffer could be used as a colour buffer on one pipeline and as an image on the second. This Cl removes the BufferType from the Buffer and always uses the BufferType on the Pipeline BufferInfo. --- src/amberscript/parser.cc | 112 ++++++++++++------------- src/amberscript/parser_bind_test.cc | 24 +++--- src/amberscript/parser_buffer_test.cc | 9 +- src/amberscript/parser_clear_color_test.cc | 2 +- src/amberscript/parser_clear_test.cc | 2 +- src/amberscript/parser_compile_options_test.cc | 6 +- src/amberscript/parser_device_feature_test.cc | 3 +- src/amberscript/parser_expect_test.cc | 8 +- src/amberscript/parser_extension_test.cc | 6 +- src/amberscript/parser_framebuffer_test.cc | 3 +- src/amberscript/parser_pipeline_set_test.cc | 2 +- src/amberscript/parser_pipeline_test.cc | 2 +- src/amberscript/parser_run_test.cc | 4 +- src/amberscript/parser_set_test.cc | 2 +- src/amberscript/parser_shader_opt_test.cc | 3 +- src/amberscript/parser_shader_test.cc | 4 +- src/buffer.cc | 2 - src/buffer.h | 8 -- src/buffer_test.cc | 36 ++++---- src/dawn/engine_dawn.cc | 10 +-- src/pipeline.cc | 48 ++++++----- src/pipeline.h | 12 ++- src/pipeline_test.cc | 34 ++++---- src/script_test.cc | 12 +-- src/vkscript/command_parser.cc | 10 +-- src/vkscript/parser.cc | 4 +- src/vkscript/parser_test.cc | 110 ++++++++++-------------- src/vulkan/device.cc | 4 +- src/vulkan/device.h | 2 +- src/vulkan/engine_vulkan.cc | 6 +- src/vulkan/engine_vulkan.h | 5 -- 31 files changed, 238 insertions(+), 257 deletions(-) diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc index 103091b..b4e6fdb 100644 --- a/src/amberscript/parser.cc +++ b/src/amberscript/parser.cc @@ -302,7 +302,8 @@ Result Parser::ValidateEndOfStatement(const std::string& name) { auto token = tokenizer_->NextToken(); if (token->IsEOL() || token->IsEOS()) return {}; - return Result("extra parameters after " + name); + return Result("extra parameters after " + name + ": " + + token->ToOriginalString()); } Result Parser::ParseShaderBlock() { @@ -520,7 +521,8 @@ Result Parser::ParsePipelineAttach(Pipeline* pipeline) { return {}; if (token->IsString()) return Result("unknown ATTACH parameter: " + token->AsString()); - return Result("extra parameters after ATTACH command"); + return Result("extra parameters after ATTACH command: " + + token->ToOriginalString()); } } } @@ -585,7 +587,8 @@ Result Parser::ParsePipelineShaderOptimizations(Pipeline* pipeline) { token = tokenizer_->NextToken(); if (!token->IsEOL()) - return Result("extra parameters after SHADER_OPTIMIZATION command"); + return Result("extra parameters after SHADER_OPTIMIZATION command: " + + token->ToOriginalString()); std::vector optimizations; while (true) { @@ -624,7 +627,8 @@ Result Parser::ParsePipelineShaderCompileOptions(Pipeline* pipeline) { token = tokenizer_->NextToken(); if (!token->IsEOL()) - return Result("extra parameters after COMPILE_OPTIONS command"); + return Result("extra parameters after COMPILE_OPTIONS command: " + + token->ToOriginalString()); std::vector options; while (true) { @@ -668,7 +672,15 @@ Result Parser::ParsePipelineFramebufferSize(Pipeline* pipeline) { Result Parser::ToBufferType(const std::string& name, BufferType* type) { assert(type); - if (name == "uniform") + if (name == "color") + *type = BufferType::kColor; + else if (name == "depth_stencil") + *type = BufferType::kDepth; + else if (name == "push_constant") + *type = BufferType::kPushConstant; + else if (name == "combined_image_sampler") + *type = BufferType::kCombinedImageSampler; + else if (name == "uniform") *type = BufferType::kUniform; else if (name == "storage") *type = BufferType::kStorage; @@ -701,13 +713,18 @@ Result Parser::ParsePipelineBind(Pipeline* pipeline) { if (!buffer) return Result("unknown buffer: " + token->AsString()); + BufferType buffer_type = BufferType::kUnknown; token = tokenizer_->NextToken(); if (token->IsString() && token->AsString() == "AS") { token = tokenizer_->NextToken(); if (!token->IsString()) return Result("invalid token for BUFFER type"); - if (token->AsString() == "color") { + Result r = ToBufferType(token->AsString(), &buffer_type); + if (!r.IsSuccess()) + return r; + + if (buffer_type == BufferType::kColor) { token = tokenizer_->NextToken(); if (!token->IsString() || token->AsString() != "LOCATION") return Result("BIND missing LOCATION"); @@ -716,28 +733,21 @@ Result Parser::ParsePipelineBind(Pipeline* pipeline) { if (!token->IsInteger()) return Result("invalid value for BIND LOCATION"); - buffer->SetBufferType(BufferType::kColor); - - Result r = pipeline->AddColorAttachment(buffer, token->AsUint32()); + r = pipeline->AddColorAttachment(buffer, token->AsUint32()); if (!r.IsSuccess()) return r; - } else if (token->AsString() == "depth_stencil") { - buffer->SetBufferType(BufferType::kDepth); - Result r = pipeline->SetDepthBuffer(buffer); + + } else if (buffer_type == BufferType::kDepth) { + r = pipeline->SetDepthBuffer(buffer); if (!r.IsSuccess()) return r; - } else if (token->AsString() == "push_constant") { - buffer->SetBufferType(BufferType::kPushConstant); - Result r = pipeline->SetPushConstantBuffer(buffer); + + } else if (buffer_type == BufferType::kPushConstant) { + r = pipeline->SetPushConstantBuffer(buffer); if (!r.IsSuccess()) return r; - } else if (token->AsString() == "storage_image") { - buffer->SetBufferType(BufferType::kStorageImage); - } else if (token->AsString() == "sampled_image") { - buffer->SetBufferType(BufferType::kSampledImage); - } else if (token->AsString() == "combined_image_sampler") { - buffer->SetBufferType(BufferType::kCombinedImageSampler); + } else if (buffer_type == BufferType::kCombinedImageSampler) { token = tokenizer_->NextToken(); if (!token->IsString() || token->AsString() != "SAMPLER") return Result("expecting SAMPLER for combined image sampler"); @@ -751,31 +761,25 @@ Result Parser::ParsePipelineBind(Pipeline* pipeline) { return Result("unknown sampler: " + token->AsString()); buffer->SetSampler(sampler); - } else { - BufferType type = BufferType::kColor; - Result r = ToBufferType(token->AsString(), &type); - if (!r.IsSuccess()) - return r; - - if (buffer->GetBufferType() == BufferType::kUnknown) - buffer->SetBufferType(type); - else if (buffer->GetBufferType() != type) - return Result("buffer type does not match intended usage"); } } - if (buffer->GetBufferType() == BufferType::kUnknown || - buffer->GetBufferType() == BufferType::kStorage || - buffer->GetBufferType() == BufferType::kUniform || - buffer->GetBufferType() == BufferType::kStorageImage || - buffer->GetBufferType() == BufferType::kSampledImage || - buffer->GetBufferType() == BufferType::kCombinedImageSampler) { - // If AS was parsed above consume the next token. - if (buffer->GetBufferType() != BufferType::kUnknown) + // The OpenCL bindings can be typeless which allows for the kUnknown + // buffer type. + if (buffer_type == BufferType::kUnknown || + buffer_type == BufferType::kStorage || + buffer_type == BufferType::kUniform || + buffer_type == BufferType::kStorageImage || + buffer_type == BufferType::kSampledImage || + buffer_type == BufferType::kCombinedImageSampler) { + // If the buffer type is known, then we proccessed the AS block above + // and have to advance to the next token. Otherwise, we're already on the + // next token and don't want to advance. + if (buffer_type != BufferType::kUnknown) token = tokenizer_->NextToken(); + // DESCRIPTOR_SET requires a buffer type to have been specified. - if (buffer->GetBufferType() != BufferType::kUnknown && - token->IsString() && token->AsString() == "DESCRIPTOR_SET") { + if (token->IsString() && token->AsString() == "DESCRIPTOR_SET") { token = tokenizer_->NextToken(); if (!token->IsInteger()) return Result("invalid value for DESCRIPTOR_SET in BIND command"); @@ -788,7 +792,9 @@ Result Parser::ParsePipelineBind(Pipeline* pipeline) { token = tokenizer_->NextToken(); if (!token->IsInteger()) return Result("invalid value for BINDING in BIND command"); - pipeline->AddBuffer(buffer, descriptor_set, token->AsUint32()); + + pipeline->AddBuffer(buffer, buffer_type, descriptor_set, + token->AsUint32()); } else if (token->IsString() && token->AsString() == "KERNEL") { token = tokenizer_->NextToken(); if (!token->IsString()) @@ -799,13 +805,13 @@ Result Parser::ParsePipelineBind(Pipeline* pipeline) { if (!token->IsString()) return Result("expected argument identifier"); - pipeline->AddBuffer(buffer, token->AsString()); + pipeline->AddBuffer(buffer, buffer_type, token->AsString()); } else if (token->AsString() == "ARG_NUMBER") { token = tokenizer_->NextToken(); if (!token->IsInteger()) return Result("expected argument number"); - pipeline->AddBuffer(buffer, token->AsUint32()); + pipeline->AddBuffer(buffer, buffer_type, token->AsUint32()); } else { return Result("missing ARG_NAME or ARG_NUMBER keyword"); } @@ -865,7 +871,6 @@ Result Parser::ParsePipelineVertexData(Pipeline* pipeline) { if (!token->IsInteger()) return Result("invalid value for VERTEX_DATA LOCATION"); - buffer->SetBufferType(BufferType::kVertex); Result r = pipeline->AddVertexBuffer(buffer, token->AsUint32()); if (!r.IsSuccess()) return r; @@ -882,7 +887,6 @@ Result Parser::ParsePipelineIndexData(Pipeline* pipeline) { if (!buffer) return Result("unknown buffer: " + token->AsString()); - buffer->SetBufferType(BufferType::kIndex); Result r = pipeline->SetIndexBuffer(buffer); if (!r.IsSuccess()) return r; @@ -1819,7 +1823,8 @@ Result Parser::ParseExpect() { } if (!token->IsEOL() && !token->IsEOS()) { - return Result("extra parameters after EXPECT command"); + return Result("extra parameters after EXPECT command: " + + token->ToOriginalString()); } command_list_.push_back(std::move(probe)); @@ -1919,16 +1924,11 @@ Result Parser::ParseCopy() { if (!buffer_to) return Result("COPY destination buffer was not declared"); - if (buffer_to->GetBufferType() == amber::BufferType::kUnknown) { - // Set destination buffer to mirror origin buffer - buffer_to->SetBufferType(buffer_from->GetBufferType()); - buffer_to->SetWidth(buffer_from->GetWidth()); - buffer_to->SetHeight(buffer_from->GetHeight()); - buffer_to->SetElementCount(buffer_from->ElementCount()); - } + // Set destination buffer to mirror origin buffer + buffer_to->SetWidth(buffer_from->GetWidth()); + buffer_to->SetHeight(buffer_from->GetHeight()); + buffer_to->SetElementCount(buffer_from->ElementCount()); - if (buffer_from->GetBufferType() != buffer_to->GetBufferType()) - return Result("cannot COPY between buffers of different types"); if (buffer_from == buffer_to) return Result("COPY origin and destination buffers are identical"); diff --git a/src/amberscript/parser_bind_test.cc b/src/amberscript/parser_bind_test.cc index e22ae8a..ffbdcc4 100644 --- a/src/amberscript/parser_bind_test.cc +++ b/src/amberscript/parser_bind_test.cc @@ -199,7 +199,7 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("12: extra parameters after BIND command", r.Error()); + EXPECT_EQ("12: extra parameters after BIND command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, BindColorBufferDuplicateLocation) { @@ -435,7 +435,7 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("12: extra parameters after BIND command", r.Error()); + EXPECT_EQ("12: extra parameters after BIND command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, BindBufferMissingBufferName) { @@ -734,7 +734,7 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("12: extra parameters after VERTEX_DATA command", r.Error()); + EXPECT_EQ("12: extra parameters after VERTEX_DATA command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, BindIndexData) { @@ -824,7 +824,7 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("12: extra parameters after INDEX_DATA command", r.Error()); + EXPECT_EQ("12: extra parameters after INDEX_DATA command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, BindIndexDataMultiple) { @@ -876,7 +876,7 @@ END const auto* pipeline = pipelines[0].get(); const auto& bufs = pipeline->GetBuffers(); ASSERT_EQ(1U, bufs.size()); - EXPECT_EQ(BufferType::kUniform, bufs[0].buffer->GetBufferType()); + EXPECT_EQ(BufferType::kUniform, bufs[0].type); EXPECT_EQ(1U, bufs[0].descriptor_set); EXPECT_EQ(2U, bufs[0].binding); EXPECT_EQ(static_cast(0), bufs[0].location); @@ -986,7 +986,7 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("12: extra parameters after BIND command", r.Error()); + EXPECT_EQ("12: extra parameters after BIND command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, BindingBufferInvalidBindingValue) { @@ -1087,7 +1087,7 @@ PIPELINE graphics my_pipeline const auto* pipeline = pipelines[0].get(); const auto& bufs = pipeline->GetBuffers(); ASSERT_EQ(1U, bufs.size()); - EXPECT_EQ(test_data.type, bufs[0].buffer->GetBufferType()); + EXPECT_EQ(test_data.type, bufs[0].type); } INSTANTIATE_TEST_SUITE_P( AmberScriptParserBufferTypeTestSamples, @@ -1146,7 +1146,7 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("12: extra parameters after BIND command", r.Error()); + EXPECT_EQ("12: extra parameters after BIND command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, BindBufferOpenCLArgName) { @@ -1349,7 +1349,7 @@ END)"; const auto* pipeline = pipelines[0].get(); const auto& bufs = pipeline->GetBuffers(); ASSERT_EQ(1U, bufs.size()); - EXPECT_EQ(BufferType::kStorageImage, bufs[0].buffer->GetBufferType()); + EXPECT_EQ(BufferType::kStorageImage, bufs[0].type); } TEST_F(AmberScriptParserTest, BindBufferStorageImageGraphics) { @@ -1380,7 +1380,7 @@ END)"; const auto* pipeline = pipelines[0].get(); const auto& bufs = pipeline->GetBuffers(); ASSERT_EQ(1U, bufs.size()); - EXPECT_EQ(BufferType::kStorageImage, bufs[0].buffer->GetBufferType()); + EXPECT_EQ(BufferType::kStorageImage, bufs[0].type); } TEST_F(AmberScriptParserTest, BindBufferStorageImageMissingDescriptorSetValue) { @@ -1487,7 +1487,7 @@ END)"; const auto* pipeline = pipelines[0].get(); const auto& bufs = pipeline->GetBuffers(); ASSERT_EQ(1U, bufs.size()); - EXPECT_EQ(BufferType::kSampledImage, bufs[0].buffer->GetBufferType()); + EXPECT_EQ(BufferType::kSampledImage, bufs[0].type); } TEST_F(AmberScriptParserTest, BindBufferSampledImageMissingDescriptorSetValue) { @@ -1603,7 +1603,7 @@ END)"; const auto* pipeline = pipelines[0].get(); const auto& bufs = pipeline->GetBuffers(); ASSERT_EQ(1U, bufs.size()); - EXPECT_EQ(BufferType::kCombinedImageSampler, bufs[0].buffer->GetBufferType()); + EXPECT_EQ(BufferType::kCombinedImageSampler, bufs[0].type); } TEST_F(AmberScriptParserTest, diff --git a/src/amberscript/parser_buffer_test.cc b/src/amberscript/parser_buffer_test.cc index 1381ec8..16609fb 100644 --- a/src/amberscript/parser_buffer_test.cc +++ b/src/amberscript/parser_buffer_test.cc @@ -650,14 +650,15 @@ INSTANTIATE_TEST_SUITE_P( BufferParseError{ "BUFFER my_index_buffer DATA_TYPE int32 DATA INVALID\n123\nEND", "1: invalid BUFFER data value: INVALID"}, - BufferParseError{"BUFFER my_index_buffer DATA_TYPE int32 SIZE 256 FILL " - "5 INVALID\n123\nEND", - "1: extra parameters after BUFFER fill command"}, + BufferParseError{ + "BUFFER my_index_buffer DATA_TYPE int32 SIZE 256 FILL " + "5 INVALID\n123\nEND", + "1: extra parameters after BUFFER fill command: INVALID"}, BufferParseError{ "BUFFER my_buffer DATA_TYPE int32 SIZE 256 SERIES_FROM 2 " "INC_BY 5 " "INVALID", - "1: extra parameters after BUFFER series_from command"}, + "1: extra parameters after BUFFER series_from command: INVALID"}, BufferParseError{"BUFFER my_buf DATA_TYPE int32 SIZE 5 FILL 5\nBUFFER " "my_buf DATA_TYPE int16 SIZE 5 FILL 2", // NOLINTNEXTLINE(whitespace/parens) diff --git a/src/amberscript/parser_clear_color_test.cc b/src/amberscript/parser_clear_color_test.cc index 95d6e46..685d67b 100644 --- a/src/amberscript/parser_clear_color_test.cc +++ b/src/amberscript/parser_clear_color_test.cc @@ -140,7 +140,7 @@ INSTANTIATE_TEST_SUITE_P( ClearColorTestData{"255 255 255 INVALID", "invalid A value for CLEAR_COLOR command: INVALID"}, ClearColorTestData{"255 255 255 255 EXTRA", - "extra parameters after CLEAR_COLOR command"}, + "extra parameters after CLEAR_COLOR command: EXTRA"}, ClearColorTestData{"-1 255 255 255", "invalid R value for CLEAR_COLOR command: -1"}, ClearColorTestData{"5.2 255 255 255", diff --git a/src/amberscript/parser_clear_test.cc b/src/amberscript/parser_clear_test.cc index 859ee2d..1eb812d 100644 --- a/src/amberscript/parser_clear_test.cc +++ b/src/amberscript/parser_clear_test.cc @@ -112,7 +112,7 @@ CLEAR my_pipeline EXTRA)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("12: extra parameters after CLEAR command", r.Error()); + EXPECT_EQ("12: extra parameters after CLEAR command: EXTRA", r.Error()); } } // namespace amberscript diff --git a/src/amberscript/parser_compile_options_test.cc b/src/amberscript/parser_compile_options_test.cc index aa181dc..fa45545 100644 --- a/src/amberscript/parser_compile_options_test.cc +++ b/src/amberscript/parser_compile_options_test.cc @@ -124,7 +124,8 @@ END Parser parser; auto r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("7: extra parameters after COMPILE_OPTIONS command", r.Error()); + EXPECT_EQ("7: extra parameters after COMPILE_OPTIONS command: extra", + r.Error()); } TEST_F(AmberScriptParserTest, PipelineShaderCompileOptionsExtraTokenEnd) { @@ -142,7 +143,8 @@ END Parser parser; auto r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("8: extra parameters after COMPILE_OPTIONS command", r.Error()); + EXPECT_EQ("8: extra parameters after COMPILE_OPTIONS command: token", + r.Error()); } TEST_F(AmberScriptParserTest, PipelineShaderCompileOptionsNotOpenCL) { diff --git a/src/amberscript/parser_device_feature_test.cc b/src/amberscript/parser_device_feature_test.cc index e7a567a..a15a602 100644 --- a/src/amberscript/parser_device_feature_test.cc +++ b/src/amberscript/parser_device_feature_test.cc @@ -70,7 +70,8 @@ TEST_F(AmberScriptParserTest, DeviceFeatureExtraParams) { Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("1: extra parameters after DEVICE_FEATURE command", r.Error()); + EXPECT_EQ("1: extra parameters after DEVICE_FEATURE command: EXTRA", + r.Error()); } } // namespace amberscript diff --git a/src/amberscript/parser_expect_test.cc b/src/amberscript/parser_expect_test.cc index 590b8f7..fccd491 100644 --- a/src/amberscript/parser_expect_test.cc +++ b/src/amberscript/parser_expect_test.cc @@ -609,7 +609,7 @@ EXPECT my_fb IDX 0 0 SIZE 250 250 EQ_RGB 0 128 255 EXTRA)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("15: extra parameters after EXPECT command", r.Error()); + EXPECT_EQ("15: extra parameters after EXPECT command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, ExpectRGBAExtraParam) { @@ -632,7 +632,7 @@ EXPECT my_fb IDX 0 0 SIZE 250 250 EQ_RGBA 0 128 255 99 EXTRA)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("15: extra parameters after EXPECT command", r.Error()); + EXPECT_EQ("15: extra parameters after EXPECT command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, ExpectEQ) { @@ -1069,7 +1069,7 @@ EXPECT buf IDX 80 80 SIZE 5 8 EQ_RGBA 128 0 128 255 TOLERANCE 3 FOO)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("3: extra parameters after EXPECT command", r.Error()); + EXPECT_EQ("3: extra parameters after EXPECT command: FOO", r.Error()); } TEST_F(AmberScriptParserTest, ExpectEqRgbToleranceOneValue) { @@ -1152,7 +1152,7 @@ EXPECT buf IDX 80 80 SIZE 5 8 EQ_RGB 128 0 128 TOLERANCE 3 FOO)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("3: extra parameters after EXPECT command", r.Error()); + EXPECT_EQ("3: extra parameters after EXPECT command: FOO", r.Error()); } TEST_F(AmberScriptParserTest, ExpectRMSEBuffer) { diff --git a/src/amberscript/parser_extension_test.cc b/src/amberscript/parser_extension_test.cc index 28354f5..0a1c8f3 100644 --- a/src/amberscript/parser_extension_test.cc +++ b/src/amberscript/parser_extension_test.cc @@ -76,7 +76,8 @@ TEST_F(AmberScriptParserTest, ExtensionInstanceExtraParams) { Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("1: extra parameters after INSTANCE_EXTENSION command", r.Error()); + EXPECT_EQ("1: extra parameters after INSTANCE_EXTENSION command: EXTRA", + r.Error()); } TEST_F(AmberScriptParserTest, ExtensionDevice) { @@ -132,7 +133,8 @@ TEST_F(AmberScriptParserTest, ExtensionDeviceExtraParams) { Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("1: extra parameters after DEVICE_EXTENSION command", r.Error()); + EXPECT_EQ("1: extra parameters after DEVICE_EXTENSION command: EXTRA", + r.Error()); } } // namespace amberscript diff --git a/src/amberscript/parser_framebuffer_test.cc b/src/amberscript/parser_framebuffer_test.cc index 5707473..27dc0ce 100644 --- a/src/amberscript/parser_framebuffer_test.cc +++ b/src/amberscript/parser_framebuffer_test.cc @@ -128,7 +128,8 @@ END Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("9: extra parameters after FRAMEBUFFER_SIZE command", r.Error()); + EXPECT_EQ("9: extra parameters after FRAMEBUFFER_SIZE command: INVALID", + r.Error()); } TEST_F(AmberScriptParserTest, FramebufferInvalidWidth) { diff --git a/src/amberscript/parser_pipeline_set_test.cc b/src/amberscript/parser_pipeline_set_test.cc index b35b35e..7d78ca7 100644 --- a/src/amberscript/parser_pipeline_set_test.cc +++ b/src/amberscript/parser_pipeline_set_test.cc @@ -153,7 +153,7 @@ END Parser parser; auto r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("7: extra parameters after SET command", r.Error()); + EXPECT_EQ("7: extra parameters after SET command: BLAH", r.Error()); } TEST_F(AmberScriptParserTest, OpenCLSetArgNameNotString) { diff --git a/src/amberscript/parser_pipeline_test.cc b/src/amberscript/parser_pipeline_test.cc index ebd2ddd..98c85ce 100644 --- a/src/amberscript/parser_pipeline_test.cc +++ b/src/amberscript/parser_pipeline_test.cc @@ -86,7 +86,7 @@ END Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("2: extra parameters after PIPELINE command", r.Error()); + EXPECT_EQ("2: extra parameters after PIPELINE command: INVALID", r.Error()); } TEST_F(AmberScriptParserTest, PipelineInvalidType) { diff --git a/src/amberscript/parser_run_test.cc b/src/amberscript/parser_run_test.cc index e4f0bd5..aa560b4 100644 --- a/src/amberscript/parser_run_test.cc +++ b/src/amberscript/parser_run_test.cc @@ -125,7 +125,7 @@ RUN my_pipeline 2 4 5 EXTRA)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - ASSERT_EQ("12: extra parameters after RUN command", r.Error()); + ASSERT_EQ("12: extra parameters after RUN command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, RunComputeInvalidZ) { @@ -486,7 +486,7 @@ RUN my_pipeline DRAW_RECT POS 2 4 SIZE 10 20 EXTRA)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - ASSERT_EQ("12: extra parameters after RUN command", r.Error()); + ASSERT_EQ("12: extra parameters after RUN command: EXTRA", r.Error()); } TEST_F(AmberScriptParserTest, RunDrawArrays) { diff --git a/src/amberscript/parser_set_test.cc b/src/amberscript/parser_set_test.cc index 1796402..f6068a2 100644 --- a/src/amberscript/parser_set_test.cc +++ b/src/amberscript/parser_set_test.cc @@ -94,7 +94,7 @@ TEST_F(AmberScriptParserTest, SetFenceTimeoutExtraParams) { Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("1: extra parameters after SET command", r.Error()); + EXPECT_EQ("1: extra parameters after SET command: EXTRA", r.Error()); } } // namespace amberscript diff --git a/src/amberscript/parser_shader_opt_test.cc b/src/amberscript/parser_shader_opt_test.cc index 5aff3e4..19eeed6 100644 --- a/src/amberscript/parser_shader_opt_test.cc +++ b/src/amberscript/parser_shader_opt_test.cc @@ -150,7 +150,8 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("5: extra parameters after SHADER_OPTIMIZATION command", r.Error()); + EXPECT_EQ("5: extra parameters after SHADER_OPTIMIZATION command: EXTRA", + r.Error()); } TEST_F(AmberScriptParserTest, PipelineShaderOptimizationNonStringParam) { diff --git a/src/amberscript/parser_shader_test.cc b/src/amberscript/parser_shader_test.cc index 4858568..4c2ca69 100644 --- a/src/amberscript/parser_shader_test.cc +++ b/src/amberscript/parser_shader_test.cc @@ -119,7 +119,7 @@ TEST_F(AmberScriptParserTest, ShaderPassThroughExtraParameters) { Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("1: extra parameters after SHADER PASSTHROUGH", r.Error()); + EXPECT_EQ("1: extra parameters after SHADER PASSTHROUGH: INVALID", r.Error()); } TEST_F(AmberScriptParserTest, Shader) { @@ -232,7 +232,7 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("2: extra parameters after SHADER command", r.Error()); + EXPECT_EQ("2: extra parameters after SHADER command: INVALID", r.Error()); } struct ShaderTypeData { diff --git a/src/buffer.cc b/src/buffer.cc index f5d2270..0b15b5f 100644 --- a/src/buffer.cc +++ b/src/buffer.cc @@ -100,8 +100,6 @@ double CalculateDiff(const Format::Segment* seg, Buffer::Buffer() = default; -Buffer::Buffer(BufferType type) : buffer_type_(type) {} - Buffer::~Buffer() = default; Result Buffer::CopyTo(Buffer* buffer) const { diff --git a/src/buffer.h b/src/buffer.h index 2c60104..d8dc109 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -61,16 +61,9 @@ class Buffer { public: /// Create a buffer of unknown type. Buffer(); - /// Create a buffer of |type_|. - explicit Buffer(BufferType type); ~Buffer(); - /// Returns the BufferType of this buffer. - BufferType GetBufferType() const { return buffer_type_; } - /// Sets the BufferType for this buffer. - void SetBufferType(BufferType type) { buffer_type_ = type; } - /// Sets the Format of the buffer to |format|. void SetFormat(Format* format) { format_is_default_ = false; @@ -228,7 +221,6 @@ class Buffer { // those stored in |buffer| and returns all the values. std::vector CalculateDiffs(const Buffer* buffer) const; - BufferType buffer_type_ = BufferType::kUnknown; std::string name_; /// max_size_in_bytes_ is the total size in bytes needed to hold the buffer /// over all ubo, ssbo size and ssbo subdata size calls. diff --git a/src/buffer_test.cc b/src/buffer_test.cc index 916e10a..9e5f614 100644 --- a/src/buffer_test.cc +++ b/src/buffer_test.cc @@ -25,7 +25,7 @@ namespace amber { using BufferTest = testing::Test; TEST_F(BufferTest, EmptyByDefault) { - Buffer b(BufferType::kColor); + Buffer b; EXPECT_EQ(static_cast(0U), b.ElementCount()); EXPECT_EQ(static_cast(0U), b.ValueCount()); EXPECT_EQ(static_cast(0U), b.GetSizeInBytes()); @@ -36,7 +36,7 @@ TEST_F(BufferTest, Size) { auto type = parser.Parse("R16_SINT"); Format fmt(type.get()); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetElementCount(10); EXPECT_EQ(10, b.ElementCount()); @@ -52,7 +52,7 @@ TEST_F(BufferTest, SizeFromData) { auto type = parser.Parse("R32_SFLOAT"); Format fmt(type.get()); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetData(std::move(values)); @@ -69,7 +69,7 @@ TEST_F(BufferTest, SizeFromDataDoesNotOverrideSize) { auto type = parser.Parse("R32_SFLOAT"); Format fmt(type.get()); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetElementCount(20); b.SetData(std::move(values)); @@ -85,7 +85,7 @@ TEST_F(BufferTest, SizeMatrixStd430) { type->SetColumnCount(3); Format fmt(type.get()); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetElementCount(10); @@ -101,7 +101,7 @@ TEST_F(BufferTest, SizeMatrixStd140) { Format fmt(type.get()); fmt.SetLayout(Format::Layout::kStd140); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetElementCount(10); @@ -116,7 +116,7 @@ TEST_F(BufferTest, SizeMatrixPaddedStd430) { type->SetColumnCount(3); Format fmt(type.get()); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetValueCount(9); @@ -138,7 +138,7 @@ TEST_F(BufferTest, GetHistogramForChannelGradient) { for (uint32_t i = 0; i < values.size(); i += 4) values[i + 2].SetIntValue(i / 4 * 25); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetData(values); @@ -158,7 +158,7 @@ TEST_F(BufferTest, GetHistogramForChannelAllBlack) { for (uint32_t i = 0; i < values.size(); i++) values[i].SetIntValue(0); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetData(values); @@ -181,7 +181,7 @@ TEST_F(BufferTest, GetHistogramForChannelAllWhite) { for (uint32_t i = 0; i < values.size(); i++) values[i].SetIntValue(std::numeric_limits::max()); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetData(values); @@ -208,11 +208,11 @@ TEST_F(BufferTest, CompareHistogramEMDToleranceFalse) { std::vector values2 = values1; values2[4].SetIntValue(values2[4].AsUint8() + 50); - Buffer b1(BufferType::kColor); + Buffer b1; b1.SetFormat(&fmt); b1.SetData(values1); - Buffer b2(BufferType::kColor); + Buffer b2; b2.SetFormat(&fmt); b2.SetData(values2); @@ -235,11 +235,11 @@ TEST_F(BufferTest, CompareHistogramEMDToleranceTrue) { std::vector values2 = values1; values2[4].SetIntValue(values2[4].AsUint8() + 50); - Buffer b1(BufferType::kColor); + Buffer b1; b1.SetFormat(&fmt); b1.SetData(values1); - Buffer b2(BufferType::kColor); + Buffer b2; b2.SetFormat(&fmt); b2.SetData(values2); @@ -259,11 +259,11 @@ TEST_F(BufferTest, CompareHistogramEMDToleranceAllBlack) { std::vector values2 = values1; - Buffer b1(BufferType::kColor); + Buffer b1; b1.SetFormat(&fmt); b1.SetData(values1); - Buffer b2(BufferType::kColor); + Buffer b2; b2.SetFormat(&fmt); b2.SetData(values2); @@ -283,11 +283,11 @@ TEST_F(BufferTest, CompareHistogramEMDToleranceAllWhite) { std::vector values2 = values1; - Buffer b1(BufferType::kColor); + Buffer b1; b1.SetFormat(&fmt); b1.SetData(values1); - Buffer b2(BufferType::kColor); + Buffer b2; b2.SetFormat(&fmt); b2.SetData(values2); diff --git a/src/dawn/engine_dawn.cc b/src/dawn/engine_dawn.cc index 868e979..ba92016 100644 --- a/src/dawn/engine_dawn.cc +++ b/src/dawn/engine_dawn.cc @@ -1616,7 +1616,7 @@ Result EngineDawn::AttachBuffersAndTextures( for (const auto& buf_info : render_pipeline->pipeline->GetBuffers()) { ::dawn::BufferUsage bufferUsage; ::dawn::BindingType bindingType; - switch (buf_info.buffer->GetBufferType()) { + switch (buf_info.type) { case BufferType::kStorage: { bufferUsage = ::dawn::BufferUsage::Storage; bindingType = ::dawn::BindingType::StorageBuffer; @@ -1629,8 +1629,7 @@ Result EngineDawn::AttachBuffersAndTextures( } default: { return Result("AttachBuffersAndTextures: unknown buffer type: " + - std::to_string(static_cast( - buf_info.buffer->GetBufferType()))); + std::to_string(static_cast(buf_info.type))); break; } } @@ -1716,7 +1715,7 @@ Result EngineDawn::AttachBuffers(ComputePipelineInfo* compute_pipeline) { for (const auto& buf_info : compute_pipeline->pipeline->GetBuffers()) { ::dawn::BufferUsage bufferUsage; ::dawn::BindingType bindingType; - switch (buf_info.buffer->GetBufferType()) { + switch (buf_info.type) { case BufferType::kStorage: { bufferUsage = ::dawn::BufferUsage::Storage; bindingType = ::dawn::BindingType::StorageBuffer; @@ -1729,8 +1728,7 @@ Result EngineDawn::AttachBuffers(ComputePipelineInfo* compute_pipeline) { } default: { return Result("AttachBuffers: unknown buffer type: " + - std::to_string(static_cast( - buf_info.buffer->GetBufferType()))); + std::to_string(static_cast(buf_info.type))); break; } } diff --git a/src/pipeline.cc b/src/pipeline.cc index 46ddea4..59f3042 100644 --- a/src/pipeline.cc +++ b/src/pipeline.cc @@ -247,6 +247,8 @@ Result Pipeline::AddColorAttachment(Buffer* buf, uint32_t location) { auto& info = color_attachments_.back(); info.location = location; + info.type = BufferType::kColor; + buf->SetWidth(fb_width_); buf->SetHeight(fb_height_); buf->SetElementCount(fb_width_ * fb_height_); @@ -267,10 +269,10 @@ Result Pipeline::GetLocationForColorAttachment(Buffer* buf, Result Pipeline::SetDepthBuffer(Buffer* buf) { if (depth_buffer_.buffer != nullptr) return Result("can only bind one depth buffer in a PIPELINE"); - if (buf->GetBufferType() != BufferType::kDepth) - return Result("expected a depth buffer"); depth_buffer_.buffer = buf; + depth_buffer_.type = BufferType::kDepth; + buf->SetWidth(fb_width_); buf->SetHeight(fb_height_); buf->SetElementCount(fb_width_ * fb_height_); @@ -292,21 +294,19 @@ Result Pipeline::AddVertexBuffer(Buffer* buf, uint32_t location) { if (vtex.buffer == buf) return Result("vertex buffer may only be bound to a PIPELINE once"); } - if (buf->GetBufferType() != BufferType::kVertex) - return Result("expected a vertex buffer"); vertex_buffers_.push_back(BufferInfo{buf}); vertex_buffers_.back().location = location; + vertex_buffers_.back().type = BufferType::kVertex; return {}; } Result Pipeline::SetPushConstantBuffer(Buffer* buf) { if (push_constant_buffer_.buffer != nullptr) return Result("can only bind one push constant buffer in a PIPELINE"); - if (buf->GetBufferType() != BufferType::kPushConstant) - return Result("expected a push constant buffer"); push_constant_buffer_.buffer = buf; + push_constant_buffer_.type = BufferType::kPushConstant; return {}; } @@ -315,7 +315,7 @@ std::unique_ptr Pipeline::GenerateDefaultColorAttachmentBuffer() { auto type = parser.Parse(kDefaultColorBufferFormat); auto fmt = MakeUnique(type.get()); - std::unique_ptr buf = MakeUnique(BufferType::kColor); + std::unique_ptr buf = MakeUnique(); buf->SetName(kGeneratedColorBuffer); buf->SetFormat(fmt.get()); @@ -329,7 +329,7 @@ std::unique_ptr Pipeline::GenerateDefaultDepthAttachmentBuffer() { auto type = parser.Parse(kDefaultDepthBufferFormat); auto fmt = MakeUnique(type.get()); - std::unique_ptr buf = MakeUnique(BufferType::kDepth); + std::unique_ptr buf = MakeUnique(); buf->SetName(kGeneratedDepthBuffer); buf->SetFormat(fmt.get()); @@ -348,6 +348,7 @@ Buffer* Pipeline::GetBufferForBinding(uint32_t descriptor_set, } void Pipeline::AddBuffer(Buffer* buf, + BufferType type, uint32_t descriptor_set, uint32_t binding) { // If this buffer binding already exists, overwrite with the new buffer. @@ -363,10 +364,12 @@ void Pipeline::AddBuffer(Buffer* buf, auto& info = buffers_.back(); info.descriptor_set = descriptor_set; info.binding = binding; - info.type = buf->GetBufferType(); + info.type = type; } -void Pipeline::AddBuffer(Buffer* buf, const std::string& arg_name) { +void Pipeline::AddBuffer(Buffer* buf, + BufferType type, + const std::string& arg_name) { // If this buffer binding already exists, overwrite with the new buffer. for (auto& info : buffers_) { if (info.arg_name == arg_name) { @@ -378,13 +381,14 @@ void Pipeline::AddBuffer(Buffer* buf, const std::string& arg_name) { buffers_.push_back(BufferInfo{buf}); auto& info = buffers_.back(); + info.type = type; info.arg_name = arg_name; info.descriptor_set = std::numeric_limits::max(); info.binding = std::numeric_limits::max(); info.arg_no = std::numeric_limits::max(); } -void Pipeline::AddBuffer(Buffer* buf, uint32_t arg_no) { +void Pipeline::AddBuffer(Buffer* buf, BufferType type, uint32_t arg_no) { // If this buffer binding already exists, overwrite with the new buffer. for (auto& info : buffers_) { if (info.arg_no == arg_no) { @@ -396,6 +400,7 @@ void Pipeline::AddBuffer(Buffer* buf, uint32_t arg_no) { buffers_.push_back(BufferInfo{buf}); auto& info = buffers_.back(); + info.type = type; info.arg_no = arg_no; info.descriptor_set = std::numeric_limits::max(); info.binding = std::numeric_limits::max(); @@ -421,8 +426,9 @@ void Pipeline::AddSampler(Sampler* sampler, Result Pipeline::UpdateOpenCLBufferBindings() { if (!IsCompute() || GetShaders().empty() || - GetShaders()[0].GetShader()->GetFormat() != kShaderFormatOpenCLC) + GetShaders()[0].GetShader()->GetFormat() != kShaderFormatOpenCLC) { return {}; + } const auto& shader_info = GetShaders()[0]; const auto& descriptor_map = shader_info.GetDescriptorMap(); @@ -440,29 +446,29 @@ Result Pipeline::UpdateOpenCLBufferBindings() { if (entry.arg_name == info.arg_name || entry.arg_ordinal == info.arg_no) { // Buffer storage class consistency checks. - if (info.buffer->GetBufferType() == BufferType::kUnknown) { + if (info.type == BufferType::kUnknown) { // Set the appropriate buffer type. switch (entry.kind) { case Pipeline::ShaderInfo::DescriptorMapEntry::Kind::UBO: case Pipeline::ShaderInfo::DescriptorMapEntry::Kind::POD_UBO: - info.buffer->SetBufferType(BufferType::kUniform); + info.type = BufferType::kUniform; break; case Pipeline::ShaderInfo::DescriptorMapEntry::Kind::SSBO: case Pipeline::ShaderInfo::DescriptorMapEntry::Kind::POD: - info.buffer->SetBufferType(BufferType::kStorage); + info.type = BufferType::kStorage; break; default: return Result("Unhandled buffer type for OPENCL-C shader"); } - } else if (info.buffer->GetBufferType() == BufferType::kUniform) { + } else if (info.type == BufferType::kUniform) { if (entry.kind != Pipeline::ShaderInfo::DescriptorMapEntry::Kind::UBO && entry.kind != Pipeline::ShaderInfo::DescriptorMapEntry::Kind::POD_UBO) { return Result("Buffer " + info.buffer->GetName() + - " must be an uniform binding"); + " must be a uniform binding"); } - } else if (info.buffer->GetBufferType() == BufferType::kStorage) { + } else if (info.type == BufferType::kStorage) { if (entry.kind != Pipeline::ShaderInfo::DescriptorMapEntry::Kind::SSBO && entry.kind != @@ -564,10 +570,10 @@ Result Pipeline::GenerateOpenCLPodBuffers() { // Add a new buffer for this descriptor set and binding. opencl_pod_buffers_.push_back(MakeUnique()); buffer = opencl_pod_buffers_.back().get(); - buffer->SetBufferType( + auto buffer_type = kind == Pipeline::ShaderInfo::DescriptorMapEntry::Kind::POD ? BufferType::kStorage - : BufferType::kUniform); + : BufferType::kUniform; // Use an 8-bit type because all the data in the descriptor map is // byte-based and it simplifies the logic for sizing below. @@ -584,7 +590,7 @@ Result Pipeline::GenerateOpenCLPodBuffers() { opencl_pod_buffer_map_.insert( buf_iter, std::make_pair(std::make_pair(descriptor_set, binding), buffer)); - AddBuffer(buffer, descriptor_set, binding); + AddBuffer(buffer, buffer_type, descriptor_set, binding); } else { buffer = buf_iter->second; } diff --git a/src/pipeline.h b/src/pipeline.h index 3b9b75c..2e5fb0c 100644 --- a/src/pipeline.h +++ b/src/pipeline.h @@ -232,12 +232,16 @@ class Pipeline { /// buffer bound. Buffer* GetIndexBuffer() const { return index_buffer_; } - /// Adds |buf| to the pipeline at the given |descriptor_set| and |binding|. - void AddBuffer(Buffer* buf, uint32_t descriptor_set, uint32_t binding); + /// Adds |buf| of |type |to the pipeline at the given |descriptor_set| + /// and |binding|. + void AddBuffer(Buffer* buf, + BufferType type, + uint32_t descriptor_set, + uint32_t binding); /// Adds |buf| to the pipeline at the given |arg_name|. - void AddBuffer(Buffer* buf, const std::string& arg_name); + void AddBuffer(Buffer* buf, BufferType type, const std::string& arg_name); /// Adds |buf| to the pipeline at the given |arg_no|. - void AddBuffer(Buffer* buf, uint32_t arg_no); + void AddBuffer(Buffer* buf, BufferType type, uint32_t arg_no); /// Returns information on all buffers in this pipeline. const std::vector& GetBuffers() const { return buffers_; } diff --git a/src/pipeline_test.cc b/src/pipeline_test.cc index dd5064b..9579dae 100644 --- a/src/pipeline_test.cc +++ b/src/pipeline_test.cc @@ -260,9 +260,9 @@ TEST_F(PipelineTest, ComputePipelineWithoutShader) { TEST_F(PipelineTest, PipelineBufferWithoutFormat) { Pipeline p(PipelineType::kCompute); - auto buf = MakeUnique(BufferType::kStorage); + auto buf = MakeUnique(); buf->SetName("MyBuffer"); - p.AddBuffer(buf.get(), 0, 0); + p.AddBuffer(buf.get(), BufferType::kStorage, 0, 0); Result r = p.Validate(); EXPECT_FALSE(r.IsSuccess()) << r.Error(); @@ -351,21 +351,21 @@ TEST_F(PipelineTest, Clone) { p.AddShader(&v, kShaderTypeVertex); p.SetShaderEntryPoint(&v, "my_main"); - auto vtex_buf = MakeUnique(BufferType::kVertex); + auto vtex_buf = MakeUnique(); vtex_buf->SetName("vertex_buffer"); p.AddVertexBuffer(vtex_buf.get(), 1); - auto idx_buf = MakeUnique(BufferType::kIndex); + auto idx_buf = MakeUnique(); idx_buf->SetName("Index Buffer"); p.SetIndexBuffer(idx_buf.get()); - auto buf1 = MakeUnique(BufferType::kStorage); + auto buf1 = MakeUnique(); buf1->SetName("buf1"); - p.AddBuffer(buf1.get(), 1, 1); + p.AddBuffer(buf1.get(), BufferType::kStorage, 1, 1); - auto buf2 = MakeUnique(BufferType::kStorage); + auto buf2 = MakeUnique(); buf2->SetName("buf2"); - p.AddBuffer(buf2.get(), 1, 2); + p.AddBuffer(buf2.get(), BufferType::kStorage, 1, 2); auto clone = p.Clone(); EXPECT_EQ("", clone->GetName()); @@ -422,13 +422,13 @@ TEST_F(PipelineTest, OpenCLUpdateBindings) { entry2.arg_ordinal = 1; p.GetShaders()[0].AddDescriptorEntry("my_main", std::move(entry2)); - auto a_buf = MakeUnique(BufferType::kStorage); + auto a_buf = MakeUnique(); a_buf->SetName("buf1"); - p.AddBuffer(a_buf.get(), "arg_a"); + p.AddBuffer(a_buf.get(), BufferType::kStorage, "arg_a"); - auto b_buf = MakeUnique(BufferType::kStorage); + auto b_buf = MakeUnique(); b_buf->SetName("buf2"); - p.AddBuffer(b_buf.get(), 1); + p.AddBuffer(b_buf.get(), BufferType::kStorage, 1); p.UpdateOpenCLBufferBindings(); @@ -467,18 +467,18 @@ TEST_F(PipelineTest, OpenCLUpdateBindingTypeMismatch) { entry2.arg_ordinal = 1; p.GetShaders()[0].AddDescriptorEntry("my_main", std::move(entry2)); - auto a_buf = MakeUnique(BufferType::kStorage); + auto a_buf = MakeUnique(); a_buf->SetName("buf1"); - p.AddBuffer(a_buf.get(), "arg_a"); + p.AddBuffer(a_buf.get(), BufferType::kStorage, "arg_a"); - auto b_buf = MakeUnique(BufferType::kUniform); + auto b_buf = MakeUnique(); b_buf->SetName("buf2"); - p.AddBuffer(b_buf.get(), 1); + p.AddBuffer(b_buf.get(), BufferType::kUniform, 1); auto r = p.UpdateOpenCLBufferBindings(); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("Buffer buf2 must be an uniform binding", r.Error()); + EXPECT_EQ("Buffer buf2 must be a uniform binding", r.Error()); } TEST_F(PipelineTest, OpenCLGeneratePodBuffers) { diff --git a/src/script_test.cc b/src/script_test.cc index 067e6e7..26fe3f9 100644 --- a/src/script_test.cc +++ b/src/script_test.cc @@ -221,7 +221,7 @@ TEST_F(ScriptTest, GetPipelines) { } TEST_F(ScriptTest, AddBuffer) { - auto buffer = MakeUnique(BufferType::kStorage); + auto buffer = MakeUnique(); buffer->SetName("my_buffer"); Script s; @@ -230,14 +230,14 @@ TEST_F(ScriptTest, AddBuffer) { } TEST_F(ScriptTest, AddDuplicateBuffer) { - auto buffer1 = MakeUnique(BufferType::kStorage); + auto buffer1 = MakeUnique(); 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(); buffer2->SetName("my_buffer"); r = s.AddBuffer(std::move(buffer2)); @@ -246,7 +246,7 @@ TEST_F(ScriptTest, AddDuplicateBuffer) { } TEST_F(ScriptTest, GetBuffer) { - auto buffer = MakeUnique(BufferType::kStorage); + auto buffer = MakeUnique(); buffer->SetName("my_buffer"); const auto* ptr = buffer.get(); @@ -270,7 +270,7 @@ TEST_F(ScriptTest, GetBuffersEmpty) { } TEST_F(ScriptTest, GetBuffers) { - auto buffer1 = MakeUnique(BufferType::kStorage); + auto buffer1 = MakeUnique(); buffer1->SetName("my_buffer1"); const auto* ptr1 = buffer1.get(); @@ -279,7 +279,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(); buffer2->SetName("my_buffer2"); const auto* ptr2 = buffer2.get(); diff --git a/src/vkscript/command_parser.cc b/src/vkscript/command_parser.cc index dd4f918..9e6b97f 100644 --- a/src/vkscript/command_parser.cc +++ b/src/vkscript/command_parser.cc @@ -571,11 +571,11 @@ Result CommandParser::ProcessSSBO() { auto* buffer = pipeline_->GetBufferForBinding(set, binding); if (!buffer) { - auto b = MakeUnique(BufferType::kStorage); + auto b = MakeUnique(); b->SetName("AutoBuf-" + std::to_string(script_->GetBuffers().size())); buffer = b.get(); script_->AddBuffer(std::move(b)); - pipeline_->AddBuffer(buffer, set, binding); + pipeline_->AddBuffer(buffer, BufferType::kStorage, set, binding); } cmd->SetBuffer(buffer); } @@ -722,11 +722,11 @@ Result CommandParser::ProcessUniform() { auto* buffer = pipeline_->GetBufferForBinding(set, binding); if (!buffer) { - auto b = MakeUnique(BufferType::kUniform); + auto b = MakeUnique(); b->SetName("AutoBuf-" + std::to_string(script_->GetBuffers().size())); buffer = b.get(); script_->AddBuffer(std::move(b)); - pipeline_->AddBuffer(buffer, set, binding); + pipeline_->AddBuffer(buffer, BufferType::kUniform, set, binding); } cmd->SetBuffer(buffer); @@ -738,7 +738,7 @@ Result CommandParser::ProcessUniform() { // Push constants don't have descriptor set and binding values. So, we do // not want to try to lookup the buffer or we'll accidentally get whatever // is bound at 0:0. - auto b = MakeUnique(BufferType::kUniform); + auto b = MakeUnique(); b->SetName("AutoBuf-" + std::to_string(script_->GetBuffers().size())); cmd->SetBuffer(b.get()); script_->AddBuffer(std::move(b)); diff --git a/src/vkscript/parser.cc b/src/vkscript/parser.cc index 12e94fb..b07f63c 100644 --- a/src/vkscript/parser.cc +++ b/src/vkscript/parser.cc @@ -298,7 +298,7 @@ Result Parser::ProcessIndicesBlock(const SectionParser::Section& section) { TypeParser parser; auto type = parser.Parse("R32_UINT"); auto fmt = MakeUnique(type.get()); - auto b = MakeUnique(BufferType::kIndex); + auto b = MakeUnique(); auto* buf = b.get(); b->SetName("indices"); b->SetFormat(fmt.get()); @@ -433,7 +433,7 @@ Result Parser::ProcessVertexDataBlock(const SectionParser::Section& section) { auto* pipeline = script_->GetPipeline(kDefaultPipelineName); for (size_t i = 0; i < headers.size(); ++i) { - auto buffer = MakeUnique(BufferType::kVertex); + auto buffer = MakeUnique(); auto* buf = buffer.get(); buffer->SetName("Vertices" + std::to_string(i)); buffer->SetFormat(headers[i].format); diff --git a/src/vkscript/parser_test.cc b/src/vkscript/parser_test.cc index 676e75e..c62052c 100644 --- a/src/vkscript/parser_test.cc +++ b/src/vkscript/parser_test.cc @@ -131,11 +131,10 @@ TEST_F(VkScriptParserTest, RequireBlockFramebuffer) { ASSERT_TRUE(r.IsSuccess()); auto script = parser.GetScript(); - const auto& buffers = script->GetBuffers(); - ASSERT_EQ(1U, buffers.size()); - EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(1U, bufs.size()); EXPECT_EQ(FormatType::kR32G32B32A32_SFLOAT, - buffers[0]->GetFormat()->GetFormatType()); + bufs[0]->GetFormat()->GetFormatType()); } TEST_F(VkScriptParserTest, RequireBlockDepthStencil) { @@ -147,11 +146,10 @@ TEST_F(VkScriptParserTest, RequireBlockDepthStencil) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - const auto& buffers = script->GetBuffers(); - ASSERT_EQ(2U, buffers.size()); - EXPECT_EQ(BufferType::kDepth, buffers[1]->GetBufferType()); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(2U, bufs.size()); EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, - buffers[1]->GetFormat()->GetFormatType()); + bufs[1]->GetFormat()->GetFormatType()); } TEST_F(VkScriptParserTest, RequireFbSize) { @@ -231,15 +229,13 @@ inheritedQueries # line comment ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - const auto& buffers = script->GetBuffers(); - ASSERT_EQ(2U, buffers.size()); - EXPECT_EQ(BufferType::kColor, buffers[0]->GetBufferType()); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(2U, bufs.size()); EXPECT_EQ(FormatType::kR32G32B32A32_SFLOAT, - buffers[0]->GetFormat()->GetFormatType()); + bufs[0]->GetFormat()->GetFormatType()); - EXPECT_EQ(BufferType::kDepth, buffers[1]->GetBufferType()); EXPECT_EQ(FormatType::kD24_UNORM_S8_UINT, - buffers[1]->GetFormat()->GetFormatType()); + bufs[1]->GetFormat()->GetFormatType()); auto feats = script->GetRequiredFeatures(); EXPECT_EQ("sparseResidency4Samples", feats[0]); @@ -255,18 +251,15 @@ TEST_F(VkScriptParserTest, IndicesBlock) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - const auto& buffers = script->GetBuffers(); - ASSERT_EQ(2U, buffers.size()); - ASSERT_EQ(BufferType::kIndex, buffers[1]->GetBufferType()); - - auto buffer_ptr = buffers[1].get(); - auto buffer = buffer_ptr; - EXPECT_TRUE(buffer->GetFormat()->IsUint32()); - EXPECT_EQ(3U, buffer->ElementCount()); - EXPECT_EQ(3U, buffer->ValueCount()); - EXPECT_EQ(3U * sizeof(uint32_t), buffer->GetSizeInBytes()); - - const auto* data = buffer->GetValues(); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(2U, bufs.size()); + + EXPECT_TRUE(bufs[1]->GetFormat()->IsUint32()); + EXPECT_EQ(3U, bufs[1]->ElementCount()); + EXPECT_EQ(3U, bufs[1]->ValueCount()); + EXPECT_EQ(3U * sizeof(uint32_t), bufs[1]->GetSizeInBytes()); + + const auto* data = bufs[1]->GetValues(); EXPECT_EQ(1, data[0]); EXPECT_EQ(2, data[1]); EXPECT_EQ(3, data[2]); @@ -286,13 +279,12 @@ TEST_F(VkScriptParserTest, IndicesBlockMultipleLines) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - auto& buffers = script->GetBuffers(); - ASSERT_EQ(2U, buffers.size()); - ASSERT_EQ(buffers[1]->GetBufferType(), BufferType::kIndex); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(2U, bufs.size()); - const auto* data = buffers[1]->GetValues(); + const auto* data = bufs[1]->GetValues(); std::vector results = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; - ASSERT_EQ(results.size(), buffers[1]->ValueCount()); + ASSERT_EQ(results.size(), bufs[1]->ValueCount()); for (size_t i = 0; i < results.size(); ++i) { EXPECT_EQ(results[i], data[i]); } @@ -337,8 +329,8 @@ TEST_F(VkScriptParserTest, VertexDataHeaderFormatString) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - const auto& buffers = script->GetBuffers(); - ASSERT_EQ(3U, buffers.size()); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(3U, bufs.size()); ASSERT_EQ(1U, script->GetPipelines().size()); const auto* pipeline = script->GetPipelines()[0].get(); @@ -346,17 +338,14 @@ TEST_F(VkScriptParserTest, VertexDataHeaderFormatString) { ASSERT_EQ(2U, pipeline->GetVertexBuffers().size()); const auto& pipeline_buffers = pipeline->GetVertexBuffers(); - ASSERT_EQ(BufferType::kVertex, buffers[1]->GetBufferType()); EXPECT_EQ(static_cast(0U), pipeline_buffers[0].location); - EXPECT_EQ(FormatType::kR32G32_SFLOAT, - buffers[1]->GetFormat()->GetFormatType()); - EXPECT_EQ(static_cast(0), buffers[1]->ElementCount()); + EXPECT_EQ(FormatType::kR32G32_SFLOAT, bufs[1]->GetFormat()->GetFormatType()); + EXPECT_EQ(static_cast(0), bufs[1]->ElementCount()); - ASSERT_EQ(BufferType::kVertex, buffers[2]->GetBufferType()); EXPECT_EQ(1U, pipeline_buffers[1].location); EXPECT_EQ(FormatType::kA8B8G8R8_UNORM_PACK32, - buffers[2]->GetFormat()->GetFormatType()); - EXPECT_EQ(static_cast(0), buffers[2]->ElementCount()); + bufs[2]->GetFormat()->GetFormatType()); + EXPECT_EQ(static_cast(0), bufs[2]->ElementCount()); } TEST_F(VkScriptParserTest, VertexDataHeaderGlslString) { @@ -368,8 +357,8 @@ TEST_F(VkScriptParserTest, VertexDataHeaderGlslString) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - const auto& buffers = script->GetBuffers(); - ASSERT_EQ(3U, buffers.size()); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(3U, bufs.size()); ASSERT_EQ(1U, script->GetPipelines().size()); const auto* pipeline = script->GetPipelines()[0].get(); @@ -377,30 +366,26 @@ TEST_F(VkScriptParserTest, VertexDataHeaderGlslString) { ASSERT_EQ(2U, pipeline->GetVertexBuffers().size()); const auto& pipeline_buffers = pipeline->GetVertexBuffers(); - ASSERT_EQ(BufferType::kVertex, buffers[1]->GetBufferType()); EXPECT_EQ(static_cast(0U), pipeline_buffers[0].location); - EXPECT_EQ(FormatType::kR32G32_SFLOAT, - buffers[1]->GetFormat()->GetFormatType()); + EXPECT_EQ(FormatType::kR32G32_SFLOAT, bufs[1]->GetFormat()->GetFormatType()); - auto& segs1 = buffers[1]->GetFormat()->GetSegments(); + auto& segs1 = bufs[1]->GetFormat()->GetSegments(); ASSERT_EQ(2U, segs1.size()); EXPECT_EQ(FormatMode::kSFloat, segs1[0].GetFormatMode()); EXPECT_EQ(FormatMode::kSFloat, segs1[1].GetFormatMode()); - EXPECT_EQ(static_cast(0), buffers[1]->ElementCount()); + EXPECT_EQ(static_cast(0), bufs[1]->ElementCount()); - ASSERT_EQ(BufferType::kVertex, buffers[2]->GetBufferType()); EXPECT_EQ(1U, pipeline_buffers[1].location); - EXPECT_EQ(FormatType::kR32G32B32_SINT, - buffers[2]->GetFormat()->GetFormatType()); + EXPECT_EQ(FormatType::kR32G32B32_SINT, bufs[2]->GetFormat()->GetFormatType()); - auto& segs2 = buffers[2]->GetFormat()->GetSegments(); + auto& segs2 = bufs[2]->GetFormat()->GetSegments(); ASSERT_EQ(4, segs2.size()); EXPECT_EQ(FormatMode::kSInt, segs2[0].GetFormatMode()); EXPECT_EQ(FormatMode::kSInt, segs2[1].GetFormatMode()); EXPECT_EQ(FormatMode::kSInt, segs2[2].GetFormatMode()); EXPECT_TRUE(segs2[3].IsPadding()); - EXPECT_EQ(static_cast(0), buffers[2]->ElementCount()); + EXPECT_EQ(static_cast(0), bufs[2]->ElementCount()); } TEST_F(VkScriptParserTest, TestBlock) { @@ -450,21 +435,17 @@ TEST_F(VkScriptParserTest, VertexDataRows) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - const auto& buffers = script->GetBuffers(); - ASSERT_EQ(3U, buffers.size()); - - ASSERT_EQ(BufferType::kVertex, buffers[1]->GetBufferType()); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(3U, bufs.size()); std::vector seg_0 = {-1.f, -1.f, 0.25f, 0, 0.25f, -1.f, 0.25f, 0}; - const auto* values_0 = buffers[1]->GetValues(); + const auto* values_0 = bufs[1]->GetValues(); for (size_t i = 0; i < seg_0.size(); ++i) { EXPECT_FLOAT_EQ(seg_0[i], values_0[i]); } - ASSERT_EQ(BufferType::kVertex, buffers[2]->GetBufferType()); - std::vector seg_1 = {255, 128, 1, 0, 255, 128, 255, 0}; - const auto* values_1 = buffers[2]->GetValues(); + const auto* values_1 = bufs[2]->GetValues(); for (size_t i = 0; i < seg_1.size(); ++i) { EXPECT_EQ(seg_1[i], values_1[i]); } @@ -509,13 +490,12 @@ TEST_F(VkScriptParserTest, VertexDataRowsWithHex) { ASSERT_TRUE(r.IsSuccess()) << r.Error(); auto script = parser.GetScript(); - const auto& buffers = script->GetBuffers(); - ASSERT_EQ(2U, buffers.size()); - ASSERT_EQ(BufferType::kVertex, buffers[1]->GetBufferType()); + const auto& bufs = script->GetBuffers(); + ASSERT_EQ(2U, bufs.size()); std::vector seg_0 = {0xff0000ff, 0xffff0000}; - const auto* values_0 = buffers[1]->GetValues(); - ASSERT_EQ(seg_0.size(), buffers[1]->ValueCount()); + const auto* values_0 = bufs[1]->GetValues(); + ASSERT_EQ(seg_0.size(), bufs[1]->ValueCount()); for (size_t i = 0; i < seg_0.size(); ++i) { EXPECT_EQ(seg_0[i], values_0[i]); diff --git a/src/vulkan/device.cc b/src/vulkan/device.cc index 698d336..f5f6e1c 100644 --- a/src/vulkan/device.cc +++ b/src/vulkan/device.cc @@ -461,7 +461,7 @@ Result Device::Initialize( } bool Device::IsFormatSupportedByPhysicalDevice(const Format& format, - Buffer* buffer) { + BufferType type) { VkFormat vk_format = GetVkFormat(format); VkFormatProperties properties = VkFormatProperties(); GetPtrs()->vkGetPhysicalDeviceFormatProperties(physical_device_, vk_format, @@ -469,7 +469,7 @@ bool Device::IsFormatSupportedByPhysicalDevice(const Format& format, VkFormatFeatureFlagBits flag = VK_FORMAT_FEATURE_VERTEX_BUFFER_BIT; bool is_buffer_type_image = false; - switch (buffer->GetBufferType()) { + switch (type) { case BufferType::kColor: case BufferType::kStorageImage: flag = VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BIT; diff --git a/src/vulkan/device.h b/src/vulkan/device.h index 700e6e8..63184af 100644 --- a/src/vulkan/device.h +++ b/src/vulkan/device.h @@ -53,7 +53,7 @@ class Device { /// Returns true if |format| and the |buffer|s buffer type combination is /// supported by the physical device. - bool IsFormatSupportedByPhysicalDevice(const Format& format, Buffer* buffer); + bool IsFormatSupportedByPhysicalDevice(const Format& format, BufferType type); VkDevice GetVkDevice() const { return device_; } VkQueue GetVkQueue() const { return queue_; } diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index f491b1c..3168f0e 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -151,7 +151,7 @@ Result EngineVulkan::CreatePipeline(amber::Pipeline* pipeline) { for (const auto& colour_info : pipeline->GetColorAttachments()) { auto fmt = colour_info.buffer->GetFormat(); - if (!device_->IsFormatSupportedByPhysicalDevice(*fmt, colour_info.buffer)) + if (!device_->IsFormatSupportedByPhysicalDevice(*fmt, colour_info.type)) return Result("Vulkan color attachment format is not supported"); } @@ -161,7 +161,7 @@ Result EngineVulkan::CreatePipeline(amber::Pipeline* pipeline) { depth_fmt = depth_info.buffer->GetFormat(); if (!device_->IsFormatSupportedByPhysicalDevice(*depth_fmt, - depth_info.buffer)) { + depth_info.type)) { return Result("Vulkan depth attachment format is not supported"); } } @@ -207,7 +207,7 @@ Result EngineVulkan::CreatePipeline(amber::Pipeline* pipeline) { for (const auto& vtex_info : pipeline->GetVertexBuffers()) { auto fmt = vtex_info.buffer->GetFormat(); - if (!device_->IsFormatSupportedByPhysicalDevice(*fmt, vtex_info.buffer)) + if (!device_->IsFormatSupportedByPhysicalDevice(*fmt, vtex_info.type)) return Result("Vulkan vertex buffer format is not supported"); if (!info.vertex_buffer) info.vertex_buffer = MakeUnique(device_.get()); diff --git a/src/vulkan/engine_vulkan.h b/src/vulkan/engine_vulkan.h index 7dab176..c875e07 100644 --- a/src/vulkan/engine_vulkan.h +++ b/src/vulkan/engine_vulkan.h @@ -78,11 +78,6 @@ class EngineVulkan : public Engine { Result GetVkShaderStageInfo( amber::Pipeline* pipeline, std::vector* out); - bool IsFormatSupportedByPhysicalDevice(BufferType type, - VkPhysicalDevice physical_device, - VkFormat format); - bool IsDescriptorSetInBounds(VkPhysicalDevice physical_device, - uint32_t descriptor_set); Result SetShader(amber::Pipeline* pipeline, ShaderType type, -- cgit v1.2.3