diff options
author | dan sinclair <dsinclair@google.com> | 2019-11-27 15:49:49 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-11-27 15:49:49 -0500 |
commit | 08ebea824879a73b9b5967f29f1d1b03d5a0abba (patch) | |
tree | 66dc516e52a6fa1af557d00ee76917c070ee270f | |
parent | 7d881bbe685f69a6a0d85abb84b5c81e7babd26c (diff) | |
download | amber-08ebea824879a73b9b5967f29f1d1b03d5a0abba.tar.gz |
Remove BufferType from the Buffer class. (#728)
The BufferType is a product of the <Pipeline, Buffer> 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.
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<std::string> 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<std::string> 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<uint32_t>(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<double> 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<size_t>(0U), b.ElementCount()); EXPECT_EQ(static_cast<size_t>(0U), b.ValueCount()); EXPECT_EQ(static_cast<size_t>(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<uint8_t>::max()); - Buffer b(BufferType::kColor); + Buffer b; b.SetFormat(&fmt); b.SetData(values); @@ -208,11 +208,11 @@ TEST_F(BufferTest, CompareHistogramEMDToleranceFalse) { std::vector<Value> 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<Value> 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<Value> 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<Value> 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<uint32_t>( - buf_info.buffer->GetBufferType()))); + std::to_string(static_cast<uint32_t>(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<uint32_t>( - buf_info.buffer->GetBufferType()))); + std::to_string(static_cast<uint32_t>(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<Buffer> Pipeline::GenerateDefaultColorAttachmentBuffer() { auto type = parser.Parse(kDefaultColorBufferFormat); auto fmt = MakeUnique<Format>(type.get()); - std::unique_ptr<Buffer> buf = MakeUnique<Buffer>(BufferType::kColor); + std::unique_ptr<Buffer> buf = MakeUnique<Buffer>(); buf->SetName(kGeneratedColorBuffer); buf->SetFormat(fmt.get()); @@ -329,7 +329,7 @@ std::unique_ptr<Buffer> Pipeline::GenerateDefaultDepthAttachmentBuffer() { auto type = parser.Parse(kDefaultDepthBufferFormat); auto fmt = MakeUnique<Format>(type.get()); - std::unique_ptr<Buffer> buf = MakeUnique<Buffer>(BufferType::kDepth); + std::unique_ptr<Buffer> buf = MakeUnique<Buffer>(); 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<uint32_t>::max(); info.binding = std::numeric_limits<uint32_t>::max(); info.arg_no = std::numeric_limits<uint32_t>::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<uint32_t>::max(); info.binding = std::numeric_limits<uint32_t>::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>()); 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<BufferInfo>& 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<Buffer>(BufferType::kStorage); + auto buf = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kVertex); + auto vtex_buf = MakeUnique<Buffer>(); vtex_buf->SetName("vertex_buffer"); p.AddVertexBuffer(vtex_buf.get(), 1); - auto idx_buf = MakeUnique<Buffer>(BufferType::kIndex); + auto idx_buf = MakeUnique<Buffer>(); idx_buf->SetName("Index Buffer"); p.SetIndexBuffer(idx_buf.get()); - auto buf1 = MakeUnique<Buffer>(BufferType::kStorage); + auto buf1 = MakeUnique<Buffer>(); buf1->SetName("buf1"); - p.AddBuffer(buf1.get(), 1, 1); + p.AddBuffer(buf1.get(), BufferType::kStorage, 1, 1); - auto buf2 = MakeUnique<Buffer>(BufferType::kStorage); + auto buf2 = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kStorage); + auto a_buf = MakeUnique<Buffer>(); a_buf->SetName("buf1"); - p.AddBuffer(a_buf.get(), "arg_a"); + p.AddBuffer(a_buf.get(), BufferType::kStorage, "arg_a"); - auto b_buf = MakeUnique<Buffer>(BufferType::kStorage); + auto b_buf = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kStorage); + auto a_buf = MakeUnique<Buffer>(); a_buf->SetName("buf1"); - p.AddBuffer(a_buf.get(), "arg_a"); + p.AddBuffer(a_buf.get(), BufferType::kStorage, "arg_a"); - auto b_buf = MakeUnique<Buffer>(BufferType::kUniform); + auto b_buf = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kStorage); + auto buffer = MakeUnique<Buffer>(); buffer->SetName("my_buffer"); Script s; @@ -230,14 +230,14 @@ TEST_F(ScriptTest, AddBuffer) { } TEST_F(ScriptTest, AddDuplicateBuffer) { - auto buffer1 = MakeUnique<Buffer>(BufferType::kStorage); + auto buffer1 = MakeUnique<Buffer>(); buffer1->SetName("my_buffer"); Script s; Result r = s.AddBuffer(std::move(buffer1)); ASSERT_TRUE(r.IsSuccess()) << r.Error(); - auto buffer2 = MakeUnique<Buffer>(BufferType::kUniform); + auto buffer2 = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kStorage); + auto buffer = MakeUnique<Buffer>(); buffer->SetName("my_buffer"); const auto* ptr = buffer.get(); @@ -270,7 +270,7 @@ TEST_F(ScriptTest, GetBuffersEmpty) { } TEST_F(ScriptTest, GetBuffers) { - auto buffer1 = MakeUnique<Buffer>(BufferType::kStorage); + auto buffer1 = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kUniform); + auto buffer2 = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kStorage); + auto b = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kUniform); + auto b = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kUniform); + auto b = MakeUnique<Buffer>(); 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<Format>(type.get()); - auto b = MakeUnique<Buffer>(BufferType::kIndex); + auto b = MakeUnique<Buffer>(); 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<Buffer>(BufferType::kVertex); + auto buffer = MakeUnique<Buffer>(); 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<uint32_t>(); + 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<uint32_t>(); 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<uint32_t>(); + const auto* data = bufs[1]->GetValues<uint32_t>(); std::vector<uint16_t> 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<uint8_t>(0U), pipeline_buffers[0].location); - EXPECT_EQ(FormatType::kR32G32_SFLOAT, - buffers[1]->GetFormat()->GetFormatType()); - EXPECT_EQ(static_cast<uint32_t>(0), buffers[1]->ElementCount()); + EXPECT_EQ(FormatType::kR32G32_SFLOAT, bufs[1]->GetFormat()->GetFormatType()); + EXPECT_EQ(static_cast<uint32_t>(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<uint32_t>(0), buffers[2]->ElementCount()); + bufs[2]->GetFormat()->GetFormatType()); + EXPECT_EQ(static_cast<uint32_t>(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<uint8_t>(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<uint32_t>(0), buffers[1]->ElementCount()); + EXPECT_EQ(static_cast<uint32_t>(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<uint32_t>(0), buffers[2]->ElementCount()); + EXPECT_EQ(static_cast<uint32_t>(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<float> seg_0 = {-1.f, -1.f, 0.25f, 0, 0.25f, -1.f, 0.25f, 0}; - const auto* values_0 = buffers[1]->GetValues<float>(); + const auto* values_0 = bufs[1]->GetValues<float>(); 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<uint8_t> seg_1 = {255, 128, 1, 0, 255, 128, 255, 0}; - const auto* values_1 = buffers[2]->GetValues<uint8_t>(); + const auto* values_1 = bufs[2]->GetValues<uint8_t>(); 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<uint32_t> seg_0 = {0xff0000ff, 0xffff0000}; - const auto* values_0 = buffers[1]->GetValues<uint32_t>(); - ASSERT_EQ(seg_0.size(), buffers[1]->ValueCount()); + const auto* values_0 = bufs[1]->GetValues<uint32_t>(); + 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<VertexBuffer>(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<VkPipelineShaderStageCreateInfo>* 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, |