Fixed Bugs/Unit Tests Pass

Fixed bugs found by unit tests and got unit tests running with RapidJSON as well as nlohmann.
This commit is contained in:
jrkoonce 2019-09-03 15:56:48 -05:00
parent 67e6160a9a
commit 06c30c4d04
2 changed files with 94 additions and 37 deletions

View File

@ -167,12 +167,12 @@ TEST_CASE("parse-integer", "[bounds-checking]") {
SECTION("parses valid numbers") { SECTION("parses valid numbers") {
std::string err; std::string err;
int result = 123; int result = 123;
CHECK(tinygltf::ParseIntegerProperty(&result, &err, {{"zero", 0}}, "zero", CHECK(tinygltf::ParseIntegerProperty(&result, &err, JsonConstruct("{\"zero\" : 0}"), "zero",
true)); true));
REQUIRE(err == ""); REQUIRE(err == "");
REQUIRE(result == 0); REQUIRE(result == 0);
CHECK(tinygltf::ParseIntegerProperty(&result, &err, {{"int", -1234}}, "int", CHECK(tinygltf::ParseIntegerProperty(&result, &err, JsonConstruct("{\"int\": -1234}"), "int",
true)); true));
REQUIRE(err == ""); REQUIRE(err == "");
REQUIRE(result == -1234); REQUIRE(result == -1234);
@ -181,7 +181,7 @@ TEST_CASE("parse-integer", "[bounds-checking]") {
SECTION("detects missing properties") { SECTION("detects missing properties") {
std::string err; std::string err;
int result = -1; int result = -1;
CHECK_FALSE(tinygltf::ParseIntegerProperty(&result, &err, {}, "int", true)); CHECK_FALSE(tinygltf::ParseIntegerProperty(&result, &err, JsonConstruct(""), "int", true));
REQUIRE_THAT(err, Catch::Contains("'int' property is missing")); REQUIRE_THAT(err, Catch::Contains("'int' property is missing"));
REQUIRE(result == -1); REQUIRE(result == -1);
} }
@ -190,7 +190,7 @@ TEST_CASE("parse-integer", "[bounds-checking]") {
std::string err; std::string err;
int result = -1; int result = -1;
CHECK_FALSE( CHECK_FALSE(
tinygltf::ParseIntegerProperty(&result, &err, {}, "int", false)); tinygltf::ParseIntegerProperty(&result, &err, JsonConstruct(""), "int", false));
REQUIRE(err == ""); REQUIRE(err == "");
REQUIRE(result == -1); REQUIRE(result == -1);
} }
@ -199,21 +199,26 @@ TEST_CASE("parse-integer", "[bounds-checking]") {
std::string err; std::string err;
int result = -1; int result = -1;
CHECK_FALSE(tinygltf::ParseIntegerProperty(&result, &err, {{"int", 0.5}}, CHECK_FALSE(tinygltf::ParseIntegerProperty(&result, &err, JsonConstruct("{\"int\": 0.5}"),
"int", true)); "int", true));
REQUIRE_THAT(err, Catch::Contains("not an integer type")); REQUIRE_THAT(err, Catch::Contains("not an integer type"));
// Excessively large values and NaN aren't allowed either. // Excessively large values and NaN aren't allowed either.
err.clear(); err.clear();
CHECK_FALSE(tinygltf::ParseIntegerProperty(&result, &err, {{"int", 1e300}}, CHECK_FALSE(tinygltf::ParseIntegerProperty(&result, &err, JsonConstruct("{\"int\": 1e300}"),
"int", true)); "int", true));
REQUIRE_THAT(err, Catch::Contains("not an integer type")); REQUIRE_THAT(err, Catch::Contains("not an integer type"));
err.clear(); err.clear();
CHECK_FALSE(tinygltf::ParseIntegerProperty( {
&result, &err, {{"int", std::numeric_limits<double>::quiet_NaN()}}, JsonDocument o;
double nan = std::numeric_limits<double>::quiet_NaN();
tinygltf::JsonAddMember(o, "int", json(nan));
CHECK_FALSE(tinygltf::ParseIntegerProperty(
&result, &err, o,
"int", true)); "int", true));
REQUIRE_THAT(err, Catch::Contains("not an integer type")); REQUIRE_THAT(err, Catch::Contains("not an integer type"));
}
} }
} }
@ -221,7 +226,7 @@ TEST_CASE("parse-unsigned", "[bounds-checking]") {
SECTION("parses valid unsigned integers") { SECTION("parses valid unsigned integers") {
// Use string-based parsing here, using the initializer list syntax doesn't // Use string-based parsing here, using the initializer list syntax doesn't
// parse 0 as unsigned. // parse 0 as unsigned.
json zero_obj = json::parse("{\"zero\": 0}"); auto zero_obj = JsonConstruct("{\"zero\": 0}");
std::string err; std::string err;
size_t result = 123; size_t result = 123;
@ -235,26 +240,31 @@ TEST_CASE("parse-unsigned", "[bounds-checking]") {
std::string err; std::string err;
size_t result = -1; size_t result = -1;
CHECK_FALSE(tinygltf::ParseUnsignedProperty(&result, &err, {{"int", -1234}}, CHECK_FALSE(tinygltf::ParseUnsignedProperty(&result, &err, JsonConstruct("{\"int\": -1234}"),
"int", true)); "int", true));
REQUIRE_THAT(err, Catch::Contains("not a positive integer")); REQUIRE_THAT(err, Catch::Contains("not a positive integer"));
err.clear(); err.clear();
CHECK_FALSE(tinygltf::ParseUnsignedProperty(&result, &err, {{"int", 0.5}}, CHECK_FALSE(tinygltf::ParseUnsignedProperty(&result, &err, JsonConstruct("{\"int\": 0.5}"),
"int", true)); "int", true));
REQUIRE_THAT(err, Catch::Contains("not a positive integer")); REQUIRE_THAT(err, Catch::Contains("not a positive integer"));
// Excessively large values and NaN aren't allowed either. // Excessively large values and NaN aren't allowed either.
err.clear(); err.clear();
CHECK_FALSE(tinygltf::ParseUnsignedProperty(&result, &err, {{"int", 1e300}}, CHECK_FALSE(tinygltf::ParseUnsignedProperty(&result, &err, JsonConstruct("{\"int\": 1e300}"),
"int", true)); "int", true));
REQUIRE_THAT(err, Catch::Contains("not a positive integer")); REQUIRE_THAT(err, Catch::Contains("not a positive integer"));
err.clear(); err.clear();
CHECK_FALSE(tinygltf::ParseUnsignedProperty( {
&result, &err, {{"int", std::numeric_limits<double>::quiet_NaN()}}, JsonDocument o;
double nan = std::numeric_limits<double>::quiet_NaN();
tinygltf::JsonAddMember(o, "int", json(nan));
CHECK_FALSE(tinygltf::ParseUnsignedProperty(
&result, &err, o,
"int", true)); "int", true));
REQUIRE_THAT(err, Catch::Contains("not a positive integer")); REQUIRE_THAT(err, Catch::Contains("not a positive integer"));
}
} }
} }
@ -263,7 +273,7 @@ TEST_CASE("parse-integer-array", "[bounds-checking]") {
std::string err; std::string err;
std::vector<int> result; std::vector<int> result;
CHECK(tinygltf::ParseIntegerArrayProperty(&result, &err, CHECK(tinygltf::ParseIntegerArrayProperty(&result, &err,
{{"x", {-1, 2, 3}}}, "x", true)); JsonConstruct("{\"x\": [-1, 2, 3]}"), "x", true));
REQUIRE(err == ""); REQUIRE(err == "");
REQUIRE(result.size() == 3); REQUIRE(result.size() == 3);
REQUIRE(result[0] == -1); REQUIRE(result[0] == -1);
@ -275,7 +285,7 @@ TEST_CASE("parse-integer-array", "[bounds-checking]") {
std::string err; std::string err;
std::vector<int> result; std::vector<int> result;
CHECK_FALSE(tinygltf::ParseIntegerArrayProperty( CHECK_FALSE(tinygltf::ParseIntegerArrayProperty(
&result, &err, {{"x", {-1, 1e300, 3}}}, "x", true)); &result, &err, JsonConstruct("{\"x\": [-1, 1e300, 3]}"), "x", true));
REQUIRE_THAT(err, Catch::Contains("not an integer type")); REQUIRE_THAT(err, Catch::Contains("not an integer type"));
} }
} }

