Update to latest tinyobjloader with patched tryParseDouble

geometry/proximity/obj_to_surface_mesh.cc

@@ -12,6 +12,7 @@
 #include <tiny_obj_loader.h>
 
 #include "drake/common/drake_assert.h"
+#include "drake/common/text_logging.h"
 #include "drake/geometry/proximity/surface_mesh.h"
 
 namespace drake {
@@ -128,17 +129,22 @@ SurfaceMesh<double> ReadObjToSurfaceMesh(std::istream* input_stream,
   tinyobj::attrib_t attrib;  // Used for vertices.
   std::vector<tinyobj::shape_t> shapes;  // Used for triangles.
   std::vector<tinyobj::material_t> materials;  // Not used.
+  std::string warn;
   std::string err;
   // Ignore material-library file.
   tinyobj::MaterialReader* readMatFn = nullptr;
   // triangulate non-triangle faces.
   bool triangulate = true;
 
-  bool ret = tinyobj::LoadObj(&attrib, &shapes, &materials, &err, input_stream,
-                              readMatFn, triangulate);
+  bool ret = tinyobj::LoadObj(
+      &attrib, &shapes, &materials, &warn, &err, input_stream, readMatFn,
+      triangulate);
   if (!ret || !err.empty()) {
     throw std::runtime_error("Error parsing Wavefront obj file : " + err);
   }
+  if (!warn.empty()) {
+    drake::log()->warn("Warning parsing Wavefront obj file : {}", warn);
+  }
   if (shapes.size() == 0) {
     throw std::runtime_error("The Wavefront obj file has no faces.");
   }

geometry/proximity_engine.cc

@@ -139,6 +139,7 @@ namespace {
     tinyobj::attrib_t attrib;
     std::vector<tinyobj::shape_t> shapes;
     std::vector<tinyobj::material_t> materials;
+    std::string warn;
     std::string err;
 
     // Tinyobj doesn't infer the search directory from the directory containing
@@ -148,12 +149,15 @@ namespace {
     const std::string obj_folder = filename.substr(0, pos + 1);
     const char* mtl_basedir = obj_folder.c_str();
 
-    bool ret = tinyobj::LoadObj(&attrib, &shapes, &materials, &err,
+    bool ret = tinyobj::LoadObj(&attrib, &shapes, &materials, &warn, &err,
                                 filename.c_str(), mtl_basedir, triangulate);
     if (!ret || !err.empty()) {
       throw std::runtime_error("Error parsing file '" + filename +
                                "' : " + err);
     }
+    if (!warn.empty()) {
+      drake::log()->warn("Warning parsing file '{}' : {}", filename, warn);
+    }
 
     if (shapes.size() != 1) {
       throw std::runtime_error(

geometry/render/gl_renderer/shape_meshes.cc

@@ -35,6 +35,7 @@ MeshData LoadMeshFromObj(std::istream* input_stream,
   tinyobj::attrib_t attrib;
   vector<tinyobj::shape_t> shapes;
   vector<tinyobj::material_t> materials;
+  string warn;
   string err;
   /* This renderer assumes everything is triangles -- we rely on tinyobj to
    triangulate for us. */
@@ -48,12 +49,12 @@ MeshData LoadMeshFromObj(std::istream* input_stream,
    Ignore material-library file.  */
   tinyobj::MaterialReader* material_reader = nullptr;
   const bool ret =
-      tinyobj::LoadObj(&attrib, &shapes, &materials, &err, input_stream,
+      tinyobj::LoadObj(&attrib, &shapes, &materials, &warn, &err, input_stream,
                        material_reader, do_tinyobj_triangulation);
-  /* As of tinyobj v1.0.6, we expect that `ret` will *always* be true. We are
-   capturing it and asserting it so that if the version advances, and false is
-   ever returned, CI will inform us so we can update the error messages.  */
-  DRAKE_DEMAND(ret == true);
+  if (!ret) {
+    throw std::runtime_error(
+        fmt::format("tinyobj::LoadObj failed to load file: {}", filename));
+  }
 
   if (shapes.empty()) {
     throw std::runtime_error(fmt::format(
@@ -134,6 +135,11 @@ MeshData LoadMeshFromObj(std::istream* input_stream,
         const int position_index = shape_mesh.indices[v_index].vertex_index;
         const int norm_index = shape_mesh.indices[v_index].normal_index;
         const int uv_index = shape_mesh.indices[v_index].texcoord_index;
+        // TODO(SeanCurtis-TRI) PR 14656 changed parse semantics. This error
+        // condition appears to no longer be reachable (it no longer appears
+        // in the unit tests) and the condition that this detects won't trigger
+        // this helpful message. Either clean up this case or find a way to give
+        // this feedback under the new tinyobj.
         if (norm_index < 0) {
           throw std::runtime_error(
               fmt::format("Not all faces reference normals: {}", filename));

geometry/render/gl_renderer/test/shape_meshes_test.cc

@@ -70,7 +70,7 @@ f 1//0 2//0 3//0
 )""");
     DRAKE_EXPECT_THROWS_MESSAGE(
         LoadMeshFromObj(&in_stream), std::runtime_error,
-        "Not all faces reference texture coordinates.+");
+        "tinyobj::LoadObj failed to load file.+");
   }
 }
 

tools/workspace/tinyobjloader/faster_float_parsing.patch

@@ -0,0 +1,154 @@
+--- tiny_obj_loader.h.orig	2017-04-24 15:34:45.000000000 -0400
++++ tiny_obj_loader.h	2019-06-19 15:57:29.479457051 -0400
+@@ -62,6 +62,14 @@ THE SOFTWARE.
+ #include <map>
+ #include <string>
+ #include <vector>
++#include <math.h>
++#include <stdlib.h>
++
++#if defined(__APPLE__)
++#include <xlocale.h>
++#else
++#include <locale.h>
++#endif
+
+ namespace tinyobj {
+
+@@ -839,125 +847,20 @@ static bool tryParseDouble(const char *s, const char *s_end, double *result) {
+     return false;
+   }
+
+-  double mantissa = 0.0;
+-  // This exponent is base 2 rather than 10.
+-  // However the exponent we parse is supposed to be one of ten,
+-  // thus we must take care to convert the exponent/and or the
+-  // mantissa to a * 2^E, where a is the mantissa and E is the
+-  // exponent.
+-  // To get the final double we will use ldexp, it requires the
+-  // exponent to be in base 2.
+-  int exponent = 0;
+-
+-  // NOTE: THESE MUST BE DECLARED HERE SINCE WE ARE NOT ALLOWED
+-  // TO JUMP OVER DEFINITIONS.
+-  char sign = '+';
+-  char exp_sign = '+';
+-  char const *curr = s;
+-
+-  // How many characters were read in a loop.
+-  int read = 0;
+-  // Tells whether a loop terminated due to reaching s_end.
+-  bool end_not_reached = false;
+-  bool leading_decimal_dots = false;
+-
+-  /*
+-          BEGIN PARSING.
+-  */
+-
+-  // Find out what sign we've got.
+-  if (*curr == '+' || *curr == '-') {
+-    sign = *curr;
+-    curr++;
+-    if ((curr != s_end) && (*curr == '.')) {
+-      // accept. Somethig like `.7e+2`, `-.5234`
+-      leading_decimal_dots = true;
+-    }
+-  } else if (IS_DIGIT(*curr)) { /* Pass through. */
+-  } else if (*curr == '.') {
+-    // accept. Somethig like `.7e+2`, `-.5234`
+-    leading_decimal_dots = true;
+-  } else {
+-    goto fail;
+-  }
+-
+-  // Read the integer part.
+-  end_not_reached = (curr != s_end);
+-  if (!leading_decimal_dots) {
+-    while (end_not_reached && IS_DIGIT(*curr)) {
+-      mantissa *= 10;
+-      mantissa += static_cast<int>(*curr - 0x30);
+-      curr++;
+-      read++;
+-      end_not_reached = (curr != s_end);
+-    }
+-
+-    // We must make sure we actually got something.
+-    if (read == 0) goto fail;
+-  }
++#if defined(__APPLE__)
++  static locale_t c_locale = newlocale(LC_ALL_MASK, NULL, NULL);
++#else
++  static locale_t c_locale = newlocale(LC_ALL_MASK, "C", (locale_t)0);
++#endif
+
+-  // We allow numbers of form "#", "###" etc.
+-  if (!end_not_reached) goto assemble;
+-
+-  // Read the decimal part.
+-  if (*curr == '.') {
+-    curr++;
+-    read = 1;
+-    end_not_reached = (curr != s_end);
+-    while (end_not_reached && IS_DIGIT(*curr)) {
+-      static const double pow_lut[] = {
+-          1.0, 0.1, 0.01, 0.001, 0.0001, 0.00001, 0.000001, 0.0000001,
+-      };
+-      const int lut_entries = sizeof pow_lut / sizeof pow_lut[0];
+-
+-      // NOTE: Don't use powf here, it will absolutely murder precision.
+-      mantissa += static_cast<int>(*curr - 0x30) *
+-                  (read < lut_entries ? pow_lut[read] : std::pow(10.0, -read));
+-      read++;
+-      curr++;
+-      end_not_reached = (curr != s_end);
+-    }
+-  } else if (*curr == 'e' || *curr == 'E') {
++  char *str_end = NULL;
++  const double val = strtod_l(s, &str_end, c_locale);
++  if (str_end != s && isfinite(val)) {
++    *result = val;
++    return true;
+   } else {
+-    goto assemble;
+-  }
+-
+-  if (!end_not_reached) goto assemble;
+-
+-  // Read the exponent part.
+-  if (*curr == 'e' || *curr == 'E') {
+-    curr++;
+-    // Figure out if a sign is present and if it is.
+-    end_not_reached = (curr != s_end);
+-    if (end_not_reached && (*curr == '+' || *curr == '-')) {
+-      exp_sign = *curr;
+-      curr++;
+-    } else if (IS_DIGIT(*curr)) { /* Pass through. */
+-    } else {
+-      // Empty E is not allowed.
+-      goto fail;
+-    }
+-
+-    read = 0;
+-    end_not_reached = (curr != s_end);
+-    while (end_not_reached && IS_DIGIT(*curr)) {
+-      exponent *= 10;
+-      exponent += static_cast<int>(*curr - 0x30);
+-      curr++;
+-      read++;
+-      end_not_reached = (curr != s_end);
+-    }
+-    exponent *= (exp_sign == '+' ? 1 : -1);
+-    if (read == 0) goto fail;
++    return false;
+   }
+-
+-assemble:
+-  *result = (sign == '+' ? 1 : -1) *
+-            (exponent ? std::ldexp(mantissa * std::pow(5.0, exponent), exponent)
+-                      : mantissa);
+-  return true;
+-fail:
+-  return false;
+ }
+
+ static inline real_t parseReal(const char **token, double default_value = 0.0) {

tools/workspace/tinyobjloader/repository.bzl

@@ -7,9 +7,9 @@ def tinyobjloader_repository(
         mirrors = None):
     github_archive(
         name = name,
-        repository = "syoyo/tinyobjloader",
-        commit = "v1.0.6",
-        sha256 = "19ee82cd201761954dd833de551edb570e33b320d6027e0d91455faf7cd4c341",  # noqa
+        repository = "tinyobjloader/tinyobjloader",
+        commit = "9173980d1de273b17eba5e10eb189e8b4be89425",
+        sha256 = "fe06bf462bf386ea7f6bf34f94c78099c849df348d4a6df681707fba5b5b643c",  # noqa
         build_file = "@drake//tools/workspace/tinyobjloader:package.BUILD.bazel",  # noqa
         mirrors = mirrors,
         patches = [
@@ -18,5 +18,8 @@ def tinyobjloader_repository(
             # dependency of Drake and we don't want the definition to leak into
             # all target that consume Drake.
             "@drake//tools/workspace/tinyobjloader:double_precision.patch",
+            # We replace tinyobjloader's implementation of float parsing with a
+            # faster call to strtod_l.
+            "@drake//tools/workspace/tinyobjloader:faster_float_parsing.patch",
         ],
     )