[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.

5 files changed, 64 insertions, 34 deletions

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<size_t>(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<uint32_t>(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<uint32_t>(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<uint32_t>(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<uint32_t>(0), cmd->GetDescriptorSet());
   EXPECT_EQ(2U, cmd->GetBinding());
-  EXPECT_EQ(1U, cmd->GetOffset());
-  EXPECT_EQ(12U, cmd->GetSize());
+  EXPECT_EQ(static_cast<uint32_t>(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());