ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

ObjImporter: rewrite without STL strings, iostreams and exceptions. 

And expand tests with corner cases that I previously forgot about. Can't
really provide any benchmarking info, since the Rungholt scene from
McGuire archives uses quads and negative indices.
master
Vladimír Vondruš 4 years ago
parent
da66ec88e8
commit
dacf37f237
10 changed files with 420 additions and 245 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 8
      src/Magnum/SceneTools/Test/SceneConverterTest.cpp
  2. 7
      src/MagnumPlugins/ObjImporter/CMakeLists.txt
  3. 441
      src/MagnumPlugins/ObjImporter/ObjImporter.cpp
  4. 1
      src/MagnumPlugins/ObjImporter/ObjImporter.h
  5. 2
      src/MagnumPlugins/ObjImporter/Test/CMakeLists.txt
  6. 115
      src/MagnumPlugins/ObjImporter/Test/ObjImporterTest.cpp
  7. 1
      src/MagnumPlugins/ObjImporter/Test/invalid-keyword.obj
  8. 6
      src/MagnumPlugins/ObjImporter/Test/invalid-keywords.obj
  9. 45
      src/MagnumPlugins/ObjImporter/Test/invalid-number-count.obj
  10. 19
      src/MagnumPlugins/ObjImporter/Test/invalid-numbers.obj

8
src/Magnum/SceneTools/Test/SceneConverterTest.cpp
Unescape View File

