Implementing Binary glTF reader for the VR controller model

chrome/browser/android/vr_shell/gltf_parser.cc

Index: chrome/browser/android/vr_shell/gltf_parser.cc
diff --git a/chrome/browser/android/vr_shell/gltf_parser.cc b/chrome/browser/android/vr_shell/gltf_parser.cc
index 6dbde77ac9e81adbab0a662e92cc178073a9c0f0..febcc2b4526680b12285573e4361584d5c03bc4d 100644
--- a/chrome/browser/android/vr_shell/gltf_parser.cc
+++ b/chrome/browser/android/vr_shell/gltf_parser.cc
@@ -9,14 +9,39 @@
 #include "base/callback_helpers.h"
 #include "base/files/file_util.h"
 #include "base/json/json_file_value_serializer.h"
+#include "base/json/json_string_value_serializer.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_util.h"
+#include "base/sys_byteorder.h"
 #include "net/base/data_url.h"
 #include "net/base/filename_util.h"
 #include "url/gurl.h"
 
 namespace vr_shell {
 
+namespace {
+constexpr const char kFailedtoReadBinaryGltfMsg[] =

[mthiesse, 2017/04/28 15:51:34]

Wrap this in #if defined(DCHECK_IS_ON)

[mthiesse, 2017/04/28 16:11:45]

I know this makes the code cleaner, but to avoid extra strings, I think you
should just hard-code it where it's used in the debug logs and delete the
constexpr here.

+    "Failed to read binary glTF: ";
+constexpr const char kGltfMagic[] = "glTF";
+constexpr const char kBinaryGltfBufferName[] = "binary_glTF";
+constexpr uint32_t kJsonGltfFormat = 0;
+
+enum {

[mthiesse, 2017/04/28 15:51:34]

Why is this an enum? Looks like just making constants would be a better fit?

[acondor_, 2017/04/28 16:07:01]

Done.

+  kVersionStart = 4,
+  kLengthStart = 8,
+  kContentLengthStart = 12,
+  kContentFormatStart = 16,
+  kContentStart = 20
+};
+
+uint32_t GetLE32(const char* data) {
+  const uint32_t* int_data = reinterpret_cast<const uint32_t*>(data);
+  return base::ByteSwapToLE32(*int_data);
+}
+
+}  // namespace
+
 GltfParser::GltfParser() {}
 
 GltfParser::~GltfParser() = default;
@@ -25,6 +50,7 @@ std::unique_ptr<gltf::Asset> GltfParser::Parse(
     const base::DictionaryValue& dict,
     std::vector<std::unique_ptr<gltf::Buffer>>* buffers,
     const base::FilePath& path) {
+  DCHECK(buffers && buffers->size() <= 1);
   path_ = path;
   asset_ = base::MakeUnique<gltf::Asset>();
 
@@ -40,6 +66,7 @@ std::unique_ptr<gltf::Asset> GltfParser::Parse(
 std::unique_ptr<gltf::Asset> GltfParser::Parse(
     const base::FilePath& gltf_path,
     std::vector<std::unique_ptr<gltf::Buffer>>* buffers) {
+  DCHECK(buffers && buffers->size() <= 1);
   JSONFileValueDeserializer json_deserializer(gltf_path);
   int error_code;
   std::string error_msg;
@@ -89,12 +116,19 @@ bool GltfParser::ParseInternal(
 bool GltfParser::SetBuffers(
     const base::DictionaryValue& dict,
     std::vector<std::unique_ptr<gltf::Buffer>>* buffers) {
-  buffers->clear();
+  size_t buffer_cnt = 0;

[mthiesse, 2017/04/28 15:51:34]

nit: avoid using abbreviations in variable names like 'cnt'

[acondor_, 2017/04/28 16:07:01]

Done.

   for (base::DictionaryValue::Iterator it(dict); !it.IsAtEnd(); it.Advance()) {
     const base::DictionaryValue* buffer_dict;
     if (!it.value().GetAsDictionary(&buffer_dict))
       return false;
 
+    if (it.key() == kBinaryGltfBufferName) {
+      if (buffers->size() == buffer_cnt)
+        return false;
+      buffer_ids_[it.key()] = 0;
+      continue;
+    }
+
     std::string uri_str;
     if (!buffer_dict->GetString("uri", &uri_str))
       return false;
@@ -109,6 +143,7 @@ bool GltfParser::SetBuffers(
 
     buffer_ids_[it.key()] = buffers->size();
     buffers->push_back(std::move(buffer));
+    ++buffer_cnt;
   }
   return true;
 }
@@ -333,4 +368,64 @@ void GltfParser::Clear() {
   scene_ids_.clear();
 }
 
+std::unique_ptr<gltf::Asset> BinaryGltfParser::Parse(
+    const base::StringPiece& glb_content,
+    std::vector<std::unique_ptr<gltf::Buffer>>* buffers,
+    const base::FilePath& path) {
+  DCHECK(buffers && buffers->empty());
+  if (glb_content.length() < kContentStart) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Incomplete data";

[mthiesse, 2017/04/28 15:51:34]

Make these DLOGs? Best to avoid adding strings to release builds.

+    return nullptr;
+  }
+  if (!glb_content.starts_with(kGltfMagic)) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Unknown magic number";
+    return nullptr;
+  }
+  if (GetLE32(glb_content.data() + kVersionStart) != 1) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Unknown version";

[mthiesse, 2017/04/28 15:51:34]

nit: s/Unknown/Unsupported

+    return nullptr;
+  }
+  if (GetLE32(glb_content.data() + kLengthStart) != glb_content.length()) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Incorrect file size";
+    return nullptr;
+  }
+  uint32_t content_length = GetLE32(glb_content.data() + kContentLengthStart);
+  if (kContentStart + content_length > glb_content.length()) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Invalid content length";
+    return nullptr;
+  }
+  if (GetLE32(glb_content.data() + kContentFormatStart) != kJsonGltfFormat) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Unknown glTF format";

[mthiesse, 2017/04/28 15:51:34]

nit: s/Unknown/Unsupported

+    return nullptr;
+  }
+
+  base::StringPiece gltf_content(glb_content.data() + kContentStart,
+                                 content_length);
+  int error_code;
+  std::string error_msg;
+  JSONStringValueDeserializer json_deserializer(gltf_content);
+  auto gltf_value = json_deserializer.Deserialize(&error_code, &error_msg);
+  if (!gltf_value) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Content not a valid JSON";
+    return nullptr;
+  }
+  base::DictionaryValue* gltf_dict;
+  if (!gltf_value->GetAsDictionary(&gltf_dict)) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Content is not a valid glTF";
+    return nullptr;
+  }
+
+  auto glb_buffer = base::MakeUnique<gltf::Buffer>(
+      glb_content.substr(kContentStart + content_length));
+  buffers->push_back(std::move(glb_buffer));
+  GltfParser gltf_parser;
+  auto gltf_asset = gltf_parser.Parse(*gltf_dict, buffers);
+  if (!gltf_asset) {
+    LOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Content is not a valid glTF";
+    buffers->clear();
+    return nullptr;
+  }
+  return gltf_asset;
+}
+
 }  // namespace vr_shell