From 86af71dc2875a1c64bcb9e418aa5ad43c7a6747d Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Sat, 30 Mar 2019 13:29:57 -0400 Subject: [vkscript] report error on invalid ssbo subdata offset. (#421) This CL adds an error message if the requested offset is not a multiple of the data type. This also updates the DatumType code to handle std140 layouts where we have to add an extra element for 3 item arrays. Currently all things are considered to be in std140. Fixes #296. --- src/datum_type.cc | 16 +++++++-- src/datum_type.h | 2 ++ src/verifier.cc | 3 +- src/vkscript/command_parser.cc | 5 +++ src/vkscript/command_parser_test.cc | 72 +++++++++++++++++++++---------------- 5 files changed, 64 insertions(+), 34 deletions(-) (limited to 'src') diff --git a/src/datum_type.cc b/src/datum_type.cc index eef85cd..8a05894 100644 --- a/src/datum_type.cc +++ b/src/datum_type.cc @@ -22,7 +22,7 @@ DatumType::~DatumType() = default; DatumType& DatumType::operator=(const DatumType&) = default; -uint32_t DatumType::SizeInBytes() const { +uint32_t DatumType::ElementSizeInBytes() const { uint32_t s; if (type_ == DataType::kInt8 || type_ == DataType::kUint8) s = 1; @@ -37,7 +37,19 @@ uint32_t DatumType::SizeInBytes() const { else s = sizeof(double); - return s * column_count_ * row_count_; + return s; +} + +uint32_t DatumType::SizeInBytes() const { + uint32_t s = ElementSizeInBytes(); + uint32_t bytes = s * column_count_ * row_count_; + + // For a vector of size 3 in std140 we need to have alignment of 4N. For a + // matrix, we update each row to 4N. + if (is_std140_ && row_count_ == 3) + bytes += (s * column_count_); + + return bytes; } } // namespace amber diff --git a/src/datum_type.h b/src/datum_type.h index 815241d..437bf80 100644 --- a/src/datum_type.h +++ b/src/datum_type.h @@ -60,12 +60,14 @@ class DatumType { void SetRowCount(uint32_t count) { row_count_ = count; } uint32_t RowCount() const { return row_count_; } + uint32_t ElementSizeInBytes() const; uint32_t SizeInBytes() const; private: DataType type_ = DataType::kUint8; uint32_t column_count_ = 1; uint32_t row_count_ = 1; + bool is_std140_ = true; }; } // namespace amber diff --git a/src/verifier.cc b/src/verifier.cc index 0014399..5754c54 100644 --- a/src/verifier.cc +++ b/src/verifier.cc @@ -650,8 +650,7 @@ Result Verifier::ProbeSSBO(const ProbeSSBOCommand* command, } const auto& datum_type = command->GetDatumType(); - size_t bytes_per_elem = datum_type.SizeInBytes() / datum_type.RowCount() / - datum_type.ColumnCount(); + size_t bytes_per_elem = datum_type.ElementSizeInBytes(); size_t offset = static_cast(command->GetOffset()); if (values.size() * bytes_per_elem + offset > size_in_bytes) { return Result( diff --git a/src/vkscript/command_parser.cc b/src/vkscript/command_parser.cc index 890576e..0fd2c69 100644 --- a/src/vkscript/command_parser.cc +++ b/src/vkscript/command_parser.cc @@ -624,6 +624,11 @@ Result CommandParser::ProcessSSBO() { return Result("offset for SSBO must be positive, got: " + std::to_string(token->AsInt32())); } + if ((token->AsUint32() % cmd->GetDatumType().SizeInBytes()) != 0) { + return Result( + "offset for SSBO must be a multiple of the data size expected " + + std::to_string(cmd->GetDatumType().SizeInBytes())); + } cmd->SetOffset(token->AsUint32()); diff --git a/src/vkscript/command_parser_test.cc b/src/vkscript/command_parser_test.cc index 230dcbc..6a1b651 100644 --- a/src/vkscript/command_parser_test.cc +++ b/src/vkscript/command_parser_test.cc @@ -3073,7 +3073,7 @@ TEST_F(CommandParserTest, SSBOMissingBinding) { } TEST_F(CommandParserTest, SSBOSubdataWithFloat) { - std::string data = "ssbo 6 subdata vec3 2 2.3 4.2 1.2"; + std::string data = "ssbo 6 subdata vec3 16 2.3 4.2 1.2"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3089,8 +3089,8 @@ TEST_F(CommandParserTest, SSBOSubdataWithFloat) { EXPECT_TRUE(cmd->IsSSBO()); EXPECT_EQ(static_cast(0), cmd->GetDescriptorSet()); EXPECT_EQ(6U, cmd->GetBinding()); - EXPECT_EQ(2U, cmd->GetOffset()); - EXPECT_EQ(12U, cmd->GetSize()); + EXPECT_EQ(16U, cmd->GetOffset()); + EXPECT_EQ(16U, cmd->GetSize()); ASSERT_TRUE(cmd->IsSubdata()); const auto& type = cmd->GetDatumType(); @@ -3118,7 +3118,7 @@ TEST_F(CommandParserTest, SSBOSubdataWithNegativeOffset) { } TEST_F(CommandParserTest, SSBOSubdataWithDescriptorSet) { - std::string data = "ssbo 5:6 subdata vec3 2 2.3 4.2 1.2"; + std::string data = "ssbo 5:6 subdata vec3 16 2.3 4.2 1.2"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3132,11 +3132,11 @@ TEST_F(CommandParserTest, SSBOSubdataWithDescriptorSet) { auto* cmd = cmds[0]->AsBuffer(); EXPECT_TRUE(cmd->IsSSBO()); + ASSERT_TRUE(cmd->IsSubdata()); EXPECT_EQ(5U, cmd->GetDescriptorSet()); EXPECT_EQ(6U, cmd->GetBinding()); - EXPECT_EQ(2U, cmd->GetOffset()); - ASSERT_TRUE(cmd->IsSubdata()); - EXPECT_EQ(12U, cmd->GetSize()); + EXPECT_EQ(16U, cmd->GetOffset()); + EXPECT_EQ(16U, cmd->GetSize()); const auto& type = cmd->GetDatumType(); EXPECT_TRUE(type.IsFloat()); @@ -3152,7 +3152,7 @@ TEST_F(CommandParserTest, SSBOSubdataWithDescriptorSet) { } TEST_F(CommandParserTest, SSBOSubdataWithInts) { - std::string data = "ssbo 6 subdata i16vec3 2 2 4 1"; + std::string data = "ssbo 6 subdata i16vec3 8 2 4 1"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3166,11 +3166,11 @@ TEST_F(CommandParserTest, SSBOSubdataWithInts) { auto* cmd = cmds[0]->AsBuffer(); EXPECT_TRUE(cmd->IsSSBO()); + ASSERT_TRUE(cmd->IsSubdata()); EXPECT_EQ(static_cast(0), cmd->GetDescriptorSet()); EXPECT_EQ(6U, cmd->GetBinding()); - EXPECT_EQ(2U, cmd->GetOffset()); - ASSERT_TRUE(cmd->IsSubdata()); - EXPECT_EQ(6U, cmd->GetSize()); + EXPECT_EQ(8U, cmd->GetOffset()); + EXPECT_EQ(8U, cmd->GetSize()); const auto& type = cmd->GetDatumType(); EXPECT_TRUE(type.IsInt16()); @@ -3186,7 +3186,7 @@ TEST_F(CommandParserTest, SSBOSubdataWithInts) { } TEST_F(CommandParserTest, SSBOSubdataWithMultipleVectors) { - std::string data = "ssbo 6 subdata i16vec3 2 2 4 1 3 6 8"; + std::string data = "ssbo 6 subdata i16vec3 8 2 4 1 3 6 8"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3200,11 +3200,11 @@ TEST_F(CommandParserTest, SSBOSubdataWithMultipleVectors) { auto* cmd = cmds[0]->AsBuffer(); EXPECT_TRUE(cmd->IsSSBO()); + ASSERT_TRUE(cmd->IsSubdata()); EXPECT_EQ(static_cast(0), cmd->GetDescriptorSet()); EXPECT_EQ(6U, cmd->GetBinding()); - EXPECT_EQ(2U, cmd->GetOffset()); - ASSERT_TRUE(cmd->IsSubdata()); - EXPECT_EQ(12U, cmd->GetSize()); + EXPECT_EQ(8U, cmd->GetOffset()); + EXPECT_EQ(16U, cmd->GetSize()); const auto& type = cmd->GetDatumType(); EXPECT_TRUE(type.IsInt16()); @@ -3220,7 +3220,7 @@ TEST_F(CommandParserTest, SSBOSubdataWithMultipleVectors) { } TEST_F(CommandParserTest, SSBOSubdataMissingBinding) { - std::string data = "ssbo subdata i16vec3 2 2 3 2"; + std::string data = "ssbo subdata i16vec3 0 2 3 2"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3286,7 +3286,7 @@ TEST_F(CommandParserTest, SSBOSubdataWithInvalidStringOffset) { } TEST_F(CommandParserTest, SSBOSubdataWithMissingData) { - std::string data = "ssbo 6 subdata i16vec3 2 2"; + std::string data = "ssbo 6 subdata i16vec3 0 2"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3298,7 +3298,7 @@ TEST_F(CommandParserTest, SSBOSubdataWithMissingData) { } TEST_F(CommandParserTest, SSBOSubdataWithMissingAllData) { - std::string data = "ssbo 6 subdata i16vec3 2"; + std::string data = "ssbo 6 subdata i16vec3 8"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3309,8 +3309,20 @@ TEST_F(CommandParserTest, SSBOSubdataWithMissingAllData) { r.Error()); } +TEST_F(CommandParserTest, SSBOSubdataWithNonDataTypeSizedOffset) { + std::string data = "ssbo 6 subdata i16vec3 2"; + + Pipeline pipeline(PipelineType::kGraphics); + Script script; + CommandParser cp(&script, &pipeline, 1, data); + Result r = cp.Parse(); + ASSERT_FALSE(r.IsSuccess()); + EXPECT_EQ("1: offset for SSBO must be a multiple of the data size expected 8", + r.Error()); +} + TEST_F(CommandParserTest, Uniform) { - std::string data = "uniform vec3 2 2.1 3.2 4.3"; + std::string data = "uniform vec3 32 2.1 3.2 4.3"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3324,8 +3336,8 @@ TEST_F(CommandParserTest, Uniform) { auto* cmd = cmds[0]->AsBuffer(); EXPECT_TRUE(cmd->IsPushConstant()); - EXPECT_EQ(2U, cmd->GetOffset()); - EXPECT_EQ(12U, cmd->GetSize()); + EXPECT_EQ(32U, cmd->GetOffset()); + EXPECT_EQ(16U, cmd->GetSize()); const auto& type = cmd->GetDatumType(); EXPECT_TRUE(type.IsFloat()); @@ -3352,7 +3364,7 @@ TEST_F(CommandParserTest, UniformOffsetMustBePositive) { } TEST_F(CommandParserTest, UniformWithContinuation) { - std::string data = "uniform vec3 2 2.1 3.2 4.3 \\\n5.4 6.7 8.9"; + std::string data = "uniform vec3 16 2.1 3.2 4.3 \\\n5.4 6.7 8.9"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3366,8 +3378,8 @@ TEST_F(CommandParserTest, UniformWithContinuation) { auto* cmd = cmds[0]->AsBuffer(); EXPECT_TRUE(cmd->IsPushConstant()); - EXPECT_EQ(2U, cmd->GetOffset()); - EXPECT_EQ(24U, cmd->GetSize()); + EXPECT_EQ(16U, cmd->GetOffset()); + EXPECT_EQ(32U, cmd->GetSize()); const auto& type = cmd->GetDatumType(); EXPECT_TRUE(type.IsFloat()); @@ -3428,7 +3440,7 @@ TEST_F(CommandParserTest, UniformMissingValues) { } TEST_F(CommandParserTest, UniformUBO) { - std::string data = "uniform ubo 2 vec3 1 2.1 3.2 4.3"; + std::string data = "uniform ubo 2 vec3 0 2.1 3.2 4.3"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3444,8 +3456,8 @@ TEST_F(CommandParserTest, UniformUBO) { EXPECT_TRUE(cmd->IsUniform()); EXPECT_EQ(static_cast(0), cmd->GetDescriptorSet()); EXPECT_EQ(2U, cmd->GetBinding()); - EXPECT_EQ(1U, cmd->GetOffset()); - EXPECT_EQ(12U, cmd->GetSize()); + EXPECT_EQ(static_cast(0), cmd->GetOffset()); + EXPECT_EQ(16U, cmd->GetSize()); const auto& type = cmd->GetDatumType(); EXPECT_TRUE(type.IsFloat()); @@ -3472,7 +3484,7 @@ TEST_F(CommandParserTest, UniformUBOOffsetMustBePositive) { } TEST_F(CommandParserTest, UniformUBOWithDescriptorSet) { - std::string data = "uniform ubo 3:2 vec3 1 2.1 3.2 4.3"; + std::string data = "uniform ubo 3:2 vec3 16 2.1 3.2 4.3"; Pipeline pipeline(PipelineType::kGraphics); Script script; @@ -3488,8 +3500,8 @@ TEST_F(CommandParserTest, UniformUBOWithDescriptorSet) { EXPECT_TRUE(cmd->IsUniform()); EXPECT_EQ(3U, cmd->GetDescriptorSet()); EXPECT_EQ(2U, cmd->GetBinding()); - EXPECT_EQ(1U, cmd->GetOffset()); - EXPECT_EQ(12U, cmd->GetSize()); + EXPECT_EQ(16U, cmd->GetOffset()); + EXPECT_EQ(16U, cmd->GetSize()); const auto& type = cmd->GetDatumType(); EXPECT_TRUE(type.IsFloat()); -- cgit v1.2.3