View File

@ -1637,9 +1637,20 @@ namespace
assert(s_pActiveDocument == nullptr); //Code assumes only one document is active at a time assert(s_pActiveDocument == nullptr); //Code assumes only one document is active at a time
s_pActiveDocument = this; s_pActiveDocument = this;
} }
~JsonDocument() { JsonDocument(const JsonDocument&) = delete;
s_pActiveDocument = nullptr; JsonDocument(JsonDocument&& rhs) noexcept : rapidjson::Document(std::move(rhs))
{
s_pActiveDocument = this;
rhs.isNil = true;
} }
~JsonDocument() {
if (!isNil)
{
s_pActiveDocument = nullptr;
}
}
private:
bool isNil = false;
}; };
#else #else
using nlohmann::json; using nlohmann::json;
@ -1656,6 +1667,13 @@ namespace
doc = json::parse(str, str + length, nullptr, throwExc); doc = json::parse(str, str + length, nullptr, throwExc);
#endif #endif
} }
JsonDocument JsonConstruct(const char* str)
{
JsonDocument doc;
JsonParse(doc, str, strlen(str));
return doc;
}
} }
#ifdef __APPLE__ #ifdef __APPLE__
@ -2878,9 +2896,13 @@ namespace
} }
bool FindMember(const json& o, const char* member, json_const_iterator& it) bool FindMember(const json& o, const char* member, json_const_iterator& it)
{ {
#ifdef TINYGLTF_USE_RAPIDJSON #ifdef TINYGLTF_USE_RAPIDJSON
if (!o.IsObject())
{
return false;
}
it = o.FindMember(member); it = o.FindMember(member);
return it != o.MemberEnd(); return it != o.MemberEnd();
#else #else
@ -3476,7 +3498,7 @@ static bool ParseExtensionsProperty(ExtensionMap *ret, std::string *err,
if (!IsObject(itObj)) continue; if (!IsObject(itObj)) continue;
std::string key(GetKey(extIt)); std::string key(GetKey(extIt));
if (!ParseJsonAsValue(&extensions[key], itObj)) { if (!ParseJsonAsValue(&extensions[key], itObj)) {
if (key.empty()) { if (!key.empty()) {
// create empty object so that an extension object is still of type // create empty object so that an extension object is still of type
// object // object
extensions[key] = Value{Value::Object{}}; extensions[key] = Value{Value::Object{}};
@ -4436,18 +4458,33 @@ static bool ParseMaterial(Material *material, std::string *err, const json &o) {
for (; it != itEnd; ++it) { for (; it != itEnd; ++it) {
std::string key(GetKey(it)); std::string key(GetKey(it));
if (key == "pbrMetallicRoughness") {
if (IsObject(GetValue(it))) {
const json &values_object = GetValue(it);
if ((key == "pbrMetallicRoughness") || json_const_iterator itVal(ObjectBegin(values_object));
(key == "extensions") || json_const_iterator itValEnd(ObjectEnd(values_object));
(key == "extras")) {
continue; //Remove duplicating these in parameters for (; itVal != itValEnd; ++itVal) {
Parameter param;
if (ParseParameterProperty(&param, err, values_object, GetKey(itVal),
false)) {
material->values.emplace(GetKey(itVal), std::move(param));
}
}
}
} }
else if (key == "extensions" || key == "extras") {
Parameter param; // done later, skip, otherwise poorly parsed contents will be saved in the
if (ParseParameterProperty(&param, err, o, key, false)) { // parametermap and serialized again later
// names of materials have already been parsed. Putting it in this map }
// doesn't correctly reflext the glTF specification else {
if (key != "name") material->additionalValues.emplace(std::move(key), std::move(param)); Parameter param;
if (ParseParameterProperty(&param, err, o, key, false)) {
// names of materials have already been parsed. Putting it in this map
// doesn't correctly reflext the glTF specification
if (key != "name") material->additionalValues.emplace(std::move(key), std::move(param));
}
} }
} }
@ -5607,8 +5644,8 @@ bool TinyGLTF::LoadBinaryFromFile(Model *model, std::string *err,
/////////////////////// ///////////////////////
// GLTF Serialization // GLTF Serialization
/////////////////////// ///////////////////////
namespace //namespace
{ //{
json JsonFromString(const char* s) json JsonFromString(const char* s)
{ {
#ifdef TINYGLTF_USE_RAPIDJSON #ifdef TINYGLTF_USE_RAPIDJSON
@ -5680,6 +5717,15 @@ namespace
#endif #endif
} }
void JsonSetObject(json& o)
{
#ifdef TINYGLTF_USE_RAPIDJSON
o.SetObject();
#else
o = o.object({});
#endif
}
void JsonReserveArray(json& o, size_t s) void JsonReserveArray(json& o, size_t s)
{ {
#ifdef TINYGLTF_USE_RAPIDJSON #ifdef TINYGLTF_USE_RAPIDJSON
@ -5689,7 +5735,7 @@ namespace
(void)(o); (void)(o);
(void)(s); (void)(s);
} }
} //}
// typedef std::pair<std::string, json> json_object_pair; // typedef std::pair<std::string, json> json_object_pair;
@ -5903,6 +5949,7 @@ static void SerializeExtensionMap(ExtensionMap &extensions, json &o) {
// create empty object so that an extension name is still included in // create empty object so that an extension name is still included in
// json. // json.
json empty; json empty;
JsonSetObject(empty);
JsonAddMember(extMap, extIt->first.c_str(), std::move(empty)); JsonAddMember(extMap, extIt->first.c_str(), std::move(empty));
} }
} }