Bugfixing and code cleanup:

1. Fixed crasher when dependent attributes were encoded.
2. Improved documentation to PointCloud::AddAttribute() methods
3. Fixed some of the inconsistencies with int64_t vs size_t usage
4. Fixed potential IO problems when the input stream does not start from 0.

CMakeLists.txt

@@ -371,6 +371,7 @@ set(draco_test_sources
     "${draco_root}/compression/attributes/prediction_schemes/prediction_scheme_normal_octahedron_transform_test.cc"
     "${draco_root}/compression/attributes/sequential_integer_attribute_encoding_test.cc"
     "${draco_root}/compression/mesh/mesh_encoder_test.cc"
+    "${draco_root}/compression/mesh/mesh_edgebreaker_encoding_test.cc"
     "${draco_root}/compression/point_cloud/point_cloud_kd_tree_encoding_test.cc"
     "${draco_root}/compression/point_cloud/point_cloud_sequential_encoding_test.cc"
     "${draco_root}/core/bit_coder_test.cc"

compression/attributes/attributes_encoder.cc

@@ -20,7 +20,7 @@ AttributesEncoder::AttributesEncoder()
     : point_cloud_encoder_(nullptr), point_cloud_(nullptr) {}
 
 AttributesEncoder::AttributesEncoder(int att_id) : AttributesEncoder() {
-  point_attribute_ids_.push_back(att_id);
+  AddAttributeId(att_id);
 }
 
 bool AttributesEncoder::Initialize(PointCloudEncoder *encoder,

compression/mesh/mesh_decoder_helpers.h

@@ -28,10 +28,10 @@ template <typename InStreamT>
 InStreamT &DecodePos3Tex2DataFromStream(InStreamT &&is,
                                         std::vector<float> *out_data) {
   // Determine the size of the encoded data and write it into a vector.
-  auto is_size = is.tellg();
+  const auto start_pos = is.tellg();
   is.seekg(0, std::ios::end);
-  is_size = is.tellg() - is_size;
-  is.seekg(0, std::ios::beg);
+  const std::streampos is_size = is.tellg() - start_pos;
+  is.seekg(start_pos);
   std::vector<char> data(is_size);
   is.read(&data[0], is_size);
 

compression/mesh/mesh_edgebreaker_encoding_test.cc

@@ -0,0 +1,152 @@
+// Copyright 2016 The Draco Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+#include <sstream>
+
+#include "compression/mesh/mesh_edgebreaker_decoder.h"
+#include "compression/mesh/mesh_edgebreaker_encoder.h"
+#include "core/draco_test_base.h"
+#include "core/draco_test_utils.h"
+#include "io/mesh_io.h"
+#include "io/obj_decoder.h"
+#include "mesh/mesh_are_equivalent.h"
+#include "mesh/mesh_cleanup.h"
+#include "mesh/triangle_soup_mesh_builder.h"
+
+namespace draco {
+
+class MeshEdgebreakerEncodingTest : public ::testing::Test {
+ protected:
+  void TestFile(const std::string &file_name) {
+    TestFile(file_name, -1);
+  }
+
+  void TestFile(const std::string &file_name, int compression_level) {
+    const std::string path = GetTestFileFullPath(file_name);
+    const std::unique_ptr<Mesh> mesh(ReadMeshFromFile(path));
+    ASSERT_NE(mesh, nullptr) << "Failed to load test model " << file_name;
+
+    TestMesh(mesh.get(), compression_level);
+  }
+
+  void TestMesh(Mesh *mesh, int compression_level) {
+    EncoderBuffer buffer;
+    MeshEdgeBreakerEncoder encoder;
+    EncoderOptions encoder_options = CreateDefaultEncoderOptions();
+    if (compression_level != -1) {
+      SetSpeedOptions(&encoder_options, 10 - compression_level,
+                      10 - compression_level);
+    }
+    encoder.SetMesh(*mesh);
+    ASSERT_TRUE(encoder.Encode(encoder_options, &buffer));
+
+    DecoderBuffer dec_buffer;
+    dec_buffer.Init(buffer.data(), buffer.size());
+    MeshEdgeBreakerDecoder decoder;
+
+    std::unique_ptr<Mesh> decoded_mesh(new Mesh());
+    ASSERT_TRUE(decoder.Decode(&dec_buffer, decoded_mesh.get()));
+
+    // Cleanup the input mesh to make sure that input and output can be
+    // compared (edgebreaker method discards degenerated triangles and isolated
+    // vertices).
+    const MeshCleanupOptions options;
+    MeshCleanup cleanup;
+    ASSERT_TRUE(cleanup(mesh, options)) << "Failed to clean the input mesh.";
+
+    MeshAreEquivalent eq;
+    ASSERT_TRUE(eq(*mesh, *decoded_mesh.get()))
+        << "Decoded mesh is not the same as the input";
+  }
+};
+
+TEST_F(MeshEdgebreakerEncodingTest, TestNmOBJ) {
+  const std::string file_name = "test_nm.obj";
+  TestFile(file_name);
+}
+
+TEST_F(MeshEdgebreakerEncodingTest, ThreeFacesOBJ) {
+  const std::string file_name = "extra_vertex.obj";
+  TestFile(file_name);
+}
+
+TEST_F(MeshEdgebreakerEncodingTest, TestPly) {
+  // Tests whether the edgebreaker successfully encodes and decodes the test
+  // file (ply with color).
+  const std::string file_name = "test_pos_color.ply";
+  TestFile(file_name);
+}
+
+TEST_F(MeshEdgebreakerEncodingTest, TestMultiAttributes) {
+  // Tests encoding of model with many attributes.
+  const std::string file_name = "cube_att.obj";
+  TestFile(file_name, 10);
+}
+
+TEST_F(MeshEdgebreakerEncodingTest, TestEncoderReuse) {
+  // Tests whether the edgebreaker encoder can be reused multiple times to
+  // encode a given mesh.
+  const std::string file_name = "test_pos_color.ply";
+  const std::string path = GetTestFileFullPath(file_name);
+  const std::unique_ptr<Mesh> mesh(ReadMeshFromFile(path));
+  ASSERT_NE(mesh, nullptr) << "Failed to load test model " << file_name;
+
+  MeshEdgeBreakerEncoder encoder;
+  EncoderOptions encoder_options = CreateDefaultEncoderOptions();
+  encoder.SetMesh(*mesh);
+  EncoderBuffer buffer_0, buffer_1;
+  ASSERT_TRUE(encoder.Encode(encoder_options, &buffer_0));
+  ASSERT_TRUE(encoder.Encode(encoder_options, &buffer_1));
+
+  // Make sure both buffer are identical.
+  ASSERT_EQ(buffer_0.size(), buffer_1.size());
+  for (int i = 0; i < buffer_0.size(); ++i) {
+    ASSERT_EQ(buffer_0.data()[i], buffer_1.data()[i]);
+  }
+}
+
+TEST_F(MeshEdgebreakerEncodingTest, TestDecoderReuse) {
+  // Tests whether the edgebreaker decoder can be reused multiple times to
+  // decode a given mesh.
+  const std::string file_name = "test_pos_color.ply";
+  const std::string path = GetTestFileFullPath(file_name);
+  const std::unique_ptr<Mesh> mesh(ReadMeshFromFile(path));
+  ASSERT_NE(mesh, nullptr) << "Failed to load test model " << file_name;
+
+  MeshEdgeBreakerEncoder encoder;
+  EncoderOptions encoder_options = CreateDefaultEncoderOptions();
+  encoder.SetMesh(*mesh);
+  EncoderBuffer buffer;
+  ASSERT_TRUE(encoder.Encode(encoder_options, &buffer));
+
+  DecoderBuffer dec_buffer;
+  dec_buffer.Init(buffer.data(), buffer.size());
+
+  MeshEdgeBreakerDecoder decoder;
+
+  // Decode the mesh two times.
+  std::unique_ptr<Mesh> decoded_mesh_0(new Mesh());
+  ASSERT_TRUE(decoder.Decode(&dec_buffer, decoded_mesh_0.get()));
+
+  dec_buffer.Init(buffer.data(), buffer.size());
+  std::unique_ptr<Mesh> decoded_mesh_1(new Mesh());
+  ASSERT_TRUE(decoder.Decode(&dec_buffer, decoded_mesh_1.get()));
+
+  // Make sure both of the meshes are identical.
+  MeshAreEquivalent eq;
+  ASSERT_TRUE(eq(*decoded_mesh_0.get(), *decoded_mesh_1.get()))
+      << "Decoded meshes are not the same";
+}
+
+}  // namespace draco

io/mesh_io.h

@@ -53,10 +53,10 @@ OutStreamT &WriteMeshIntoStream(const Mesh *mesh, OutStreamT &&os) {
 template <typename InStreamT>
 InStreamT &ReadMeshFromStream(std::unique_ptr<Mesh> *mesh, InStreamT &&is) {
   // Determine size of stream and write into a vector
-  auto is_size = is.tellg();
+  const auto start_pos = is.tellg();
   is.seekg(0, std::ios::end);
-  is_size = is.tellg() - is_size;
-  is.seekg(0, std::ios::beg);
+  const std::streampos is_size = is.tellg() - start_pos;
+  is.seekg(start_pos);
   std::vector<char> data(is_size);
   is.read(&data[0], is_size);
 

io/point_cloud_io.h

@@ -54,10 +54,10 @@ template <typename InStreamT>
 InStreamT &ReadPointCloudFromStream(std::unique_ptr<PointCloud> *point_cloud,
                                     InStreamT &&is) {
   // Determine size of stream and write into a vector
-  auto is_size = is.tellg();
+  const auto start_pos = is.tellg();
   is.seekg(0, std::ios::end);
-  is_size = is.tellg() - is_size;
-  is.seekg(0, std::ios::beg);
+  const std::streampos is_size = is.tellg() - start_pos;
+  is.seekg(start_pos);
   std::vector<char> data(is_size);
   is.read(&data[0], is_size);
 

mesh/mesh.h

@@ -56,7 +56,7 @@ class Mesh : public PointCloud {
 
   // Sets the total number of faces. Creates new empty faces or deletes
   // existings ones if necessary.
-  void SetNumFaces(int64_t num_faces) { faces_.resize(num_faces, Face()); }
+  void SetNumFaces(size_t num_faces) { faces_.resize(num_faces, Face()); }
 
   FaceIndex::ValueType num_faces() const { return faces_.size(); }
   const Face &face(FaceIndex face_id) const {

point_cloud/point_attribute.h

@@ -69,7 +69,7 @@ class PointAttribute : public GeometryAttribute {
   }
   // This function sets the mapping to be explicitly using the indices_map_
   // array that needs to be initialized by the caller.
-  void SetExplicitMapping(int64_t num_points) {
+  void SetExplicitMapping(size_t num_points) {
     identity_mapping_ = false;
     indices_map_.resize(num_points, kInvalidAttributeValueIndex);
   }

point_cloud/point_cloud.h

@@ -64,12 +64,29 @@ class PointCloud {
     return attributes_[att_id].get();
   }
 
+  // Adds a new attribute to the point cloud.
+  // Returns the attribute id.
   int AddAttribute(std::unique_ptr<PointAttribute> pa);
+
+  // Creates and adds a new attribute to the point cloud. The attribute has
+  // properties derived from the provided GeometryAttribute |att|.
+  // If |identity_mapping| is set to true, the attribute will use identity
+  // mapping between point indices and attribute value indices (i.e., each point
+  // has a unique attribute value).
+  // If |identity_mapping| is false, the mapping between point indices and
+  // attribute value indices is set to explicit, and it needs to be initialized
+  // manually using the PointAttribute::SetPointMapEntry() method.
+  // |num_attribute_values| can be used to specify the number of attribute
+  // values that are going to be stored in the newly created attribute.
+  // Returns attribute id of the newly created attribute.
   int AddAttribute(const GeometryAttribute &att, bool identity_mapping,
                    AttributeValueIndex::ValueType num_attribute_values);
+
+  // Assigns an attribute id to a given PointAttribute. If an attribute with the
+  // same attribute id already exists, it is deleted.
   virtual void SetAttribute(int att_id, std::unique_ptr<PointAttribute> pa);
 
-  // Deduplicate all attribute values (all attribute entries with the same
+  // Deduplicates all attribute values (all attribute entries with the same
   // value are merged into a single entry).
   virtual bool DeduplicateAttributeValues();
 
@@ -86,7 +103,7 @@ class PointCloud {
   void set_num_points(PointIndex::ValueType num) { num_points_ = num; }
 
  protected:
-  // Apply id mapping of deduplicated points (called by DeduplicatePointIds).
+  // Applies id mapping of deduplicated points (called by DeduplicatePointIds).
   virtual void ApplyPointIdDeduplication(
       const IndexTypeVector<PointIndex, PointIndex> &id_map,
       const std::vector<PointIndex> &unique_point_ids);

testdata/cube_att.obj

@@ -0,0 +1,33 @@
+v  0.0  0.0  0.0
+v  0.0  0.0  1.0
+v  0.0  1.0  0.0
+v  0.0  1.0  1.0
+v  1.0  0.0  0.0
+v  1.0  0.0  1.0
+v  1.0  1.0  0.0
+v  1.0  1.0  1.0
+
+vn  0.0  0.0  1.0
+vn  0.0  0.0 -1.0
+vn  0.0  1.0  0.0
+vn  0.0 -1.0  0.0
+vn  1.0  0.0  0.0
+vn -1.0  0.0  0.0
+
+vt 0.0 0.0
+vt 0.0 1.0
+vt 1.0 0.0
+vt 1.0 1.0
+
+f  1/4/2  7/1/2  5/2/2
+f  1/4/2  3/3/2  7/1/2
+f  1/2/6  4/3/6  3/1/6
+f  1/2/6  2/4/6  4/3/6
+f  3/1/3  8/4/3  7/3/3
+f  3/1/3  4/2/3  8/4/3
+f  5/4/5  7/3/5  8/1/5
+f  5/4/5  8/1/5  6/2/5
+f  1/2/4  5/4/4  6/3/4
+f  1/2/4  6/3/4  2/1/4
+f  2/2/1  6/4/1  8/3/1
+f  2/2/1  8/3/1  4/1/1