Skip to content

Commit

Permalink
Update to latest tinyobjloader and patch tryParseDouble (#14656)
Browse files Browse the repository at this point in the history
  • Loading branch information
calderpg-tri committed Feb 18, 2021
1 parent 9905f8a commit cfb842a
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 12 deletions.
10 changes: 8 additions & 2 deletions geometry/proximity/obj_to_surface_mesh.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.");
}
Expand Down
6 changes: 5 additions & 1 deletion geometry/proximity_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
16 changes: 11 additions & 5 deletions geometry/render/gl_renderer/shape_meshes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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(
Expand Down Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion geometry/render/gl_renderer/test/shape_meshes_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.+");
}
}

Expand Down
154 changes: 154 additions & 0 deletions tools/workspace/tinyobjloader/faster_float_parsing.patch
Original file line number Diff line number Diff line change
@@ -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) {
9 changes: 6 additions & 3 deletions tools/workspace/tinyobjloader/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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",
],
)

0 comments on commit cfb842a

Please sign in to comment.