@ -1109,7 +1109,7 @@ const struct {
Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/whatever.ply")
}},
"ObjImporter", nullptr, nullptr, nullptr,
"Trade::ObjImporter::mesh(): wrong index count for point\n"
"Trade::ObjImporter::mesh(): expected exactly 1 position index tuple for a point, got 5 5\n"
"Cannot import the mesh\n"},
{"can't import a mesh for concatenation", {InPlaceInit, {
"-I", "ObjImporter", "--concatenate-meshes",
@ -1117,7 +1117,7 @@ const struct {
Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/whatever.ply")
}},
"ObjImporter", nullptr, nullptr, nullptr,
"Trade::ObjImporter::mesh(): wrong index count for point\n"
"Trade::ObjImporter::mesh(): expected exactly 1 position index tuple for a point, got 5 5\n"
"Cannot import mesh 0\n"},
{"can't import a scene for concatenation", {InPlaceInit, {
/** @todo change to an OBJ once ObjImporter imports materials (and
@ -1135,7 +1135,7 @@ const struct {
Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/whatever.ply")
}},
"ObjImporter", nullptr, nullptr, nullptr,
"Trade::ObjImporter::mesh(): wrong index count for point\n"
"Trade::ObjImporter::mesh(): expected exactly 1 position index tuple for a point, got 5 5\n"
"Cannot import mesh 0\n"},
{"invalid mesh attribute filter", {InPlaceInit, {
/** @todo drop --mesh once it's not needed anymore again */
@ -1188,7 +1188,7 @@ const struct {
Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/whatever.ply")
}},
"ObjImporter", nullptr, "StanfordSceneConverter", nullptr,
"Trade::ObjImporter::mesh(): wrong index count for point\n"
"Trade::ObjImporter::mesh(): expected exactly 1 position index tuple for a point, got 5 5\n"
"Cannot add importer contents\n"},
{"can't add processed meshes", {InPlaceInit, {
"-I", "ObjImporter", "-C", "StanfordSceneConverter",

7
src/MagnumPlugins/ObjImporter/CMakeLists.txt
Unescape View File

@ -45,13 +45,6 @@ if(MAGNUM_OBJIMPORTER_BUILD_STATIC AND MAGNUM_BUILD_STATIC_PIC)
set_target_properties(ObjImporter PROPERTIES POSITION_INDEPENDENT_CODE ON)
endif()
target_link_libraries(ObjImporter PUBLIC MagnumTrade MagnumMeshTools)
if(CORRADE_TARGET_EMSCRIPTEN)
# Since 1.39.0, this needs to be enabled on the plugin. Before (on 1.38.44
# at least) this wasn't needed, at least for the test -- having it enabled
# for the test binary and in the linker was enough. Most probably related
# to fastcomp->llvm backend switch.
set_property(TARGET ObjImporter APPEND_STRING PROPERTY COMPILE_FLAGS " -s DISABLE_EXCEPTION_CATCHING=0")
endif()
install(FILES ObjImporter.h ${CMAKE_CURRENT_BINARY_DIR}/configure.h
DESTINATION ${MAGNUM_PLUGINS_INCLUDE_INSTALL_DIR}/ObjImporter)

441
src/MagnumPlugins/ObjImporter/ObjImporter.cpp
Unescape View File

@ -26,17 +26,13 @@
#include "ObjImporter.h"
#include <fstream>
#include <limits>
#include <sstream>
#include <unordered_map>
#include <Corrade/Containers/GrowableArray.h>
#include <Corrade/Containers/Optional.h>
#include <Corrade/Containers/StridedArrayView.h>
#include <Corrade/Containers/String.h>
#include <Corrade/Containers/StringStl.h> /** @todo remove once iostream is dropped */
#include <Corrade/Utility/DebugStl.h>
#include <Corrade/Utility/String.h>
#include <Corrade/Containers/StringStlHash.h>
#include <Corrade/Utility/Algorithms.h>
#include "Magnum/Mesh.h"
#include "Magnum/MeshTools/RemoveDuplicates.h"
@ -46,52 +42,103 @@
namespace Magnum { namespace Trade {
using namespace Containers::Literals;
namespace {
struct Mesh {
std::streampos begin;
std::streampos end;
/* Points to File::fileData, the end is implicitly `(this + 1)->begin` */
const char* begin;
/* Name of the mesh. The first mesh has a name only if it appears before
any data line (v/vt/vn/... or p/l/f/...). */
Containers::StringView name;
/* Offset of the first position, texture coordinate and normal index.
Assuming that not only vertex data but also index data follow the mesh
name, this way we don't have to parse the whole file if just a single
mesh out of many is requested. */
UnsignedInt positionIndexOffset;
UnsignedInt textureCoordinateIndexOffset;
UnsignedInt normalIndexOffset;
std::string name;
};
}
struct ObjImporter::File {
std::unordered_map<std::string, UnsignedInt> meshesForName;
std::unordered_map<Containers::StringView, UnsignedInt> meshesForName;
/* Contains always n + 1 entries, with the last entry being an upper bound
on the file range and index offsets */
Containers::Array<Mesh> meshes;
Containers::Pointer<std::istream> in;
Containers::Array<char> fileData;
};
namespace {
void ignoreLine(std::istream& in) {
in.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
/* The spec doesn't say anything explicit about whitespace, but at the very
least the examples at http://paulbourke.net/dataformats/obj/ pad the values
with whitespace and various tools are also producing such files, as reported
at https://forum.babylonjs.com/t/extra-whitespace-breaks-obj-parsing/5244
Besides space I'm considering a tab and a CR character. *Not* a newline,
since that is a significant delimiter that has to be treated separately. */
constexpr Containers::StringView Whitespace = " \t\r"_s;
/* Mostly just a copy of Corrade's Utility::Json::parseFloatInternal() and
parseUnsignedIntInternal() */
/** @todo make a common API in Corrade once we have something that can parse
numbers like a grownup */
inline bool parseFloat(const char* const errorPrefix, const Containers::StringView string, Float& out) {
/** @todo replace with something that can parse non-null-terminated stuff,
then drop this "too long" error */
char buffer[128];
const std::size_t size = string.size();
if(size > Containers::arraySize(buffer) - 1) {
Error{} << errorPrefix << "too long numeric literal" << string;
return false;
}
template<std::size_t size> Math::Vector<size, Float> extractFloatData(const std::string& str, Float* extra = nullptr) {
std::vector<std::string> data = Utility::String::splitWithoutEmptyParts(str, ' ');
if(data.size() < size || data.size() > size + (extra ? 1 : 0)) {
Error() << "Trade::ObjImporter::mesh(): invalid float array size";
throw 0;
std::memcpy(buffer, string.data(), size);
buffer[size] = '\0';
char* end;
out = std::strtof(buffer, &end);
if(!string || std::size_t(end - buffer) != size) {
Error{} << errorPrefix << "invalid floating-point literal" << string;
return false;
}
Math::Vector<size, Float> output;
for(std::size_t i = 0; i != size; ++i)
output[i] = std::stof(data[i]);
/* Success; value already written above */
return true;
}
if(data.size() == size+1) {
/* This should be obvious from the first if, but add this just to make
Clang Analyzer happy */
CORRADE_INTERNAL_ASSERT(extra);
inline bool parseUnsignedInt(const char* const errorPrefix, const Containers::StringView string, UnsignedInt& out) {
/** @todo replace with something that can parse non-null-terminated stuff,
then drop this "too long" error */
char buffer[128];
const std::size_t size = string.size();
if(size > Containers::arraySize(buffer) - 1) {
Error{} << errorPrefix << "too long numeric literal" << string;
return false;
}
*extra = std::stof(data.back());
std::memcpy(buffer, string.data(), size);
buffer[size] = '\0';
char* end;
/* Not using strtoul() here as on Windows it's 32-bit and we wouldn't be
able to detect overflows */
/** @todo replace with something that can report errors in a non-insane
way */
const std::uint64_t outLong = std::strtoull(buffer, &end, 10);
if(!string || std::size_t(end - buffer) != size) {
Error{} << errorPrefix << "invalid integer literal" << string;
return false;
}
if(outLong > ~std::uint32_t{}) {
Error{} << errorPrefix << "too large integer literal" << string;
return false;
}
return output;
/* On success convert the value to 32 bits */
out = outLong;
return true;
}
}
@ -104,84 +151,72 @@ ObjImporter::~ObjImporter() = default;
ImporterFeatures ObjImporter::doFeatures() const { return ImporterFeature::OpenData; }
void ObjImporter::doClose() { _file.reset(); }
bool ObjImporter::doIsOpened() const { return !!_file; }
void ObjImporter::doOpenFile(const Containers::StringView filename) {
/** @todo ARGH clean this up, won't work with UTF-8 */
Containers::Pointer<std::istream> in{new std::ifstream{filename, std::ios::binary}};
if(!in->good()) {
Error() << "Trade::ObjImporter::openFile(): cannot open file" << filename;
return;
void ObjImporter::doClose() {
_file = {};
}
_file.reset(new File);
_file->in = Utility::move(in);
parseMeshNames();
}
bool ObjImporter::doIsOpened() const { return !!_file; }
void ObjImporter::doOpenData(Containers::Array<char>&& data, DataFlags) {
void ObjImporter::doOpenData(Containers::Array<char>&& data, const DataFlags dataFlags) {
_file.reset(new File);
/** @todo ARGH MY EYES what is this cursed thing, burn it to the ground */
_file->in.reset(new std::istringstream{{data.begin(), data.size()}});
parseMeshNames();
/* Copy file content. Take over the existing array or copy the data if we
can't. We need to keep the data around as JSON tokens are views onto it
and also for the GLB binary chunk. */
if(dataFlags & (DataFlag::Owned|DataFlag::ExternallyOwned)) {
_file->fileData = std::move(data);
} else {
_file->fileData = Containers::Array<char>{NoInit, data.size()};
Utility::copy(data, _file->fileData);
}
void ObjImporter::parseMeshNames() {
/* First mesh starts at the beginning, its indices start from 1. The end
offset will be updated to proper value later. */
/* First mesh starts at the beginning, its indices start from 1 */
UnsignedInt positionIndexOffset = 1;
UnsignedInt normalIndexOffset = 1;
UnsignedInt textureCoordinateIndexOffset = 1;
arrayAppend(_file->meshes, InPlaceInit, _file->fileData.begin(), Containers::StringView{}, positionIndexOffset, textureCoordinateIndexOffset, normalIndexOffset);
/* The first mesh doesn't have name by default but we might find it later,
so we need to track whether there are any data before first name */
bool thisIsFirstMeshAndItHasNoData = true;
arrayAppend(_file->meshes, InPlaceInit, 0, 0, positionIndexOffset, normalIndexOffset, textureCoordinateIndexOffset, std::string{});
while(_file->in->good()) {
/* The previous object might end at the beginning of this line */
const std::streampos end = _file->in->tellg();
/** @todo check size < 1G on 32b? currently it'd just assert, but it's
unlikely that such amount of contiguous memory would even be available
there, so ¯\_(ツ)_/¯ */
Containers::StringView in{_file->fileData, _file->fileData.size()};
while(in) {
/* Get a (trimmed) line from the input */
const Containers::StringView lineEnd = in.findOr('\n', in.end());
const Containers::StringView line = in.prefix(lineEnd.begin()).trimmed(Whitespace);
in = in.suffix(lineEnd.end());
/* Comment line */
if(_file->in->peek() == '#') {
ignoreLine(*_file->in);
continue;
}
/* Comment or empty line, skip */
if(!line || line.hasPrefix('#')) continue;
/* Parse the keyword */
std::string keyword;
*_file->in >> keyword;
const Containers::StringView keywordEnd = line.findAnyOr(Whitespace, line.end());
const Containers::StringView keyword = line.prefix(keywordEnd.begin());
/* Mesh name */
if(keyword == "o") {
std::string name;
std::getline(*_file->in, name);
name = Utility::String::trim(name);
if(keyword == "o"_s) {
const Containers::StringView name = line.suffix(keywordEnd.end()).trimmed(Whitespace);
/* This is the name of first mesh */
if(thisIsFirstMeshAndItHasNoData) {
thisIsFirstMeshAndItHasNoData = false;
/* Update its name and add it to name map */
if(!name.empty())
if(name)
_file->meshesForName.emplace(name, _file->meshes.size() - 1);
_file->meshes.back().name = Utility::move(name);
/* Update its begin offset to be more precise */
_file->meshes.back().begin = _file->in->tellg();
_file->meshes.back().name = name;
/* Otherwise this is a name of new mesh */
} else {
/* Set end of the previous one */
_file->meshes.back().end = end;
/* Save name and offset of the new one. The end offset will be
updated later. */
if(!name.empty())
if(name)
_file->meshesForName.emplace(name, _file->meshes.size());
arrayAppend(_file->meshes, InPlaceInit, _file->in->tellg(), 0, positionIndexOffset, textureCoordinateIndexOffset, normalIndexOffset, Utility::move(name));
arrayAppend(_file->meshes, InPlaceInit, in.begin(), name, positionIndexOffset, textureCoordinateIndexOffset, normalIndexOffset);
}
continue;
@ -190,43 +225,40 @@ void ObjImporter::parseMeshNames() {
the first object is unnamed. We need to check for them. */
/* Vertex data, update index offset for the following meshes */
} else if(keyword == "v") {
} else if(keyword == "v"_s) {
++positionIndexOffset;
thisIsFirstMeshAndItHasNoData = false;
} else if(keyword == "vt") {
} else if(keyword == "vt"_s) {
++textureCoordinateIndexOffset;
thisIsFirstMeshAndItHasNoData = false;
} else if(keyword == "vn") {
} else if(keyword == "vn"_s) {
++normalIndexOffset;
thisIsFirstMeshAndItHasNoData = false;
/* Index data, just mark that we found something for first unnamed
object */
} else if(thisIsFirstMeshAndItHasNoData) for(const std::string data: {"p", "l", "f"}) {
if(keyword == data) {
} else if(keyword == "p"_s ||
keyword == "l"_s ||
keyword == "f"_s) {
thisIsFirstMeshAndItHasNoData = false;
break;
}
}
/* Ignore the rest of the line */
ignoreLine(*_file->in);
/* Save the final offset so we have an upper bound on index offsets */
arrayAppend(_file->meshes, InPlaceInit, in.begin(), Containers::StringView{}, positionIndexOffset, textureCoordinateIndexOffset, normalIndexOffset);
}
/* Set end of the last object */
_file->in->clear();
_file->in->seekg(0, std::ios::end);
_file->meshes.back().end = _file->in->tellg();
UnsignedInt ObjImporter::doMeshCount() const {
/* There's always one more item for an upper bound */
return _file->meshes.size() - 1;
}
UnsignedInt ObjImporter::doMeshCount() const { return _file->meshes.size(); }
Int ObjImporter::doMeshForName(const Containers::StringView name) {
const auto it = _file->meshesForName.find(name);
return it == _file->meshesForName.end() ? -1 : it->second;
}
Containers::String ObjImporter::doMeshName(UnsignedInt id) {
Containers::String ObjImporter::doMeshName(const UnsignedInt id) {
return _file->meshes[id].name;
}
@ -246,10 +278,9 @@ template<class T> bool checkAndDuplicateInto(const Containers::StridedArrayView1
}
Containers::Optional<MeshData> ObjImporter::doMesh(UnsignedInt id, UnsignedInt) {
Containers::Optional<MeshData> ObjImporter::doMesh(const UnsignedInt id, UnsignedInt) {
/* Seek the file, set mesh parsing parameters */
const Mesh& mesh = _file->meshes[id];
_file->in->seekg(mesh.begin);
Containers::Optional<MeshPrimitive> primitive;
Containers::Array<Vector3> positions;
@ -260,68 +291,151 @@ Containers::Optional<MeshData> ObjImporter::doMesh(UnsignedInt id, UnsignedInt)
Containers::Array<Vector3ui> indices;
std::size_t textureCoordinateIndexCount = 0, normalIndexCount = 0;
try { while(_file->in->good() && _file->in->tellg() < mesh.end) {
/* Ignore comments */
if(_file->in->peek() == '#') {
ignoreLine(*_file->in);
Containers::StringView in{mesh.begin, std::size_t(_file->meshes[id + 1].begin - mesh.begin)};
while(in) {
/* Get a line from the input */
const Containers::StringView lineEnd = in.findOr('\n', in.end());
const Containers::StringView line = in.prefix(lineEnd.begin()).trimmed(Whitespace);
in = in.suffix(lineEnd.end());
/* Comment or empty line, skip */
if(!line || line.hasPrefix('#')) continue;
/* Parse the keyword */
const Containers::StringView keywordEnd = line.findAnyOr(Whitespace, line.end());
const Containers::StringView keyword = line.prefix(keywordEnd.begin());
/* Skip keywords that are not interesting to us or that were parsed
earlier. In particular, the `o` can be here because the mesh range
is everything until the next mesh data start, so including the next mesh name. */
if(keyword == "o"_s ||
keyword == "g"_s ||
keyword == "s"_s ||
keyword == "mtllib"_s ||
keyword == "usemtl"_s)
continue;
}
/* Get the line */
std::string line;
std::getline(*_file->in, line);
line = Utility::String::trim(line);
/* Keyword contents */
Containers::StringView contents = line.suffix(keywordEnd.end()).trimmedPrefix(Whitespace);
/* Ignore empty lines */
if(line.empty()) continue;
/* Vertex data */
if(keyword == "v"_s || keyword == "vt"_s || keyword == "vn"_s) {
/* Decide on how many components we expect at most. There's
optional behavior for four-component positions and
three-component texture coordinates, so it can't be an exact
count. */
std::size_t maxComponentCount;
if(keyword == "v"_s) maxComponentCount = 4;
else if(keyword == "vt"_s) maxComponentCount = 3;
else if(keyword == "vn"_s) maxComponentCount = 3;
else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
/* Split the line into keyword and contents */
const std::size_t keywordEnd = line.find(' ');
const std::string keyword = line.substr(0, keywordEnd);
const std::string contents = keywordEnd != std::string::npos ?
Utility::String::ltrim(line.substr(keywordEnd+1)) : "";
/* Parse them all. If there's less than expected, `i` would be too
small; if there's more then `contents` would stay non-empty. */
Float data[4];
std::size_t i = 0;
for(; i != maxComponentCount && contents; ++i) {
const Containers::StringView foundSpace = contents.findAnyOr(Whitespace, contents.end());
/* Vertex position */
if(keyword == "v") {
Float extra{1.0f};
const Vector3 data = extractFloatData<3>(contents, &extra);
if(!Math::TypeTraits<Float>::equals(extra, 1.0f)) {
Error() << "Trade::ObjImporter::mesh(): homogeneous coordinates are not supported";
return Containers::NullOpt;
if(!parseFloat("Trade::ObjImporter::mesh():", contents.prefix(foundSpace.begin()), data[i]))
return {};
contents = contents.suffix(foundSpace.end()).trimmedPrefix(Whitespace);
}
arrayAppend(positions, data);
/* Position */
if(keyword == "v"_s) {
if(i < 3 || contents) {
Error{} << "Trade::ObjImporter::mesh(): expected 3 or 4 position coordinates, got" << line.suffix(keywordEnd.end());
return {};
}
if(i == 4 && !Math::equal(data[3], 1.0f)) {
Error{} << "Trade::ObjImporter::mesh(): homogeneous coordinates are not supported";
return {};
}
arrayAppend(positions, Vector3::from(data));
/* Texture coordinate */
} else if(keyword == "vt") {
Float extra{0.0f};
const Vector2 data = extractFloatData<2>(contents, &extra);
if(!Math::TypeTraits<Float>::equals(extra, 0.0f)) {
Error() << "Trade::ObjImporter::mesh(): 3D texture coordinates are not supported";
return Containers::NullOpt;
} else if(keyword == "vt"_s) {
if(i < 2 || contents) {
Error{} << "Trade::ObjImporter::mesh(): expected 2 or 3 texture coordinates, got" << line.suffix(keywordEnd.end());
return {};
}
if(i == 3 && !Math::equal(data[2], 0.0f)) {
Error{} << "Trade::ObjImporter::mesh(): 3D texture coordinates are not supported";
return {};
}
arrayAppend(textureCoordinates, data);
arrayAppend(textureCoordinates, Vector2::from(data));
/* Normal */
} else if(keyword == "vn") {
arrayAppend(normals, Vector3{extractFloatData<3>(contents)});
} else if(keyword == "vn"_s) {
if(i < 3 || contents) {
Error{} << "Trade::ObjImporter::mesh(): expected 3 normal coordinates, got" << line.suffix(keywordEnd.end());
return {};
}
arrayAppend(normals, Vector3::from(data));
} else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
/* Indices */
} else if(keyword == "p" || keyword == "l" || keyword == "f") {
const std::vector<std::string> indexTuples = Utility::String::splitWithoutEmptyParts(contents, ' ');
} else if(keyword == "p"_s || keyword == "l"_s || keyword == "f"_s) {
/* Decide on how many tuples we expect */
std::size_t indexTupleCount;
if(keyword == "p"_s) indexTupleCount = 1;
else if(keyword == "l"_s) indexTupleCount = 2;
else if(keyword == "f"_s) indexTupleCount = 3;
else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
/* Parse them all. If there's less than expected, `i` would be too
small; if there's more then `contents` would stay non-empty. */
Vector3ui data[3];
std::size_t i = 0;
for(; i != indexTupleCount && contents; ++i) {
const Containers::StringView foundSpace = contents.findAnyOr(Whitespace, contents.end());
Containers::StringView indexTuple = contents.prefix(foundSpace.begin());
/* The number before first slash is a position index */
const Containers::StringView foundSlash1 = indexTuple.findOr('/', indexTuple.end());
if(!parseUnsignedInt("Trade::ObjImporter::mesh():", indexTuple.prefix(foundSlash1.begin()), data[i][0]))
return {};
data[i][0] -= mesh.positionIndexOffset;
/* If there was a slash, next is a texture coordinate or
empty */
if(foundSlash1) {
indexTuple = indexTuple.suffix(foundSlash1.end());
const Containers::StringView foundSlash2 = indexTuple.findOr('/', indexTuple.end());
if(!foundSlash2 || foundSlash2.begin() != indexTuple.begin()) {
if(!parseUnsignedInt("Trade::ObjImporter::mesh():", indexTuple.prefix(foundSlash2.begin()), data[i][2]))
return {};
data[i][2] -= mesh.textureCoordinateIndexOffset;
++textureCoordinateIndexCount;
}
/* If there was a second slash, last is a normal */
if(foundSlash2) {
indexTuple = indexTuple.suffix(foundSlash2.end());
if(!parseUnsignedInt("Trade::ObjImporter::mesh():", indexTuple, data[i][1]))
return {};
data[i][1] -= mesh.normalIndexOffset;
++normalIndexCount;
}
}
contents = contents.suffix(foundSpace.end()).trimmedPrefix(Whitespace);
}
/* Points */
if(keyword == "p") {
/* Check that we don't mix the primitives in one mesh */
if(primitive && primitive != MeshPrimitive::Points) {
Error() << "Trade::ObjImporter::mesh(): mixed primitive" << *primitive << "and" << MeshPrimitive::Points;
return Containers::NullOpt;
}
/* Check vertex count per primitive */
if(indexTuples.size() != 1) {
Error() << "Trade::ObjImporter::mesh(): wrong index count for point";
if(i < 1 || contents) {
Error() << "Trade::ObjImporter::mesh(): expected exactly 1 position index tuple for a point, got" << line.suffix(keywordEnd.end());
return Containers::NullOpt;
}
@ -329,15 +443,12 @@ Containers::Optional<MeshData> ObjImporter::doMesh(UnsignedInt id, UnsignedInt)
/* Lines */
} else if(keyword == "l") {
/* Check that we don't mix the primitives in one mesh */
if(primitive && primitive != MeshPrimitive::Lines) {
Error() << "Trade::ObjImporter::mesh(): mixed primitive" << *primitive << "and" << MeshPrimitive::Lines;
return Containers::NullOpt;
}
/* Check vertex count per primitive */
if(indexTuples.size() != 2) {
Error() << "Trade::ObjImporter::mesh(): wrong index count for line";
if(i < 2 || contents) {
Error() << "Trade::ObjImporter::mesh(): expected exactly 2 position index tuples for a line, got" << line.suffix(keywordEnd.end());
return Containers::NullOpt;
}
@ -345,18 +456,12 @@ Containers::Optional<MeshData> ObjImporter::doMesh(UnsignedInt id, UnsignedInt)
/* Faces */
} else if(keyword == "f") {
/* Check that we don't mix the primitives in one mesh */
if(primitive && primitive != MeshPrimitive::Triangles) {
Error() << "Trade::ObjImporter::mesh(): mixed primitive" << *primitive << "and" << MeshPrimitive::Triangles;
return Containers::NullOpt;
}
/* Check vertex count per primitive */
if(indexTuples.size() < 3) {
Error() << "Trade::ObjImporter::mesh(): wrong index count for triangle";
return Containers::NullOpt;
} else if(indexTuples.size() != 3) {
Error() << "Trade::ObjImporter::mesh(): polygons are not supported";
if(i < 3 || contents) {
Error() << "Trade::ObjImporter::mesh(): expected exactly 3 position index tuples for a triangle, got" << line.suffix(keywordEnd.end());
return Containers::NullOpt;
}
@ -364,50 +469,14 @@ Containers::Optional<MeshData> ObjImporter::doMesh(UnsignedInt id, UnsignedInt)
} else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
for(const std::string& indexTuple: indexTuples) {
std::vector<std::string> indexStrings = Utility::String::split(indexTuple, '/');
if(indexStrings.size() > 3) {
Error() << "Trade::ObjImporter::mesh(): invalid index data";
return Containers::NullOpt;
}
Vector3ui index;
/* Position indices */
index[0] = std::stoul(indexStrings[0]) - mesh.positionIndexOffset;
/* Texture coordinates */
if(indexStrings.size() == 2 || (indexStrings.size() == 3 && !indexStrings[1].empty())) {
index[2] = std::stoul(indexStrings[1]) - mesh.textureCoordinateIndexOffset;
++textureCoordinateIndexCount;
}
/* Normal indices */
if(indexStrings.size() == 3) {
index[1] = std::stoul(indexStrings[2]) - mesh.normalIndexOffset;
++normalIndexCount;
}
/** @todo fix arrayAppend() to not need the cast here */
arrayAppend(indices, Containers::ArrayView<const Vector3ui>{data}.prefix(i));
arrayAppend(indices, index);
}
/* Ignore unsupported keywords, error out on unknown keywords */
} else if(![&keyword](){
/* Using lambda to emulate for-else construct like in Python */
for(const std::string expected: {"mtllib", "usemtl", "g", "s"})
if(keyword == expected) return true;
return false;
}()) {
Error() << "Trade::ObjImporter::mesh(): unknown keyword" << keyword;
return Containers::NullOpt;
/* Unknown keyword */
} else {
Error{} << "Trade::ObjImporter::mesh(): unknown keyword" << keyword;
return {};
}
}} catch(const std::exception&) {
Error() << "Trade::ObjImporter::mesh(): error while converting numeric data";
return Containers::NullOpt;
} catch(...) {
/* Error message already printed */
return Containers::NullOpt;
}
/* There should be at least indexed position data */

1
src/MagnumPlugins/ObjImporter/ObjImporter.h
Unescape View File

@ -124,7 +124,6 @@ class MAGNUM_OBJIMPORTER_EXPORT ObjImporter: public AbstractImporter {
MAGNUM_OBJIMPORTER_LOCAL bool doIsOpened() const override;
MAGNUM_OBJIMPORTER_LOCAL void doOpenData(Containers::Array<char>&& data, DataFlags dataFlags) override;
MAGNUM_OBJIMPORTER_LOCAL void doOpenFile(Containers::StringView filename) override;
MAGNUM_OBJIMPORTER_LOCAL void doClose() override;
MAGNUM_OBJIMPORTER_LOCAL UnsignedInt doMeshCount() const override;

2
src/MagnumPlugins/ObjImporter/Test/CMakeLists.txt
Unescape View File

@ -50,7 +50,7 @@ corrade_add_test(ObjImporterTest ObjImporterTest.cpp
empty.obj
invalid-incomplete-data.obj
invalid-inconsistent-index-tuple.obj
invalid-keyword.obj
invalid-keywords.obj
invalid-mixed-primitives.obj
invalid-number-count.obj
invalid-numbers.obj

115
src/MagnumPlugins/ObjImporter/Test/ObjImporterTest.cpp
Unescape View File

@ -68,7 +68,7 @@ struct ObjImporterTest: TestSuite::Tester {
maintain. So it's instead grouped into files by a common error scenario
with each case testing one file and just the invalid() case testing
separate files. */
void invalid();
void invalidKeywords();
void invalidMixedPrimitives();
void invalidNumbers();
void invalidNumberCount();
@ -76,6 +76,8 @@ struct ObjImporterTest: TestSuite::Tester {
void invalidIncompleteData();
void invalidOptionalCoordinate();
void whitespace();
void openTwice();
void importTwice();
@ -93,11 +95,11 @@ const struct {
const struct {
const char* name;
const char* filename;
const char* message;
} InvalidData[]{
{"unknown keyword", "invalid-keyword.obj",
"unknown keyword bleh"}
} InvalidKeywordsData[]{
{"unknown keyword", "unknown keyword bleh"},
{"no space between vertex keyword and number", "unknown keyword vt3"},
{"no space between index keyword and number", "unknown keyword p6"}
};
const struct {
@ -116,9 +118,12 @@ const struct {
const char* name;
const char* message;
} InvalidNumbersData[]{
{"invalid float literal", "error while converting numeric data"},
{"invalid integer literal", "error while converting numeric data"},
{"position index out of range", "index 1 out of range for 1 vertices"},
{"too long float literal", "too long numeric literal 1234.567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567"},
{"too long integer literal", "too long numeric literal 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678"},
{"too large integer literal", "too large integer literal 4294967296"},
{"invalid float literal", "invalid floating-point literal bleh"},
{"invalid integer literal", "invalid integer literal bleh"},
{"position index out of range", "index 3 out of range for 1 vertices"},
{"texture index out of range", "index 4 out of range for 3 vertices"},
{"normal index out of range", "index 3 out of range for 2 vertices"},
{"zero index", "index 0 out of range for 1 vertices"}
@ -128,14 +133,25 @@ const struct {
const char* name;
const char* message;
} InvalidNumberCountData[]{
{"two-component position", "invalid float array size"},
{"five-component position with optional fourth component", "invalid float array size"},
{"four-component normal", "invalid float array size"},
{"four-component index tuple", "invalid index data"},
{"point with two indices", "wrong index count for point"},
{"line with one index", "wrong index count for line"},
{"triangle with two indices", "wrong index count for triangle"},
{"quad", "polygons are not supported"}
{"no position component at all", "expected 3 or 4 position coordinates, got "},
{"two-component position", "expected 3 or 4 position coordinates, got 0.5 1.0"},
{"five-component position with optional fourth component", "expected 3 or 4 position coordinates, got 0.5 1 2 1.0 3.5"},
{"no texture coordinate component at all", "expected 2 or 3 texture coordinates, got "},
{"one-component texture coordinate", "expected 2 or 3 texture coordinates, got 0.5"},
{"four-component texture coordinate with optional third component", "expected 2 or 3 texture coordinates, got 0.5 1.0 0.0 7.4"},
{"no normal component at all", "expected 3 normal coordinates, got "},
{"two-component normal", "expected 3 normal coordinates, got 0.5 0.0"},
{"four-component normal", "expected 3 normal coordinates, got 0.5 1.0 2.3 7.4"},
{"no index at all", "expected exactly 1 position index tuple for a point, got "},
/** @todo better error message (the literal is empty) */
{"no index before first slash", "invalid integer literal "},
{"no index after first slash", "invalid integer literal "},
{"no index after second slash", "invalid integer literal "},
{"four-component index tuple", "invalid integer literal 1/1"},
{"point with two indices", "expected exactly 1 position index tuple for a point, got 9 9"},
{"line with one index", "expected exactly 2 position index tuples for a line, got 10"},
{"triangle with two indices", "expected exactly 3 position index tuples for a triangle, got 11 11"},
{"quad", "expected exactly 3 position index tuples for a triangle, got 12 12 12 12"}
};
const struct {
@ -188,8 +204,8 @@ ObjImporterTest::ObjImporterTest() {
addTests({&ObjImporterTest::moreMeshes});
addInstancedTests({&ObjImporterTest::invalid},
Containers::arraySize(InvalidData));
addInstancedTests({&ObjImporterTest::invalidKeywords},
Containers::arraySize(InvalidKeywordsData));
addInstancedTests({&ObjImporterTest::invalidMixedPrimitives},
Containers::arraySize(InvalidMixedPrimitivesData));
@ -209,7 +225,9 @@ ObjImporterTest::ObjImporterTest() {
addInstancedTests({&ObjImporterTest::invalidOptionalCoordinate},
Containers::arraySize(InvalidOptionalCoordinateData));
addTests({&ObjImporterTest::openTwice,
addTests({&ObjImporterTest::whitespace,
&ObjImporterTest::openTwice,
&ObjImporterTest::importTwice});
#ifdef OBJIMPORTER_PLUGIN_FILENAME
@ -554,18 +572,19 @@ void ObjImporterTest::moreMeshes() {
TestSuite::Compare::Container);
}
void ObjImporterTest::invalid() {
auto&& data = InvalidData[testCaseInstanceId()];
void ObjImporterTest::invalidKeywords() {
auto&& data = InvalidKeywordsData[testCaseInstanceId()];
setTestCaseDescription(data.name);
Containers::Pointer<AbstractImporter> importer = _manager.instantiate("ObjImporter");
CORRADE_VERIFY(importer->openFile(Utility::Path::join(OBJIMPORTER_TEST_DIR, data.filename)));
CORRADE_VERIFY(importer->openFile(Utility::Path::join(OBJIMPORTER_TEST_DIR, "invalid-keywords.obj")));
CORRADE_COMPARE(importer->meshCount(), 1);
/* Ensure we didn't forget to test any case */
CORRADE_COMPARE(importer->meshCount(), Containers::arraySize(InvalidKeywordsData));
Containers::String out;
Error redirectError{&out};
CORRADE_VERIFY(!importer->mesh(0));
CORRADE_VERIFY(!importer->mesh(data.name));
CORRADE_COMPARE(out, Utility::format("Trade::ObjImporter::mesh(): {}\n", data.message));
}
@ -665,6 +684,54 @@ void ObjImporterTest::invalidOptionalCoordinate() {
CORRADE_COMPARE(out, Utility::format("Trade::ObjImporter::mesh(): {}\n", data.message));
}
void ObjImporterTest::whitespace() {
/* Using an embedded string to have the whitespace visualized clearer */
Containers::StringView data =
" \t\r # This file has various \t\n"
" #whitespace that should be treated properly \n"
" \r\t o\t \tObject \t name \r\n"
" \t v\r \t 3 5 \t7 \t1\n"
" \rvt 8\t 9\t\t\r0\n"
"\n \r \n"
" vn \t 2 \t 4 \r6\n"
"\n \t \r\n"
/* There's no space expected inside the index tuple */
" f\t 1/1/1 \r1/1/1 \t1/1/1 \t\r\n"
"\n\n"
"\n";
Containers::Pointer<AbstractImporter> importer = _manager.instantiate("ObjImporter");
CORRADE_VERIFY(importer->openData(data));
CORRADE_COMPARE(importer->meshCount(), 1);
/* The name is trimmed from both ends, but internal whitespace is left
intact */
CORRADE_COMPARE(importer->meshName(0), "Object \t name");
const Containers::Optional<MeshData> mesh = importer->mesh(0);
CORRADE_VERIFY(mesh);
CORRADE_COMPARE(mesh->primitive(), MeshPrimitive::Triangles);
CORRADE_COMPARE(mesh->attributeCount(), 3);
CORRADE_COMPARE_AS(mesh->attribute<Vector3>(MeshAttribute::Position),
Containers::arrayView<Vector3>({
{3.0f, 5.0f, 7.0f}
}), TestSuite::Compare::Container);
CORRADE_COMPARE_AS(mesh->attribute<Vector2>(MeshAttribute::TextureCoordinates),
Containers::arrayView<Vector2>({
{8.0f, 9.0f}
}), TestSuite::Compare::Container);
CORRADE_COMPARE_AS(mesh->attribute<Vector3>(MeshAttribute::Normal),
Containers::arrayView<Vector3>({
{2.0f, 4.0f, 6.0f}
}), TestSuite::Compare::Container);
CORRADE_VERIFY(mesh->isIndexed());
CORRADE_COMPARE(mesh->indexType(), MeshIndexType::UnsignedInt);
CORRADE_COMPARE_AS(mesh->indices<UnsignedInt>(),
Containers::arrayView<UnsignedInt>({0, 0, 0}),
TestSuite::Compare::Container);
}
void ObjImporterTest::openTwice() {
Containers::Pointer<AbstractImporter> importer = _manager.instantiate("ObjImporter");

1
src/MagnumPlugins/ObjImporter/Test/invalid-keyword.obj
Unescape View File

@ -1 +0,0 @@
bleh

6
src/MagnumPlugins/ObjImporter/Test/invalid-keywords.obj
Unescape View File

@ -0,0 +1,6 @@
o unknown keyword
bleh
o no space between vertex keyword and number
vt3 1
o no space between index keyword and number
p6

45
src/MagnumPlugins/ObjImporter/Test/invalid-number-count.obj
Unescape View File

@ -1,29 +1,62 @@
o no position component at all
v
o two-component position
v 0.5 1.0
o five-component position with optional fourth component
v 0.5 1 2 0.0 3.5
v 0.5 1 2 1.0 3.5
o no texture coordinate component at all
vt
o one-component texture coordinate
v 0.5 1 2
vt 0.5
o four-component texture coordinate with optional third component
v 0.5 1 2
vt 0.5 1.0 0.0 7.4
o no normal component at all
vn
o two-component normal
v 0.5 1 2
vn 0.5 0.0
o four-component normal
v 0.5 1 2
vn 0.5 1.0 2.3 7.4
o no index at all
p
o no index before first slash
p /
o no index after first slash
p 8/
o no index after second slash
p 8//
o four-component index tuple
v 1 2 3
p 4/1/1/1
p 8/1/1/1
o point with two indices
v 1 2 3
p 5 5
p 9 9
o line with one index
v 1 2 3
l 6
l 10
o triangle with two indices
v 1 2 3
f 7 7
f 11 11
o quad
v 1 2 3
f 8 8 8 8
f 12 12 12 12

19
src/MagnumPlugins/ObjImporter/Test/invalid-numbers.obj
Unescape View File

@ -1,6 +1,15 @@
o too long float literal
v 1234.567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567 0 0
o too long integer literal
p 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678
o too large integer literal
p 4294967296
o invalid float literal
v 1 bleh 2
p 1
p 2
o invalid integer literal
v 1 0 2
@ -8,8 +17,8 @@ p bleh
o position index out of range
v 1 0 2
# Should be 3
p 1
# Should be 4
p 3
o texture index out of range
v 1 0 2
@ -17,14 +26,14 @@ vt 0 1
vt 0 1
vt 0 1
# Should be 4/3
p 4/4
p 5/4
o normal index out of range
v 1 0 2
vn 0 0 1
vn 0 0 1
# Should be 5/2
p 5//3
p 6//3
o zero index
v 1 0 2

Write Preview
Loading…
Cancel
Save