Revert of Implementing Binary glTF reader for the VR controller model...

Revert of Implementing Binary glTF reader for the VR controller model (patchset #3 id:40001 of https://codereview.chromium.org/2852533004/ )

Reason for revert:
Test added in this patch fails on downstream bot (see http://crbug.com/717088)

BUG=717088

Original issue's description:
> Implementing Binary glTF reader for the VR controller model
>
> The binary glTF format enables us to deliver 3D models on a single file with minimal size.
>
> Binary glTF Specification in: https://github.com/KhronosGroup/glTF/tree/master/extensions/Khronos/KHR_binary_glTF
>
> BUG=644562
>
> Review-Url: https://codereview.chromium.org/2852533004
> Cr-Commit-Position: refs/heads/master@{#468198}
> Committed: https://chromium.googlesource.com/chromium/src/+/b416391e0ae62217afb7ea5ffeb04ab2640a96ed

TBR=mthiesse@chromium.org,bajones@chromium.org,acondor@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=644562

Review-Url: https://codereview.chromium.org/2852103002
Cr-Commit-Position: refs/heads/master@{#468338}

chrome/browser/android/vr_shell/BUILD.gn

@@ -146,7 +146,6 @@ if (enable_vr) {
       "test/data/sample_inline.gltf",
       "test/data/sample_external.gltf",
       "test/data/sample.bin",
-      "test/data/sample.glb",
     ]
   }
 }

chrome/browser/android/vr_shell/gltf_parser.cc

@@ -9,36 +9,14 @@
 #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[] =
-    "Failed to read binary glTF: ";
-constexpr const char kGltfMagic[] = "glTF";
-constexpr const char kBinaryGltfBufferName[] = "binary_glTF";
-constexpr uint32_t kJsonGltfFormat = 0;
-constexpr size_t kVersionStart = 4;
-constexpr size_t kLengthStart = 8;
-constexpr size_t kContentLengthStart = 12;
-constexpr size_t kContentFormatStart = 16;
-constexpr size_t 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;
@@ -47,7 +25,6 @@ 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>();
 
@@ -63,7 +40,6 @@ 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;
@@ -113,19 +89,12 @@ bool GltfParser::ParseInternal(
 bool GltfParser::SetBuffers(
     const base::DictionaryValue& dict,
     std::vector<std::unique_ptr<gltf::Buffer>>* buffers) {
-  size_t buffer_count = 0;
+  buffers->clear();
   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_count)
-        return false;
-      buffer_ids_[it.key()] = 0;
-      continue;
-    }
-
     std::string uri_str;
     if (!buffer_dict->GetString("uri", &uri_str))
       return false;
@@ -140,7 +109,6 @@ bool GltfParser::SetBuffers(
 
     buffer_ids_[it.key()] = buffers->size();
     buffers->push_back(std::move(buffer));
-    ++buffer_count;
   }
   return true;
 }
@@ -365,64 +333,4 @@ 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) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Incomplete data";
-    return nullptr;
-  }
-  if (!glb_content.starts_with(kGltfMagic)) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Unknown magic number";
-    return nullptr;
-  }
-  if (GetLE32(glb_content.data() + kVersionStart) != 1) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Unsupported version";
-    return nullptr;
-  }
-  if (GetLE32(glb_content.data() + kLengthStart) != glb_content.length()) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Incorrect file size";
-    return nullptr;
-  }
-  uint32_t content_length = GetLE32(glb_content.data() + kContentLengthStart);
-  if (kContentStart + content_length > glb_content.length()) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Invalid content length";
-    return nullptr;
-  }
-  if (GetLE32(glb_content.data() + kContentFormatStart) != kJsonGltfFormat) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Unsupported glTF format";
-    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) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Content not a valid JSON";
-    return nullptr;
-  }
-  base::DictionaryValue* gltf_dict;
-  if (!gltf_value->GetAsDictionary(&gltf_dict)) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Content is not a JSON object";
-    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) {
-    DLOG(ERROR) << kFailedtoReadBinaryGltfMsg << "Content is not a valid glTF";
-    buffers->clear();
-    return nullptr;
-  }
-  return gltf_asset;
-}
-
 }  // namespace vr_shell

chrome/browser/android/vr_shell/gltf_parser.h

@@ -67,16 +67,6 @@ class GltfParser {
   DISALLOW_COPY_AND_ASSIGN(GltfParser);
 };
 
-class BinaryGltfParser {
- public:
-  // Note: If your glTF references external files, this function will perform
-  // IO, and a base path must be specified.
-  static std::unique_ptr<gltf::Asset> Parse(
-      const base::StringPiece& glb_content,
-      std::vector<std::unique_ptr<gltf::Buffer>>* buffers,
-      const base::FilePath& path = base::FilePath());
-};
-
 }  // namespace vr_shell
 
 #endif  // CHROME_BROWSER_ANDROID_VR_SHELL_GLTF_PARSER_H_

chrome/browser/android/vr_shell/gltf_parser_unittest.cc

@@ -17,26 +17,18 @@
 
 namespace vr_shell {
 
-class DataDrivenTest : public testing::Test {
+class GltfParserTest : public testing::Test {
  protected:
   static void SetUpTestCase() { test::RegisterPathProvider(); }
 
   void SetUp() override { PathService::Get(test::DIR_TEST_DATA, &data_dir_); }
 
   base::FilePath data_dir_;
-};
 
-class GltfParserTest : public DataDrivenTest {
- protected:
   std::unique_ptr<base::DictionaryValue> Deserialize(
       const base::FilePath& gltf_path);
 };
 
-class BinaryGltfParserTest : public DataDrivenTest {
- protected:
-  base::StringPiece Read(const base::FilePath& path);
-};
-
 std::unique_ptr<base::DictionaryValue> GltfParserTest::Deserialize(
     const base::FilePath& gltf_path) {
   int error_code;
@@ -50,12 +42,6 @@ std::unique_ptr<base::DictionaryValue> GltfParserTest::Deserialize(
   return std::unique_ptr<base::DictionaryValue>(asset);
 }
 
-base::StringPiece BinaryGltfParserTest::Read(const base::FilePath& path) {
-  std::string data;
-  base::ReadFileToString(path, &data);
-  return base::StringPiece(data);
-}
-
 TEST_F(GltfParserTest, Parse) {
   auto asset = Deserialize(data_dir_.Append("sample_inline.gltf"));
   GltfParser parser;
@@ -171,15 +157,4 @@ TEST_F(GltfParserTest, ParseExternalNoPath) {
   EXPECT_EQ(nullptr, parser.Parse(*asset, &buffers));
 }
 
-TEST_F(BinaryGltfParserTest, ParseBinary) {
-  auto glb_data = Read(data_dir_.Append("sample.glb"));
-  std::vector<std::unique_ptr<gltf::Buffer>> buffers;
-  auto asset = BinaryGltfParser::Parse(glb_data, &buffers);
-  EXPECT_TRUE(asset);
-  EXPECT_EQ(1u, buffers.size());
-  const gltf::BufferView* buffer_view = asset->GetBufferView(0);
-  EXPECT_TRUE(buffer_view);
-  EXPECT_EQ(0, buffer_view->buffer);
-}
-
 }  // namespace vr_shell

chrome/browser/android/vr_shell/vr_controller_model.cc

@@ -20,8 +20,8 @@ namespace vr_shell {
 namespace {
 
 enum {
-  ELEMENTS_BUFFER_ID = 2,
-  INDICES_BUFFER_ID = 3,
+  ELEMENTS_BUFFER_ID = 0,
+  INDICES_BUFFER_ID = 1,
 };
 
 constexpr char kPosition[] = "POSITION";
@@ -33,7 +33,7 @@ constexpr char const kComponentName[] = "VrShell";
 constexpr char const kDefaultVersion[] = "0";
 
 constexpr char const kModelsDirectory[] = "models";
-constexpr char const kModelFilename[] = "ddcontroller.glb";
+constexpr char const kModelFilename[] = "controller.gltf";
 constexpr char const kTexturesDirectory[] = "tex";
 constexpr char const kBaseTextureFilename[] = "ddcontroller_idle.png";
 constexpr char const* kTexturePatchesFilenames[] = {
@@ -167,11 +167,9 @@ std::unique_ptr<VrControllerModel> VrControllerModel::LoadFromComponent() {
     return nullptr;
   }
 
+  GltfParser gltf_parser;
   std::vector<std::unique_ptr<gltf::Buffer>> buffers;
-
-  std::string model_data;
-  base::ReadFileToString(model_path, &model_data);
-  auto asset = BinaryGltfParser::Parse(base::StringPiece(model_data), &buffers);
+  auto asset = gltf_parser.Parse(model_path, &buffers);
   if (!asset) {
     LOG(ERROR) << "Failed to read controller model";
     return nullptr;